Re: [PATCH] extras: New test/build infrastructure

[Florian Weimer <fweimer at redhat dot com>]

On 11/25/2016 07:44 PM, Zack Weinberg wrote:
> On Fri, Nov 25, 2016 at 12:45 PM, Florian Weimer <fweimer@redhat.com> wrote:
> > On 11/25/2016 05:14 PM, Zack Weinberg wrote:
> > > Can I ask why the new directory is called "extras"?  That makes it sound
> > > like a home for extra features that we want to provide but not in the
> > > core C library.  Something more obviously internal-use and
> > > build/test-related would be better, I think.
> >
> > I plan to use bits of it for fixing localedef bugs and contributing Fedora
> > changes upstream.  Those bits would then end up in installed binaries.
> >
> > The immediate need is for testing only and generic test support code
> > (container setup, a fake DNS server implementation, and so on).
>
> So maybe "support/" or "build-test-support/"?

support/ works for me.  Any objects to that?

> > > > +libextras-static-only-routines := $(libextras-routines)
> > > > +# Only build one variant of the library.
> > > > +libextras-inhibit-o := .os
> > > > +ifeq ($(build-shared),yes)
> > > > +libextras-inhibit-o += .o
> > > > +endif
> > >
> > > This doesn't look right if the goal is to build only the .a version of
> > > the library.
> >
> > Could you clarify what worries you?  If there's just one variant, it has to
> > be PIC, unless it's a static-only build.
>
> I may well not understand what this combination of
> -static-only-routines and -inhibit-o does, but what it *looks* like it
> does is disable generation of .os (PIC) object files unconditionally,
> and if shared libraries are enabled, it also disables .o (static)
> object files. I would expect this to wind up either not working at
> all, or producing only a *shared* library, or possibly only a
> profiling library!

For shared builds, we still build .oS, that is lib_*nonshared.a.  Sure, 
it's rather strange, but it looks like this is the way it is expected to 
work.

> Also I don't know why this code would need to be PIC.

We already have one use of write_message from a test DSO, in 
dlfcn/bug-atexit3-lib.cc.  This was impossible with the existing test 
skeleton.

> > > This library is _not_ part of the implementation and should not be using
> > > __ names.  And I'm not sure it ought to be using features.h either.
> >
> > <features.h> is needed for __BEGIN_DECLS.  Including <sys/cdefs.h> would be
> > even more extreme, I think.
>
> Honestly I think sys/cdefs.h has a better claim to be a public
> interface than features.h, since it exists on *BSD (and does in fact
> define compatible __BEGIN_DECLS/__END_DECLS there too: see
> https://github.com/freebsd/freebsd/blob/master/sys/sys/cdefs.h)

Fine, <sys/cdefs.h> it is.

Thanks,
Florian