[prev in list] [next in list] [prev in thread] [next in thread] 

List:       libvir-list
Subject:    Re: [libvirt] [PATCH] build: fix linking libqemutestdriver with LTO enabled
From:       Andrea Bolognani <abologna () redhat ! com>
Date:       2019-05-30 16:00:54
Message-ID: d128670c08fecfa327b43061d9d36495aa7e16da.camel () redhat ! com
[Download RAW message or body]

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 :)

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[prev in list] [next in list] [prev in thread] [next in thread]