| Message ID | 1426093401-23108-1-git-send-email-johan.oudinet@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Dear Johan Oudinet, On Wed, 11 Mar 2015 18:03:21 +0100, Johan Oudinet wrote: > Adding this flag when BR2_ENABLE_DEBUG is activated make several > packages to produce binaries that do not work as expected (e.g., dhcp, > lame, nano). Moreover, the help message of BR2_ENABLE_DEBUG does not > say it is adding this flag. It is supposed to build packages with > debugging symbols enabled. So, let it do that only. > > Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com> I am personally in favor of this change, so thanks for bringing it up. > -ifeq ($(BR2_ENABLE_DEBUG),y) > -ENABLE_DEBUG := --enable-debug > -else > +ifneq ($(BR2_ENABLE_DEBUG),y) > ENABLE_DEBUG := --disable-debug > endif So if we have BR2_ENABLE_DEBUG enabled, then we don't pass --disable-debug. And when BR2_ENABLE_DEBUG is disabled, we're passing it. I'm not sure to understand the logic here. Shouldn't we simply unconditionally pass --disable-debug, or not pass anything at all? Also, a number of packages had workarounds in their specific .mk file to avoid --enable-debug. It would be good to get rid of such workarounds as well. Thanks, Thomas
Thomas, All, On Wed, Mar 11, 2015 at 9:42 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: >> -ifeq ($(BR2_ENABLE_DEBUG),y) >> -ENABLE_DEBUG := --enable-debug >> -else >> +ifneq ($(BR2_ENABLE_DEBUG),y) >> ENABLE_DEBUG := --disable-debug >> endif > > So if we have BR2_ENABLE_DEBUG enabled, then we don't pass > --disable-debug. And when BR2_ENABLE_DEBUG is disabled, we're passing > it. I'm not sure to understand the logic here. > > Shouldn't we simply unconditionally pass --disable-debug, or not pass > anything at all? I felt bad in passing --disable-debug when BR2_ENABLE_DEBUG is set, and I didn't want to remove it otherwise (in case a package expects it). Still, the result is not logic. Actually, there is no good reason for a package to depends on a --disable-debug flag to produce a version without debugging information (see for example http://stackoverflow.com/a/4680578/1448926) I'm going to remove --disable-debug too. > > Also, a number of packages had workarounds in their specific .mk file > to avoid --enable-debug. It would be good to get rid of such > workarounds as well. Indeed, I'm going to get rid of such workarounds and send another version for this patch. Thanks for the quick review.
diff --git a/package/Makefile.in b/package/Makefile.in index 803b162..2995222 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -387,9 +387,7 @@ ifneq ($(BR2_INSTALL_LIBSTDCPP),y) TARGET_CONFIGURE_OPTS += CXX=false endif -ifeq ($(BR2_ENABLE_DEBUG),y) -ENABLE_DEBUG := --enable-debug -else +ifneq ($(BR2_ENABLE_DEBUG),y) ENABLE_DEBUG := --disable-debug endif
Adding this flag when BR2_ENABLE_DEBUG is activated make several packages to produce binaries that do not work as expected (e.g., dhcp, lame, nano). Moreover, the help message of BR2_ENABLE_DEBUG does not say it is adding this flag. It is supposed to build packages with debugging symbols enabled. So, let it do that only. Signed-off-by: Johan Oudinet <johan.oudinet@gmail.com> --- package/Makefile.in | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)