| Message ID | 20220416211323.3200669-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] package/pkgconf: fix BR2_SHARED_STATIC_LIBS build | expand |
On 16/04/2022 23:13, Fabrice Fontaine wrote: > Add --static when calling pkg-config with BR2_SHARED_STATIC_LIBS to > avoid the following build failure if a package (e.g. dash or zabbix) > decide to use the static library of of its dependency instead of the > shared library (e.g. edit or openssl) resulting in the following build > failures: > > /home/autobuild/autobuild/instance-3/output-1/host/lib/gcc/powerpc64-buildroot-linux-gnu/10.3.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: /home/autobuild/autobuild/instance-3/output-1/host/bin/../powerpc64-buildroot-linux-gnu/sysroot/usr/lib/libedit.a(terminal.o): in function `terminal_tputs': > terminal.c:(.text+0x1d4): undefined reference to `tputs' > > /nvmedata/autobuild/instance-28/output-1/host/lib/gcc/powerpc64le-buildroot-linux-gnu/10.3.0/../../../../powerpc64le-buildroot-linux-gnu/bin/ld: /nvmedata/autobuild/instance-28/output-1/host/powerpc64le-buildroot-linux-gnu/sysroot/usr/lib/libcrypto.a(dso_dlfcn.o): in function `dlfcn_globallookup': > dso_dlfcn.c:(.text+0x28): warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking > > Fixes: > - http://autobuild.buildroot.org/results/2032d6b1233ce5c79a0c9421052ab1b9184c5b89 > - http://autobuild.buildroot.org/results/b0e1bd19f0612a0e90d89ad8fe9e294f57871f6b > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > --- > package/pkgconf/pkgconf.mk | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk > index 5d65f69c10..c40c8b9433 100644 > --- a/package/pkgconf/pkgconf.mk > +++ b/package/pkgconf/pkgconf.mk > @@ -35,10 +35,10 @@ endef > PKGCONF_POST_INSTALL_TARGET_HOOKS += PKGCONF_LINK_PKGCONFIG > HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_INSTALL_WRAPPER > > -ifeq ($(BR2_STATIC_LIBS),y) > -HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC > -else > +ifeq ($(BR2_SHARED_LIBS),y) > HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_SHARED > +else > +HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC Err, so this would mean that we almost always link statically in the SHARED_STATIC case? I don't think that that's what we want, really... If a package wants to link statically and is using pkg-config, why is not calling pkg-config with --static? Something seems to be really off here... Regards, Arnout > endif > > $(eval $(autotools-package))
Le sam. 23 avr. 2022 à 16:40, Arnout Vandecappelle <arnout@mind.be> a écrit : > > > > On 16/04/2022 23:13, Fabrice Fontaine wrote: > > Add --static when calling pkg-config with BR2_SHARED_STATIC_LIBS to > > avoid the following build failure if a package (e.g. dash or zabbix) > > decide to use the static library of of its dependency instead of the > > shared library (e.g. edit or openssl) resulting in the following build > > failures: > > > > /home/autobuild/autobuild/instance-3/output-1/host/lib/gcc/powerpc64-buildroot-linux-gnu/10.3.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: /home/autobuild/autobuild/instance-3/output-1/host/bin/../powerpc64-buildroot-linux-gnu/sysroot/usr/lib/libedit.a(terminal.o): in function `terminal_tputs': > > terminal.c:(.text+0x1d4): undefined reference to `tputs' > > > > /nvmedata/autobuild/instance-28/output-1/host/lib/gcc/powerpc64le-buildroot-linux-gnu/10.3.0/../../../../powerpc64le-buildroot-linux-gnu/bin/ld: /nvmedata/autobuild/instance-28/output-1/host/powerpc64le-buildroot-linux-gnu/sysroot/usr/lib/libcrypto.a(dso_dlfcn.o): in function `dlfcn_globallookup': > > dso_dlfcn.c:(.text+0x28): warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking > > > > Fixes: > > - http://autobuild.buildroot.org/results/2032d6b1233ce5c79a0c9421052ab1b9184c5b89 > > - http://autobuild.buildroot.org/results/b0e1bd19f0612a0e90d89ad8fe9e294f57871f6b > > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > > --- > > package/pkgconf/pkgconf.mk | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk > > index 5d65f69c10..c40c8b9433 100644 > > --- a/package/pkgconf/pkgconf.mk > > +++ b/package/pkgconf/pkgconf.mk > > @@ -35,10 +35,10 @@ endef > > PKGCONF_POST_INSTALL_TARGET_HOOKS += PKGCONF_LINK_PKGCONFIG > > HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_INSTALL_WRAPPER > > > > -ifeq ($(BR2_STATIC_LIBS),y) > > -HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC > > -else > > +ifeq ($(BR2_SHARED_LIBS),y) > > HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_SHARED > > +else > > +HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC > > Err, so this would mean that we almost always link statically in the > SHARED_STATIC case? I don't think that that's what we want, really... > > If a package wants to link statically and is using pkg-config, why is not > calling pkg-config with --static? Something seems to be really off here... To my knowledge most packages are not adding --static when calling pkg-config. --static is added by buildroot through HOST_PKGCONF_POST_INSTALL_HOOKS. dash fails to build because -static is passed to gcc. -static is passed to gcc because package/Makefile.in is setting --enable-static (and --enable-shared): /home/fabrice/buildroot/output/host/bin/powerpc64-buildroot-linux-gnu-gcc -Wall -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -O1 -g0 -D_FORTIFY_SOURCE=1 -static -Wl,--fatal-warnings -o dash alias.o arith_yacc.o arith_yylex.o cd.o error.o eval.o exec.o expand.o histedit.o input.o jobs.o mail.o main.o memalloc.o miscbltin.o mystring.o options.o parser.o redir.o show.o trap.o output.o bltin/printf.o system.o bltin/test.o bltin/times.o var.o builtins.o init.o nodes.o signames.o syntax.o -ledit /home/fabrice/buildroot/output/host/lib/gcc/powerpc64-buildroot-linux-gnu/10.3.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: /home/fabrice/buildroot/output/host/powerpc64-buildroot-linux-gnu/sysroot/usr/lib/../lib64/libedit.a(terminal.o): in function `terminal_tputs': terminal.c:(.text+0x1d4): undefined reference to `tputs' dash doesn't not handle --enable-shared. It adds -static to gcc call depending on --{en,dis}able-static. So, the 'onlyy' other option would be to add DASH_CONF_OPTS += --disable-static to override what is set by package/Makefile.in I can send a v2 if you think that this solution is better. However, it'll have to be applied to dash and zabbix but perhaps to a large number of "application" packages ... > > Regards, > Arnout > > > > endif > > > > $(eval $(autotools-package)) Best Regards, Fabrice
On 23/04/2022 18:49, Fabrice Fontaine wrote: > Le sam. 23 avr. 2022 à 16:40, Arnout Vandecappelle <arnout@mind.be> a écrit : >> >> >> >> On 16/04/2022 23:13, Fabrice Fontaine wrote: >>> Add --static when calling pkg-config with BR2_SHARED_STATIC_LIBS to >>> avoid the following build failure if a package (e.g. dash or zabbix) >>> decide to use the static library of of its dependency instead of the >>> shared library (e.g. edit or openssl) resulting in the following build >>> failures: [snip] >>> -ifeq ($(BR2_STATIC_LIBS),y) >>> -HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC >>> -else >>> +ifeq ($(BR2_SHARED_LIBS),y) >>> HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_SHARED >>> +else >>> +HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC >> >> Err, so this would mean that we almost always link statically in the >> SHARED_STATIC case? I don't think that that's what we want, really... >> >> If a package wants to link statically and is using pkg-config, why is not >> calling pkg-config with --static? Something seems to be really off here... > > To my knowledge most packages are not adding --static when calling pkg-config. > --static is added by buildroot through HOST_PKGCONF_POST_INSTALL_HOOKS. > > dash fails to build because -static is passed to gcc. > -static is passed to gcc because package/Makefile.in is setting > --enable-static (and --enable-shared): > > /home/fabrice/buildroot/output/host/bin/powerpc64-buildroot-linux-gnu-gcc > -Wall -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 > -O1 -g0 -D_FORTIFY_SOURCE=1 -static -Wl,--fatal-warnings -o dash > alias.o arith_yacc.o arith_yylex.o cd.o error.o eval.o exec.o expand.o > histedit.o input.o jobs.o mail.o main.o memalloc.o miscbltin.o > mystring.o options.o parser.o redir.o show.o trap.o output.o > bltin/printf.o system.o bltin/test.o bltin/times.o var.o builtins.o > init.o nodes.o signames.o syntax.o -ledit > /home/fabrice/buildroot/output/host/lib/gcc/powerpc64-buildroot-linux-gnu/10.3.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: > /home/fabrice/buildroot/output/host/powerpc64-buildroot-linux-gnu/sysroot/usr/lib/../lib64/libedit.a(terminal.o): > in function `terminal_tputs': > terminal.c:(.text+0x1d4): undefined reference to `tputs' > > dash doesn't not handle --enable-shared. > It adds -static to gcc call depending on --{en,dis}able-static. > > So, the 'onlyy' other option would be to add > DASH_CONF_OPTS += --disable-static > to override what is set by package/Makefile.in This is what was done for dropbear 4 years ago in commit c9922a4d2fc79e. > I can send a v2 if you think that this solution is better. > However, it'll have to be applied to dash and zabbix but perhaps to a > large number of "application" packages ... Very good point. I think perhaps we should simply remove BR2_SHARED_STATIC completely. The idea is to build both static and shared libraries, but link with shared libraries by default. However, it turns out not to be working: cmake and meson packages don't support it, for all the non-C languages it's not relevant, and even for autotools packages it doesn't work consistently, as shown by these issues. So I think this option is just creating problems for us without added value. I've put a bunch of other Buildroot contributors in Cc to weigh in in this question. But for me, it is creating problems without adding much value. Regards, Arnout
Arnout, All, On 2022-04-24 17:03 +0200, Arnout Vandecappelle spake thusly: > On 23/04/2022 18:49, Fabrice Fontaine wrote: > >Le sam. 23 avr. 2022 à 16:40, Arnout Vandecappelle <arnout@mind.be> a écrit : > >>On 16/04/2022 23:13, Fabrice Fontaine wrote: > >>>Add --static when calling pkg-config with BR2_SHARED_STATIC_LIBS to > >>>avoid the following build failure if a package (e.g. dash or zabbix) > >>>decide to use the static library of of its dependency instead of the > >>>shared library (e.g. edit or openssl) resulting in the following build > >>>failures: > [snip] > >>>-ifeq ($(BR2_STATIC_LIBS),y) > >>>-HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC > >>>-else > >>>+ifeq ($(BR2_SHARED_LIBS),y) > >>> HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_SHARED > >>>+else > >>>+HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC > >> Err, so this would mean that we almost always link statically in the > >>SHARED_STATIC case? I don't think that that's what we want, really... Agreed: in the shared+static case, we want to link dynamically by default. > >> If a package wants to link statically and is using pkg-config, why is not > >>calling pkg-config with --static? Something seems to be really off here... > >To my knowledge most packages are not adding --static when calling pkg-config. > >--static is added by buildroot through HOST_PKGCONF_POST_INSTALL_HOOKS. > >dash fails to build because -static is passed to gcc. > >-static is passed to gcc because package/Makefile.in is setting > >--enable-static (and --enable-shared): [--SNIP--] > >dash doesn't not handle --enable-shared. > >It adds -static to gcc call depending on --{en,dis}able-static. > > > >So, the 'onlyy' other option would be to add > >DASH_CONF_OPTS += --disable-static > >to override what is set by package/Makefile.in > This is what was done for dropbear 4 years ago in commit c9922a4d2fc79e. That was not exactly the same reason. dropbear misuses --enable-static. --enable-{static,shared} is meant for building libraries, not for linking an executable statically or not [0]. [0] https://www.gnu.org/software/libtool/manual/libtool.html#LT_005fINIT Still, I think in that case, and unless the package really needs a static link, we should fix it with ad-hoc warts, like the one for dropbear. So, we should work around the issue in dash and zabbix*. [*] now that we have fixed the a and z packages, what is there left to fix? ;-) > >I can send a v2 if you think that this solution is better. > >However, it'll have to be applied to dash and zabbix but perhaps to a > >large number of "application" packages ... > Very good point. I am not sure there would be so many packages that misbehave in that situation, in fact... > I think perhaps we should simply remove BR2_SHARED_STATIC completely. The > idea is to build both static and shared libraries, but link with shared > libraries by default. However, it turns out not to be working: cmake and > meson packages don't support it, for all the non-C languages it's not > relevant, and even for autotools packages it doesn't work consistently, as > shown by these issues. So I think this option is just creating problems for > us without added value. > > I've put a bunch of other Buildroot contributors in Cc to weigh in in this > question. But for me, it is creating problems without adding much value. I have always been a bit confused on how BR2_SHARED_STATIC_LIBS was supposed to work, to be honest... Sure, it meant we wanted to _build_ both the static and shared libs. But how were we going to tell packages whether they were supposed to link staticially or not? Surely we do not want a per-package option... Then we're left with deciding at the package's .mk level, by hard-coding some heuristic to decide. In upstream, we have no such case where we'd want a package to be statically linked even in the presence of shared libraries (maybe the the exception being busybox, for those who want to cheaply build an initrd before pivoting in the final rootfs, but that can be done with a config fragment or a custom config file in any case). This leaves out-of-tree packages. In that case, there can be tons of reasons to prefer a static link even in the presence of shared libraries. So, the real quesiton is whether we want to support that use-case or not. If we do, then we need to keep BR2_SHARED_STATIC_LIBS and fix those packages that misbehave in its presence, like done for dropbear. Regards, Yann E. MORIN.
On 24/04/2022 18:33, Yann E. MORIN wrote: [snip] > I have always been a bit confused on how BR2_SHARED_STATIC_LIBS was > supposed to work, to be honest... Sure, it meant we wanted to _build_ > both the static and shared libs. The problem is that it only actually works for autotools packages. They are still dominant (more than 1/3 of all packages), but diminishing fast and many "headline" packages are no longer autotools. I don't really like to have an option that doesn't even work for most packages. Regards, Arnout > But how were we going to tell packages > whether they were supposed to link staticially or not? > > Surely we do not want a per-package option... Then we're left with > deciding at the package's .mk level, by hard-coding some heuristic to > decide. > > In upstream, we have no such case where we'd want a package to be > statically linked even in the presence of shared libraries (maybe the > the exception being busybox, for those who want to cheaply build an > initrd before pivoting in the final rootfs, but that can be done with > a config fragment or a custom config file in any case). > > This leaves out-of-tree packages. > > In that case, there can be tons of reasons to prefer a static link even > in the presence of shared libraries. > > So, the real quesiton is whether we want to support that use-case or > not. > > If we do, then we need to keep BR2_SHARED_STATIC_LIBS and fix those > packages that misbehave in its presence, like done for dropbear. > > Regards, > Yann E. MORIN. > >
> On 24/04/2022 18:33, Yann E. MORIN wrote: > [snip] > > I have always been a bit confused on how BR2_SHARED_STATIC_LIBS was > > supposed to work, to be honest... Sure, it meant we wanted to _build_ > > both the static and shared libs. > The problem is that it only actually works for autotools packages. They are > still dominant (more than 1/3 of all packages), but diminishing fast and > many "headline" packages are no longer autotools. I don't really like to > have an option that doesn't even work for most packages. That IMHO justifies removing this option. Kind regards, Petr > Regards, > Arnout > > But how were we going to tell packages > > whether they were supposed to link staticially or not? > > Surely we do not want a per-package option... Then we're left with > > deciding at the package's .mk level, by hard-coding some heuristic to > > decide. > > In upstream, we have no such case where we'd want a package to be > > statically linked even in the presence of shared libraries (maybe the > > the exception being busybox, for those who want to cheaply build an > > initrd before pivoting in the final rootfs, but that can be done with > > a config fragment or a custom config file in any case). > > This leaves out-of-tree packages. > > In that case, there can be tons of reasons to prefer a static link even > > in the presence of shared libraries. > > So, the real quesiton is whether we want to support that use-case or > > not. > > If we do, then we need to keep BR2_SHARED_STATIC_LIBS and fix those > > packages that misbehave in its presence, like done for dropbear. > > Regards, > > Yann E. MORIN.
diff --git a/package/pkgconf/pkgconf.mk b/package/pkgconf/pkgconf.mk index 5d65f69c10..c40c8b9433 100644 --- a/package/pkgconf/pkgconf.mk +++ b/package/pkgconf/pkgconf.mk @@ -35,10 +35,10 @@ endef PKGCONF_POST_INSTALL_TARGET_HOOKS += PKGCONF_LINK_PKGCONFIG HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_INSTALL_WRAPPER -ifeq ($(BR2_STATIC_LIBS),y) -HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC -else +ifeq ($(BR2_SHARED_LIBS),y) HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_SHARED +else +HOST_PKGCONF_POST_INSTALL_HOOKS += HOST_PKGCONF_STATIC endif $(eval $(autotools-package))
Add --static when calling pkg-config with BR2_SHARED_STATIC_LIBS to avoid the following build failure if a package (e.g. dash or zabbix) decide to use the static library of of its dependency instead of the shared library (e.g. edit or openssl) resulting in the following build failures: /home/autobuild/autobuild/instance-3/output-1/host/lib/gcc/powerpc64-buildroot-linux-gnu/10.3.0/../../../../powerpc64-buildroot-linux-gnu/bin/ld: /home/autobuild/autobuild/instance-3/output-1/host/bin/../powerpc64-buildroot-linux-gnu/sysroot/usr/lib/libedit.a(terminal.o): in function `terminal_tputs': terminal.c:(.text+0x1d4): undefined reference to `tputs' /nvmedata/autobuild/instance-28/output-1/host/lib/gcc/powerpc64le-buildroot-linux-gnu/10.3.0/../../../../powerpc64le-buildroot-linux-gnu/bin/ld: /nvmedata/autobuild/instance-28/output-1/host/powerpc64le-buildroot-linux-gnu/sysroot/usr/lib/libcrypto.a(dso_dlfcn.o): in function `dlfcn_globallookup': dso_dlfcn.c:(.text+0x28): warning: Using 'dlopen' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking Fixes: - http://autobuild.buildroot.org/results/2032d6b1233ce5c79a0c9421052ab1b9184c5b89 - http://autobuild.buildroot.org/results/b0e1bd19f0612a0e90d89ad8fe9e294f57871f6b Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> --- package/pkgconf/pkgconf.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)