Re: [PATCH] tools/qemu-xen: remove CFLAGS for qemu build

[Olaf Hering <olaf@aepfle.de>]

On Wed, Mar 14, Ian Jackson wrote:

> Olaf Hering writes ("Re: [Xen-devel] [PATCH] tools/qemu-xen: remove CFLAGS for qemu build"):
> > I think that there should be a way to pass individual external CFLAGS to
> > the tools, ipxe, qemu-traditional, qemu-xen, etc builds.
> 
> Yes, but I think that's already supposed to exist.  PREPEND_CFLAGS and
> APPEND_CFLAGS.  Look in tools/Rules.mk.

PREPEND_CFLAGS sets just CFLAGS inside configure. APPEND_CFLAGS is the
wrong place, the Makefiles must be able to override whats already in
CFLAGS.

> > From a distro perspective, its required to build libraries and binaries
> > with certain global cflags. Up to the point when qemu-upstream was
> > imported it worked as expected by exporting CFLAGS before 'make tools'.
> > Now qemu-upstream reuses these CFLAGS, but it cant deal with the result.
> > 
> > How about something like this:
> > env \
> >         EXTRA_CFLAGS_XEN_TOOLS="$RPM_OPT_FLAGS" \
> >         EXTRA_CFLAGS_QEMU_UPSTREAM="" \
> >         EXTRA_CFLAGS_IPXE="" \
> >         ./configure <more options>
> 
> It doesn't need to be plumbed through configure; it can just be used
> directly by make, although configure should use it for its tests I
> guess and probably doesn't right now.

My idea is to use configure to write the desired strings into the
Makefiles. The change below mostly works.
I'm using it like this in xen.spec:

 env \
 EXTRA_CFLAGS_XEN_TOOLS="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_XEN_TOOLS" \
 EXTRA_CFLAGS_QEMU_TRADITIONAL="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_QEMU_TRADITIONAL" \
 EXTRA_CFLAGS_QEMU_XEN="${RPM_OPT_FLAGS} -DEXTRA_CFLAGS_QEMU_XEN" \
 ./configure \
        --libdir=%{_libdir} \
        --prefix=/usr

However, there are a few issues with this approach:

Now that CFLAGS starts as empty variable in config/Tools.mk, this file
has to be included first. Therefore the order of include in
tools/Rules.mk is changed. I will send patches to fix affected
Makefiles, so far its only tools/libfsimage/zfs/Makefile. I will prepare
a patch to move the include tools/libfsimage/Rules.mk in all libfsimage
Makefiles.

I noticed that @F is empty now, leading to ..d files in all directories.
No idea how to fix that.
tools/Rules.mk:CFLAGS += -MMD -MF .$(@F).d

Unrelated to this change: in config/Tools.mk debug is forced to 'y'. I
think the logic in tools/m4/{enable,disable}_feature.m4 is flipped.

What do you think about the general approach of this patch?

Olaf


# HG changeset patch
# Parent 773d0367087212c43faf8cdcc21cf443b1ea0046

diff -r 773d03670872 config/Tools.mk.in
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -23,6 +23,10 @@ PREPEND_LIB         := @PREPEND_LIB@
 APPEND_INCLUDES     := @APPEND_INCLUDES@
 APPEND_LIB          := @APPEND_LIB@
 
+CFLAGS                        := @EXTRA_CFLAGS_XEN_TOOLS@
+EXTRA_CFLAGS_QEMU_TRADITIONAL := @EXTRA_CFLAGS_QEMU_TRADITIONAL@
+EXTRA_CFLAGS_QEMU_XEN         := @EXTRA_CFLAGS_QEMU_XEN@
+
 # Download GIT repositories via HTTP or GIT's own protocol?
 # GIT's protocol is faster and more robust, when it works at all (firewalls
 # may block it). We make it the default, but if your GIT repository downloads
diff -r 773d03670872 tools/Makefile
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -122,6 +122,8 @@ subdir-all-qemu-xen-traditional-dir subd
 	set -e; \
 		$(buildmakevars2shellvars); \
 		cd qemu-xen-traditional-dir; \
+		env CFLAGS="$(EXTRA_CFLAGS_QEMU_TRADITIONAL)" \
+		PREFIX="$(PREFIX)" \
 		$(QEMU_ROOT)/xen-setup $(IOEMU_CONFIGURE_CROSS); \
 		$(MAKE) install
 
@@ -147,6 +149,7 @@ subdir-all-qemu-xen-dir subdir-install-q
 		source=.; \
 	fi; \
 	cd qemu-xen-dir; \
+	env CFLAGS="$(EXTRA_CFLAGS_QEMU_XEN)" \
 	$$source/configure --enable-xen --target-list=i386-softmmu \
 		--source-path=$$source \
 		--extra-cflags="-I$(XEN_ROOT)/tools/include \
diff -r 773d03670872 tools/Rules.mk
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -3,8 +3,8 @@
 # `all' is the default target
 all:
 
+include $(XEN_ROOT)/config/Tools.mk
 include $(XEN_ROOT)/Config.mk
-include $(XEN_ROOT)/config/Tools.mk
 
 export _INSTALL := $(INSTALL)
 INSTALL = $(XEN_ROOT)/tools/cross-install
diff -r 773d03670872 tools/configure.ac
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -54,6 +54,12 @@ AC_ARG_VAR([APPEND_INCLUDES],
     [List of include folders to append to CFLAGS (without -I)])
 AC_ARG_VAR([APPEND_LIB],
     [List of library folders to append to LDFLAGS (without -L)])
+AC_ARG_VAR([EXTRA_CFLAGS_XEN_TOOLS],
+    [Extra CFLAGS to build tools])
+AC_ARG_VAR([EXTRA_CFLAGS_QEMU_TRADITIONAL],
+    [Extra CFLAGS to build qemu-traditional])
+AC_ARG_VAR([EXTRA_CFLAGS_QEMU_XEN],
+    [Extra CFLAGS to build qemu-xen])
 
 AX_SET_FLAGS