* [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains
@ 2014-10-21 13:21 Vicente Olivert Riera
2014-10-21 20:50 ` Arnout Vandecappelle
0 siblings, 1 reply; 5+ messages in thread
From: Vicente Olivert Riera @ 2014-10-21 13:21 UTC (permalink / raw)
To: buildroot
...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.
Related:
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 <Vincent.Riera@imgtec.com>
---
package/bash/bash.mk | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/package/bash/bash.mk b/package/bash/bash.mk
index 34a3a73..9a13f69 100644
--- a/package/bash/bash.mk
+++ b/package/bash/bash.mk
@@ -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)
--
1.7.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains
2014-10-21 13:21 [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains Vicente Olivert Riera
@ 2014-10-21 20:50 ` Arnout Vandecappelle
2014-10-22 9:17 ` Vicente Olivert Riera
0 siblings, 1 reply; 5+ messages in thread
From: Arnout Vandecappelle @ 2014-10-21 20:50 UTC (permalink / raw)
To: buildroot
On 21/10/14 15:21, Vicente Olivert Riera wrote:
> ...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.
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?
>
> Related:
> 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 <Vincent.Riera@imgtec.com>
> ---
> package/bash/bash.mk | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/package/bash/bash.mk b/package/bash/bash.mk
> index 34a3a73..9a13f69 100644
> --- a/package/bash/bash.mk
> +++ b/package/bash/bash.mk
> @@ -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
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
> endif
>
> # Make /bin/sh -> bash (no other shell, better than busybox shells)
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains
2014-10-21 20:50 ` Arnout Vandecappelle
@ 2014-10-22 9:17 ` Vicente Olivert Riera
2014-10-22 9:35 ` Arnout Vandecappelle
0 siblings, 1 reply; 5+ messages in thread
From: Vicente Olivert Riera @ 2014-10-22 9:17 UTC (permalink / raw)
To: buildroot
Dear Arnout Vandecappelle,
On 10/21/2014 09:50 PM, Arnout Vandecappelle wrote:
> On 21/10/14 15:21, Vicente Olivert Riera wrote:
>> ...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.
>
> 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?
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.
>>
>> Related:
>> 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 <Vincent.Riera@imgtec.com>
>> ---
>> package/bash/bash.mk | 5 ++++-
>> 1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/package/bash/bash.mk b/package/bash/bash.mk
>> index 34a3a73..9a13f69 100644
>> --- a/package/bash/bash.mk
>> +++ b/package/bash/bash.mk
>> @@ -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
>
> It's probably better to also set it to yes explicitly in the else branch.
No problem.
> Also, a short comment would be good.
No problem.
> 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.
>
>
> Regards,
> Arnout
>
>> endif
>>
>> # Make /bin/sh -> bash (no other shell, better than busybox shells)
>>
>
>
Best regards,
--
Vicente Olivert Riera
Graduate Software Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains
2014-10-22 9:17 ` Vicente Olivert Riera
@ 2014-10-22 9:35 ` Arnout Vandecappelle
2014-10-22 9:41 ` Vicente Olivert Riera
0 siblings, 1 reply; 5+ messages in thread
From: Arnout Vandecappelle @ 2014-10-22 9:35 UTC (permalink / raw)
To: buildroot
On 22/10/14 11:17, Vicente Olivert Riera wrote:
> Dear Arnout Vandecappelle,
>
> On 10/21/2014 09:50 PM, Arnout Vandecappelle wrote:
>> 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]
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains
2014-10-22 9:35 ` Arnout Vandecappelle
@ 2014-10-22 9:41 ` Vicente Olivert Riera
0 siblings, 0 replies; 5+ messages in thread
From: Vicente Olivert Riera @ 2014-10-22 9:41 UTC (permalink / raw)
To: buildroot
Dear Arnout Vandecappelle,
On 10/22/2014 10:35 AM, Arnout Vandecappelle wrote:
> On 22/10/14 11:17, Vicente Olivert Riera wrote:
>> Dear Arnout Vandecappelle,
>>
>> On 10/21/2014 09:50 PM, Arnout Vandecappelle wrote:
>>> 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]
>
Done: http://patchwork.ozlabs.org/patch/401979/
Best regards,
--
Vicente Olivert Riera
Graduate Software Engineer, MIPS Processor IP
Imagination Technologies Limited
t: +44 (0)113 2429814
www.imgtec.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-22 9:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-21 13:21 [Buildroot] [PATCH] bash: fix linking for static builds with uClibc toolchains Vicente Olivert Riera
2014-10-21 20:50 ` Arnout Vandecappelle
2014-10-22 9:17 ` Vicente Olivert Riera
2014-10-22 9:35 ` Arnout Vandecappelle
2014-10-22 9:41 ` Vicente Olivert Riera
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox