[Buildroot] [PATCHv3 00/15] Introduce BR2_ENABLE_RUNTIME_DEBUG

[Buildroot] [PATCHv3 00/15] Introduce BR2_ENABLE_RUNTIME_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Hello,

This patch series started with as main goal to fix the performance degradation
found when testing gRPC in combination with BR2_ENABLE_DEBUG.

Its implementation strives to settle a discussion that has happened several
times, regarding the CMAKE_BUILD_TYPE that Buildroot should set, by not forcing
anything upon the user but allowing the choice.

Additionally, it cleans up some related changes done in specific packages.

After this series, there are still packages that set CMAKE_BUILD_TYPE
explicitly, but the associated comments indicate that they are needed to avoid
build failures, so they are not touched.

v2: fix sysrepo after comment by Jan Kundr?t (thanks!)
v3:
- new patch setting -g0 (Arnout)
- don't set -DNDEBUG globally anymore, and drop related libjson change
- flare-engine, supertux: retain disabling of -pg option (Arnout)
- sysrepo: new patch for host-sysrepo, disable tests for target (Arnout)
- update several packages to use BR2_ENABLE_RUNTIME_DEBUG instead of
  BR2_ENABLE_DEBUG


Thomas De Schampheleire (15):
  package/Makefile.in: pass '-g0' explicitly if !BR2_ENABLE_DEBUG
  core: introduce BR2_ENABLE_RUNTIME_DEBUG
  package/pkg-cmake.mk: determine CMAKE_BUILD_TYPE depending on
    BR2_ENABLE_RUNTIME_DEBUG
  package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE
  package/sysrepo: use default CMAKE_BUILD_TYPE for host package
  package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE
  package/boost: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/oracle-mysql: use BR2_ENABLE_RUNTIME_DEBUG iso
    BR2_ENABLE_DEBUG
  package/qt5: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/ripgrep: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/uclibc: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/zmqpp: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  package/pkg-meson.mk: determine 'buildtype' based on
    BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
  utils/genrandconfig: also test BR2_ENABLE_RUNTIME_DEBUG

 Config.in                             | 13 +++++++++++++
 docs/manual/adding-packages-cmake.txt |  2 +-
 package/Makefile.in                   |  3 +++
 package/boost/boost.mk                |  2 +-
 package/flare-engine/flare-engine.mk  |  2 +-
 package/oracle-mysql/oracle-mysql.mk  |  2 +-
 package/pkg-cmake.mk                  |  2 +-
 package/pkg-meson.mk                  |  2 +-
 package/qt5/qt5base/qt5base.mk        |  2 +-
 package/ripgrep/ripgrep.mk            |  2 +-
 package/sofia-sip/sofia-sip.mk        |  2 +-
 package/sysrepo/sysrepo.mk            |  9 ++++++---
 package/uclibc/uclibc.mk              |  2 +-
 package/zmqpp/zmqpp.mk                |  2 +-
 utils/genrandconfig                   |  2 ++
 15 files changed, 35 insertions(+), 14 deletions(-)

-- 
2.26.3

[Buildroot] [PATCHv3 01/15] package/Makefile.in: pass '-g0' explicitly if !BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

If BR2_ENABLE_DEBUG is not set, Buildroot did not pass any flag
to control debug level. This means that the build system of the package
itself would control it.

Instead, provide an explicit '-g0' (no debugging symbols) to get consistent
behavior across packages.

Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/Makefile.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/Makefile.in b/package/Makefile.in
index f4028bc67c..86db62ba5b 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -136,6 +136,9 @@ endif
 ifeq ($(BR2_OPTIMIZE_FAST),y)
 TARGET_OPTIMIZATION = -Ofast
 endif
+ifeq ($(BR2_ENABLE_DEBUG),)
+TARGET_DEBUGGING = -g0
+endif
 ifeq ($(BR2_DEBUG_1),y)
 TARGET_DEBUGGING = -g1
 endif
-- 
2.26.3

[Buildroot] [PATCHv3 02/15] core: introduce BR2_ENABLE_RUNTIME_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Some packages have optional runtime assertions, extra traces, or other
elements that can help in debugging problems. However, such runtime elements
can negatively influence performance.

In a test program performing 100K gRPC calls from a client to a local server
and receiving the returned response, we see following execution time:

    - runtime debug enabled: 1065 seconds
    - runtime debug disabled:  48 seconds

This is more than a factor 20 (!) difference. Analysis shows that the
problem mostly stems from libabseil-cpp (a dependency of gRPC) which enables
mutex deadlock analysis when the preprocessor flag 'NDEBUG' is not set,
which adds a 'backtrace()' call on every lock/unlock.  Potentially worse,
when libunwind is enabled and linked with the test program, 'backtrace()' is
not provided by glibc but by libunwind itself.

For production systems, users expect good performance out-of-the-box. In the
example above, the difference is huge and unless explicitly tested and
analyzed, users may not realize that the performance could be much better.

Address this problem by introducing a new option BR2_ENABLE_RUNTIME_DEBUG,
which can be used by packages or package infrastructures to set the
necessary flags.

Note that BR2_ENABLE_RUNTIME_DEBUG is orthogonal to BR2_ENABLE_DEBUG: the
former changes runtime behavior, while the latter is only expected to add
debug symbols to the build. Today, the cmake build system does introduce a
runtime impact when BR2_ENABLE_DEBUG is set, but that will be rectified in a
subsequent commit.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 Config.in | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Config.in b/Config.in
index c65e34bd5e..a008425688 100644
--- a/Config.in
+++ b/Config.in
@@ -412,6 +412,19 @@ config BR2_DEBUG_3
 endchoice
 endif
 
+config BR2_ENABLE_RUNTIME_DEBUG
+	bool "build packages with runtime debugging info"
+	help
+	  Some packages may have runtime assertions, extra traces, and
+	  similar runtime elements that can help debugging. However,
+	  these elements may negatively influence performance so should
+	  normally not be enabled on production systems.
+
+	  Enable this option to enable such runtime debugging.
+
+	  Note: disabling this option is not a guarantee that all
+	  packages effectively removed these runtime debugging elements.
+
 config BR2_STRIP_strip
 	bool "strip target binaries"
 	default y
-- 
2.26.3

[Buildroot] [PATCHv3 03/15] package/pkg-cmake.mk: determine CMAKE_BUILD_TYPE depending on BR2_ENABLE_RUNTIME_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The CMAKE_BUILD_TYPE is currently set as 'Debug' in case BR2_ENABLE_DEBUG is
set, and as 'Release' in other cases. However, while the description of
BR2_ENABLE_DEBUG is to enable debug symbols (no runtime impact), the 'Debug'
build type in CMake can actually have runtime impact. For one, because it
does not set -DNDEBUG like is done for 'Release', but also because packages
may do custom things based on it.

The question of which CMAKE_BUILD_TYPE Buildroot should set, be it 'Debug',
'Release', 'RelWithDebInfo' or others, has come up several times in the
past. See some references below:

- July 2016: switch from Debug to RelWithDebInfo:
  https://git.buildroot.org/buildroot/commit/?id=4b0120183404913f7f7788ef4f0f6b51498ef363

- October 2016: switch from RelWithDebInfo back to Debug:
  https://git.buildroot.org/buildroot/commit/?id=104bb29e0490bfb487e2e665448dd3ca07fcc2b5
  and changes to make sure Buildroot's flags are respected:
  https://git.buildroot.org/buildroot/commit/?id=12494ef48f893684d0800e7f6fe39a2ceaed0451

- August 2017: bug #10246 - "BR2_ENABLE_DEBUG does not have the expected
  effect for cmake packages"
  https://bugs.busybox.net/show_bug.cgi?id=10246

- August 2017: mail thread following bug #10246:
  http://lists.busybox.net/pipermail/buildroot/2017-August/200778.html

In the last mail thread, Samuel Martin confirmed that the 'Release' build
type could be used in all cases, because Buildroot is actually making sure
that the optimization flags are those determined by Buildroot, not the
defaults of cmake, thanks to commit 12494ef48f.
But Arnout Vandecappelle objected to using always 'Release', stating that
users may actually want the extra assertions.

With the introduction of BR2_ENABLE_RUNTIME_DEBUG, Buildroot can now cater
for all cases:

- use CMAKE_BUILD_TYPE=Release by default. This makes sure that there is no
  unexpected performance degradation triggered by enabling BR2_ENABLE_DEBUG.

- users can optionally enable BR2_ENABLE_RUNTIME_DEBUG if they want runtime
  debug info like assertions, at the risk of introducing performance
  degradation. In this case, we switch to CMAKE_BUILD_TYPE=Debug.

- orthogonally to the above, BR2_ENABLE_DEBUG still determines passing the
  '-g' flag to enable debug symbols, and BR2_OPTIMIZE_X still determines the
  used optimization flags.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 docs/manual/adding-packages-cmake.txt | 2 +-
 package/pkg-cmake.mk                  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt
index 73f0943024..541d7422cf 100644
--- a/docs/manual/adding-packages-cmake.txt
+++ b/docs/manual/adding-packages-cmake.txt
@@ -100,7 +100,7 @@ typical packages will therefore only use a few of them.
   necessary to set them in the package's +*.mk+ file unless you want
   to override them:
 
-** +CMAKE_BUILD_TYPE+ is driven by +BR2_ENABLE_DEBUG+;
+** +CMAKE_BUILD_TYPE+ is driven by +BR2_ENABLE_RUNTIME_DEBUG+;
 ** +CMAKE_INSTALL_PREFIX+;
 ** +BUILD_SHARED_LIBS+ is driven by +BR2_STATIC_LIBS+;
 ** +BUILD_DOC+, +BUILD_DOCS+ are disabled;
diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
index 37078f3c34..4ee100a0c6 100644
--- a/package/pkg-cmake.mk
+++ b/package/pkg-cmake.mk
@@ -266,7 +266,7 @@ define TOOLCHAIN_CMAKE_INSTALL_FILES
 		-e 's#@@CMAKE_SYSTEM_PROCESSOR@@#$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR))#' \
 		-e 's#@@TOOLCHAIN_HAS_CXX@@#$(if $(BR2_INSTALL_LIBSTDCPP),1,0)#' \
 		-e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \
-		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
+		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_RUNTIME_DEBUG),Debug,Release)#' \
 		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
 		> $(HOST_DIR)/share/buildroot/toolchainfile.cmake
 	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake \
-- 
2.26.3

[Buildroot] [PATCHv3 04/15] package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

The flare-engine package forces CMAKE_BUILD_TYPE=RelWithDebInfo in case
Buildroot would normally set CMAKE_BUILD_TYPE=Debug. Previously, this would
happen if BR2_ENABLE_DEBUG is set, but now we should check
BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/flare-engine/flare-engine.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/flare-engine/flare-engine.mk b/package/flare-engine/flare-engine.mk
index e2f3eefa28..503eb0825f 100644
--- a/package/flare-engine/flare-engine.mk
+++ b/package/flare-engine/flare-engine.mk
@@ -15,7 +15,7 @@ FLARE_ENGINE_DEPENDENCIES += sdl2 sdl2_image sdl2_mixer sdl2_ttf
 FLARE_ENGINE_CONF_OPTS += -DBINDIR=bin -DDATADIR=share/flare
 
 # Don't use the default Debug type as it adds -pg (gprof)
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 FLARE_ENGINE_CONF_OPTS += -DCMAKE_BUILD_TYPE=RelWithDebInfo
 endif
 
-- 
2.26.3

[Buildroot] [PATCHv3 05/15] package/sysrepo: use default CMAKE_BUILD_TYPE for host package

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Use the default build type for host-sysrepo.
In the current version of sysrepo, this happens to be 'Debug'.
As 'Debug' also enables tests, explicitly disable them.

Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/sysrepo/sysrepo.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
index dbc7b16b63..f5be29d2c9 100644
--- a/package/sysrepo/sysrepo.mk
+++ b/package/sysrepo/sysrepo.mk
@@ -26,8 +26,9 @@ define SYSREPO_INSTALL_INIT_SYSV
 endef
 
 HOST_SYSREPO_CONF_OPTS = \
-	-DCMAKE_BUILD_TYPE=Release \
 	-DBUILD_EXAMPLES=OFF \
+	-DENABLE_TESTS=OFF \
+	-DENABLE_VALGRIND_TESTS=OFF \
 	-DREPO_PATH=$(TARGET_DIR)/etc/sysrepo
 
 $(eval $(cmake-package))
-- 
2.26.3

[Buildroot] [PATCHv3 06/15] package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

sysrepo explicitly sets CMAKE_BUILD_TYPE=Release, ignoring any possible
value of BR2_ENABLE_DEBUG (previously) or BR2_ENABLE_RUNTIME_DEBUG (now).

With the introduction of BR2_ENABLE_RUNTIME_DEBUG, this change should no
longer be necessary. Users that do not wish to have additional runtime
debugging just keep BR2_ENABLE_RUNTIME_DEBUG disabled (default value).

As the 'Debug' build type enables tests, disable them explicitly.
As the 'Debug' build type uses a custom REPO_PATH which does not exist on
target, force /etc/sysrepo like in the 'Release' build type.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/sysrepo/sysrepo.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
index f5be29d2c9..c0db05e52c 100644
--- a/package/sysrepo/sysrepo.mk
+++ b/package/sysrepo/sysrepo.mk
@@ -13,8 +13,10 @@ SYSREPO_DEPENDENCIES = libyang pcre host-sysrepo
 HOST_SYSREPO_DEPENDENCIES = host-libyang host-pcre
 
 SYSREPO_CONF_OPTS = \
-	-DCMAKE_BUILD_TYPE=Release \
-	-DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF)
+	-DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF) \
+	-DENABLE_TESTS=OFF \
+	-DENABLE_VALGRIND_TESTS=OFF \
+	-DREPO_PATH=/etc/sysrepo
 
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
 SYSREPO_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
-- 
2.26.3

[Buildroot] [PATCHv3 07/15] package/boost: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/boost/boost.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/boost/boost.mk b/package/boost/boost.mk
index 8d745ecc72..e72ddf897c 100644
--- a/package/boost/boost.mk
+++ b/package/boost/boost.mk
@@ -95,7 +95,7 @@ BOOST_OPTS += --no-cmake-config \
 	     toolset=gcc \
 	     threading=multi \
 	     abi=$(BOOST_ABI) \
-	     variant=$(if $(BR2_ENABLE_DEBUG),debug,release)
+	     variant=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)
 
 ifeq ($(BR2_sparc64),y)
 BOOST_OPTS += architecture=sparc instruction-set=ultrasparc
-- 
2.26.3

[Buildroot] [PATCHv3 08/15] package/oracle-mysql: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/oracle-mysql/oracle-mysql.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/oracle-mysql/oracle-mysql.mk b/package/oracle-mysql/oracle-mysql.mk
index ccfa40cfb1..e7d6354ed8 100644
--- a/package/oracle-mysql/oracle-mysql.mk
+++ b/package/oracle-mysql/oracle-mysql.mk
@@ -95,7 +95,7 @@ ORACLE_MYSQL_CONF_OPTS += \
 
 # Debugging is only available for the server, so no need for
 # this if-block outside of the server if-block
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 ORACLE_MYSQL_CONF_OPTS += --with-debug=full
 else
 ORACLE_MYSQL_CONF_OPTS += --without-debug
-- 
2.26.3

[Buildroot] [PATCHv3 09/15] package/qt5: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/qt5/qt5base/qt5base.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
index 84e9fa4edb..8b55aa3098 100644
--- a/package/qt5/qt5base/qt5base.mk
+++ b/package/qt5/qt5base/qt5base.mk
@@ -86,7 +86,7 @@ else
 QT5BASE_CONFIGURE_OPTS += -no-gbm
 endif
 
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 QT5BASE_CONFIGURE_OPTS += -debug
 else
 QT5BASE_CONFIGURE_OPTS += -release
-- 
2.26.3

[Buildroot] [PATCHv3 10/15] package/ripgrep: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/ripgrep/ripgrep.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/ripgrep/ripgrep.mk b/package/ripgrep/ripgrep.mk
index 9dd8d58de1..c3c12fd185 100644
--- a/package/ripgrep/ripgrep.mk
+++ b/package/ripgrep/ripgrep.mk
@@ -18,7 +18,7 @@ RIPGREP_CARGO_OPTS = \
 	--target=$(RUSTC_TARGET_NAME) \
 	--manifest-path=$(@D)/Cargo.toml
 
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 RIPGREP_CARGO_BIN_SUBDIR = debug
 else
 RIPGREP_CARGO_OPTS += --release
-- 
2.26.3

[Buildroot] [PATCHv3 11/15] package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/sofia-sip/sofia-sip.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/sofia-sip/sofia-sip.mk b/package/sofia-sip/sofia-sip.mk
index 5c383400ff..cb867ba0dc 100644
--- a/package/sofia-sip/sofia-sip.mk
+++ b/package/sofia-sip/sofia-sip.mk
@@ -30,7 +30,7 @@ SOFIA_SIP_CONF_OPTS += \
 	--without-openssl
 endif
 
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 SOFIA_SIP_CONF_OPTS += --enable-ndebug
 endif
 
-- 
2.26.3

[Buildroot] [PATCHv3 12/15] package/uclibc: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/uclibc/uclibc.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
index 3d253d93bc..61bdbd50a0 100644
--- a/package/uclibc/uclibc.mk
+++ b/package/uclibc/uclibc.mk
@@ -205,7 +205,7 @@ endif
 #
 # Debug
 #
-ifeq ($(BR2_ENABLE_DEBUG),y)
+ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
 define UCLIBC_DEBUG_CONFIG
 	$(call KCONFIG_ENABLE_OPT,DODEBUG)
 endef
-- 
2.26.3

[Buildroot] [PATCHv3 13/15] package/zmqpp: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/zmqpp/zmqpp.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
index 3cd19d644a..cfef9874d0 100644
--- a/package/zmqpp/zmqpp.mk
+++ b/package/zmqpp/zmqpp.mk
@@ -19,7 +19,7 @@ ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
 # -ffast-math -finline-functions -fomit-frame-pointer are disabled,
 # so only set CONFIG for the non-affected cases.
 ifneq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
-ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
+ZMQPP_CONFIG = $(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)
 endif
 
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
-- 
2.26.3

[Buildroot] [PATCHv3 14/15] package/pkg-meson.mk: determine 'buildtype' based on BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
should have no negative effect on performance.

Introduction of 'assert' statements, 'debug'-type builds with additional
logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 package/pkg-meson.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
index a57820d4d2..dafad3b1eb 100644
--- a/package/pkg-meson.mk
+++ b/package/pkg-meson.mk
@@ -89,7 +89,7 @@ define $(2)_CONFIGURE_CMDS
 		--prefix=/usr \
 		--libdir=lib \
 		--default-library=$(if $(BR2_STATIC_LIBS),static,shared) \
-		--buildtype=$(if $(BR2_ENABLE_DEBUG),debug,release) \
+		--buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \
 		--cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \
 		-Dstrip=false \
 		-Dbuild.pkg_config_path=$$(HOST_DIR)/lib/pkgconfig \
-- 
2.26.3

[Buildroot] [PATCHv3 15/15] utils/genrandconfig: also test BR2_ENABLE_RUNTIME_DEBUG

[Thomas De Schampheleire]

From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

... similar to BR2_ENABLE_DEBUG.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 utils/genrandconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/utils/genrandconfig b/utils/genrandconfig
index 93dc6d898b..ca3e77e3e3 100755
--- a/utils/genrandconfig
+++ b/utils/genrandconfig
@@ -357,6 +357,8 @@ def gen_config(args):
     # Amend the configuration with a few things.
     if randint(0, 20) == 0:
         configlines.append("BR2_ENABLE_DEBUG=y\n")
+    if randint(0, 20) == 0:
+        configlines.append("BR2_ENABLE_RUNTIME_DEBUG=y\n")
     if randint(0, 1) == 0:
         configlines.append("BR2_INIT_BUSYBOX=y\n")
     elif randint(0, 15) == 0:
-- 
2.26.3

[Buildroot] [PATCHv3 01/15] package/Makefile.in: pass '-g0' explicitly if !BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> If BR2_ENABLE_DEBUG is not set, Buildroot did not pass any flag
> to control debug level. This means that the build system of the package
> itself would control it.
> 
> Instead, provide an explicit '-g0' (no debugging symbols) to get consistent
> behavior across packages.
> 
> Suggested-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

 Applied to next, thanks.

 Regards,
 Arnout

> ---
>  package/Makefile.in | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index f4028bc67c..86db62ba5b 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -136,6 +136,9 @@ endif
>  ifeq ($(BR2_OPTIMIZE_FAST),y)
>  TARGET_OPTIMIZATION = -Ofast
>  endif
> +ifeq ($(BR2_ENABLE_DEBUG),)
> +TARGET_DEBUGGING = -g0
> +endif
>  ifeq ($(BR2_DEBUG_1),y)
>  TARGET_DEBUGGING = -g1
>  endif
> 

[Buildroot] [PATCHv3 00/15] Introduce BR2_ENABLE_RUNTIME_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Hello,
> 
> This patch series started with as main goal to fix the performance degradation
> found when testing gRPC in combination with BR2_ENABLE_DEBUG.
> 
> Its implementation strives to settle a discussion that has happened several
> times, regarding the CMAKE_BUILD_TYPE that Buildroot should set, by not forcing
> anything upon the user but allowing the choice.
> 
> Additionally, it cleans up some related changes done in specific packages.
> 
> After this series, there are still packages that set CMAKE_BUILD_TYPE
> explicitly, but the associated comments indicate that they are needed to avoid
> build failures, so they are not touched.

 There are probably also lots of (non-cmake) packages that have options that
should depend on this new config option, but we don't need to fix the world
right away.

 For the record: I'm in agreement with this series (with some small comments
which I'll make in individual patches). However, it's a bit too controversial
for me to apply outright, so I'm waiting for acks from other developers.

 Regards,
 Arnout

> 
> v2: fix sysrepo after comment by Jan Kundr?t (thanks!)
> v3:
> - new patch setting -g0 (Arnout)
> - don't set -DNDEBUG globally anymore, and drop related libjson change
> - flare-engine, supertux: retain disabling of -pg option (Arnout)
> - sysrepo: new patch for host-sysrepo, disable tests for target (Arnout)
> - update several packages to use BR2_ENABLE_RUNTIME_DEBUG instead of
>   BR2_ENABLE_DEBUG
> 
> 
> Thomas De Schampheleire (15):
>   package/Makefile.in: pass '-g0' explicitly if !BR2_ENABLE_DEBUG
>   core: introduce BR2_ENABLE_RUNTIME_DEBUG
>   package/pkg-cmake.mk: determine CMAKE_BUILD_TYPE depending on
>     BR2_ENABLE_RUNTIME_DEBUG
>   package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE
>   package/sysrepo: use default CMAKE_BUILD_TYPE for host package
>   package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE
>   package/boost: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/oracle-mysql: use BR2_ENABLE_RUNTIME_DEBUG iso
>     BR2_ENABLE_DEBUG
>   package/qt5: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/ripgrep: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/uclibc: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/zmqpp: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   package/pkg-meson.mk: determine 'buildtype' based on
>     BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG
>   utils/genrandconfig: also test BR2_ENABLE_RUNTIME_DEBUG
> 
>  Config.in                             | 13 +++++++++++++
>  docs/manual/adding-packages-cmake.txt |  2 +-
>  package/Makefile.in                   |  3 +++
>  package/boost/boost.mk                |  2 +-
>  package/flare-engine/flare-engine.mk  |  2 +-
>  package/oracle-mysql/oracle-mysql.mk  |  2 +-
>  package/pkg-cmake.mk                  |  2 +-
>  package/pkg-meson.mk                  |  2 +-
>  package/qt5/qt5base/qt5base.mk        |  2 +-
>  package/ripgrep/ripgrep.mk            |  2 +-
>  package/sofia-sip/sofia-sip.mk        |  2 +-
>  package/sysrepo/sysrepo.mk            |  9 ++++++---
>  package/uclibc/uclibc.mk              |  2 +-
>  package/zmqpp/zmqpp.mk                |  2 +-
>  utils/genrandconfig                   |  2 ++
>  15 files changed, 35 insertions(+), 14 deletions(-)
> 

[Buildroot] [PATCHv3 02/15] core: introduce BR2_ENABLE_RUNTIME_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Some packages have optional runtime assertions, extra traces, or other
> elements that can help in debugging problems. However, such runtime elements
> can negatively influence performance.
> 
> In a test program performing 100K gRPC calls from a client to a local server
> and receiving the returned response, we see following execution time:
> 
>     - runtime debug enabled: 1065 seconds
>     - runtime debug disabled:  48 seconds
> 
> This is more than a factor 20 (!) difference. Analysis shows that the
> problem mostly stems from libabseil-cpp (a dependency of gRPC) which enables
> mutex deadlock analysis when the preprocessor flag 'NDEBUG' is not set,
> which adds a 'backtrace()' call on every lock/unlock.  Potentially worse,
> when libunwind is enabled and linked with the test program, 'backtrace()' is
> not provided by glibc but by libunwind itself.
> 
> For production systems, users expect good performance out-of-the-box. In the
> example above, the difference is huge and unless explicitly tested and
> analyzed, users may not realize that the performance could be much better.
> 
> Address this problem by introducing a new option BR2_ENABLE_RUNTIME_DEBUG,
> which can be used by packages or package infrastructures to set the
> necessary flags.
> 
> Note that BR2_ENABLE_RUNTIME_DEBUG is orthogonal to BR2_ENABLE_DEBUG: the
> former changes runtime behavior, while the latter is only expected to add
> debug symbols to the build. Today, the cmake build system does introduce a
> runtime impact when BR2_ENABLE_DEBUG is set, but that will be rectified in a
> subsequent commit.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  Config.in | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Config.in b/Config.in
> index c65e34bd5e..a008425688 100644
> --- a/Config.in
> +++ b/Config.in
> @@ -412,6 +412,19 @@ config BR2_DEBUG_3
>  endchoice
>  endif
>  
> +config BR2_ENABLE_RUNTIME_DEBUG

 I'm still not convinced that RUNTIME_DEBUG is the best name, but I can't think
of anything better.

> +	bool "build packages with runtime debugging info"

 It's not debugging info (that is actually covered by BR2_ENABLE_DEBUG) - it's
debugging support. Or maybe just "debugging".

 Nitpick, of course, so:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> +	help
> +	  Some packages may have runtime assertions, extra traces, and
> +	  similar runtime elements that can help debugging. However,
> +	  these elements may negatively influence performance so should
> +	  normally not be enabled on production systems.
> +
> +	  Enable this option to enable such runtime debugging.
> +
> +	  Note: disabling this option is not a guarantee that all
> +	  packages effectively removed these runtime debugging elements.
> +
>  config BR2_STRIP_strip
>  	bool "strip target binaries"
>  	default y
> 

[Buildroot] [PATCHv3 03/15] package/pkg-cmake.mk: determine CMAKE_BUILD_TYPE depending on BR2_ENABLE_RUNTIME_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The CMAKE_BUILD_TYPE is currently set as 'Debug' in case BR2_ENABLE_DEBUG is
> set, and as 'Release' in other cases. However, while the description of
> BR2_ENABLE_DEBUG is to enable debug symbols (no runtime impact), the 'Debug'
> build type in CMake can actually have runtime impact. For one, because it
> does not set -DNDEBUG like is done for 'Release', but also because packages
> may do custom things based on it.
> 
> The question of which CMAKE_BUILD_TYPE Buildroot should set, be it 'Debug',
> 'Release', 'RelWithDebInfo' or others, has come up several times in the
> past. See some references below:
> 
> - July 2016: switch from Debug to RelWithDebInfo:
>   https://git.buildroot.org/buildroot/commit/?id=4b0120183404913f7f7788ef4f0f6b51498ef363
> 
> - October 2016: switch from RelWithDebInfo back to Debug:
>   https://git.buildroot.org/buildroot/commit/?id=104bb29e0490bfb487e2e665448dd3ca07fcc2b5
>   and changes to make sure Buildroot's flags are respected:
>   https://git.buildroot.org/buildroot/commit/?id=12494ef48f893684d0800e7f6fe39a2ceaed0451
> 
> - August 2017: bug #10246 - "BR2_ENABLE_DEBUG does not have the expected
>   effect for cmake packages"
>   https://bugs.busybox.net/show_bug.cgi?id=10246
> 
> - August 2017: mail thread following bug #10246:
>   http://lists.busybox.net/pipermail/buildroot/2017-August/200778.html
> 
> In the last mail thread, Samuel Martin confirmed that the 'Release' build
> type could be used in all cases, because Buildroot is actually making sure
> that the optimization flags are those determined by Buildroot, not the
> defaults of cmake, thanks to commit 12494ef48f.
> But Arnout Vandecappelle objected to using always 'Release', stating that
> users may actually want the extra assertions.
> 
> With the introduction of BR2_ENABLE_RUNTIME_DEBUG, Buildroot can now cater
> for all cases:
> 
> - use CMAKE_BUILD_TYPE=Release by default. This makes sure that there is no
>   unexpected performance degradation triggered by enabling BR2_ENABLE_DEBUG.
> 
> - users can optionally enable BR2_ENABLE_RUNTIME_DEBUG if they want runtime
>   debug info like assertions, at the risk of introducing performance
>   degradation. In this case, we switch to CMAKE_BUILD_TYPE=Debug.
> 
> - orthogonally to the above, BR2_ENABLE_DEBUG still determines passing the
>   '-g' flag to enable debug symbols, and BR2_OPTIMIZE_X still determines the
>   used optimization flags.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> --->  docs/manual/adding-packages-cmake.txt | 2 +-
>  package/pkg-cmake.mk                  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-cmake.txt b/docs/manual/adding-packages-cmake.txt
> index 73f0943024..541d7422cf 100644
> --- a/docs/manual/adding-packages-cmake.txt
> +++ b/docs/manual/adding-packages-cmake.txt
> @@ -100,7 +100,7 @@ typical packages will therefore only use a few of them.
>    necessary to set them in the package's +*.mk+ file unless you want
>    to override them:
>  
> -** +CMAKE_BUILD_TYPE+ is driven by +BR2_ENABLE_DEBUG+;
> +** +CMAKE_BUILD_TYPE+ is driven by +BR2_ENABLE_RUNTIME_DEBUG+;
>  ** +CMAKE_INSTALL_PREFIX+;
>  ** +BUILD_SHARED_LIBS+ is driven by +BR2_STATIC_LIBS+;
>  ** +BUILD_DOC+, +BUILD_DOCS+ are disabled;
> diff --git a/package/pkg-cmake.mk b/package/pkg-cmake.mk
> index 37078f3c34..4ee100a0c6 100644
> --- a/package/pkg-cmake.mk
> +++ b/package/pkg-cmake.mk
> @@ -266,7 +266,7 @@ define TOOLCHAIN_CMAKE_INSTALL_FILES
>  		-e 's#@@CMAKE_SYSTEM_PROCESSOR@@#$(call qstrip,$(CMAKE_SYSTEM_PROCESSOR))#' \
>  		-e 's#@@TOOLCHAIN_HAS_CXX@@#$(if $(BR2_INSTALL_LIBSTDCPP),1,0)#' \
>  		-e 's#@@TOOLCHAIN_HAS_FORTRAN@@#$(if $(BR2_TOOLCHAIN_HAS_FORTRAN),1,0)#' \
> -		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_DEBUG),Debug,Release)#' \
> +		-e 's#@@CMAKE_BUILD_TYPE@@#$(if $(BR2_ENABLE_RUNTIME_DEBUG),Debug,Release)#' \
>  		$(TOPDIR)/support/misc/toolchainfile.cmake.in \
>  		> $(HOST_DIR)/share/buildroot/toolchainfile.cmake
>  	$(Q)$(INSTALL) -D -m 0644 support/misc/Buildroot.cmake \
> 

[Buildroot] [PATCHv3 04/15] package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> The flare-engine package forces CMAKE_BUILD_TYPE=RelWithDebInfo in case
> Buildroot would normally set CMAKE_BUILD_TYPE=Debug. Previously, this would
> happen if BR2_ENABLE_DEBUG is set, but now we should check
> BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/flare-engine/flare-engine.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/flare-engine/flare-engine.mk b/package/flare-engine/flare-engine.mk
> index e2f3eefa28..503eb0825f 100644
> --- a/package/flare-engine/flare-engine.mk
> +++ b/package/flare-engine/flare-engine.mk
> @@ -15,7 +15,7 @@ FLARE_ENGINE_DEPENDENCIES += sdl2 sdl2_image sdl2_mixer sdl2_ttf
>  FLARE_ENGINE_CONF_OPTS += -DBINDIR=bin -DDATADIR=share/flare
>  
>  # Don't use the default Debug type as it adds -pg (gprof)

 Debug type is no longer default :-)

> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>  FLARE_ENGINE_CONF_OPTS += -DCMAKE_BUILD_TYPE=RelWithDebInfo

 Since CMAKE_BUILD_TYPE is only used by flare-engine to set -O2 -g flags, maybe
it would be better/cleaner to just unconditionally set build type to Release?

 Regards,
 Arnout

>  endif
>  
> 

[Buildroot] [PATCHv3 05/15] package/sysrepo: use default CMAKE_BUILD_TYPE for host package

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Use the default build type for host-sysrepo.
> In the current version of sysrepo, this happens to be 'Debug'.
> As 'Debug' also enables tests, explicitly disable them.
> 
> Suggested-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  package/sysrepo/sysrepo.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
> index dbc7b16b63..f5be29d2c9 100644
> --- a/package/sysrepo/sysrepo.mk
> +++ b/package/sysrepo/sysrepo.mk
> @@ -26,8 +26,9 @@ define SYSREPO_INSTALL_INIT_SYSV
>  endef
>  
>  HOST_SYSREPO_CONF_OPTS = \
> -	-DCMAKE_BUILD_TYPE=Release \
>  	-DBUILD_EXAMPLES=OFF \
> +	-DENABLE_TESTS=OFF \
> +	-DENABLE_VALGRIND_TESTS=OFF \
>  	-DREPO_PATH=$(TARGET_DIR)/etc/sysrepo
>  
>  $(eval $(cmake-package))
> 

[Buildroot] [PATCHv3 06/15] package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> sysrepo explicitly sets CMAKE_BUILD_TYPE=Release, ignoring any possible
> value of BR2_ENABLE_DEBUG (previously) or BR2_ENABLE_RUNTIME_DEBUG (now).
> 
> With the introduction of BR2_ENABLE_RUNTIME_DEBUG, this change should no
> longer be necessary. Users that do not wish to have additional runtime
> debugging just keep BR2_ENABLE_RUNTIME_DEBUG disabled (default value).
> 
> As the 'Debug' build type enables tests, disable them explicitly.
> As the 'Debug' build type uses a custom REPO_PATH which does not exist on
> target, force /etc/sysrepo like in the 'Release' build type.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Although I think this could have been squashed with the previous patch.

 Regards,
 Arnout

> ---
>  package/sysrepo/sysrepo.mk | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
> index f5be29d2c9..c0db05e52c 100644
> --- a/package/sysrepo/sysrepo.mk
> +++ b/package/sysrepo/sysrepo.mk
> @@ -13,8 +13,10 @@ SYSREPO_DEPENDENCIES = libyang pcre host-sysrepo
>  HOST_SYSREPO_DEPENDENCIES = host-libyang host-pcre
>  
>  SYSREPO_CONF_OPTS = \
> -	-DCMAKE_BUILD_TYPE=Release \
> -	-DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF)
> +	-DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF) \
> +	-DENABLE_TESTS=OFF \
> +	-DENABLE_VALGRIND_TESTS=OFF \
> +	-DREPO_PATH=/etc/sysrepo
>  
>  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>  SYSREPO_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
> 

[Buildroot] [PATCHv3 07/15] package/boost: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/boost/boost.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/boost/boost.mk b/package/boost/boost.mk
> index 8d745ecc72..e72ddf897c 100644
> --- a/package/boost/boost.mk
> +++ b/package/boost/boost.mk
> @@ -95,7 +95,7 @@ BOOST_OPTS += --no-cmake-config \
>  	     toolset=gcc \
>  	     threading=multi \
>  	     abi=$(BOOST_ABI) \
> -	     variant=$(if $(BR2_ENABLE_DEBUG),debug,release)
> +	     variant=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)

It looks like the variant does the following things:

<optimization>off <debug-symbols>on <inlining>off <runtime-debugging>on

 Perhaps it would make more sense to control these individually in
user-config.jam. E.g. inlining should be controller by optimization level >= 2 IMHO.

 We should probably also check if our -g and -O options are properly propagated
through the stuff that is set by bjam itself.

 But that's probably for someone with actual boost expertise to tackle, so I'm
OK with applying as-is.

 Regards,
 Arnout

>  
>  ifeq ($(BR2_sparc64),y)
>  BOOST_OPTS += architecture=sparc instruction-set=ultrasparc
> 

[Buildroot] [PATCHv3 08/15] package/oracle-mysql: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  package/oracle-mysql/oracle-mysql.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/oracle-mysql/oracle-mysql.mk b/package/oracle-mysql/oracle-mysql.mk
> index ccfa40cfb1..e7d6354ed8 100644
> --- a/package/oracle-mysql/oracle-mysql.mk
> +++ b/package/oracle-mysql/oracle-mysql.mk
> @@ -95,7 +95,7 @@ ORACLE_MYSQL_CONF_OPTS += \
>  
>  # Debugging is only available for the server, so no need for
>  # this if-block outside of the server if-block
> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>  ORACLE_MYSQL_CONF_OPTS += --with-debug=full
>  else
>  ORACLE_MYSQL_CONF_OPTS += --without-debug
> 

[Buildroot] [PATCHv3 09/15] package/qt5: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  package/qt5/qt5base/qt5base.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/qt5/qt5base/qt5base.mk b/package/qt5/qt5base/qt5base.mk
> index 84e9fa4edb..8b55aa3098 100644
> --- a/package/qt5/qt5base/qt5base.mk
> +++ b/package/qt5/qt5base/qt5base.mk
> @@ -86,7 +86,7 @@ else
>  QT5BASE_CONFIGURE_OPTS += -no-gbm
>  endif
>  
> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>  QT5BASE_CONFIGURE_OPTS += -debug
>  else
>  QT5BASE_CONFIGURE_OPTS += -release
> 

[Buildroot] [PATCHv3 10/15] package/ripgrep: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/ripgrep/ripgrep.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/ripgrep/ripgrep.mk b/package/ripgrep/ripgrep.mk
> index 9dd8d58de1..c3c12fd185 100644
> --- a/package/ripgrep/ripgrep.mk
> +++ b/package/ripgrep/ripgrep.mk
> @@ -18,7 +18,7 @@ RIPGREP_CARGO_OPTS = \
>  	--target=$(RUSTC_TARGET_NAME) \
>  	--manifest-path=$(@D)/Cargo.toml
>  
> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>  RIPGREP_CARGO_BIN_SUBDIR = debug
>  else
>  RIPGREP_CARGO_OPTS += --release

 This stuff should probably be raised to the pkg-cargo level, but for now:

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

[Buildroot] [PATCHv3 11/15] package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/sofia-sip/sofia-sip.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/sofia-sip/sofia-sip.mk b/package/sofia-sip/sofia-sip.mk
> index 5c383400ff..cb867ba0dc 100644
> --- a/package/sofia-sip/sofia-sip.mk
> +++ b/package/sofia-sip/sofia-sip.mk
> @@ -30,7 +30,7 @@ SOFIA_SIP_CONF_OPTS += \
>  	--without-openssl
>  endif
>  
> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>  SOFIA_SIP_CONF_OPTS += --enable-ndebug

 Actually, the logic was inverted here (already wrong in the current situation):
--enable-ndebug should be given when RUNTIME_DEBUG is *not* set.

 However, I think we should define -DNDEBUG in TARGET_CPPFLAGS, so it wouldn't
be necessary to pass it explicitly here (--enable-ndebug does nothing more than
defining NDEBUG).


 Regards,
 Arnout

>  endif
>  
> 

[Buildroot] [PATCHv3 12/15] package/uclibc: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/uclibc/uclibc.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
> index 3d253d93bc..61bdbd50a0 100644
> --- a/package/uclibc/uclibc.mk
> +++ b/package/uclibc/uclibc.mk
> @@ -205,7 +205,7 @@ endif
>  #
>  # Debug
>  #
> -ifeq ($(BR2_ENABLE_DEBUG),y)
> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)

 This one I'm not so sure... In UClibc, we don't (want to) enforce the -g/-O
options as strictly. In fact, we don't pass them down into uClibc at all. And
indeed, the DODEBUG option mostly controls debug symbols and optimisation. The
only "runtime debugging" difference that I can see are:

- different way of handling SSP errors;
- debug messages in linuxthreads.

 So I don't think this patch should be applied.

 Regards,
 Arnout


>  define UCLIBC_DEBUG_CONFIG
>  	$(call KCONFIG_ENABLE_OPT,DODEBUG)
>  endef
> 

[Buildroot] [PATCHv3 13/15] package/zmqpp: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/zmqpp/zmqpp.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
> index 3cd19d644a..cfef9874d0 100644
> --- a/package/zmqpp/zmqpp.mk
> +++ b/package/zmqpp/zmqpp.mk
> @@ -19,7 +19,7 @@ ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
>  # -ffast-math -finline-functions -fomit-frame-pointer are disabled,
>  # so only set CONFIG for the non-affected cases.
>  ifneq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
> -ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
> +ZMQPP_CONFIG = $(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)

 This is the effect of CONFIG:

CONFIG_FLAGS =
ifeq ($(CONFIG),debug)
        CONFIG_FLAGS = -g -fno-inline -ftemplate-depth-1000
endif
ifeq ($(CONFIG),valgrind)
        CONFIG_FLAGS = -g -O1 -DNO_DEBUG_LOG -DNO_TRACE_LOG
endif
ifeq ($(CONFIG),max)
        CONFIG_FLAGS = -O3 -funroll-loops -ffast-math -finline-functions
-fomit-frame-pointer -DNDEBUG
endif
ifneq (,$(findstring $(CONFIG),release loadtest))
        CONFIG_FLAGS = -O3 -funroll-loops -ffast-math -finline-functions
-fomit-frame-pointer -DNO_DEBUG_LOG -DNO_TRACE_LOG -DNDEBUG
endif


 So I'm OK with the release case, but I think maybe in the RUNTIME_DEBUG case we
don't want to set anything at all. Neither -fno-inline nor -ftemplate-depth is
what we want, I think. So maybe we should set nothing at all?


 Regards,
 Arnout



>  endif
>  
>  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> 

[Buildroot] [PATCHv3 15/15] utils/genrandconfig: also test BR2_ENABLE_RUNTIME_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> ... similar to BR2_ENABLE_DEBUG.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


> ---
>  utils/genrandconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/utils/genrandconfig b/utils/genrandconfig
> index 93dc6d898b..ca3e77e3e3 100755
> --- a/utils/genrandconfig
> +++ b/utils/genrandconfig
> @@ -357,6 +357,8 @@ def gen_config(args):
>      # Amend the configuration with a few things.
>      if randint(0, 20) == 0:
>          configlines.append("BR2_ENABLE_DEBUG=y\n")
> +    if randint(0, 20) == 0:

 Although actually, I think we should increase the frequency of all these
options quite a lot...

 Regards,
 Arnout

> +        configlines.append("BR2_ENABLE_RUNTIME_DEBUG=y\n")
>      if randint(0, 1) == 0:
>          configlines.append("BR2_INIT_BUSYBOX=y\n")
>      elif randint(0, 15) == 0:
> 

[Buildroot] [PATCHv3 05/15] package/sysrepo: use default CMAKE_BUILD_TYPE for host package

[Heiko Thiery]

Hi Thomas,

Am Di., 25. Mai 2021 um 22:54 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Use the default build type for host-sysrepo.
> > In the current version of sysrepo, this happens to be 'Debug'.
> > As 'Debug' also enables tests, explicitly disable them.
> >
> > Suggested-by: Arnout Vandecappelle <arnout@mind.be>
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Acked-by: Heiko Thiery <heiko.thiery@gmail.com>

>
>  Regards,
>  Arnout
>
> > ---
> >  package/sysrepo/sysrepo.mk | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
> > index dbc7b16b63..f5be29d2c9 100644
> > --- a/package/sysrepo/sysrepo.mk
> > +++ b/package/sysrepo/sysrepo.mk
> > @@ -26,8 +26,9 @@ define SYSREPO_INSTALL_INIT_SYSV
> >  endef
> >
> >  HOST_SYSREPO_CONF_OPTS = \
> > -     -DCMAKE_BUILD_TYPE=Release \
> >       -DBUILD_EXAMPLES=OFF \
> > +     -DENABLE_TESTS=OFF \
> > +     -DENABLE_VALGRIND_TESTS=OFF \
> >       -DREPO_PATH=$(TARGET_DIR)/etc/sysrepo
> >
> >  $(eval $(cmake-package))
> >

[Buildroot] [PATCHv3 06/15] package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE

[Heiko Thiery]

Hi Thomas,

Am Di., 25. Mai 2021 um 22:56 Uhr schrieb Arnout Vandecappelle <arnout@mind.be>:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > sysrepo explicitly sets CMAKE_BUILD_TYPE=Release, ignoring any possible
> > value of BR2_ENABLE_DEBUG (previously) or BR2_ENABLE_RUNTIME_DEBUG (now).
> >
> > With the introduction of BR2_ENABLE_RUNTIME_DEBUG, this change should no
> > longer be necessary. Users that do not wish to have additional runtime
> > debugging just keep BR2_ENABLE_RUNTIME_DEBUG disabled (default value).
> >
> > As the 'Debug' build type enables tests, disable them explicitly.
> > As the 'Debug' build type uses a custom REPO_PATH which does not exist on
> > target, force /etc/sysrepo like in the 'Release' build type.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Acked-by: Heiko Thiery <heiko.thiery@gmail.com>

>
> Although I think this could have been squashed with the previous patch.
>
>  Regards,
>  Arnout
>
> > ---
> >  package/sysrepo/sysrepo.mk | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/package/sysrepo/sysrepo.mk b/package/sysrepo/sysrepo.mk
> > index f5be29d2c9..c0db05e52c 100644
> > --- a/package/sysrepo/sysrepo.mk
> > +++ b/package/sysrepo/sysrepo.mk
> > @@ -13,8 +13,10 @@ SYSREPO_DEPENDENCIES = libyang pcre host-sysrepo
> >  HOST_SYSREPO_DEPENDENCIES = host-libyang host-pcre
> >
> >  SYSREPO_CONF_OPTS = \
> > -     -DCMAKE_BUILD_TYPE=Release \
> > -     -DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF)
> > +     -DBUILD_EXAMPLES=$(if $(BR2_PACKAGE_SYSREPO_EXAMPLES),ON,OFF) \
> > +     -DENABLE_TESTS=OFF \
> > +     -DENABLE_VALGRIND_TESTS=OFF \
> > +     -DREPO_PATH=/etc/sysrepo
> >
> >  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
> >  SYSREPO_CONF_OPTS += -DCMAKE_EXE_LINKER_FLAGS=-latomic
> >

[Buildroot] [PATCHv3 04/15] package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 22:51, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > The flare-engine package forces CMAKE_BUILD_TYPE=RelWithDebInfo in case
> > Buildroot would normally set CMAKE_BUILD_TYPE=Debug. Previously, this would
> > happen if BR2_ENABLE_DEBUG is set, but now we should check
> > BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/flare-engine/flare-engine.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/flare-engine/flare-engine.mk b/package/flare-engine/flare-engine.mk
> > index e2f3eefa28..503eb0825f 100644
> > --- a/package/flare-engine/flare-engine.mk
> > +++ b/package/flare-engine/flare-engine.mk
> > @@ -15,7 +15,7 @@ FLARE_ENGINE_DEPENDENCIES += sdl2 sdl2_image sdl2_mixer sdl2_ttf
> >  FLARE_ENGINE_CONF_OPTS += -DBINDIR=bin -DDATADIR=share/flare
> >
> >  # Don't use the default Debug type as it adds -pg (gprof)
>
>  Debug type is no longer default :-)
>
> > -ifeq ($(BR2_ENABLE_DEBUG),y)
> > +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
> >  FLARE_ENGINE_CONF_OPTS += -DCMAKE_BUILD_TYPE=RelWithDebInfo
>
>  Since CMAKE_BUILD_TYPE is only used by flare-engine to set -O2 -g flags, maybe
> it would be better/cleaner to just unconditionally set build type to Release?
>

I was checking the situation with BR2_ENABLE_DEBUG=y and see that the
flags of Buildroot do not survive. E.g. compilation commands are:

... -Os -g2 -D_FORTIFY_SOURCE=1 -O2 -g0 ...

The '-O2 -g0' are from the 'Release' build type, set by flare-engine
itself, and '-Os -g2' is what Buildroot sets.

So it means that in the current state of things, this patch would
actually remove debugging symbols when requested, and so would using
'Release' unconditionally.
The pkg-cmake.mk intends to pass its flags to each cmake package and
they should survive. In flare-engine this principle is violated.
So unless there is a better way to fix that, we may actually need to
keep the existing check on BR2_ENABLE_DEBUG and set RelWithDebInfo in
that case.

/Thomas

[Buildroot] [PATCHv3 04/15] package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE

[Arnout Vandecappelle]

On 26/05/2021 16:28, Thomas De Schampheleire wrote:
> El mar, 25 may 2021 a las 22:51, Arnout Vandecappelle
> (<arnout@mind.be>) escribi?:
>>
>>
>>
>> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
>>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>
>>> The flare-engine package forces CMAKE_BUILD_TYPE=RelWithDebInfo in case
>>> Buildroot would normally set CMAKE_BUILD_TYPE=Debug. Previously, this would
>>> happen if BR2_ENABLE_DEBUG is set, but now we should check
>>> BR2_ENABLE_RUNTIME_DEBUG instead.
>>>
>>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>> ---
>>>  package/flare-engine/flare-engine.mk | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/package/flare-engine/flare-engine.mk b/package/flare-engine/flare-engine.mk
>>> index e2f3eefa28..503eb0825f 100644
>>> --- a/package/flare-engine/flare-engine.mk
>>> +++ b/package/flare-engine/flare-engine.mk
>>> @@ -15,7 +15,7 @@ FLARE_ENGINE_DEPENDENCIES += sdl2 sdl2_image sdl2_mixer sdl2_ttf
>>>  FLARE_ENGINE_CONF_OPTS += -DBINDIR=bin -DDATADIR=share/flare
>>>
>>>  # Don't use the default Debug type as it adds -pg (gprof)
>>
>>  Debug type is no longer default :-)
>>
>>> -ifeq ($(BR2_ENABLE_DEBUG),y)
>>> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>>>  FLARE_ENGINE_CONF_OPTS += -DCMAKE_BUILD_TYPE=RelWithDebInfo
>>
>>  Since CMAKE_BUILD_TYPE is only used by flare-engine to set -O2 -g flags, maybe
>> it would be better/cleaner to just unconditionally set build type to Release?
>>
> 
> I was checking the situation with BR2_ENABLE_DEBUG=y and see that the
> flags of Buildroot do not survive. E.g. compilation commands are:
> 
> ... -Os -g2 -D_FORTIFY_SOURCE=1 -O2 -g0 ...
> 
> The '-O2 -g0' are from the 'Release' build type, set by flare-engine
> itself, and '-Os -g2' is what Buildroot sets.
> 
> So it means that in the current state of things, this patch would
> actually remove debugging symbols when requested, and so would using
> 'Release' unconditionally.
> The pkg-cmake.mk intends to pass its flags to each cmake package and
> they should survive. In flare-engine this principle is violated.
> So unless there is a better way to fix that, we may actually need to
> keep the existing check on BR2_ENABLE_DEBUG and set RelWithDebInfo in
> that case.

 There is actually a different way - set CMAKE_BUILD_TYPE to Buildroot (or
anything else that is not covered by the CMakeLists.txt).

 Regards,
 Arnout

[Buildroot] [PATCHv3 07/15] package/boost: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 23:17, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> > should have no negative effect on performance.
> >
> > Introduction of 'assert' statements, 'debug'-type builds with additional
> > logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/boost/boost.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/boost/boost.mk b/package/boost/boost.mk
> > index 8d745ecc72..e72ddf897c 100644
> > --- a/package/boost/boost.mk
> > +++ b/package/boost/boost.mk
> > @@ -95,7 +95,7 @@ BOOST_OPTS += --no-cmake-config \
> >            toolset=gcc \
> >            threading=multi \
> >            abi=$(BOOST_ABI) \
> > -          variant=$(if $(BR2_ENABLE_DEBUG),debug,release)
> > +          variant=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)
>
> It looks like the variant does the following things:
>
> <optimization>off <debug-symbols>on <inlining>off <runtime-debugging>on
>
>  Perhaps it would make more sense to control these individually in
> user-config.jam. E.g. inlining should be controller by optimization level >= 2 IMHO.

Yes, in some packages the debugging flags and other stuff are mixed
and it may be needed that the .mk does something special to handle
BR2_ENABLE_DEBUG and BR2_ENABLE_RUNTIME_DEBUG correctly.
But normally Buildroot expects that our debug and optimization flags
are passed in any case via cflags, so package build systems should not
concern themselves with them anymore. But of course, there will be
violations in some packages.

>
>  We should probably also check if our -g and -O options are properly propagated
> through the stuff that is set by bjam itself.

I checked on top of this patch, with BR2_ENABLE_DEBUG=y, -O2 -g is
correctly passed.
(cross-checked on one of the boost libs, not all, but I think it will
be global rules).

>
>  But that's probably for someone with actual boost expertise to tackle, so I'm
> OK with applying as-is.

Thanks,
Thomas

[Buildroot] [PATCHv3 04/15] package/flare-engine: update explicit setting of CMAKE_BUILD_TYPE

[Thomas De Schampheleire]

El mi?, 26 may 2021 a las 17:18, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 26/05/2021 16:28, Thomas De Schampheleire wrote:
> > El mar, 25 may 2021 a las 22:51, Arnout Vandecappelle
> > (<arnout@mind.be>) escribi?:
> >>
> >>
> >>
> >> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> >>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>>
> >>> The flare-engine package forces CMAKE_BUILD_TYPE=RelWithDebInfo in case
> >>> Buildroot would normally set CMAKE_BUILD_TYPE=Debug. Previously, this would
> >>> happen if BR2_ENABLE_DEBUG is set, but now we should check
> >>> BR2_ENABLE_RUNTIME_DEBUG instead.
> >>>
> >>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>> ---
> >>>  package/flare-engine/flare-engine.mk | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/package/flare-engine/flare-engine.mk b/package/flare-engine/flare-engine.mk
> >>> index e2f3eefa28..503eb0825f 100644
> >>> --- a/package/flare-engine/flare-engine.mk
> >>> +++ b/package/flare-engine/flare-engine.mk
> >>> @@ -15,7 +15,7 @@ FLARE_ENGINE_DEPENDENCIES += sdl2 sdl2_image sdl2_mixer sdl2_ttf
> >>>  FLARE_ENGINE_CONF_OPTS += -DBINDIR=bin -DDATADIR=share/flare
> >>>
> >>>  # Don't use the default Debug type as it adds -pg (gprof)
> >>
> >>  Debug type is no longer default :-)
> >>
> >>> -ifeq ($(BR2_ENABLE_DEBUG),y)
> >>> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
> >>>  FLARE_ENGINE_CONF_OPTS += -DCMAKE_BUILD_TYPE=RelWithDebInfo
> >>
> >>  Since CMAKE_BUILD_TYPE is only used by flare-engine to set -O2 -g flags, maybe
> >> it would be better/cleaner to just unconditionally set build type to Release?
> >>
> >
> > I was checking the situation with BR2_ENABLE_DEBUG=y and see that the
> > flags of Buildroot do not survive. E.g. compilation commands are:
> >
> > ... -Os -g2 -D_FORTIFY_SOURCE=1 -O2 -g0 ...
> >
> > The '-O2 -g0' are from the 'Release' build type, set by flare-engine
> > itself, and '-Os -g2' is what Buildroot sets.
> >
> > So it means that in the current state of things, this patch would
> > actually remove debugging symbols when requested, and so would using
> > 'Release' unconditionally.
> > The pkg-cmake.mk intends to pass its flags to each cmake package and
> > they should survive. In flare-engine this principle is violated.
> > So unless there is a better way to fix that, we may actually need to
> > keep the existing check on BR2_ENABLE_DEBUG and set RelWithDebInfo in
> > that case.
>
>  There is actually a different way - set CMAKE_BUILD_TYPE to Buildroot (or
> anything else that is not covered by the CMakeLists.txt).

Thanks, this indeed works.

[Buildroot] [PATCHv3 11/15] package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 23:32, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> > should have no negative effect on performance.
> >
> > Introduction of 'assert' statements, 'debug'-type builds with additional
> > logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/sofia-sip/sofia-sip.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/sofia-sip/sofia-sip.mk b/package/sofia-sip/sofia-sip.mk
> > index 5c383400ff..cb867ba0dc 100644
> > --- a/package/sofia-sip/sofia-sip.mk
> > +++ b/package/sofia-sip/sofia-sip.mk
> > @@ -30,7 +30,7 @@ SOFIA_SIP_CONF_OPTS += \
> >       --without-openssl
> >  endif
> >
> > -ifeq ($(BR2_ENABLE_DEBUG),y)
> > +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
> >  SOFIA_SIP_CONF_OPTS += --enable-ndebug
>
>  Actually, the logic was inverted here (already wrong in the current situation):
> --enable-ndebug should be given when RUNTIME_DEBUG is *not* set.

Indeed, you're right. I'll add a patch to fix that first.

>
>  However, I think we should define -DNDEBUG in TARGET_CPPFLAGS, so it wouldn't
> be necessary to pass it explicitly here (--enable-ndebug does nothing more than
> defining NDEBUG).
>

In a previous iteration I basically did that, but dropped that patch
because it also had effect in the toolchain wrapper, which may be used
to build project-specific projects, and which caused compilation
failures when NDEBUG was unexpectedly set (e.g. variables only used
for assert statements, which got compiled away with NDEBUG). Granted,
this is actually a problem in such code, but nevertheless it was my
reality and I expect those of others too.
See http://patchwork.ozlabs.org/project/buildroot/patch/20210212135451.22786-3-patrickdepinguin at gmail.com/
   and my last comment on it.

Best regards,
Thomas

[Buildroot] [PATCHv3 12/15] package/uclibc: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 23:43, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> > should have no negative effect on performance.
> >
> > Introduction of 'assert' statements, 'debug'-type builds with additional
> > logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/uclibc/uclibc.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/uclibc/uclibc.mk b/package/uclibc/uclibc.mk
> > index 3d253d93bc..61bdbd50a0 100644
> > --- a/package/uclibc/uclibc.mk
> > +++ b/package/uclibc/uclibc.mk
> > @@ -205,7 +205,7 @@ endif
> >  #
> >  # Debug
> >  #
> > -ifeq ($(BR2_ENABLE_DEBUG),y)
> > +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>
>  This one I'm not so sure... In UClibc, we don't (want to) enforce the -g/-O
> options as strictly. In fact, we don't pass them down into uClibc at all. And
> indeed, the DODEBUG option mostly controls debug symbols and optimisation. The
> only "runtime debugging" difference that I can see are:
>
> - different way of handling SSP errors;
> - debug messages in linuxthreads.
>
>  So I don't think this patch should be applied.

OK, I'll drop it from the series, thanks.

/Thomas

[Buildroot] [PATCHv3 14/15] package/pkg-meson.mk: determine 'buildtype' based on BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> should have no negative effect on performance.
> 
> Introduction of 'assert' statements, 'debug'-type builds with additional
> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  package/pkg-meson.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
> index a57820d4d2..dafad3b1eb 100644
> --- a/package/pkg-meson.mk
> +++ b/package/pkg-meson.mk
> @@ -89,7 +89,7 @@ define $(2)_CONFIGURE_CMDS
>  		--prefix=/usr \
>  		--libdir=lib \
>  		--default-library=$(if $(BR2_STATIC_LIBS),static,shared) \
> -		--buildtype=$(if $(BR2_ENABLE_DEBUG),debug,release) \
> +		--buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \

 I haven't checked yet in the source code what the effect of buildtype is, nor
have I tested it, but the documentation says:


For setting optimization levels and
toggling debug, you can either set the `buildtype` option, or you can
set the `optimization` and `debug` options which give finer control
over the same. Whichever you decide to use, the other will be deduced
from it. For example, `-Dbuildtype=debugoptimized` is the same as
`-Ddebug=true -Doptimization=2` and vice-versa. This table documents
the two-way mapping:

| buildtype      | debug | optimization |
| ---------      | ----- | ------------ |
| plain          | false | 0            |
| debug          | true  | 0            |
| debugoptimized | true  | 2            |
| release        | false | 3            |
| minsize        | true  | s            |

All other combinations of `debug` and `optimization` set `buildtype` to `'custom'`.


So I think this should be just 'custom', regardless of RUNTIME_DEBUG.


 That said, it *is* possible that the debug option has more effect than just
'-g'... So I checked the meson source. There are a bunch of effects.

* For gcc, it just sets -g/-O options.

* On windows and mac, there are a bunch of other effects but I didn't look at that.

* For boost, it selects the "g" suffix if necessary - but we install without
that suffix anyway.

* For rust, it only has effect on optimisation flags. Note that we currently
don't set optimisation flags correctly for rust, so we have something to learn
there.

* For fortran, java, cuda, it als only affects optimisation and debug flags.

* There is one more relevant effect: there is an option "b_ndebug", which
defaults to false. A project can set it to "if-release" - in that case, -DNDEBUG
is passed if the buildtype if 'release'.


 So it sounds like setting buildtype to release in the RUNTIME_DEBUG case is the
right thing to do. There is one caveat (but this is already the case now): it
will force the optimisation to -O3, and I believe that that will override the
optimisation option we set in cflags. But that's essentially what happens now
already, so not something that needs to be addressed by this series.

 In the non-RUNTIME_DEBUG case, however, I don't think we should set to "debug",
because that will add a -g option. Maybe "plain" is the best option.

 Alternatively, we leave buildtype alone, and instead set debug to false,
optimization to the value corresponding to the optimisation level chosen, and
b_ndebug to true or false based on RUNTIME_DEBUG.


 Regards,
 Arnout

>  		--cross-file=$$($$(PKG)_SRCDIR)/build/cross-compilation.conf \
>  		-Dstrip=false \
>  		-Dbuild.pkg_config_path=$$(HOST_DIR)/lib/pkgconfig \
> 

[Buildroot] [PATCHv3 11/15] package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Arnout Vandecappelle]

On 28/05/2021 20:35, Thomas De Schampheleire wrote:
> El mar, 25 may 2021 a las 23:32, Arnout Vandecappelle
> (<arnout@mind.be>) escribi?:
>>
>>
>> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
>>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>
>>> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
>>> should have no negative effect on performance.
>>>
>>> Introduction of 'assert' statements, 'debug'-type builds with additional
>>> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
>>>
>>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>> ---
>>>  package/sofia-sip/sofia-sip.mk | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/package/sofia-sip/sofia-sip.mk b/package/sofia-sip/sofia-sip.mk
>>> index 5c383400ff..cb867ba0dc 100644
>>> --- a/package/sofia-sip/sofia-sip.mk
>>> +++ b/package/sofia-sip/sofia-sip.mk
>>> @@ -30,7 +30,7 @@ SOFIA_SIP_CONF_OPTS += \
>>>       --without-openssl
>>>  endif
>>>
>>> -ifeq ($(BR2_ENABLE_DEBUG),y)
>>> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
>>>  SOFIA_SIP_CONF_OPTS += --enable-ndebug
>>  Actually, the logic was inverted here (already wrong in the current situation):
>> --enable-ndebug should be given when RUNTIME_DEBUG is *not* set.
> Indeed, you're right. I'll add a patch to fix that first.
> 
>>  However, I think we should define -DNDEBUG in TARGET_CPPFLAGS, so it wouldn't
>> be necessary to pass it explicitly here (--enable-ndebug does nothing more than
>> defining NDEBUG).
> In a previous iteration I basically did that, but dropped that patch
> because it also had effect in the toolchain wrapper,

 No it doesn't... TARGET_CFLAGS isn't set in the toolchain wrapper. Only
arch-specific options are there, and BR2_TARGET_OPTIMIZATION (which is intended
for arch-specific -m flags).

> which may be used
> to build project-specific projects, and which caused compilation
> failures when NDEBUG was unexpectedly set

 If it's built by Buildroot, then of course it *is* going to be passed in
through CFLAGS.

> (e.g. variables only used
> for assert statements, which got compiled away with NDEBUG). Granted,
> this is actually a problem in such code, but nevertheless it was my
> reality and I expect those of others too.

 If you don't want to fix the actual issue, you can always add -UNDEBUG in the
package-specific CFLAGS or in the package build system.

 Regards,
 Arnout

> See http://patchwork.ozlabs.org/project/buildroot/patch/20210212135451.22786-3-patrickdepinguin at gmail.com/
>    and my last comment on it.

[Buildroot] [PATCHv3 14/15] package/pkg-meson.mk: determine 'buildtype' based on BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

Hi Arnout,

El vie, 28 may 2021 a las 23:01, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> > should have no negative effect on performance.
> >
> > Introduction of 'assert' statements, 'debug'-type builds with additional
> > logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/pkg-meson.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/pkg-meson.mk b/package/pkg-meson.mk
> > index a57820d4d2..dafad3b1eb 100644
> > --- a/package/pkg-meson.mk
> > +++ b/package/pkg-meson.mk
> > @@ -89,7 +89,7 @@ define $(2)_CONFIGURE_CMDS
> >               --prefix=/usr \
> >               --libdir=lib \
> >               --default-library=$(if $(BR2_STATIC_LIBS),static,shared) \
> > -             --buildtype=$(if $(BR2_ENABLE_DEBUG),debug,release) \
> > +             --buildtype=$(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release) \
>
>  I haven't checked yet in the source code what the effect of buildtype is, nor
> have I tested it, but the documentation says:
>
>
> For setting optimization levels and
> toggling debug, you can either set the `buildtype` option, or you can
> set the `optimization` and `debug` options which give finer control
> over the same. Whichever you decide to use, the other will be deduced
> from it. For example, `-Dbuildtype=debugoptimized` is the same as
> `-Ddebug=true -Doptimization=2` and vice-versa. This table documents
> the two-way mapping:
>
> | buildtype      | debug | optimization |
> | ---------      | ----- | ------------ |
> | plain          | false | 0            |
> | debug          | true  | 0            |
> | debugoptimized | true  | 2            |
> | release        | false | 3            |
> | minsize        | true  | s            |
>
> All other combinations of `debug` and `optimization` set `buildtype` to `'custom'`.
>
>
> So I think this should be just 'custom', regardless of RUNTIME_DEBUG.
>
>
>  That said, it *is* possible that the debug option has more effect than just
> '-g'... So I checked the meson source. There are a bunch of effects.
>
> * For gcc, it just sets -g/-O options.
>
> * On windows and mac, there are a bunch of other effects but I didn't look at that.
>
> * For boost, it selects the "g" suffix if necessary - but we install without
> that suffix anyway.
>
> * For rust, it only has effect on optimisation flags. Note that we currently
> don't set optimisation flags correctly for rust, so we have something to learn
> there.
>
> * For fortran, java, cuda, it als only affects optimisation and debug flags.
>
> * There is one more relevant effect: there is an option "b_ndebug", which
> defaults to false. A project can set it to "if-release" - in that case, -DNDEBUG
> is passed if the buildtype if 'release'.
>
>
>  So it sounds like setting buildtype to release in the RUNTIME_DEBUG case is the
> right thing to do. There is one caveat (but this is already the case now): it
> will force the optimisation to -O3, and I believe that that will override the
> optimisation option we set in cflags. But that's essentially what happens now
> already, so not something that needs to be addressed by this series.

I think you swapped the RUNTIME_DEBUG value:
The above is for 'RUNTIME_DEBUG not set', i.e. default case --> release


>
>  In the non-RUNTIME_DEBUG case, however, I don't think we should set to "debug",
> because that will add a -g option. Maybe "plain" is the best option.

And this is about RUNTIME_DEBUG=y .

I tested with 'libsigc', and the compilation flags Buildroot passes
explicitly for optimization and debugging are put last, so the '-g'
passed due to buildtype=debug is overridden by the '-g0' we now pass
explicitly if BR2_ENABLE_DEBUG is off.

So I think the proposed patch is actually fine.

Of course, packages where the buildroot flags do not 'survive' will
have a different outcome, but this is a problem in its own right, and
to be fixed for these packages.


>
>  Alternatively, we leave buildtype alone, and instead set debug to false,
> optimization to the value corresponding to the optimisation level chosen, and
> b_ndebug to true or false based on RUNTIME_DEBUG.
>

At this moment I don't have a compelling reason to change it, but
please let me know if you think otherwise.

Thanks,
Thomas

[Buildroot] [PATCHv3 13/15] package/zmqpp: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 23:48, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> > should have no negative effect on performance.
> >
> > Introduction of 'assert' statements, 'debug'-type builds with additional
> > logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  package/zmqpp/zmqpp.mk | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/package/zmqpp/zmqpp.mk b/package/zmqpp/zmqpp.mk
> > index 3cd19d644a..cfef9874d0 100644
> > --- a/package/zmqpp/zmqpp.mk
> > +++ b/package/zmqpp/zmqpp.mk
> > @@ -19,7 +19,7 @@ ZMQPP_LDFLAGS = $(TARGET_LDFLAGS) -lpthread
> >  # -ffast-math -finline-functions -fomit-frame-pointer are disabled,
> >  # so only set CONFIG for the non-affected cases.
> >  ifneq ($(BR2_or1k):$(BR2_TOOLCHAIN_GCC_AT_LEAST_6),y:)
> > -ZMQPP_CONFIG = $(if $(BR2_ENABLE_DEBUG),debug,release)
> > +ZMQPP_CONFIG = $(if $(BR2_ENABLE_RUNTIME_DEBUG),debug,release)
>
>  This is the effect of CONFIG:
>
> CONFIG_FLAGS =
> ifeq ($(CONFIG),debug)
>         CONFIG_FLAGS = -g -fno-inline -ftemplate-depth-1000
> endif
> ifeq ($(CONFIG),valgrind)
>         CONFIG_FLAGS = -g -O1 -DNO_DEBUG_LOG -DNO_TRACE_LOG
> endif
> ifeq ($(CONFIG),max)
>         CONFIG_FLAGS = -O3 -funroll-loops -ffast-math -finline-functions
> -fomit-frame-pointer -DNDEBUG
> endif
> ifneq (,$(findstring $(CONFIG),release loadtest))
>         CONFIG_FLAGS = -O3 -funroll-loops -ffast-math -finline-functions
> -fomit-frame-pointer -DNO_DEBUG_LOG -DNO_TRACE_LOG -DNDEBUG
> endif
>
>
>  So I'm OK with the release case, but I think maybe in the RUNTIME_DEBUG case we
> don't want to set anything at all. Neither -fno-inline nor -ftemplate-depth is
> what we want, I think. So maybe we should set nothing at all?

Ok, I will set 'buildroot' instead of 'debug'   (empty value could be
confusing, as the CONFIG variable is also passed to 'BUILD_ENV', even
though that is currently unused).

Thanks,
Thomas

[Buildroot] [PATCHv3 11/15] package/sofia-sip: use BR2_ENABLE_RUNTIME_DEBUG iso BR2_ENABLE_DEBUG

[Thomas De Schampheleire]

Hello,

El vie, 28 may 2021 a las 23:07, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 28/05/2021 20:35, Thomas De Schampheleire wrote:
> > El mar, 25 may 2021 a las 23:32, Arnout Vandecappelle
> > (<arnout@mind.be>) escribi?:
> >>
> >>
> >> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> >>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>>
> >>> BR2_ENABLE_DEBUG should just steer the availability of debug symbols and
> >>> should have no negative effect on performance.
> >>>
> >>> Introduction of 'assert' statements, 'debug'-type builds with additional
> >>> logging, etc. should be steered by BR2_ENABLE_RUNTIME_DEBUG instead.
> >>>
> >>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>> ---
> >>>  package/sofia-sip/sofia-sip.mk | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/package/sofia-sip/sofia-sip.mk b/package/sofia-sip/sofia-sip.mk
> >>> index 5c383400ff..cb867ba0dc 100644
> >>> --- a/package/sofia-sip/sofia-sip.mk
> >>> +++ b/package/sofia-sip/sofia-sip.mk
> >>> @@ -30,7 +30,7 @@ SOFIA_SIP_CONF_OPTS += \
> >>>       --without-openssl
> >>>  endif
> >>>
> >>> -ifeq ($(BR2_ENABLE_DEBUG),y)
> >>> +ifeq ($(BR2_ENABLE_RUNTIME_DEBUG),y)
> >>>  SOFIA_SIP_CONF_OPTS += --enable-ndebug
> >>  Actually, the logic was inverted here (already wrong in the current situation):
> >> --enable-ndebug should be given when RUNTIME_DEBUG is *not* set.
> > Indeed, you're right. I'll add a patch to fix that first.
> >
> >>  However, I think we should define -DNDEBUG in TARGET_CPPFLAGS, so it wouldn't
> >> be necessary to pass it explicitly here (--enable-ndebug does nothing more than
> >> defining NDEBUG).
> > In a previous iteration I basically did that, but dropped that patch
> > because it also had effect in the toolchain wrapper,
>
>  No it doesn't... TARGET_CFLAGS isn't set in the toolchain wrapper. Only
> arch-specific options are there, and BR2_TARGET_OPTIMIZATION (which is intended
> for arch-specific -m flags).

I had to reproduce the issue in order to understand where my
conclusion was wrong.

The compiler wrapper indeed does not pass these flags.
The problem in our case is that the -DNDEBUG is passed via a
<project>-config script to external compilations. In our case, xenomai
is used, and it provides a 'xeno-config' script to get the correct
compilation and linker flags. The generated file contains the flags
set by Buildroot in XENO_BASE_CFLAGS . With the mentioned patch, this
would include -DNDEBUG.
And the output of 'xeno-config [...] --cflags' was used in the
compilation of the external project.

We can work around that by filtering the output of 'xeno-config' in
our local makefiles.

Note that Thomas Petazzoni had initial doubts on the mentioned patch
(http://patchwork.ozlabs.org/project/buildroot/patch/20210212135451.22786-3-patrickdepinguin at gmail.com/).

If you think we should re-add it to the series, I will do it.

Thanks,
Thomas

[Buildroot] [PATCHv3 02/15] core: introduce BR2_ENABLE_RUNTIME_DEBUG

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 22:34, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > Some packages have optional runtime assertions, extra traces, or other
> > elements that can help in debugging problems. However, such runtime elements
> > can negatively influence performance.
> >
> > In a test program performing 100K gRPC calls from a client to a local server
> > and receiving the returned response, we see following execution time:
> >
> >     - runtime debug enabled: 1065 seconds
> >     - runtime debug disabled:  48 seconds
> >
> > This is more than a factor 20 (!) difference. Analysis shows that the
> > problem mostly stems from libabseil-cpp (a dependency of gRPC) which enables
> > mutex deadlock analysis when the preprocessor flag 'NDEBUG' is not set,
> > which adds a 'backtrace()' call on every lock/unlock.  Potentially worse,
> > when libunwind is enabled and linked with the test program, 'backtrace()' is
> > not provided by glibc but by libunwind itself.
> >
> > For production systems, users expect good performance out-of-the-box. In the
> > example above, the difference is huge and unless explicitly tested and
> > analyzed, users may not realize that the performance could be much better.
> >
> > Address this problem by introducing a new option BR2_ENABLE_RUNTIME_DEBUG,
> > which can be used by packages or package infrastructures to set the
> > necessary flags.
> >
> > Note that BR2_ENABLE_RUNTIME_DEBUG is orthogonal to BR2_ENABLE_DEBUG: the
> > former changes runtime behavior, while the latter is only expected to add
> > debug symbols to the build. Today, the cmake build system does introduce a
> > runtime impact when BR2_ENABLE_DEBUG is set, but that will be rectified in a
> > subsequent commit.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> >  Config.in | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/Config.in b/Config.in
> > index c65e34bd5e..a008425688 100644
> > --- a/Config.in
> > +++ b/Config.in
> > @@ -412,6 +412,19 @@ config BR2_DEBUG_3
> >  endchoice
> >  endif
> >
> > +config BR2_ENABLE_RUNTIME_DEBUG
>
>  I'm still not convinced that RUNTIME_DEBUG is the best name, but I can't think
> of anything better.
>
> > +     bool "build packages with runtime debugging info"
>
>  It's not debugging info (that is actually covered by BR2_ENABLE_DEBUG) - it's
> debugging support. Or maybe just "debugging".

It's hard to describe it well because the exact influence will depend
on the package.
For some it will only be asserts that enabled/disabled, others will
have extra traces, yet others will compile additional code for
developers.

I feel that "build packages with runtime debugging support" may give
the false impression that this option is needed in order to be able to
debug the package.

But likewise, the original title of this option could also be
interpreted incorrectly.

So I suggest that the person that applies the patch can make the final call ;-)

Thanks,
Thomas

[Buildroot] [PATCHv3 06/15] package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE

[Thomas De Schampheleire]

El mar, 25 may 2021 a las 22:56, Arnout Vandecappelle
(<arnout@mind.be>) escribi?:
>
>
>
> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >
> > sysrepo explicitly sets CMAKE_BUILD_TYPE=Release, ignoring any possible
> > value of BR2_ENABLE_DEBUG (previously) or BR2_ENABLE_RUNTIME_DEBUG (now).
> >
> > With the introduction of BR2_ENABLE_RUNTIME_DEBUG, this change should no
> > longer be necessary. Users that do not wish to have additional runtime
> > debugging just keep BR2_ENABLE_RUNTIME_DEBUG disabled (default value).
> >
> > As the 'Debug' build type enables tests, disable them explicitly.
> > As the 'Debug' build type uses a custom REPO_PATH which does not exist on
> > target, force /etc/sysrepo like in the 'Release' build type.
> >
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>
> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> Although I think this could have been squashed with the previous patch.

I thought a separate patch was intended based on your comment:
http://patchwork.ozlabs.org/comment/2633317/

In v4 I'm sending the patch still separated but feel free to squash it.

Thanks,
Thomas

[Buildroot] [PATCHv3 06/15] package/sysrepo: remove explicit setting of CMAKE_BUILD_TYPE

[Arnout Vandecappelle]

On 01/06/2021 16:24, Thomas De Schampheleire wrote:
> El mar, 25 may 2021 a las 22:56, Arnout Vandecappelle
> (<arnout@mind.be>) escribi?:
>>
>>
>>
>> On 25/05/2021 14:27, Thomas De Schampheleire wrote:
>>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>
>>> sysrepo explicitly sets CMAKE_BUILD_TYPE=Release, ignoring any possible
>>> value of BR2_ENABLE_DEBUG (previously) or BR2_ENABLE_RUNTIME_DEBUG (now).
>>>
>>> With the introduction of BR2_ENABLE_RUNTIME_DEBUG, this change should no
>>> longer be necessary. Users that do not wish to have additional runtime
>>> debugging just keep BR2_ENABLE_RUNTIME_DEBUG disabled (default value).
>>>
>>> As the 'Debug' build type enables tests, disable them explicitly.
>>> As the 'Debug' build type uses a custom REPO_PATH which does not exist on
>>> target, force /etc/sysrepo like in the 'Release' build type.
>>>
>>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>
>> Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>> Although I think this could have been squashed with the previous patch.
> 
> I thought a separate patch was intended based on your comment:
> http://patchwork.ozlabs.org/comment/2633317/

 Good point! I should have already applied patch 5.

 Regards,
 Arnout

> 
> In v4 I'm sending the patch still separated but feel free to squash it.
> 
> Thanks,
> Thomas
>