| Message ID | 20180407162234.12848-1-fontaine.fabrice@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] lxc: fix build with static libcap and shared gnutls | expand |
Hello Fabrice, Sorry for the slow feedback on this one. On Sat, 7 Apr 2018 18:22:34 +0200, Fabrice Fontaine wrote: > Fixes: > - http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ffb6c654fc90 > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> So, this happens because when BR2_SHARED_LIBS=y, gnutls installs only a shared library, while libcap installs both a static library and a shared library. But really, the crux of the problem is that LXC shouldn't try to build a statically linked program when BR2_SHARED_LIBS=y. It doesn't make sense, because we don't guarantee to provide static libraries when BR2_SHARED_LIBS=y: some packages do build static libraries unconditionally, but for examples the autotools packages that obey to --disable-static will not build/install static libraries when BR2_SHARED_LIBS=y. And in fact, I'm wondering how your patch resolves that really. I guess it disables GnuTLS support in such a situation, but that isn't really the right fix IMO. I would rather disable the build of the static version of the LXC program. Do you think you could have a look into this ? Thomas
Dear Thomas, 2018-05-03 21:40 GMT+02:00 Thomas Petazzoni <thomas.petazzoni@bootlin.com>: > Hello Fabrice, > > Sorry for the slow feedback on this one. > > On Sat, 7 Apr 2018 18:22:34 +0200, Fabrice Fontaine wrote: > > Fixes: > > - http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ff > b6c654fc90 > > > > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> > > So, this happens because when BR2_SHARED_LIBS=y, gnutls installs only a > shared library, while libcap installs both a static library and a > shared library. > > But really, the crux of the problem is that LXC shouldn't try to build > a statically linked program when BR2_SHARED_LIBS=y. It doesn't make > sense, because we don't guarantee to provide static libraries when > BR2_SHARED_LIBS=y: some packages do build static libraries > unconditionally, but for examples the autotools packages that obey to > --disable-static will not build/install static libraries when > BR2_SHARED_LIBS=y. > > And in fact, I'm wondering how your patch resolves that really. I guess > it disables GnuTLS support in such a situation, but that isn't really > the right fix IMO. I would rather disable the build of the static > version of the LXC program. > > Do you think you could have a look into this ? > The issue was more subtle, init_lxc is a simple binary that LXC wants to be statically linked if it founds a static version of the only library that it needs: libcap. Indeed, this binary doesn't need gnutls or any other dependencies such as selinux or seccomp that are needed for liblxc. However, because AC_CHECK_LIB was added with its default behavior of adding gnutls to the global LIBS variable, linking of init_lxc was failing because gnutls was not static. So, I think my patch is the right fix and it has been merged upstream: https://github.com/lxc/lxc/commit/49bc916b1daa79cffe38fae32059bcdd985c8c8e. Moreover, the issue was raised because when BR2_SHARED_LIBS is set, the libcap package installs the static and the shared version of the library, I made a patch to change this behavior but I've not send it as it didn't fix the root cause of the issue. > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com > Best Regards, Fabrice <div dir="ltr">Dear Thomas,<br><div class="gmail_extra"><br><div class="gmail_quote">2018-05-03 21:40 GMT+02:00 Thomas Petazzoni <span dir="ltr"><<a href="mailto:thomas.petazzoni@bootlin.com" target="_blank">thomas.petazzoni@bootlin.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello Fabrice,<br> <br> Sorry for the slow feedback on this one.<br> <span class="gmail-"><br> On Sat, 7 Apr 2018 18:22:34 +0200, Fabrice Fontaine wrote:<br> > Fixes:<br> > - <a href="http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ffb6c654fc90" rel="noreferrer" target="_blank">http://autobuild.buildroot.<wbr>net/results/<wbr>b655d6853c25a195df28d91512b3ff<wbr>b6c654fc90</a><br> > <br> > Signed-off-by: Fabrice Fontaine <<a href="mailto:fontaine.fabrice@gmail.com">fontaine.fabrice@gmail.com</a>><br> <br> </span>So, this happens because when BR2_SHARED_LIBS=y, gnutls installs only a<br> shared library, while libcap installs both a static library and a<br> shared library.<br> <br> But really, the crux of the problem is that LXC shouldn't try to build<br> a statically linked program when BR2_SHARED_LIBS=y. It doesn't make<br> sense, because we don't guarantee to provide static libraries when<br> BR2_SHARED_LIBS=y: some packages do build static libraries<br> unconditionally, but for examples the autotools packages that obey to<br> --disable-static will not build/install static libraries when<br> BR2_SHARED_LIBS=y.<br> <br> And in fact, I'm wondering how your patch resolves that really. I guess<br> it disables GnuTLS support in such a situation, but that isn't really<br> the right fix IMO. I would rather disable the build of the static<br> version of the LXC program.<br> <br> Do you think you could have a look into this ?<br></blockquote><div>The issue was more subtle, init_lxc is a simple binary that LXC wants to be statically linked if it founds a static version of the only library that it needs: libcap. Indeed, this binary doesn't need gnutls or any other dependencies such as selinux or seccomp that are needed for liblxc. However, because AC_CHECK_LIB was added with its default behavior of adding gnutls to the global LIBS variable, linking of init_lxc was failing because gnutls was not static.<br></div><div>So, I think my patch is the right fix and it has been merged upstream: <a href="https://github.com/lxc/lxc/commit/49bc916b1daa79cffe38fae32059bcdd985c8c8e">https://github.com/lxc/lxc/commit/49bc916b1daa79cffe38fae32059bcdd985c8c8e</a>.<br></div><div>Moreover, the issue was raised because when BR2_SHARED_LIBS is set, the libcap package installs the static and the shared version of the library, I made a patch to change this behavior but I've not send it as it didn't fix the root cause of the issue. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <span class="gmail-HOEnZb"><font color="#888888"><br> Thomas<br> -- <br> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)<br> Embedded Linux and Kernel engineering<br> <a href="https://bootlin.com" rel="noreferrer" target="_blank">https://bootlin.com</a><br> </font></span></blockquote></div>Best Regards,<br><br></div><div class="gmail_extra">Fabrice<br></div></div>
Hello, On Thu, 3 May 2018 23:47:17 +0200, Fabrice Fontaine wrote: > > Do you think you could have a look into this ? > > > The issue was more subtle, init_lxc is a simple binary that LXC wants to be > statically linked if it founds a static version of the only library that it > needs: libcap. Indeed, this binary doesn't need gnutls or any other > dependencies such as selinux or seccomp that are needed for liblxc. > However, because AC_CHECK_LIB was added with its default behavior of adding > gnutls to the global LIBS variable, linking of init_lxc was failing because > gnutls was not static. > So, I think my patch is the right fix and it has been merged upstream: > https://github.com/lxc/lxc/commit/49bc916b1daa79cffe38fae32059bcdd985c8c8e. > Moreover, the issue was raised because when BR2_SHARED_LIBS is set, the > libcap package installs the static and the shared version of the library, I > made a patch to change this behavior but I've not send it as it didn't fix > the root cause of the issue. OK, I understand better. It makes sense now thanks! Thomas
Hello, On Sat, 7 Apr 2018 18:22:34 +0200, Fabrice Fontaine wrote: > diff --git a/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch b/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch > new file mode 100644 > index 0000000000..9a0dcbe55a > --- /dev/null > +++ b/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch There was already a 0001 patch, so I renumbered this one to 0002. I've also added the upstream status. And applied. Thanks! Best regards, Thomas
diff --git a/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch b/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch new file mode 100644 index 0000000000..9a0dcbe55a --- /dev/null +++ b/package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch @@ -0,0 +1,72 @@ +From 49bc916b1daa79cffe38fae32059bcdd985c8c8e Mon Sep 17 00:00:00 2001 +From: Fabrice Fontaine <fontaine.fabrice@gmail.com> +Date: Sat, 7 Apr 2018 15:48:46 +0200 +Subject: [PATCH] Fix compilation with static libcap and shared gnutls + +Commit c06ed219c47098f34485d408410b6ecc94a40877 has broken +compilation with a static libcap and a shared gnutls. +This results in a build failure on init_lxc_static if gnutls is +a shared library as init_lxc_static is built with -all-static option +(see src/lxc/Makefile.am) and AC_CHECK_LIB adds gnutls to LIBS. + +This commit fix the issue by removing default behavior of AC_CHECK_LIB +and handling manually GNUTLS_LIBS and HAVE_LIBGNUTLS + +Fixes: + - http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ffb6c654fc90 + +Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> +--- + configure.ac | 2 +- + src/lxc/Makefile.am | 8 ++++++-- + 2 files changed, 7 insertions(+), 3 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 50c99836..2467bb54 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -263,7 +263,7 @@ AM_CONDITIONAL([ENABLE_GNUTLS], [test "x$enable_gnutls" = "xyes"]) + + AM_COND_IF([ENABLE_GNUTLS], + [AC_CHECK_HEADER([gnutls/gnutls.h],[],[AC_MSG_ERROR([You must install the GnuTLS development package in order to compile lxc])]) +- AC_CHECK_LIB([gnutls], [gnutls_hash_fast],[],[AC_MSG_ERROR([You must install the GnuTLS development package in order to compile lxc])]) ++ AC_CHECK_LIB([gnutls], [gnutls_hash_fast],[true],[AC_MSG_ERROR([You must install the GnuTLS development package in order to compile lxc])]) + AC_SUBST([GNUTLS_LIBS], [-lgnutls])]) + + # SELinux +diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am +index c8d76836..0662d83d 100644 +--- a/src/lxc/Makefile.am ++++ b/src/lxc/Makefile.am +@@ -175,6 +175,10 @@ if ENABLE_APPARMOR + AM_CFLAGS += -DHAVE_APPARMOR + endif + ++if ENABLE_GNUTLS ++AM_CFLAGS += -DHAVE_LIBGNUTLS ++endif ++ + if ENABLE_SELINUX + AM_CFLAGS += -DHAVE_SELINUX + endif +@@ -196,7 +200,7 @@ liblxc_la_LDFLAGS = \ + -Wl,-soname,liblxc.so.$(firstword $(subst ., ,@LXC_ABI@)) \ + -version-info @LXC_ABI_MAJOR@ + +-liblxc_la_LIBADD = $(CAP_LIBS) $(SELINUX_LIBS) $(SECCOMP_LIBS) ++liblxc_la_LIBADD = $(CAP_LIBS) $(GNUTLS_LIBS) $(SELINUX_LIBS) $(SECCOMP_LIBS) + + bin_SCRIPTS= + +@@ -243,7 +247,7 @@ AM_LDFLAGS = -Wl,-E + if ENABLE_RPATH + AM_LDFLAGS += -Wl,-rpath -Wl,$(libdir) + endif +-LDADD=liblxc.la @CAP_LIBS@ @SELINUX_LIBS@ @SECCOMP_LIBS@ ++LDADD=liblxc.la @CAP_LIBS@ @GNUTLS_LIBS@ @SELINUX_LIBS@ @SECCOMP_LIBS@ + + if ENABLE_TOOLS + lxc_attach_SOURCES = tools/lxc_attach.c tools/arguments.c tools/tool_utils.c +-- +2.14.1 +
Fixes: - http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ffb6c654fc90 Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> --- ...tion-with-static-libcap-and-shared-gnutls.patch | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 package/lxc/0001-Fix-compilation-with-static-libcap-and-shared-gnutls.patch