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

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

[Matt Wilson]

I've removed all the LIBLEAF bits in this version, but kept passing
the libfsimage plugin location via compiler command line.

If there's a better way to do this, I'm certainly open to it. But
looking at it further today I think this isn't too horrible.

Matt

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

[Matt Wilson]

Currently shared libraries are automatically installed into /usr/lib
or /usr/lib64, depending on the supplied --prefix value and
$(XEN_TARGET_ARCH). Some systems, like recent Debian and Ubuntu releases,
do not use /usr/lib64, but instead /usr/lib/x86_64-linux-gnu.

With this change, packagers can supply the desired location for shared
libraries on the ./configure command line. Packagers need to note that
the default behaviour on 64-bit Linux systems will be to install shared
libraries in /usr/lib, not /usr/lib64, unless a --libdir value is provided
to ./configure.

Additionally, the libfsimage plugins are now loaded explicitly from
$LIBDIR/fs, removing platform-based decision trees in code.

Signed-off-by: Matt Wilson <msw@amazon.com>

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
 
 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))
 endif
 
 PYTHON      ?= python
diff -r 32034d1914a6 -r 4ba90ad04596 config/NetBSD.mk
--- a/config/NetBSD.mk	Thu Jun 07 19:46:57 2012 +0100
+++ b/config/NetBSD.mk	Wed Jun 20 00:40:15 2012 +0000
@@ -1,7 +1,6 @@
 include $(XEN_ROOT)/config/StdGNU.mk
 
 # Override settings for this OS
-LIBLEAFDIR_x86_64 = lib
 LIBEXEC = $(PREFIX)/libexec
 PRIVATE_BINDIR = $(BINDIR)
 
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
 SHAREDIR = $(PREFIX)/share
 MANDIR = $(SHAREDIR)/man
 MAN1DIR = $(MANDIR)/man1
diff -r 32034d1914a6 -r 4ba90ad04596 config/SunOS.mk
--- a/config/SunOS.mk	Thu Jun 07 19:46:57 2012 +0100
+++ b/config/SunOS.mk	Wed Jun 20 00:40:15 2012 +0000
@@ -22,10 +22,6 @@ INSTALL_PROG = $(INSTALL) -m0755 -p
 PREFIX ?= /usr
 BINDIR = $(PREFIX)/bin
 INCLUDEDIR = $(PREFIX)/include
-LIBLEAFDIR = lib
-LIBLEAFDIR_x86_64 = lib/amd64
-LIBDIR = $(PREFIX)/$(LIBLEAFDIR)
-LIBDIR_x86_64 = $(PREFIX)/$(LIBLEAFDIR_x86_64)
 MANDIR = $(PREFIX)/share/man
 MAN1DIR = $(MANDIR)/man1
 MAN8DIR = $(MANDIR)/man8
diff -r 32034d1914a6 -r 4ba90ad04596 config/Tools.mk.in
--- a/config/Tools.mk.in	Thu Jun 07 19:46:57 2012 +0100
+++ b/config/Tools.mk.in	Wed Jun 20 00:40:15 2012 +0000
@@ -1,6 +1,7 @@
 # Prefix and install folder
 PREFIX              := @prefix@
-LIBLEAFDIR_x86_64   := @LIB_PATH@
+exec_prefix         := @exec_prefix@
+LIBDIR              := @libdir@
 
 # A debug build of tools?
 debug               := @debug@
diff -r 32034d1914a6 -r 4ba90ad04596 config/x86_64.mk
--- a/config/x86_64.mk	Thu Jun 07 19:46:57 2012 +0100
+++ b/config/x86_64.mk	Wed Jun 20 00:40:15 2012 +0000
@@ -10,9 +10,6 @@ CONFIG_IOEMU := y
 
 CFLAGS += -m64
 
-LIBLEAFDIR = $(LIBLEAFDIR_x86_64)
-LIBDIR = $(LIBDIR_x86_64)
-
 SunOS_LIBDIR = $(SunOS_LIBDIR_x86_64)
 
 # Use only if calling $(LD) directly.
diff -r 32034d1914a6 -r 4ba90ad04596 tools/libfsimage/Rules.mk
--- a/tools/libfsimage/Rules.mk	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libfsimage/Rules.mk	Wed Jun 20 00:40:15 2012 +0000
@@ -1,17 +1,12 @@
 include $(XEN_ROOT)/tools/Rules.mk
 
-CFLAGS += -Wno-unknown-pragmas -I$(XEN_ROOT)/tools/libfsimage/common/
+CFLAGS += -Wno-unknown-pragmas -I$(XEN_ROOT)/tools/libfsimage/common/ -DFSIMAGE_FSDIR=\"$(FSDIR)\"
 CFLAGS += -Werror -D_GNU_SOURCE
 LDFLAGS += -L../common/
 
 PIC_OBJS := $(patsubst %.c,%.opic,$(LIB_SRCS-y))
 
-FSDIR-$(CONFIG_Linux) = $(LIBDIR)/fs/$(FS)
-FSDIR-$(CONFIG_SunOS)-x86_64 = $(PREFIX)/lib/fs/$(FS)/64
-FSDIR-$(CONFIG_SunOS)-x86_32 = $(PREFIX)/lib/fs/$(FS)/
-FSDIR-$(CONFIG_SunOS) = $(FSDIR-$(CONFIG_SunOS)-$(XEN_TARGET_ARCH))
-FSDIR-$(CONFIG_NetBSD) = $(LIBDIR)/fs/$(FS)
-FSDIR = $(FSDIR-y)
+FSDIR = $(LIBDIR)/fs
 
 FSLIB = fsimage.so
 
@@ -20,8 +15,8 @@ fs-all: $(FSLIB)
 
 .PHONY: fs-install
 fs-install: fs-all
-	$(INSTALL_DIR) $(DESTDIR)$(FSDIR)
-	$(INSTALL_PROG) $(FSLIB) $(DESTDIR)$(FSDIR)
+	$(INSTALL_DIR) $(DESTDIR)$(FSDIR)/$(FS)
+	$(INSTALL_PROG) $(FSLIB) $(DESTDIR)$(FSDIR)/$(FS)
 
 $(FSLIB): $(PIC_OBJS)
 	$(CC) $(LDFLAGS) $(SHLIB_LDFLAGS) -o $@ $^ -lfsimage $(FS_LIBDEPS) $(APPEND_LDFLAGS)
diff -r 32034d1914a6 -r 4ba90ad04596 tools/libfsimage/common/Makefile
--- a/tools/libfsimage/common/Makefile	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libfsimage/common/Makefile	Wed Jun 20 00:40:15 2012 +0000
@@ -1,5 +1,5 @@
 XEN_ROOT = $(CURDIR)/../../..
-include $(XEN_ROOT)/tools/Rules.mk
+include $(XEN_ROOT)/tools/libfsimage/Rules.mk
 
 MAJOR = 1.0
 MINOR = 0
diff -r 32034d1914a6 -r 4ba90ad04596 tools/libfsimage/common/fsimage_plugin.c
--- a/tools/libfsimage/common/fsimage_plugin.c	Thu Jun 07 19:46:57 2012 +0100
+++ b/tools/libfsimage/common/fsimage_plugin.c	Wed Jun 20 00:40:15 2012 +0000
@@ -122,7 +122,6 @@ fail:
 static int load_plugins(void)
 {
 	const char *fsdir = getenv("FSIMAGE_FSDIR");
-	const char *isadir = "";
 	struct dirent *dp = NULL;
 	struct dirent *dpp;
 	DIR *dir = NULL;
@@ -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
+#else
 	if (fsdir == NULL)
 		fsdir = FSIMAGE_FSDIR;
-#elif defined(__sun__)
-	if (fsdir == NULL)
-		fsdir = "/usr/lib/fs";
-
-	if (sizeof(void *) == 8)
-		isadir = "64/";
-#elif defined(__ia64__)
-	if (fsdir == NULL)
-		fsdir = "/usr/lib/fs";
-#else
-	if (fsdir == NULL) {
-		if (sizeof(void *) == 8)
-			fsdir = "/usr/lib64/fs";
-		else
-			fsdir = "/usr/lib/fs";
-	}
 #endif
-
 	if ((name_max = pathconf(fsdir, _PC_NAME_MAX)) == -1)
 		goto fail;
 
@@ -172,8 +156,8 @@ static int load_plugins(void)
 		if (strcmp(dpp->d_name, "..") == 0)
 			continue;
 
-		(void) snprintf(tmp, name_max, "%s/%s/%sfsimage.so", fsdir,
-		    dpp->d_name, isadir);
+		(void) snprintf(tmp, name_max, "%s/%s/fsimage.so", fsdir,
+			dpp->d_name);
 
 		if (init_plugin(tmp) != 0)
 			goto fail;

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

[Ian Campbell]

On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
> Currently shared libraries are automatically installed into /usr/lib
> or /usr/lib64, depending on the supplied --prefix value and
> $(XEN_TARGET_ARCH). Some systems, like recent Debian and Ubuntu releases,
> do not use /usr/lib64, but instead /usr/lib/x86_64-linux-gnu.
> 
> With this change, packagers can supply the desired location for shared
> libraries on the ./configure command line. Packagers need to note that
> the default behaviour on 64-bit Linux systems will be to install shared
> libraries in /usr/lib, not /usr/lib64, unless a --libdir value is provided
> to ./configure.
> 
> Additionally, the libfsimage plugins are now loaded explicitly from
> $LIBDIR/fs, removing platform-based decision trees in code.
> 
> Signed-off-by: Matt Wilson <msw@amazon.com>
> 
> 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.

>  
>  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. :-/

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.

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?

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 ?

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

> @@ -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.

Ian.

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

[Roger Pau Monne]

Ian Campbell wrote:
> On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
>>   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. :-/
>
> 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.
>
> 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?
>
> CCing Ian (who wrote 16950) and Olaf, whose been doing stuff in this
> area.

EXTRA_LIB was keep to maintain backward compatibility, but the use of 
the APPEND_ and PREPEND_ flags should provide the same functionality 
with more flexibility (see 24141:078392e5078d).

I don't think the hypervisor needs any special libs to build, at least I 
never had to pass any extra libs to build it on the several not so 
common systems I've build Xen on (NetBSD and uClibc).

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

[Olaf Hering]

On Thu, Jun 21, Ian Campbell wrote:

> On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
> > Currently shared libraries are automatically installed into /usr/lib
> > or /usr/lib64, depending on the supplied --prefix value and
> > $(XEN_TARGET_ARCH). Some systems, like recent Debian and Ubuntu releases,
> > do not use /usr/lib64, but instead /usr/lib/x86_64-linux-gnu.
> > 
> > With this change, packagers can supply the desired location for shared
> > libraries on the ./configure command line. Packagers need to note that
> > the default behaviour on 64-bit Linux systems will be to install shared
> > libraries in /usr/lib, not /usr/lib64, unless a --libdir value is provided
> > to ./configure.

Perhaps that should be stated in the README, which states to call just
configure without options.

> >  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. :-/
> 
> 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.
> 
> 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?

I'm not sure what the purpose of EXTRA_INCLUDES and EXTRA_LIB is, now
that EXTRA_CFLAGS can be specified, since changeset 25464:75a2bb5db228.

Perhaps its use case should also be added to the README?


Olaf

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

[Ian Campbell]

On Thu, 2012-06-21 at 09:53 +0100, Olaf Hering wrote:
> On Thu, Jun 21, Ian Campbell wrote:
> 
> > On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
> > > Currently shared libraries are automatically installed into /usr/lib
> > > or /usr/lib64, depending on the supplied --prefix value and
> > > $(XEN_TARGET_ARCH). Some systems, like recent Debian and Ubuntu releases,
> > > do not use /usr/lib64, but instead /usr/lib/x86_64-linux-gnu.
> > > 
> > > With this change, packagers can supply the desired location for shared
> > > libraries on the ./configure command line. Packagers need to note that
> > > the default behaviour on 64-bit Linux systems will be to install shared
> > > libraries in /usr/lib, not /usr/lib64, unless a --libdir value is provided
> > > to ./configure.
> 
> Perhaps that should be stated in the README, which states to call just
> configure without options.

I'd have assumed that it was well understood what options one
could/should pass to configure? Anybody who's ever built anything on a
system which uses lib64 must know it, right?

Anyway, README already says:
   If you want, you can run ./configure --help to see the list of
   options available options when building and installing Xen.

> 
> > >  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. :-/
> > 
> > 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.
> > 
> > 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?
> 
> I'm not sure what the purpose of EXTRA_INCLUDES and EXTRA_LIB is, now
> that EXTRA_CFLAGS can be specified, since changeset 25464:75a2bb5db228.
> 
> Perhaps its use case should also be added to the README?

It sounds to me like it could be deleted instead.

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

[Olaf Hering]

On Thu, Jun 21, Ian Campbell wrote:

> On Thu, 2012-06-21 at 09:53 +0100, Olaf Hering wrote:
> > On Thu, Jun 21, Ian Campbell wrote:
> > 
> > > On Wed, 2012-06-20 at 23:51 +0100, Matt Wilson wrote:
> > > > Currently shared libraries are automatically installed into /usr/lib
> > > > or /usr/lib64, depending on the supplied --prefix value and
> > > > $(XEN_TARGET_ARCH). Some systems, like recent Debian and Ubuntu releases,
> > > > do not use /usr/lib64, but instead /usr/lib/x86_64-linux-gnu.
> > > > 
> > > > With this change, packagers can supply the desired location for shared
> > > > libraries on the ./configure command line. Packagers need to note that
> > > > the default behaviour on 64-bit Linux systems will be to install shared
> > > > libraries in /usr/lib, not /usr/lib64, unless a --libdir value is provided
> > > > to ./configure.
> > 
> > Perhaps that should be stated in the README, which states to call just
> > configure without options.
> 
> I'd have assumed that it was well understood what options one
> could/should pass to configure? Anybody who's ever built anything on a
> system which uses lib64 must know it, right?

I think you are right, those who build from source will most likely know
that already. I'm building rpm packages so all this configure stuff is
kind of hidden from me.

Olaf

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

[Matt Wilson]

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

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

[Ian Campbell]

On Thu, 2012-06-21 at 19:43 +0100, Matt Wilson wrote:
> 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:
> > >  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. 

Oh, right, yes!

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

OK.

> 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

... weird!

[...]

> > 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.

Lets leave that until 4.3, I'm already a little uncomfortable messing
with the build system at this stage in 4.2 (there's always unforeseen
consequences to these sorts of changes, no matter how diligent we all
are :-/). For the lib vs. lib64 breakage on multiarch I think the risk
is worth it, but for more "cosmetic" issues like libexec I'd rather pass
for now.

Ian.

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

[Matt Wilson]

On Fri, Jun 22, 2012 at 01:26:49AM -0700, Ian Campbell wrote:
> On Thu, 2012-06-21 at 19:43 +0100, Matt Wilson wrote:
> > 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
> 
> ... weird!
> 
> [...]
> 
> > > 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.
> 
> Lets leave that until 4.3, I'm already a little uncomfortable messing
> with the build system at this stage in 4.2 (there's always unforeseen
> consequences to these sorts of changes, no matter how diligent we all
> are :-/). For the lib vs. lib64 breakage on multiarch I think the risk
> is worth it, but for more "cosmetic" issues like libexec I'd rather pass
> for now.

Agreed. It deserves a little extra attention, since some of the
binaries that might should move to /usr/libexec live outside the
tools/ tree.

Matt