diff mbox series

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

Message ID 20210525122750.5022-8-patrickdepinguin@gmail.com
State Changes Requested
Headers show
Series Introduce BR2_ENABLE_RUNTIME_DEBUG | expand

Commit Message

Thomas De Schampheleire May 25, 2021, 12:27 p.m. UTC 
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(-)

Comments

Arnout Vandecappelle May 25, 2021, 9:17 p.m. UTC | #1 
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
>
Thomas De Schampheleire May 28, 2021, 10:11 a.m. UTC | #2 
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
diff mbox series

Patch

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