bash: fix linking for static builds with uClibc toolchains

PATCH New Vicente Olivert Riera 2 participants 5 messages Oct 21, 2014 – Oct 22, 2014

lore

Share & tools

lore

Vicente Olivert Riera

author

2014-10-21 13:21 UTC #0 lore raw

PATCH 1/1 bash: fix linking for static builds with uClibc toolchains

...and also use configure options instead of environment variables.

bash fails to link for static builds with uClibc toolchains due to getenv redefinitions. This is caused because bash is unable to check if getenv is already defined when cross-compiling, so it defaults to 'yes':

configure:14438: WARNING: cannot check getenv redefinition if cross compiling -- defaulting to yes

We can avoid this redefinition by adding bash_cv_getenv_redef=no to the configure options.

At the same time, we use configure options instead of environment variables because there is no need to pass those options to the configure script as environment variables. Doing it in this way we avoid the risk of environment variables leaking into places we don't expect.

	http://lists.gnu.org/archive/html/bug-bash/2012-03/msg00052.html

Fixes:

	http://autobuild.buildroot.net/results/a20/a2007e6dbcfe53e7cd837ae642869ee26376826a/

Signed-off-by Vicente Olivert Riera <[email protected]>

---
 package/bash/bash.mk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

package/bash/bash.mk +4 -1

index 34a3a73..9a13f69 100644
@@ -13,7 +13,7 @@ BASH_CONF_OPTS = --with-installed-readline
 BASH_LICENSE = GPLv3+
 BASH_LICENSE_FILES = COPYING
 
-BASH_CONF_ENV += \
+BASH_CONF_OPTS += \
 	ac_cv_rl_prefix="$(STAGING_DIR)" \
 	ac_cv_rl_version="$(READLINE_VERSION)" \
 	bash_cv_job_control_missing=present \

@@ -28,6 +28,9 @@ BASH_MAKE = $(MAKE1)
 # The static build needs some trickery
 ifeq ($(BR2_PREFER_STATIC_LIB),y)
 BASH_CONF_OPTS += --enable-static-link --without-bash-malloc
+ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)
+BASH_CONF_OPTS += bash_cv_getenv_redef=no
+endif
 endif
 
 # Make /bin/sh -> bash (no other shell, better than busybox shells)

Signature · 1 line

1.7.1

Arnout Vandecappelle

2014-10-21 20:50 UTC #1 lore raw

PATCH 1/1 bash: fix linking for static builds with uClibc toolchains

Quoted 9 lines · Vicente Olivert Riera

...and also use configure options instead of environment variables.
bash fails to link for static builds with uClibc toolchains due to getenv redefinitions. This is caused because bash is unable to check if getenv is already defined when cross-compiling, so it defaults to 'yes':
configure:14438: WARNING: cannot check getenv redefinition if cross compiling -- defaulting to yes

Quoted 8 lines

We can avoid this redefinition by adding bash_cv_getenv_redef=no to the configure options.
At the same time, we use configure options instead of environment variables because there is no need to pass those options to the configure script as environment variables. Doing it in this way we avoid the risk of environment variables leaking into places we don't expect.

That change from _ENV to _OPTS should definitely be a separate patch.
Especially because I don't see the reason to do it. The environment variables will leak into exactly one place: the configure script. And that script will just export all assignments passed on the command line into its environment. So the effect is exactly the same.

If you do make this change, then it should probably be done for all packages,
which means it should probably be done in pkg-autotools.mk.

So you have a concrete example of where the _ENV causes trouble?

Quoted 26 lines

	http://lists.gnu.org/archive/html/bug-bash/2012-03/msg00052.html

Fixes:

	http://autobuild.buildroot.net/results/a20/a2007e6dbcfe53e7cd837ae642869ee26376826a/

Signed-off-by Vicente Olivert Riera <[email protected]>

---
 package/bash/bash.mk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

package/bash/bash.mk +1 -1

index 34a3a73..9a13f69 100644
@@ -13,7 +13,7 @@ BASH_CONF_OPTS = --with-installed-readline
 BASH_LICENSE = GPLv3+
 BASH_LICENSE_FILES = COPYING
 
-BASH_CONF_ENV += \
+BASH_CONF_OPTS += \
 	ac_cv_rl_prefix="$(STAGING_DIR)" \
 	ac_cv_rl_version="$(READLINE_VERSION)" \
 	bash_cv_job_control_missing=present \

package/bash/bash.mk BASH_MAKE = $(MAKE1) · line 33

Quoted 6 lines

package/bash/bash.mk +3 -0

@@ -28,6 +28,9 @@ BASH_MAKE = $(MAKE1)
 # The static build needs some trickery
 ifeq ($(BR2_PREFER_STATIC_LIB),y)
 BASH_CONF_OPTS += --enable-static-link --without-bash-malloc
+ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)
+BASH_CONF_OPTS += bash_cv_getenv_redef=no
+endif

It's probably better to also set it to yes explicitly in the else branch.

Also, a short comment would be good.

Also, does musl allow redefining getenv?

Regards,
Arnout

Quoted 4 lines

package/bash/bash.mk +0 -0

 endif
 
 # Make /bin/sh -> bash (no other shell, better than busybox shells)

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

Vicente Olivert Riera

author

2014-10-22 09:17 UTC #2 lore raw

PATCH 1/1 bash: fix linking for static builds with uClibc toolchains

Dear Arnout Vandecappelle,

Quoted 25 lines · Arnout Vandecappelle

Quoted 9 lines · Vicente Olivert Riera
...and also use configure options instead of environment variables.
bash fails to link for static builds with uClibc toolchains due to getenv redefinitions. This is caused because bash is unable to check if getenv is already defined when cross-compiling, so it defaults to 'yes':
configure:14438: WARNING: cannot check getenv redefinition if cross compiling -- defaulting to yes
Quoted 8 lines
We can avoid this redefinition by adding bash_cv_getenv_redef=no to the configure options.
At the same time, we use configure options instead of environment variables because there is no need to pass those options to the configure script as environment variables. Doing it in this way we avoid the risk of environment variables leaking into places we don't expect.
That change from _ENV to _OPTS should definitely be a separate patch.
Especially because I don't see the reason to do it. The environment variables will leak into exactly one place: the configure script. And that script will just export all assignments passed on the command line into its environment. So the effect is exactly the same.

Quoted 4 lines

If you do make this change, then it should probably be done for all packages,
which means it should probably be done in pkg-autotools.mk.
So you have a concrete example of where the _ENV causes trouble?

No, I haven't. I just prefer using configure options rather than environment variables. Anyway..., no problem, I will send another patch without that change.

Quoted 20 lines

	http://lists.gnu.org/archive/html/bug-bash/2012-03/msg00052.html

Fixes:

	http://autobuild.buildroot.net/results/a20/a2007e6dbcfe53e7cd837ae642869ee26376826a/

Signed-off-by Vicente Olivert Riera <[email protected]>

---
  package/bash/bash.mk |    5 ++++-
  1 files changed, 4 insertions(+), 1 deletions(-)

package/bash/bash.mk +0 -0

index 34a3a73..9a13f69 100644
@@ -13,7 +13,7 @@ BASH_CONF_OPTS = --with-installed-readline
  BASH_LICENSE = GPLv3+
  BASH_LICENSE_FILES = COPYING

Quoted 14 lines

package/bash/bash.mk BASH_MAKE = $(MAKE1) · line 33

Quoted 12 lines

package/bash/bash.mk +4 -1

-BASH_CONF_ENV += \
+BASH_CONF_OPTS += \
  	ac_cv_rl_prefix="$(STAGING_DIR)" \
  	ac_cv_rl_version="$(READLINE_VERSION)" \
  	bash_cv_job_control_missing=present \

@@ -28,6 +28,9 @@ BASH_MAKE = $(MAKE1)
  # The static build needs some trickery
  ifeq ($(BR2_PREFER_STATIC_LIB),y)
  BASH_CONF_OPTS += --enable-static-link --without-bash-malloc
+ifeq ($(BR2_TOOLCHAIN_USES_UCLIBC),y)
+BASH_CONF_OPTS += bash_cv_getenv_redef=no
+endif

It's probably better to also set it to yes explicitly in the else branch.

No problem.

Quoted 1 line

Also, a short comment would be good.

No problem.

Quoted 1 line

Also, does musl allow redefining getenv?

Well, I did a test with a static build using the musl toolchain and it worked fine. So, yes, it seems that musl is ok with the getenv redefinition.

Quoted 11 lines

Regards,
Arnout
Quoted 4 lines
package/bash/bash.mk +0 -0
  endif
 
  # Make /bin/sh -> bash (no other shell, better than busybox shells)

Best regards,

Signature · 5 lines

Vicente Olivert Riera
Graduate Software Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com

Arnout Vandecappelle

2014-10-22 09:35 UTC #3 lore raw

PATCH 1/1 bash: fix linking for static builds with uClibc toolchains

Quoted 7 lines · Vicente Olivert Riera

Dear Arnout Vandecappelle,
Quoted 1 line · Arnout Vandecappelle
Also, does musl allow redefining getenv?
Well, I did a test with a static build using the musl toolchain and it worked fine. So, yes, it seems that musl is ok with the getenv redefinition.

Perhaps it's a good idea to mention this analysis explicitly in comments.
Something like:

# bash wants to redefine the getenv() function. To check whether this is
# possible, AC_TRY_RUN is used which is not possible in cross-compilation.
# On uClibc, redefining getenv is not possible; on glibc and musl it is.

Regards,
Arnout

[snip]

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

Vicente Olivert Riera

author

2014-10-22 09:41 UTC #4 lore raw

PATCH 1/1 bash: fix linking for static builds with uClibc toolchains

Dear Arnout Vandecappelle,

Quoted 17 lines · Arnout Vandecappelle

Quoted 7 lines · Vicente Olivert Riera
Dear Arnout Vandecappelle,
Quoted 1 line · Arnout Vandecappelle
Also, does musl allow redefining getenv?
Well, I did a test with a static build using the musl toolchain and it worked fine. So, yes, it seems that musl is ok with the getenv redefinition.
Perhaps it's a good idea to mention this analysis explicitly in comments.
Something like:
# bash wants to redefine the getenv() function. To check whether this is
# possible, AC_TRY_RUN is used which is not possible in cross-compilation.
# On uClibc, redefining getenv is not possible; on glibc and musl it is.

Quoted 5 lines

Regards,
Arnout
[snip]

Done: http://patchwork.ozlabs.org/patch/401979/

Best regards,

Signature · 5 lines

Vicente Olivert Riera
Graduate Software Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com