Re: [Buildroot] [PATCH] package/Makefile.in: work around buggy filesystem wrt LDFLAGS

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

On Fri, 9 Aug 2024 15:08:22 +0200
<yann.morin@orange.com> wrote:

> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> Some buildsystems (or their use of it by packages) will cause flags in
> LDFLAGS to be re-orderded, or even dropped, causing some incompremsible
> mayhem... For example, gpsd [0] has this in its SConscript [1](typoes
> not mines, for once):
> 
>     591 # scons uses gcc, or clang, to link. Thus LDFLAGS does not serve its
>     592 # traditional function of providing arguments to ln. LDFLAGS set in the
>     593 # environment before running scons get moved into CCFLAGS by scons.
>     594 # LDFLAGS set while running scons get ignored.
>     [--SNIP--]
>     611 for i in ["ARFLAGS",
>     612           "CCFLAGS",
>     613           "CFLAGS",
>     614           "CPPFLAGS",
>     615           "CXXFLAGS",
>     616           "LDFLAGS",
>     617           "LINKFLAGS",
>     618           "SHLINKFLAGS",
>     619           ]:
>     620     if i in os.environ:
>     621         # MergeFlags() puts the options where scons wants them, not
>     622         # where you asked them to go.
>     623         env.MergeFlags(Split(os.getenv(i)))
> 
> So, when LDFLAGS (our TARGET_LDFLAGS) contains "-z text"  (without the
> quotes), that gets turned into a command line like (line-splitted for
> readability):
> 
>     [...]/buildroot/output/host/bin/aarch64_be-buildroot-linux-musl-gcc \
>         -o gpsd-3.25/drivers/driver_rtcm2.os \
>         -c \
>         --sysroot=[...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot \
>         -O3 -g0 \
>         -z \
>         -fPIC -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 \
>         gpsd-3.25/drivers/driver_rtcm2.c
> 
> Notice how there is a lone "-z" without any following keyword.
> 
> This then causes a build failure that looks totally unrelated [2]:
> 
>     In file included from gpsd-3.25/drivers/../include/gpsd.h:36,
>                      from gpsd-3.25/drivers/driver_rtcm2.c:65:
>     gpsd-3.25/drivers/../include/os_compat.h:40:8: error: redefinition of ‘struct timespec’
>        40 | struct timespec {
>           |        ^~~~~~~~
>     In file included from [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/sys/select.h:16,
>                      from gpsd-3.25/drivers/../include/gpsd.h:31:
>     [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/bits/alltypes.h:237:8: note: originally defined here
>       237 | struct timespec { time_t tv_sec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER==4321); long tv_nsec; int :8*(sizeof(time_t)-sizeof(long))*(__BYTE_ORDER!=4321); };
>           |        ^~~~~~~~
>     gpsd-3.25/drivers/../include/os_compat.h:48:5: error: conflicting types for ‘clock_gettime’; have ‘int(clockid_t,  struct timespec *)’ {aka ‘int(int,  struct timespec *)’}
>        48 | int clock_gettime(clockid_t, struct timespec *);
>           |     ^~~~~~~~~~~~~
>     In file included from gpsd-3.25/drivers/../include/gpsd.h:33:
>     [...]/buildroot/output/host/aarch64_be-buildroot-linux-musl/sysroot/usr/include/time.h:104:5: note: previous declaration of ‘clock_gettime’ with type ‘int(clockid_t,  struct timespec *)’ {aka ‘int(int,  struct timespec *)’}
>       104 | int clock_gettime (clockid_t, struct timespec *);
>           |     ^~~~~~~~~~~~~
>     scons: *** [gpsd-3.25/drivers/driver_rtcm2.os] Error 1
>     scons: building terminated because of errors.
>     make[1]: *** [package/pkg-generic.mk:289: [...]/buildroot/output/build/gpsd-3.25/.stamp_built] Error 2
>     make: *** [Makefile:83: _all] Error 2
> 
> Although undocumented, neither in gcc not ld (clang unchecked by lack of
> a clang toolchain here, and by lack of clang knowledge), -z accepts the
> keyword to be snatch-glued onto it, like -zkeyword, rather than be
> spearated with a space. So, use that to pass -ztext.
> 
> Fixes:
>     http://autobuild.buildroot.org/results/c03/c039989947b960ac6af17c87090366abc26dcb6d/
>     http://autobuild.buildroot.net/results/bc35d3e7b0e8c59c776652070650af3c749250ee/
> 
> [0] Our other scons-based package, mongodb, does not build for another
> reason that probably hides the same issue as seen with gpsd.
> 
> [1] As explained in gpsd's SConscript (se above), Scons does play tricks
> with variables:
>     https://scons.org/doc/production/HTML/scons-man.html#f-MergeFlags
>     https://scons.org/doc/production/HTML/scons-man.html#f-ParseFlags
> 
> Quoting:
>     Flag values are translated according to the prefix found, and added
>     to the following construction variables:
>     [...]
>     -Wl,                    LINKFLAGS
>     [...]
>     -                       CCFLAGS
>     [...]
>     Any other strings not associated with options are assumed to be the
>     names of libraries and added to the $LIBS construction variable.
> 
> So in our case, it finds that -z is an unknown option that matches the
> '-' prefix, so it is added to CFLAGS, while 'text' is a string on its
> own, so added to LIBS, and thus it would try to link with -ltext
> (supposedly, because we do not even go that far). Funnily enough, we can
> se that "-Wl," is a known option prefix, that is added to LINKFLAGS (to
> properly be used with gcc or clang, not ld).
> 
> As a consequence, gpsd's buildsystem does drop -ztext from the link
> flags, and only passes it to the compile flags, which brings us back to
> before we banned textrels in a1a2f498d7ec (package/Makefile.in: ban
> textrels on musl toolchains). Fixing gpsd is a task for another,
> separate patch...
> 
> [2] I spent quite some time to look at recent, time-related changes in
> Buildroot, especially due to the infamous time64_t issues... Alas, that
> was not related, and only a git-bisect pinpointed the actual issue. Poor
> polar bears...
> 
> Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
> Cc: J. Neuschäfer <j.neuschaefer@gmx.net>
> ---
>  package/Makefile.in | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot