Re: [PATCH 1 of 1 v2] tools: honour --libdir when it is passed to ./configure

[Matt Wilson <msw@amazon.com>]

On Thu, Jun 21, 2012 at 01:33:25AM -0700, Ian Campbell wrote:
> On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
> > diff -r 32034d1914a6 -r 4ba90ad04596 Config.mk
> > --- a/Config.mk	Thu Jun 07 19:46:57 2012 +0100
> > +++ b/Config.mk	Wed Jun 20 00:40:15 2012 +0000
> > @@ -43,6 +43,7 @@ endif
> >  
> >  include $(XEN_ROOT)/config/$(XEN_OS).mk
> >  include $(XEN_ROOT)/config/$(XEN_TARGET_ARCH).mk
> > +include $(XEN_ROOT)/config/Tools.mk
> 
> Unfortunately this won't work, our policy is that you only need to
> invoke configure in order to build the tools -- so the top-level
> Config.mk cannot include configure generated files.

Aah, sorry. Good to know.

> >  SHAREDIR    ?= $(PREFIX)/share
> >  DOCDIR      ?= $(SHAREDIR)/doc/xen
> > @@ -67,7 +68,7 @@ endef
> >  
> >  ifneq ($(EXTRA_PREFIX),)
> >  EXTRA_INCLUDES += $(EXTRA_PREFIX)/include
> > -EXTRA_LIB += $(EXTRA_PREFIX)/$(LIBLEAFDIR)
> > +EXTRA_LIB += $(EXTRA_PREFIX)/$(shell basename $(LIBDIR))
> 
> since we are sort of reverting 16950:0faf620bc749 here this could in
> theory $(EXTRA_PREFIX)/$(LIBDIR)? That doesn't remove the need to
> include Tools.mk though. :-/

That would result in looking in /some/extra/prefix/usr/lib, which is
not what I'd expect is desired. 

> Does anyone know if this EXTRA_PREFIX stuff is intended to be used for
> hypervisor and other non-tools builds? If not then we could consider
> pushing it down a level.

That stuff has been around since the bitkeeper import. No idea why
they'd be needed.

> In the tools case I think we already have a way to inject arbitrary -L
> and -I options -- so maybe this can just go away?

Sounds good to me. :-)

> CCing Ian (who wrote 16950) and Olaf, whose been doing stuff in this
> area.
> 
> > diff -r 32034d1914a6 -r 4ba90ad04596 config/StdGNU.mk
> > --- a/config/StdGNU.mk	Thu Jun 07 19:46:57 2012 +0100
> > +++ b/config/StdGNU.mk	Wed Jun 20 00:40:15 2012 +0000
> > @@ -32,13 +32,7 @@ INSTALL_PROG = $(INSTALL) -m0755 -p
> >  PREFIX ?= /usr
> >  BINDIR = $(PREFIX)/bin
> >  INCLUDEDIR = $(PREFIX)/include
> > -LIBLEAFDIR = lib
> > -LIBLEAFDIR_x86_32 = lib
> > -LIBLEAFDIR_x86_64 ?= lib64
> > -LIBDIR = $(PREFIX)/$(LIBLEAFDIR)
> > -LIBDIR_x86_32 = $(PREFIX)/$(LIBLEAFDIR_x86_32)
> > -LIBDIR_x86_64 = $(PREFIX)/$(LIBLEAFDIR_x86_64)
> > -LIBEXEC = $(LIBDIR_x86_32)/xen/bin
> > +LIBEXEC = $(PREFIX)/lib/xen/bin
> 
> Wouldn't this be $(LIBDIR)/xen/bin ?

No, the old behavior (which I retained) is to always install
$(LIBEXEC) files to /usr/lib/xen/bin (since it was defined to
$(LIBDIR_x86_32), which expands to /usr/lib), even on non-Itanium
64-bit Linux platforms. This results in having paths like:

/usr/lib/xen/bin/qemu-dm
/usr/lib/xen/bin/stubdom-dm
/usr/lib/xen/bin/stubdompath.sh

Confusingly, we also install "private" binaries to $(PRIVATE_BINDIR),
which expands to $(PRIVATE_PREFIX)/bin which expands to $(LIBDIR)/xen/bin,
which expands to /usr/lib64/xen/bin. This results in paths like:

/usr/lib64/xen/bin/lsevtchn
/usr/lib64/xen/bin/qemu-dm
/usr/lib64/xen/bin/readnotes
/usr/lib64/xen/bin/xc_restore
/usr/lib64/xen/bin/xc_save
/usr/lib64/xen/bin/xenconsole
/usr/lib64/xen/bin/xenctx

This split doesn't match my personal taste, which is for all
"internal" binaries to live in /usr/libexec. Now, with my FHS / LSB
hat on (which is admittedly very dusty, full of holes, and smells a
bit), there's no current ratified standard guidance for using
/usr/libexec on Linux systems, but the first FHS 3.0 [1] includes it.

> I suppose configure defines a libexec but it isn't the one we want?

By default, configure will define libexec to be /usr/libexec, which I
like. If we switched to using the value provided from configure, we
people who don't like /usr/libexec could just provide a different
value at ./configure time.

> > @@ -131,27 +130,12 @@ static int load_plugins(void)
> >  	int err;
> >  	int ret = -1;
> >  
> > -#if defined(FSIMAGE_FSDIR)
> > +#if !defined(FSIMAGE_FSDIR)
> > +#error FSIMAGE_FSDIR not defined
> 
> FWIW I'd be happy with the regular message you get from using an
> undefined symbol, if you want to ditch this #error.

Sounds good to me. Thanks for the review!

Matt

[1] http://www.linuxbase.org/betaspecs/fhs/fhs.txt