[Libosinfo] [libosinfo PATCH v2 1/2] osinfo: Create an intermediate convenience library

[Fabiano Fidêncio]

On Wed, 2018-11-21 at 09:08 +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 21, 2018 at 09:44:47AM +0100, Christophe Fergeau wrote:
> > Ok, 2 more comments actually,
> > 
> > On Tue, Nov 20, 2018 at 03:11:23PM +0100, Fabiano Fidêncio wrote:
> > > Let's create libosinfo-impl.la which is nothing else than
> > > libosinfo-1.0.la without stripping out its non-public symbols.
> > > 
> > > libosinfo-impl.la can be used to link directly against our tests
> > > (as
> > > those may use private functions that we do *not* want to expose),
> > > while
> > > the "official" one (libosinfo-1.0.la) will remain the same.
> > > 
> > > Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> > > ---
> > >  osinfo/Makefile.am | 48 ++++++++++++++++++++++++++------------
> > > --------
> > >  1 file changed, 27 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/osinfo/Makefile.am b/osinfo/Makefile.am
> > > index 058653a..60b601f 100644
> > > --- a/osinfo/Makefile.am
> > > +++ b/osinfo/Makefile.am
> > > @@ -24,9 +24,9 @@ endif
> > >  pkgconfigdir = $(libdir)/pkgconfig
> > >  pkgconfig_DATA = libosinfo-1.0.pc
> > >  
> > > -lib_LTLIBRARIES = libosinfo-1.0.la
> > > +noinst_LTLIBRARIES = libosinfo-impl.la
> > 
> > I would keep libosinfo-1.0.la near the top as that is the most
> > interesting output from that Makefile.am (ie what actually gets
> > installed).
> > 
> > > -nodist_libosinfo_1_0_la_SOURCES =	\
> > > +nodist_libosinfo_impl_la_SOURCES =	\
> > >    osinfo_enum_types.c			\
> > >    $(NULL)
> > >  
> > > +lib_LTLIBRARIES = libosinfo-1.0.la
> > > +
> > > +libosinfo_1_0_la_SOURCES =
> > > +
> > > +libosinfo_1_0_la_LIBADD = libosinfo-impl.la
> > > +libosinfo_1_0_la_LIBADD += $(libosinfo_impl_la_LIBADD)
> > > +
> > > +libosinfo_1_0_la_LDFLAGS = \
> > > +	$(COVERAGE_LDFLAGS) \
> > > +	$(VERSION_SCRIPT_FLAGS)$(LIBOSINFO_VERSION_FILE) \
> > > +        -version-info $(LIBOSINFO_VERSION_INFO) \
> > > +	$(NO_UNDEFINED_FLAGS)
> > > +
> > > +libosinfo_1_0_la_DEPENDENCIES = libosinfo.syms libosinfo-impl.la
> > 
> > Here, I would expect libtool to be able to infer the libosinfo-
> > impl.la
> > dependency from 'libosinfo_1_0_la_LIBADD = libosinfo-impl.la' above
> 
> IIRC there is a problem - if you specify _DEPENDENCIES = ... at all,
> as needed for the libosinfo.syms file, then I believe it discards the
> default dependancy on libosinfo-impl.la

That's exactly what happens. If you specify _DEPENDENCIES = ..., but
doesn't specify libosinfo-impl.la, libosinfo-1.0.la will be built first
and then it would just break.

I've fixed the other comments raised up by Christophe and will push the
patches after another run of the tests.

Best Regards,
-- 
Fabiano Fidêncio