[1/1] lxc: fix build with static libcap and shared gnutls

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
Related show

Commit Message

Fabrice Fontaine April 7, 2018, 4:22 p.m.
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

Comments

Thomas Petazzoni May 3, 2018, 7:40 p.m. | #1
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
Fabrice Fontaine May 3, 2018, 9:47 p.m. | #2
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">&lt;<a href="mailto:thomas.petazzoni@bootlin.com" target="_blank">thomas.petazzoni@bootlin.com</a>&gt;</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>
&gt; Fixes:<br>
&gt;  - <a href="http://autobuild.buildroot.net/results/b655d6853c25a195df28d91512b3ffb6c654fc90" rel="noreferrer" target="_blank">http://autobuild.buildroot.<wbr>net/results/<wbr>b655d6853c25a195df28d91512b3ff<wbr>b6c654fc90</a><br>
&gt; <br>
&gt; Signed-off-by: Fabrice Fontaine &lt;<a href="mailto:fontaine.fabrice@gmail.com">fontaine.fabrice@gmail.com</a>&gt;<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&#39;t try to build<br>
a statically linked program when BR2_SHARED_LIBS=y. It doesn&#39;t make<br>
sense, because we don&#39;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&#39;m wondering how your patch resolves that really. I guess<br>
it disables GnuTLS support in such a situation, but that isn&#39;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&#39;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&#39;ve not send it as it didn&#39;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>
Thomas Petazzoni May 7, 2018, 3:44 p.m. | #3
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
Thomas Petazzoni May 7, 2018, 3:45 p.m. | #4
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

Patch

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
+