| Message ID | 1490128516-31437-1-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Accepted |
| Commit | ba72d71d79f623ae2a7c62a6fa32a77b951666ac |
| Headers | show |
Thomas, All, On 2017-03-21 21:35 +0100, Thomas Petazzoni spake thusly: > /usr/share normally should not contain binaries executable for the > target platform. However, it might contain ELF binaries for other > platforms, such as firmware files installed by Qemu or > pru-software-support. > > Instead of special-casing each package, let's simply ignore /usr/share. > > Fixes: > > http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/ > (pru-software-support) > > http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/ > (qemu) > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN. > --- > support/scripts/check-bin-arch | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch > index 2c619ad..be2f371 100755 > --- a/support/scripts/check-bin-arch > +++ b/support/scripts/check-bin-arch > @@ -27,6 +27,13 @@ for f in ${pkg_files} ; do > continue > fi > > + # Skip files in /usr/share, several packages (qemu, > + # pru-software-support) legitimately install ELF binaries that > + # are not for the target architecture > + if [[ "${f}" =~ ^\./usr/share/.* ]]; then > + continue > + fi > + > # Get architecture using readelf. We pipe through 'head -1' so > # that when the file is a static library (.a), we only take > # into account the architecture of the first object file. > -- > 2.7.4 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > /usr/share normally should not contain binaries executable for the > target platform. However, it might contain ELF binaries for other > platforms, such as firmware files installed by Qemu or > pru-software-support. > Instead of special-casing each package, let's simply ignore /usr/share. > Fixes: > http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/ > (pru-software-support) > http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/ > (qemu) > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Committed, thanks.
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > /usr/share normally should not contain binaries executable for the > target platform. However, it might contain ELF binaries for other > platforms, such as firmware files installed by Qemu or > pru-software-support. > Instead of special-casing each package, let's simply ignore /usr/share. > Fixes: > http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/ > (pru-software-support) > http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/ > (qemu) Hmm, this doesn't seem to work? http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log I wonder if we shouldn't turn the tests around, E.G. instead of searching for elf files with a machine different from target, search for a files with machine == host. If we do this then we need to disable the test when host == target (but the current tests aren't really helpful either for such setups). This can naturally still fail if somebody adds a package (E.G. in br2-external) that installs a i386/x86-64 binary. Presumably this could happen if you want to include a PC application to inside the firmware (E.G. downloadable through the web interface and used for controlling the firmware or similar), but it doesn't get caught up in all these build issues about various other firmware files or slightly different machine strings (sparc / sparcv9, arcompat / arcv2)..
Hello, On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote: > http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log > http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log > > I wonder if we shouldn't turn the tests around, E.G. instead of > searching for elf files with a machine different from target, search for > a files with machine == host. Indeed, that's an option. We would need to have only the readelf machine string for the most common build architectures (x86, x86-64), and if Buildroot is used on a different architecture, we simply don't do the check. > If we do this then we need to disable the test when host == target (but > the current tests aren't really helpful either for such setups). Correct. > This can naturally still fail if somebody adds a package (E.G. in > br2-external) that installs a i386/x86-64 binary. Presumably this could > happen if you want to include a PC application to inside the firmware > (E.G. downloadable through the web interface and used for controlling > the firmware or similar), but it doesn't get caught up in all these > build issues about various other firmware files or slightly different > machine strings (sparc / sparcv9, arcompat / arcv2).. I must say I still like the fact that we detect sparc vs. sparcv9, arcompat vs arcv2. I.e why do we have some binaries that have a different machine number than most of the binaries being produced? But I agree it's of limited usefulness, so if you want me to change the mechanism by inverting the logic, I can cook some patches. Best regards, Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Hello, > On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote: >> http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log >> http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log >> >> I wonder if we shouldn't turn the tests around, E.G. instead of >> searching for elf files with a machine different from target, search for >> a files with machine == host. > Indeed, that's an option. We would need to have only the readelf > machine string for the most common build architectures (x86, x86-64), > and if Buildroot is used on a different architecture, we simply don't > do the check. Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC) to detect the machine string. >> This can naturally still fail if somebody adds a package (E.G. in >> br2-external) that installs a i386/x86-64 binary. Presumably this could >> happen if you want to include a PC application to inside the firmware >> (E.G. downloadable through the web interface and used for controlling >> the firmware or similar), but it doesn't get caught up in all these >> build issues about various other firmware files or slightly different >> machine strings (sparc / sparcv9, arcompat / arcv2).. > I must say I still like the fact that we detect sparc vs. sparcv9, > arcompat vs arcv2. I.e why do we have some binaries that have a > different machine number than most of the binaries being produced? Sorry, I don't know enough details about Sparc and Arc. It indeed seems strange. > But I agree it's of limited usefulness, so if you want me to change the > mechanism by inverting the logic, I can cook some patches. Well, just like for all other BR features I want to ensure that the complexity/gain relation is OK. If we need to add more and more exceptions and perhaps more complicated/fuzzy matching to handle these machine string variantions then I think it would make sense to invert the test to simplify - But lets see how it goes.
Hello, On Wed, 22 Mar 2017 22:43:48 +0100, Peter Korsgaard wrote: > Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC) > to detect the machine string. Indeed. > > I must say I still like the fact that we detect sparc vs. sparcv9, > > arcompat vs arcv2. I.e why do we have some binaries that have a > > different machine number than most of the binaries being produced? > > Sorry, I don't know enough details about Sparc and Arc. It indeed seems > strange. The ARC is resolved. I'll look into the SPARC one. Best regards, Thomas
On 22-03-17 22:43, Peter Korsgaard wrote: >>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > > > Hello, > > On Wed, 22 Mar 2017 09:24:56 +0100, Peter Korsgaard wrote: > > >> http://autobuild.buildroot.net/results/4e2/4e27559827f3ed75a12f13bd595998bf661b2aaf/build-end.log > >> http://autobuild.buildroot.net/results/99e/99e4ed21116c721faf9f1d0349f312b357d333ee/build-end.log Looks like the regexes in the script are all wrong: they match against ./something, but the file names are /something. Ah, that's Yann's doing: when moving the pkg_files to the while read loop, he sneaked in the removal of the initial . - without mentioning it in the commit log. Tsk tsk. > >> I wonder if we shouldn't turn the tests around, E.G. instead of > >> searching for elf files with a machine different from target, search for > >> a files with machine == host. I don't agree. We don't only want to detect host-target confusion, we also want to detect wrong compilation, e.g. missing -EB. By the way, we shouldn't be afraid to exclude files and directories. Take for example /usr/share: some packages may actually install binaries there (e.g. gstreamer installs a bash-completion helper there). However, all packages also install something somewhere else, and it's very likely that if it does something wrong, it's wrong everywhere. BTW, we should also manually check if there are no problems with the boot loaders, which don't get triggered in the autobuilders... > > Indeed, that's an option. We would need to have only the readelf > > machine string for the most common build architectures (x86, x86-64), > > and if Buildroot is used on a different architecture, we simply don't > > do the check. > > Exactly, or alternatively we run readelf on a host utility (E.G. HOSTCC) > to detect the machine string. > > > >> This can naturally still fail if somebody adds a package (E.G. in > >> br2-external) that installs a i386/x86-64 binary. Presumably this could > >> happen if you want to include a PC application to inside the firmware > >> (E.G. downloadable through the web interface and used for controlling > >> the firmware or similar), but it doesn't get caught up in all these > >> build issues about various other firmware files or slightly different > >> machine strings (sparc / sparcv9, arcompat / arcv2).. > > > I must say I still like the fact that we detect sparc vs. sparcv9, > > arcompat vs arcv2. I.e why do we have some binaries that have a > > different machine number than most of the binaries being produced? > > Sorry, I don't know enough details about Sparc and Arc. It indeed seems > strange. > > > > But I agree it's of limited usefulness, so if you want me to change the > > mechanism by inverting the logic, I can cook some patches. > > Well, just like for all other BR features I want to ensure that the > complexity/gain relation is OK. If we need to add more and more > exceptions and perhaps more complicated/fuzzy matching to handle these > machine string variantions then I think it would make sense to invert > the test to simplify - But lets see how it goes. Indeed, if it becomes too complicated we have to find a simpler approach. Regards, Arnout
Hello, On Thu, 23 Mar 2017 00:03:57 +0100, Arnout Vandecappelle wrote: > > >> I wonder if we shouldn't turn the tests around, E.G. instead of > > >> searching for elf files with a machine different from target, search for > > >> a files with machine == host. > > I don't agree. We don't only want to detect host-target confusion, we also want > to detect wrong compilation, e.g. missing -EB. This was exactly the case of the gst-ffmpeg package, for which I've sent a fix. Everything was built for Sparc, but two libraries were built for a slightly different version of Sparc, which was actually incorrect. Since for now, autobuilders issue caused by this seems to have all been handled, I don't think there's much need to find a different solution. Let's give it some more time and see if we need to change our mind. Best regards, Thomas
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes: > Hello, > On Thu, 23 Mar 2017 00:03:57 +0100, Arnout Vandecappelle wrote: >> > >> I wonder if we shouldn't turn the tests around, E.G. instead of >> > >> searching for elf files with a machine different from target, search for >> > >> a files with machine == host. >> >> I don't agree. We don't only want to detect host-target confusion, we also want >> to detect wrong compilation, e.g. missing -EB. > This was exactly the case of the gst-ffmpeg package, for which I've > sent a fix. Everything was built for Sparc, but two libraries were > built for a slightly different version of Sparc, which was actually > incorrect. > Since for now, autobuilders issue caused by this seems to have all been > handled, I don't think there's much need to find a different solution. > Let's give it some more time and see if we need to change our mind. Ok, sounds good to me. Thanks!
diff --git a/support/scripts/check-bin-arch b/support/scripts/check-bin-arch index 2c619ad..be2f371 100755 --- a/support/scripts/check-bin-arch +++ b/support/scripts/check-bin-arch @@ -27,6 +27,13 @@ for f in ${pkg_files} ; do continue fi + # Skip files in /usr/share, several packages (qemu, + # pru-software-support) legitimately install ELF binaries that + # are not for the target architecture + if [[ "${f}" =~ ^\./usr/share/.* ]]; then + continue + fi + # Get architecture using readelf. We pipe through 'head -1' so # that when the file is a static library (.a), we only take # into account the architecture of the first object file.
/usr/share normally should not contain binaries executable for the target platform. However, it might contain ELF binaries for other platforms, such as firmware files installed by Qemu or pru-software-support. Instead of special-casing each package, let's simply ignore /usr/share. Fixes: http://autobuild.buildroot.net/results/6f3fea9f6adaef1573fbb0dd6903b5d99e470610/ (pru-software-support) http://autobuild.buildroot.net/results/fe8892bc22a03299fc41e30bfea5e42166838f88/ (qemu) Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- support/scripts/check-bin-arch | 7 +++++++ 1 file changed, 7 insertions(+)