Re: [libvirt] [PATCH] build: fix linking libqemutestdriver with LTO enabled

On Thu, 2019-05-30 at 16:56 +0200, Michal Privoznik wrote: > On 5/30/19 4:44 PM, Jim Fehlig wrote: >> On 5/30/19 3:08 AM, Michal Privoznik wrote: >>> On 5/29/19 7:44 PM, Jim Fehlig wrote: >>>> -libqemutestdriver_la_LIBADD = $(qemu_LDADDS) >>>> +libqemutestdriver_la_LIBADD = $(qemu_LDADDS) $(GNULIB_LIBS) >>>> qemucpumock_la_SOURCES = \ >>>> qemucpumock.c testutilshostcpus.h >>> >>> ACK and safe for freeze to this hunk. Alternatively, we might go with >>> $(LDADDS) which includes $(GNULIB_LIBS). >> >> Do you have a preference? LDADDS includes some other things which AFAIK >> are not needed. > > Well, other test libs use LDADDS and I'd say that LTO doesn't link > anything that's not needed. But maybe I'm mistaken. So, no, I don't have > any preference. I think we don't really care about potentially overlinking when it comes to test programs, so there's no need to have the kind of granularity your patch implements. Actually I'd go one further and adopt what Xen tests are doing: there's an explicit libxl_LDADDS += $(LDADDS) and then most tests include at least $(libxl_LDADDS) in their _(LD|LIB)ADDs, whereas most QEMU tests need to use _(LD|LIB)ADD = $(qemu_LDADDS) $(LDADDS) My suggestion would be to copy that approach, have qemu_LDADDS += $(LDADDS) and then drop the extra $(LDADDS) from QEMU tests, which will not only fix your linkin problem but also clean up Makefile.am pretty nicely :)