Re: [libvirt] [PATCH 2/2] makefile: fix build on systems where gnutls is not in /usr/include

On 23.05.2016 17:39, Michal Privoznik wrote: > On 23.05.2016 17:06, Pavel Hrdina wrote: >> On Mon, May 23, 2016 at 04:42:43PM +0200, Michal Privoznik wrote: >>> On 23.05.2016 10:18, Pavel Hrdina wrote: >>>> We need to append GNUTLS_CFLAGS while building utils because virtcrypto >>>> is using it. This fixes build on freebsd where gnutuls is in >>>> /usr/local/include. >>>> >>>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >>>> --- >>>> src/Makefile.am | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/src/Makefile.am b/src/Makefile.am >>>> index 314f6df..12b66c2 100644 >>>> --- a/src/Makefile.am >>>> +++ b/src/Makefile.am >>>> @@ -1106,7 +1106,7 @@ libvirt_util_la_SOURCES = \ >>>> libvirt_util_la_CFLAGS = $(CAPNG_CFLAGS) $(YAJL_CFLAGS) $(LIBNL_CFLAGS) \ >>>> $(AM_CFLAGS) $(AUDIT_CFLAGS) $(DEVMAPPER_CFLAGS) \ >>>> $(DBUS_CFLAGS) $(LDEXP_LIBM) $(NUMACTL_CFLAGS) \ >>>> - $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) \ >>>> + $(SYSTEMD_DAEMON_CFLAGS) $(POLKIT_CFLAGS) $(GNUTLS_CFLAGS) \ >>>> -I$(srcdir)/conf >>>> libvirt_util_la_LIBADD = $(CAPNG_LIBS) $(YAJL_LIBS) $(LIBNL_LIBS) \ >>>> $(THREAD_LIBS) $(AUDIT_LIBS) $(DEVMAPPER_LIBS) \ >>>> >>> >>> I think we need to update _LIBADD too so that it contains GNUTLS_LIBS >>> because now libvirt_uitls lib is depending on gnutls lib. This, however, >> >> Yes, I've realized that too few minutes ago. >> >>> make commandtest fail as gnutls upon its init opens /dev/urandom and >>> keeps it opened for the rest of binary runtime which makes commandhelper >>> report the FD and in turn commandtest sad. >>> I'm not sure I have proper solution for that - other than what you did >>> in 1/2. >> >> We could wrap all gnutls functions and for tests mock those wrapper functions. >> > > That won't fly. Problem is, this is in gnutls_init which is ran when the library is linked. It's not that we are calling something that leaves FD open. > > #0 open64 () at ../sysdeps/unix/syscall-template.S:84 > #1 0x00007ffff5808a7a in open (__oflag=0, __path=0x7ffff581c4c2 "/dev/urandom") at /usr/include/bits/fcntl2.h:53 > #2 _rnd_system_entropy_init () at /var/tmp/portage/net-libs/gnutls-3.3.17.1/work/gnutls-3.3.17.1/lib/nettle/rnd-common.c:216 > #3 0x00007ffff580903c in wrap_nettle_rnd_init (ctx=<optimized out>) at /var/tmp/portage/net-libs/gnutls-3.3.17.1/work/gnutls-3.3.17.1/lib/nettle/rnd.c:230 > #4 0x00007ffff577fff9 in _gnutls_rnd_init () at /var/tmp/portage/net-libs/gnutls-3.3.17.1/work/gnutls-3.3.17.1/lib/random.c:49 > #5 0x00007ffff5774a26 in gnutls_global_init () at /var/tmp/portage/net-libs/gnutls-3.3.17.1/work/gnutls-3.3.17.1/lib/gnutls_global.c:272 > #6 0x00007ffff57569c9 in lib_init () at /var/tmp/portage/net-libs/gnutls-3.3.17.1/work/gnutls-3.3.17.1/lib/gnutls_global.c:434 > #7 0x00007ffff7dea26a in call_init (l=<optimized out>, argc=argc@entry=1, argv=argv@entry=0x7fffffffdee8, env=env@entry=0x7fffffffdef8) at dl-init.c:72 > #8 0x00007ffff7dea37b in call_init (env=0x7fffffffdef8, argv=0x7fffffffdee8, argc=1, l=<optimized out>) at dl-init.c:30 > #9 _dl_init (main_map=0x7ffff7ffe148, argc=1, argv=0x7fffffffdee8, env=0x7fffffffdef8) at dl-init.c:120 > #10 0x00007ffff7ddbc7a in _dl_start_user () from /lib64/ld-linux-x86-64.so.2 > #11 0x0000000000000001 in ?? () > #12 0x00007fffffffe2f4 in ?? () > #13 0x0000000000000000 in ?? () > > But making commandtest dependent on WITH_GNULTS just doesn't sound right. > > Michal > Okay, so I gave this some thinking and I don't see a better solution for now than the one you've proposed. Moreover, we are getting closer and closer to the release therefore we should fix this in whatever way we can and leave the good looking solution for the future (if there ever will be one). ACK series.