[PATCH 1/2 v2] Add configure --with-extra-cflags-*

[PATCH 1/2 v2] Add configure --with-extra-cflags-*

[Olaf Hering]

Since commit 338c4375153e39b95ddc82f2ed95f85dd73e0245 ("tools: pass
EXTRA_CFLAGS via environment") it was possible to specify additional
CFLAGS in the environment. That commit gives a good explanation how that
is useful.

Now that we have configure move the knobs from the middle of the source
code to a more prominent place. If the variables are set once via
configure it is possible to rerun make from another shell which may not
have these environment variables. That can happen if one tries to resume
a failed build with either doing chroot into a rpmbuild tree or from
another xterm.

This change adds the configure options --with-extra-cflags-tools=,
--with-extra-cflags-qemu-traditional= and
--with-extra-cflags-qemu-upstream= which will assign their values to the
existing make variables.

It is now required to use configure instead of environment variables to
have a consistent set of CFLAGS across make invocations.

A new knob "skip_extra_cflags" in Rules.mk is recognized to turn off the
assignment to CFLAGS. This is required to build the firmware.

Please rerun autogen.sh after applying this patch.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---

v2:
handle also --without-opt by handling withval=no

 config/Tools.mk.in      |  3 +++
 tools/Rules.mk          |  2 ++
 tools/configure.ac      | 30 ++++++++++++++++++++++++++++++
 tools/firmware/Rules.mk |  2 +-
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/config/Tools.mk.in b/config/Tools.mk.in
index 89de5bd..6d0ac1a 100644
--- a/config/Tools.mk.in
+++ b/config/Tools.mk.in
@@ -59,6 +59,9 @@ CONFIG_QEMU_TRAD    := @qemu_traditional@
 CONFIG_QEMU_XEN     := @qemu_xen@
 CONFIG_BLKTAP1      := @blktap1@
 CONFIG_BLKTAP2      := @blktap2@
+EXTRA_CFLAGS_XEN_TOOLS        := @EXTRA_CFLAGS_XEN_TOOLS@
+EXTRA_CFLAGS_QEMU_TRADITIONAL := @EXTRA_CFLAGS_QEMU_TRADITIONAL@
+EXTRA_CFLAGS_QEMU_XEN         := @EXTRA_CFLAGS_QEMU_XEN@
 CONFIG_QEMUU_EXTRA_ARGS:= @EXTRA_QEMUU_CONFIGURE_ARGS@
 CONFIG_REMUS_NETBUF := @remus_netbuf@
 
diff --git a/tools/Rules.mk b/tools/Rules.mk
index 87a56dc..228f9a2 100644
--- a/tools/Rules.mk
+++ b/tools/Rules.mk
@@ -88,7 +88,9 @@ endif
 CFLAGS-$(CONFIG_X86_32) += $(call cc-option,$(CC),-mno-tls-direct-seg-refs)
 CFLAGS += $(CFLAGS-y)
 
+ifneq ($(skip_extra_cflags),y)
 CFLAGS += $(EXTRA_CFLAGS_XEN_TOOLS)
+endif
 
 INSTALL_PYTHON_PROG = \
 	$(XEN_ROOT)/tools/python/install-wrap "$(PYTHON_PATH)" $(INSTALL_PROG)
diff --git a/tools/configure.ac b/tools/configure.ac
index f584798..12b8824 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -222,6 +222,36 @@ AC_ARG_WITH([system-ovmf],
 ],[])
 AC_SUBST(ovmf_path)
 
+AC_ARG_WITH([extra-cflags-tools],
+    AS_HELP_STRING([--with-extra-cflags-tools="EXTRA_CFLAGS"],
+       [Additional CFLAGS to be used to build tools.]),[
+    case "$withval" in
+        no) EXTRA_CFLAGS_XEN_TOOLS= ;;
+        *)  EXTRA_CFLAGS_XEN_TOOLS=$withval ;;
+    esac
+],[EXTRA_CFLAGS_XEN_TOOLS=])
+AC_SUBST(EXTRA_CFLAGS_XEN_TOOLS)
+
+AC_ARG_WITH([extra-cflags-qemu-traditional],
+    AS_HELP_STRING([--with-extra-cflags-qemu-traditional="EXTRA_CFLAGS"],
+       [Additional CFLAGS to be used to build qemu-traditional.]),[
+    case "$withval" in
+        no) EXTRA_CFLAGS_QEMU_TRADITIONAL= ;;
+        *)  EXTRA_CFLAGS_QEMU_TRADITIONAL=$withval ;;
+    esac
+],[EXTRA_CFLAGS_QEMU_TRADITIONAL=])
+AC_SUBST(EXTRA_CFLAGS_QEMU_TRADITIONAL)
+
+AC_ARG_WITH([extra-cflags-qemu-upstream],
+    AS_HELP_STRING([--with-extra-cflags-qemu-upstream="EXTRA_CFLAGS"],
+       [Additional CFLAGS to be used to build qemu-upstream.]),[
+    case "$withval" in
+        no) EXTRA_CFLAGS_QEMU_XEN= ;;
+        *)  EXTRA_CFLAGS_QEMU_XEN=$withval ;;
+    esac
+],[EXTRA_CFLAGS_QEMU_XEN=])
+AC_SUBST(EXTRA_CFLAGS_QEMU_XEN)
+
 AC_ARG_WITH([extra-qemuu-configure-args],
     AS_HELP_STRING([--with-extra-qemuu-configure-args@<:@="--ARG1 ..."@:>@],
        [List of additional configure options for upstream qemu]),[
diff --git a/tools/firmware/Rules.mk b/tools/firmware/Rules.mk
index 26bbddc..e3bf480 100644
--- a/tools/firmware/Rules.mk
+++ b/tools/firmware/Rules.mk
@@ -3,7 +3,7 @@ override XEN_TARGET_ARCH = x86_32
 
 # User-supplied CFLAGS are not useful here.
 CFLAGS =
-EXTRA_CFLAGS_XEN_TOOLS =
+skip_extra_cflags := y
 
 include $(XEN_ROOT)/tools/Rules.mk

[PATCH 2/2] tools: Clear private variables from configure --with-opt

[Olaf Hering]

Configure will use variables from environment when substituting a
private variable if the corresponding --with*-foo option is omited.
This happens for seabios_path, ovmf_path and EXTRA_QEMUU_CONFIGURE_ARGS:

git clean -dfx
env EXTRA_QEMUU_CONFIGURE_ARGS=XXXXXXXXXXXXXXXX ./configure
grep XXXXXXXXXXXXXXXX config/Tools.mk
CONFIG_QEMUU_EXTRA_ARGS:= XXXXXXXXXXXXXXXX

The reason is the empty "action-if-not-given" case. Fix the bug by
clearing the variables in this case.

Please rerun autogen.sh after applying this patch.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/configure.ac | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/configure.ac b/tools/configure.ac
index 12b8824..c359339 100644
--- a/tools/configure.ac
+++ b/tools/configure.ac
@@ -208,7 +208,7 @@ AC_ARG_WITH([system-seabios],
         no) seabios_path= ;;
         *)  seabios_path=$withval ;;
     esac
-],[])
+],[seabios_path=])
 AC_SUBST(seabios_path)
 
 AC_ARG_WITH([system-ovmf],
@@ -219,7 +219,7 @@ AC_ARG_WITH([system-ovmf],
         no) ovmf_path= ;;
         *)  ovmf_path=$withval ;;
     esac
-],[])
+],[ovmf_path=])
 AC_SUBST(ovmf_path)
 
 AC_ARG_WITH([extra-cflags-tools],
@@ -259,7 +259,7 @@ AC_ARG_WITH([extra-qemuu-configure-args],
         no) EXTRA_QEMUU_CONFIGURE_ARGS= ;;
         *)  EXTRA_QEMUU_CONFIGURE_ARGS=$withval ;;
     esac
-],[])
+],[EXTRA_QEMUU_CONFIGURE_ARGS=])
 AC_SUBST(EXTRA_QEMUU_CONFIGURE_ARGS)
 
 AC_ARG_VAR([PREPEND_INCLUDES],

Re: [PATCH 2/2] tools: Clear private variables from configure --with-opt

[Ian Jackson]

Olaf Hering writes ("[PATCH 2/2] tools: Clear private variables from configure --with-opt"):
> Configure will use variables from environment when substituting a
> private variable if the corresponding --with*-foo option is omited.
> This happens for seabios_path, ovmf_path and EXTRA_QEMUU_CONFIGURE_ARGS:
> 
> git clean -dfx
> env EXTRA_QEMUU_CONFIGURE_ARGS=XXXXXXXXXXXXXXXX ./configure
> grep XXXXXXXXXXXXXXXX config/Tools.mk
> CONFIG_QEMUU_EXTRA_ARGS:= XXXXXXXXXXXXXXXX
> 
> The reason is the empty "action-if-not-given" case. Fix the bug by
> clearing the variables in this case.

Why on earth is this a problem ?  My answer is `don't do that then'.

Ian.

Re: [PATCH 2/2] tools: Clear private variables from configure --with-opt

[Ian Campbell]

On Wed, 2014-10-22 at 14:06 +0100, Ian Jackson wrote:
> Olaf Hering writes ("[PATCH 2/2] tools: Clear private variables from configure --with-opt"):
> > Configure will use variables from environment when substituting a
> > private variable if the corresponding --with*-foo option is omited.
> > This happens for seabios_path, ovmf_path and EXTRA_QEMUU_CONFIGURE_ARGS:
> > 
> > git clean -dfx
> > env EXTRA_QEMUU_CONFIGURE_ARGS=XXXXXXXXXXXXXXXX ./configure
> > grep XXXXXXXXXXXXXXXX config/Tools.mk
> > CONFIG_QEMUU_EXTRA_ARGS:= XXXXXXXXXXXXXXXX
> > 
> > The reason is the empty "action-if-not-given" case. Fix the bug by
> > clearing the variables in this case.
> 
> Why on earth is this a problem ?  My answer is `don't do that then'.

Is the patch harmful though? Do you think people are relying on this
behaviour?

The autoconf docs for AC_ARG_WITH don't really say what the default is
if you omit one of the option shell scripts, presumably it is to do
nothing.

https://autotools.io/autoconf/arguments.html talks about AC_ARG_VAR
which is supposed to be used for this sort of thing.

I've not confirmed this but
http://stackoverflow.com/questions/13848154/passing-environment-variables-to-autoconfs-configure suggests that passing something via the env causes config.status --recheck to do the wrong thing, which seems very plausible.

Ian.