diff mbox series

[1/1] package/bitcoin: unset the NDEBUG flag

Message ID eIIz4qpp1iCjP9F8wSbhB85GxfrjYh4LnxsTUDodko@cp4-web-032.plabs.ch
State Changes Requested
Headers show
Series [1/1] package/bitcoin: unset the NDEBUG flag | expand

Commit Message

D. Olsson June 11, 2021, 8:06 p.m. UTC
Since https://git.buildroot.net/buildroot/commit/?id=5a8c50fe05afacc3cbe8e7347e238da9f242fab0
all packages are now built with NDEBUG, which broke Bitcoin builds.

Bitcoin is using assert(...) extensively with the assumption of it
never being a noop at runtime. So we cannot build with NDEBUG.
See: https://github.com/bitcoin/bitcoin/blob/0.21/src/compat/assumptions.h

Signed-off-by: Dick Olsson <hi@senzilla.io>
---
 package/bitcoin/bitcoin.mk | 3 +++
 1 file changed, 3 insertions(+)

Comments

Yann E. MORIN June 12, 2021, 9:51 a.m. UTC | #1
Dick, All,

On 2021-06-11 20:06 +0000, Dick Olsson via buildroot spake thusly:
> Since https://git.buildroot.net/buildroot/commit/?id=5a8c50fe05afacc3cbe8e7347e238da9f242fab0
> all packages are now built with NDEBUG, which broke Bitcoin builds.
> 
> Bitcoin is using assert(...) extensively with the assumption of it
> never being a noop at runtime. So we cannot build with NDEBUG.
> See: https://github.com/bitcoin/bitcoin/blob/0.21/src/compat/assumptions.h
> 
> Signed-off-by: Dick Olsson <hi@senzilla.io>
> ---
>  package/bitcoin/bitcoin.mk | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/package/bitcoin/bitcoin.mk b/package/bitcoin/bitcoin.mk
> index 5f1684879c..27fc2d5051 100644
> --- a/package/bitcoin/bitcoin.mk
> +++ b/package/bitcoin/bitcoin.mk
> @@ -13,6 +13,9 @@ BITCOIN_CPE_ID_VENDOR = bitcoin
>  BITCOIN_CPE_ID_PRODUCT = bitcoin_core
>  BITCOIN_DEPENDENCIES = host-pkgconf boost libevent
>  BITCOIN_MAKE_ENV = BITCOIN_GENBUILD_NO_GIT=1
> +# Bitcoin is using assert(...) extensively with the assumption of it
> +# never being a noop at runtime. So we cannot build with NDEBUG.
> +BITCOIN_CONF_ENV = CFLAGS="-UNDEBUG" CXXFLAGS="-UNDEBUG"

That's insufficient, because that will entirely override the original
CFLAGS/CXXFLAGS the autotools infra is passing:

    https://git.buildroot.org/buildroot/tree/package/pkg-autotools.mk#n180

with CFLAGS et al. inherited from TARGET_CONFIGURE_OPTS:

    https://git.buildroot.org/buildroot/tree/package/Makefile.in#n252

In this case, since you override the infra-provided flags, the -DNDEBUG
is not present in the flags, so it is not surprising that it works.

So, the proper solution is to extend them.

Additionally, I thing we should also override CPPFLAGS.

    BITCOIN_CONF_ENV = \
        CPPFLAGS="$(TARGET_CPPFLAGS) -UNDEBUG" \
        CFLAGS="$(TARGET_CFLAGS) -UNDEBUG" \
        CXXFLAGS="$(TARGET_CXXFLAGS) -UNDEBUG"

Can you check this is still working, and if so respin a patch?

An alternative solution, *iff* the above does not work, would be to
filter out -DNDEBUG, like so:

    BITCOIN_CONF_ENV = \
        CPPFLAGS="$(filter-out -DNDEBUG,$(TARGET_CPPFLAGS))" \
        CFLAGS="$(filter-out -DNDEBUG,$(TARGET_CFLAGS))" \
        CXXFLAGS="$(filter-out -DNDEBUG,$(TARGET_CXXFLAGS))"

Regards,
Yann E. MORIN.

>  BITCOIN_CONF_OPTS = \
>  	--disable-bench \
>  	--disable-wallet \
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
diff mbox series

Patch

diff --git a/package/bitcoin/bitcoin.mk b/package/bitcoin/bitcoin.mk
index 5f1684879c..27fc2d5051 100644
--- a/package/bitcoin/bitcoin.mk
+++ b/package/bitcoin/bitcoin.mk
@@ -13,6 +13,9 @@  BITCOIN_CPE_ID_VENDOR = bitcoin
 BITCOIN_CPE_ID_PRODUCT = bitcoin_core
 BITCOIN_DEPENDENCIES = host-pkgconf boost libevent
 BITCOIN_MAKE_ENV = BITCOIN_GENBUILD_NO_GIT=1
+# Bitcoin is using assert(...) extensively with the assumption of it
+# never being a noop at runtime. So we cannot build with NDEBUG.
+BITCOIN_CONF_ENV = CFLAGS="-UNDEBUG" CXXFLAGS="-UNDEBUG"
 BITCOIN_CONF_OPTS = \
 	--disable-bench \
 	--disable-wallet \