diff mbox

[RFC,1/1] package/libpthsem: cannot be sanely cross-compiled

Message ID 1438285530-320-1-git-send-email-brendanheading@gmail.com
State Rejected, archived
Headers show

Commit Message

Brendan Heading July 30, 2015, 7:45 p.m. UTC
Fixes the following :
http://autobuild.buildroot.net/results/675df26d7b47e02bad0b22f3e1cb404076a84a91/
http://autobuild.buildroot.net/results/003c4313676cc79c72a9296159ff63dce177f079/
http://autobuild.buildroot.net/results/86bd9fd83278a5468856f4abf700df57749d8e09/
http://autobuild.buildroot.net/results/97205d8fce8d0851208d167fc3fc7ccce40d391a/
http://autobuild.buildroot.net/results/1764c8f7f84fc5bad950fdb41ecadff571ceb36b/
http://autobuild.buildroot.net/results/e733cb648398df7efd7ccae6493af3c92713a9bd/

I'm submitting this as RFC as it seems strange that I've come across this
only now, but I don't see how libpthsem ever cross compiled in its current
form.

During the configure stage, libpthsem attempts to compile and run an
executable to detect capabilities such as which direction the stack grows,
behaviour of setjmp, etc. Obviously these fail to execute on non-Intel
architectures, leading to defaults which cause the build to fail even
on architectures where libpthsem is supposed to work.

I've tested mipsel, sparcv8 and aarch64 here but I expect it will fail
for all the other non-Intel architectures too.

There are flags to force some (but not all) of these settings to certain
fallbacks. It's not clear, without access to each of these architectures,
what the correct default fallback settings should be.

This patch therefore disables libpthsem and its dependents on non-Intel
architectures.

Signed-off-by: Brendan Heading <brendanheading@gmail.com>
---
 package/bcusdk/Config.in    | 3 +++
 package/gnupg2/Config.in    | 3 +++
 package/libpthsem/Config.in | 2 ++
 package/linknx/Config.in    | 3 +++
 4 files changed, 11 insertions(+)

Comments

Yann E. MORIN Aug. 2, 2015, 10:14 p.m. UTC | #1
Brendan, All,

On 2015-07-30 20:45 +0100, Brendan Heading spake thusly:
> Fixes the following :
> http://autobuild.buildroot.net/results/675df26d7b47e02bad0b22f3e1cb404076a84a91/
> http://autobuild.buildroot.net/results/003c4313676cc79c72a9296159ff63dce177f079/
> http://autobuild.buildroot.net/results/86bd9fd83278a5468856f4abf700df57749d8e09/
> http://autobuild.buildroot.net/results/97205d8fce8d0851208d167fc3fc7ccce40d391a/
> http://autobuild.buildroot.net/results/1764c8f7f84fc5bad950fdb41ecadff571ceb36b/
> http://autobuild.buildroot.net/results/e733cb648398df7efd7ccae6493af3c92713a9bd/
> 
> I'm submitting this as RFC as it seems strange that I've come across this
> only now, but I don't see how libpthsem ever cross compiled in its current
> form.

I've tested the first reported build failure... And it builds fine for
me... :-/

Not to say that the result is correct, just that I can't reproduce the
build failure.

> During the configure stage, libpthsem attempts to compile and run an
> executable to detect capabilities such as which direction the stack grows,
> behaviour of setjmp, etc. Obviously these fail to execute on non-Intel
> architectures, leading to defaults which cause the build to fail even
> on architectures where libpthsem is supposed to work.
> 
> I've tested mipsel, sparcv8 and aarch64 here but I expect it will fail
> for all the other non-Intel architectures too.
> 
> There are flags to force some (but not all) of these settings to certain
> fallbacks. It's not clear, without access to each of these architectures,
> what the correct default fallback settings should be.
> 
> This patch therefore disables libpthsem and its dependents on non-Intel
> architectures.

That's definitiely not acceptable that we have to disable it.

All those tests that can not be determined at build time because they
need to run programs should be forced-guessed,with the appropriate ac_cv
variables.

See for example the atk package, which has quite a bunch of those;
    package/atk/atk.mk

Excerpt;

    ATK_CONF_ENV = \
        ac_cv_func_posix_getpwuid_r=yes \
        glib_cv_stack_grows=no \
        glib_cv_uscore=no \
        ac_cv_func_strtod=yes \
        ac_fsusage_space=yes \
        fu_cv_sys_stat_statfs2_bsize=yes \
        ac_cv_func_closedir_void=no \
        [...]

Which should be done in this case, too.

----

For the records, I've diffed the config.log from the first build failure
with the config.log I git on my machine, and there are a few interesting
differences ('-' is mine, '+' is the autobuild failure):

First, it's not using the same awk implementation:

    -configure:2743: found /usr/bin/gawk
    -configure:2754: result: gawk
    +configure:2757: result: no
    +configure:2727: checking for mawk
    +configure:2743: found /usr/bin/mawk
    +configure:2754: result: mawk

Second, running programs fail with different errors; the error on my
machine is probably the dynamic linker /lib/ld.so whinning that it can
not recognise the exectuable (expected, it's PPC, my machine is x86_64),
while the error on the autobuilder is probably becasue the dynamic
linker does not exist (probably because it is not in the same location):

    -./configure: line 1828: ./conftest: cannot execute binary file: Exec format error
    -configure:12578: $? = 126
    -configure: program exited with status 126
    +./configure: line 1828: ./conftest: No such file or directory
    +configure:12578: $? = 127
    +configure: program exited with status 127

Third, and much more interesting, is the mis-detection of the sjlj to
use:

    -configure:14176: result: yes: ssjlj
    +configure:14176: result: yes: sjljlx
    [--SNIP--]
    -configure:15098: result: decision on mctx implementation...  sjlj/ssjlj/sas
    +configure:15098: result: decision on mctx implementation...  sjlj/sjljlx/none
    [--SNIP--]
    -PTH_MCTX_ID='sjlj/ssjlj/sas'
    +PTH_MCTX_ID='sjlj/sjljlx/none'

Ah, and my system has my login shell set to /bin/bash , not /bin/sh .

Now, if you look closely (not too closely, even!), you'll see that those
errors are all occuring on a single autobuilder, the one from Nathaniel:

    http://autobuild.buildroot.org/?reason=libpthsem-2.0.8

So, I guess something from the environment causes the build failures.

Not to say that we should not fix them, just that probably the proposed
fix is not the correct solution...

Regards,
Yann E. MORIN.
Brendan Heading Aug. 2, 2015, 10:39 p.m. UTC | #2
> I've tested the first reported build failure... And it builds fine for
> me... :-/
>
> Not to say that the result is correct, just that I can't reproduce the
> build failure.

Yann,

Thanks for reviewing my patch. I knew it would not fly, so I put it as
RFC to start a discussion on the best approach.

> All those tests that can not be determined at build time because they
> need to run programs should be forced-guessed,with the appropriate ac_cv
> variables.
>
> See for example the atk package, which has quite a bunch of those;
>     package/atk/atk.mk

Noted. I agree that this is a better approach.

> Now, if you look closely (not too closely, even!), you'll see that those
> errors are all occuring on a single autobuilder, the one from Nathaniel:
>
>     http://autobuild.buildroot.org/?reason=libpthsem-2.0.8
>
> So, I guess something from the environment causes the build failures.

Yes, I agree. I found the same problem that you did (issues with
reproducibility). Sometimes you get "Exec format error", others "file
not found" (which happens when there is a dynamic link problem).
Strangely, I could reproduce it on some architectures but not others.
But irrespective of this - the way that it is detecting features
cannot work in a cross-compiled environment. So I didn't waste a lot
of time characterising this in detail.

The big problem is the misdetection of the sjlj as you noted. To fix
this properly we're going to have to go through all the arches and
figure out what the correct value is ..

Brendan
Brendan Heading Aug. 3, 2015, 3:11 p.m. UTC | #3
>> All those tests that can not be determined at build time because they
>> need to run programs should be forced-guessed,with the appropriate ac_cv
>> variables.
>>
>> See for example the atk package, which has quite a bunch of those;
>>     package/atk/atk.mk
>
> Noted. I agree that this is a better approach.

Yann,

[cc Thomas, list]

I thought about this a bit more today.

The state of play, right now, is that we can't be completely sure that
cross-compiling libpthsem will work correctly. We know for sure that
on several architectures it definitely does not work. In other cases
it may appear to compile, by accident, but there's no way to be sure
that it works at run time. Of course this isn't buildroot's fault.

I don't have hardware available to test the right force-guess values,
so I don't think I'm the right person to fix it. Moreover, even with
the right hardware, any bugs caused by using the wrong force-guess
value could be very subtle. Maybe someone else can solve this, but so
far, nobody has, and these packages must have been broken for quite a
long time now (years ?).

So, given that there is no interest in the architectures, and there is
nobody with time to fix the problems properly, isn't there a case for
explicitly disabling libpthsem (and its dependents) for all
cross-compiled builds ? Then, when someone comes along to complain, or
even better proposes a patch, we can selectively re-enable the
configurations that work ?

Just my two cents .. since as things stand right now this package and
related packages are just adding noise in the autobuild logs, and it
might save time for people if the erroneous cases where expressly
disabled, rather than giving the false impression that it works and is
a supportable configuration.

regards

Brendan
Yann E. MORIN Aug. 3, 2015, 4:38 p.m. UTC | #4
Brendan, All,

On 2015-08-03 16:11 +0100, Brendan Heading spake thusly:
> >> All those tests that can not be determined at build time because they
> >> need to run programs should be forced-guessed,with the appropriate ac_cv
> >> variables.
> >>
> >> See for example the atk package, which has quite a bunch of those;
> >>     package/atk/atk.mk
> >
> > Noted. I agree that this is a better approach.
> 
> I thought about this a bit more today.
> 
> The state of play, right now, is that we can't be completely sure that
> cross-compiling libpthsem will work correctly. We know for sure that
> on several architectures it definitely does not work. In other cases
> it may appear to compile, by accident, but there's no way to be sure
> that it works at run time. Of course this isn't buildroot's fault.

What are those archs which libpthsem does *not* support by design?

I.e. is it because there are arch-specific assembly code which is not
ported to some archs? Or because of some requirements (like an MMU or
some such)? Other reasons?

> I don't have hardware available to test the right force-guess values,
> so I don't think I'm the right person to fix it. Moreover, even with
> the right hardware, any bugs caused by using the wrong force-guess
> value could be very subtle. Maybe someone else can solve this, but so
> far, nobody has, and these packages must have been broken for quite a
> long time now (years ?).

Well, the problem can be quite easily solved. Nowadays, virtually all
architectures have the their stack all growing down (or is it up? I
never remember, but AFAIK, all archs in Buildroot are in the same
direction) so we can just skip over the test and force-guess it.

I guess a lot of the other tests can be just answered the same.

Again, pretty much all architectures nowadays are very similar in their
"standard" behaviour anyway. There might be a few exceptions for some
corner-cases tests but overall, hard-coding them would just be a matter
of identifying tho special cases.

However, it does not really matter if you do not have access to the
hardware just to decide the force-guess for those tests. And thos you
don't know or can't find the answer fo, just say so in the commit log.
Some knowledgeable individual will step up and tell you.

> So, given that there is no interest in the architectures, and there is
> nobody with time to fix the problems properly, isn't there a case for
> explicitly disabling libpthsem (and its dependents) for all
> cross-compiled builds ? Then, when someone comes along to complain, or
> even better proposes a patch, we can selectively re-enable the
> configurations that work ?

Well, there are a few packages that need libpthsem, of which gnupg2, so
it would be a shame and quite a loss to have to make without those
packages, especialy gnupg2...

Regards,
Yann E. MORIN.
Thomas Petazzoni Aug. 19, 2015, 11:10 a.m. UTC | #5
Brendan,

On Thu, 30 Jul 2015 20:45:30 +0100, Brendan Heading wrote:
> Fixes the following :
> http://autobuild.buildroot.net/results/675df26d7b47e02bad0b22f3e1cb404076a84a91/
> http://autobuild.buildroot.net/results/003c4313676cc79c72a9296159ff63dce177f079/
> http://autobuild.buildroot.net/results/86bd9fd83278a5468856f4abf700df57749d8e09/
> http://autobuild.buildroot.net/results/97205d8fce8d0851208d167fc3fc7ccce40d391a/
> http://autobuild.buildroot.net/results/1764c8f7f84fc5bad950fdb41ecadff571ceb36b/
> http://autobuild.buildroot.net/results/e733cb648398df7efd7ccae6493af3c92713a9bd/
> 
> I'm submitting this as RFC as it seems strange that I've come across this
> only now, but I don't see how libpthsem ever cross compiled in its current
> form.
> 
> During the configure stage, libpthsem attempts to compile and run an
> executable to detect capabilities such as which direction the stack grows,
> behaviour of setjmp, etc. Obviously these fail to execute on non-Intel
> architectures, leading to defaults which cause the build to fail even
> on architectures where libpthsem is supposed to work.
> 
> I've tested mipsel, sparcv8 and aarch64 here but I expect it will fail
> for all the other non-Intel architectures too.
> 
> There are flags to force some (but not all) of these settings to certain
> fallbacks. It's not clear, without access to each of these architectures,
> what the correct default fallback settings should be.
> 
> This patch therefore disables libpthsem and its dependents on non-Intel
> architectures.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>

I don't think this is the right solution, as Yann noted. I've posted a
different patch, which is shorter and allows libpthsem to be built on
all architectures, after doing a more in-depth analysis of the
build problem.

See:

  http://patchwork.ozlabs.org/patch/508707/

Consequently, I've marked your patch as Rejected in patchwork.

Best regards,

Thomas
diff mbox

Patch

diff --git a/package/bcusdk/Config.in b/package/bcusdk/Config.in
index 72deef1..a712941 100644
--- a/package/bcusdk/Config.in
+++ b/package/bcusdk/Config.in
@@ -2,6 +2,8 @@  config BR2_PACKAGE_BCUSDK
 	bool "bcusdk"
 	depends on BR2_USE_MMU # libpthsem
 	depends on BR2_INSTALL_LIBSTDCPP
+	# libpthsem cannot be sensibly cross compiled
+	depends on BR2_i386 || BR2_x86_64
 	select BR2_PACKAGE_LIBPTHSEM
 	select BR2_PACKAGE_ARGP_STANDALONE if BR2_TOOLCHAIN_USES_UCLIBC
 	help
@@ -22,4 +24,5 @@  config BR2_PACKAGE_BCUSDK
 
 comment "bcusdk needs a toolchain w/ C++"
 	depends on BR2_USE_MMU
+	depends on BR2_i386 || BR2_x86_64
 	depends on !BR2_INSTALL_LIBSTDCPP
diff --git a/package/gnupg2/Config.in b/package/gnupg2/Config.in
index e246fd7..0a9dd30 100644
--- a/package/gnupg2/Config.in
+++ b/package/gnupg2/Config.in
@@ -1,6 +1,7 @@ 
 comment "gnupg2 needs a toolchain w/ dynamic library"
 	depends on BR2_USE_MMU
 	depends on BR2_STATIC_LIBS
+	depends on BR2_i386 || BR2_x86_64
 
 config BR2_PACKAGE_GNUPG2
 	bool "gnupg2"
@@ -13,6 +14,8 @@  config BR2_PACKAGE_GNUPG2
 	select BR2_PACKAGE_LIBPTHSEM_COMPAT
 	select BR2_PACKAGE_LIBICONV if !BR2_ENABLE_LOCALE
 	depends on BR2_USE_MMU # libassuan
+	# libpthsem cannot be sensibly cross compiled
+	depends on BR2_i386 || BR2_x86_64
 	depends on !BR2_STATIC_LIBS
 	help
 	  GnuPG is the GNU project's complete and free implementation
diff --git a/package/libpthsem/Config.in b/package/libpthsem/Config.in
index 3219de1..6ebc173 100644
--- a/package/libpthsem/Config.in
+++ b/package/libpthsem/Config.in
@@ -1,6 +1,8 @@ 
 config BR2_PACKAGE_LIBPTHSEM
 	bool "libpthsem"
 	depends on BR2_USE_MMU # fork()
+	# libpthsem cannot be sensibly cross compiled
+	depends on BR2_i386 || BR2_x86_64
 	help
 	  GNU pth is a user mode multi threading library. pthsem is an extend
 	  version, with support for semaphores added.
diff --git a/package/linknx/Config.in b/package/linknx/Config.in
index 9982f31..c2ea713 100644
--- a/package/linknx/Config.in
+++ b/package/linknx/Config.in
@@ -4,6 +4,8 @@  config BR2_PACKAGE_LINKNX
 	select BR2_PACKAGE_ARGP_STANDALONE if BR2_TOOLCHAIN_USES_UCLIBC
 	depends on BR2_INSTALL_LIBSTDCPP
 	depends on BR2_USE_MMU # libpthsem
+	# libpthsem cannot be sensibly cross compiled
+	depends on BR2_i386 || BR2_x86_64
 	help
 	  Linknx is an automation platform providing high level functionalities
 	  to EIB/KNX installation.
@@ -12,4 +14,5 @@  config BR2_PACKAGE_LINKNX
 
 comment "linknx needs a toolchain w/ C++"
 	depends on BR2_USE_MMU
+	depends on BR2_i386 || BR2_x86_64
 	depends on !BR2_INSTALL_LIBSTDCPP