[Xen-devel] [PATCH 0/3] build: honor toolchain related environment vars

[Xen-devel] [PATCH 0/3] build: honor toolchain related environment vars

[Roger Pau Monne]

Hello,

Current Xen build system will ignore any toolchain related variables on
the environment when building (ie: CC, LD, CXX...), and the only way to
set those is to assign them directly on the make command line (ie: make
CC=foo CXX=bar ...).

The following series attempts to fix this, by removing the hardcoding of
the toolchain variables previously done in StdGNU.mk.

Note that this has the side effect that the build system will no longer
prepend CROSS_COMPILE to the toolchain variables if those are already
set. So if you are building Xen and setting CROSS_COMPILE make sure
toolchain variables are unset, or if set they should contain
CROSS_COMPILE. The Travis CI script is updated in patch 2/3 in order to
comply with the above.

The series can be found at:

git://xenbits.xen.org/people/royger/xen.git env_tools.wip

Results from Travis and gitlab CI loops are at:

https://travis-ci.org/royger/xen/builds/563972832
https://gitlab.com/xen-project/people/royger/xen/pipelines/73130338

Thanks, Roger.

Roger Pau Monne (3):
  kconfig: include default toolchain values
  build: allow picking the env values for compiler variables
  build: allow picking the env values for toolchain utilities

 config/StdGNU.mk                   | 55 ++++++++++++++++--------------
 scripts/travis-build               |  8 +++++
 xen/Makefile                       |  6 ++--
 xen/tools/kconfig/Makefile.kconfig |  7 ++--
 4 files changed, 43 insertions(+), 33 deletions(-)

-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 1/3] kconfig: include default toolchain values

[Roger Pau Monne]

Include config/$(OS).mk which contains the default values for the
toolchain variables. This removes the need to pass HOST{CC/CXX} as
parameters from the high level make target or to default them to
gcc/g++ if unset.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
Cc: Doug Goldstein <cardoe@cardoe.com>
---
 xen/Makefile                       | 6 +++---
 xen/tools/kconfig/Makefile.kconfig | 7 +++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/Makefile b/xen/Makefile
index c80914c31d..e9f700f9e7 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -267,14 +267,14 @@ kconfig := silentoldconfig oldconfig config menuconfig defconfig \
 	randconfig $(notdir $(wildcard arch/$(SRCARCH)/configs/*_defconfig))
 .PHONY: $(kconfig)
 $(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" $@
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) $@
 
 include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" silentoldconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) silentoldconfig
 
 # Allow people to just run `make` as before and not force them to configure
 $(KCONFIG_CONFIG):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) HOSTCC="$(HOSTCC)" HOSTCXX="$(HOSTCXX)" defconfig
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) defconfig
 
 # Break the dependency chain for the first run
 include/config/auto.conf.cmd: ;
diff --git a/xen/tools/kconfig/Makefile.kconfig b/xen/tools/kconfig/Makefile.kconfig
index dbd8912015..138bf3f1b7 100644
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -35,15 +35,14 @@ KBUILD_DEFCONFIG := $(ARCH)_defconfig
 # provide our shell
 CONFIG_SHELL := $(SHELL)
 
-# provide the host compiler
-HOSTCC ?= gcc
-HOSTCXX ?= g++
-
 # force target
 PHONY += FORCE
 
 FORCE:
 
+# Sets toolchain binaries to use
+include $(XEN_ROOT)/config/$(shell uname -s).mk
+
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Roger Pau Monne]

Don't force the usage of the hardcoded compiler values if those are
already set on the environment. This allows the Xen build system to
correctly pick CC/CXX values present on the environment, and fixes the
usage of those by the Gitlab CI test system.

Note that without this fix the Xen build system will completely ignore
any CC or CXX values set on the environment, and the only way to pass
a different CC or CXX is to overwrite it on the make command line.

Due to this change, Travis CI needs to be updated in order to pass a
CC and CXX that also contains the CROSS_COMPILE path, since Xen will
no longer overwrite the CC or CXX value if those are set on the
environment.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
---
 config/StdGNU.mk     | 35 +++++++++++++++++++----------------
 scripts/travis-build |  8 ++++++++
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index 7a6159021b..b3072f5b13 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -1,28 +1,31 @@
 # Use Clang/LLVM instead of GCC?
 clang     ?= n
 
-# If we are not cross-compiling, default HOSTC{C/XX} to C{C/XX}
-ifeq ($(XEN_TARGET_ARCH), $(XEN_COMPILE_ARCH))
-HOSTCC    ?= $(CC)
-HOSTCXX   ?= $(CXX)
-endif
-
 AS         = $(CROSS_COMPILE)as
 LD         = $(CROSS_COMPILE)ld
 ifeq ($(clang),y)
 gcc       := n
-CC         = $(CROSS_COMPILE)clang
-CXX        = $(CROSS_COMPILE)clang++
-LD_LTO     = $(CROSS_COMPILE)llvm-ld
-HOSTCC    ?= clang
-HOSTCXX   ?= clang++
+DEF_CC     = clang
+DEF_CXX    = clang++
+LD_LTO    ?= $(CROSS_COMPILE)llvm-ld
 else
 gcc       := y
-CC         = $(CROSS_COMPILE)gcc
-CXX        = $(CROSS_COMPILE)g++
-LD_LTO     = $(CROSS_COMPILE)ld
-HOSTCC    ?= gcc
-HOSTCXX   ?= g++
+DEF_CC     = gcc
+DEF_CXX    = g++
+LD_LTO    ?= $(CROSS_COMPILE)ld
+endif
+
+CC        ?= $(CROSS_COMPILE)$(DEF_CC)
+CXX       ?= $(CROSS_COMPILE)$(DEF_CXX)
+
+# If we are not cross-compiling, default HOSTC{C/XX} to C{C/XX}
+# else use the default values if unset
+ifeq ($(XEN_TARGET_ARCH), $(XEN_COMPILE_ARCH))
+HOSTCC    ?= $(CC)
+HOSTCXX   ?= $(CXX)
+else
+HOSTCC    ?= $(DEF_CC)
+HOSTCXX   ?= $(DEF_CXX)
 endif
 
 CPP        = $(CC) -E
diff --git a/scripts/travis-build b/scripts/travis-build
index 0cb15a89e4..a264e286b2 100755
--- a/scripts/travis-build
+++ b/scripts/travis-build
@@ -1,6 +1,14 @@
 #!/bin/bash -ex
 
+# Set HOST{CC/CXX} in case we are cross building
+export HOSTCC=${CC}
+export HOSTCXX=${CXX}
+# Prefix environment CC/CXX with CROSS_COMPILE if present
+export CC=${CROSS_COMPILE}${CC}
+export CXX=${CROSS_COMPILE}${CXX}
+
 $CC --version
+[[ "${CC}" != "${HOSTCC}" ]] && $HOSTCC --version
 
 # random config or default config
 if [[ "${RANDCONFIG}" == "y" ]]; then
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH 3/3] build: allow picking the env values for toolchain utilities

[Roger Pau Monne]

Don't force the usage of the hardcoded toolchain values if those are
already set on the environment.

Note that as part of the change the definition of AS and LD is moved
after the setting of compiler related variables.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wl@xen.org>
---
 config/StdGNU.mk | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/config/StdGNU.mk b/config/StdGNU.mk
index b3072f5b13..cab7369f12 100644
--- a/config/StdGNU.mk
+++ b/config/StdGNU.mk
@@ -1,8 +1,6 @@
 # Use Clang/LLVM instead of GCC?
 clang     ?= n
 
-AS         = $(CROSS_COMPILE)as
-LD         = $(CROSS_COMPILE)ld
 ifeq ($(clang),y)
 gcc       := n
 DEF_CC     = clang
@@ -28,19 +26,21 @@ HOSTCC    ?= $(DEF_CC)
 HOSTCXX   ?= $(DEF_CXX)
 endif
 
-CPP        = $(CC) -E
-AR         = $(CROSS_COMPILE)ar
-RANLIB     = $(CROSS_COMPILE)ranlib
-NM         = $(CROSS_COMPILE)nm
-STRIP      = $(CROSS_COMPILE)strip
-OBJCOPY    = $(CROSS_COMPILE)objcopy
-OBJDUMP    = $(CROSS_COMPILE)objdump
-SIZEUTIL   = $(CROSS_COMPILE)size
+AS        ?= $(CROSS_COMPILE)as
+LD        ?= $(CROSS_COMPILE)ld
+CPP       ?= $(CC) -E
+AR        ?= $(CROSS_COMPILE)ar
+RANLIB    ?= $(CROSS_COMPILE)ranlib
+NM        ?= $(CROSS_COMPILE)nm
+STRIP     ?= $(CROSS_COMPILE)strip
+OBJCOPY   ?= $(CROSS_COMPILE)objcopy
+OBJDUMP   ?= $(CROSS_COMPILE)objdump
+SIZEUTIL  ?= $(CROSS_COMPILE)size
 
 # Allow git to be wrappered in the environment
 GIT        ?= git
 
-INSTALL      = install
+INSTALL     ?= install
 INSTALL_DIR  = $(INSTALL) -d -m0755 -p
 INSTALL_DATA = $(INSTALL) -m0644 -p
 INSTALL_PROG = $(INSTALL) -m0755 -p
-- 
2.20.1 (Apple Git-117)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/3] kconfig: include default toolchain values

[Jan Beulich]

On 26.07.2019 15:33, Roger Pau Monne wrote:
> Include config/$(OS).mk which contains the default values for the
> toolchain variables. This removes the need to pass HOST{CC/CXX} as
> parameters from the high level make target or to default them to
> gcc/g++ if unset.

 From this description I thought reviewing would be straightforward.
However, ...

> --- a/xen/tools/kconfig/Makefile.kconfig
> +++ b/xen/tools/kconfig/Makefile.kconfig
> @@ -35,15 +35,14 @@ KBUILD_DEFCONFIG := $(ARCH)_defconfig
>   # provide our shell
>   CONFIG_SHELL := $(SHELL)
>   
> -# provide the host compiler
> -HOSTCC ?= gcc
> -HOSTCXX ?= g++
> -
>   # force target
>   PHONY += FORCE
>   
>   FORCE:
>   
> +# Sets toolchain binaries to use
> +include $(XEN_ROOT)/config/$(shell uname -s).mk
> +
>   # include the original Makefile and Makefile.host from Linux
>   include $(src)/Makefile
>   include $(src)/Makefile.host

... neither the make file here nor the two ones included (in
context above) include any other file (afaics) that would lead to
HOSTCC being defined. And nothing under $(XEN_ROOT)/config/ looks
to define it either. I guess I'm missing something, as I'm sure
this is working for you.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Jan Beulich]

On 26.07.2019 15:33, Roger Pau Monne wrote:
> Don't force the usage of the hardcoded compiler values if those are
> already set on the environment. This allows the Xen build system to
> correctly pick CC/CXX values present on the environment, and fixes the
> usage of those by the Gitlab CI test system.
> 
> Note that without this fix the Xen build system will completely ignore
> any CC or CXX values set on the environment, and the only way to pass
> a different CC or CXX is to overwrite it on the make command line.

Now the question is: Do we possibly want it to be that way? I've always
been of the opinion that inheriting something that happens to be (left?)
set in the environment is not a good idea. Hence I've been welcoming all
changes that removed dependencies on settings possibly coming from the
environment. (Exceptions of course are XEN_* environment variables we
specifically evaluate.)

As a result I'm inclined to nak this patch, but I'm open to arguments.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 3/3] build: allow picking the env values for toolchain utilities

[Jan Beulich]

On 26.07.2019 15:33, Roger Pau Monne wrote:
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -1,8 +1,6 @@
>   # Use Clang/LLVM instead of GCC?
>   clang     ?= n
>   
> -AS         = $(CROSS_COMPILE)as
> -LD         = $(CROSS_COMPILE)ld
>   ifeq ($(clang),y)
>   gcc       := n
>   DEF_CC     = clang
> @@ -28,19 +26,21 @@ HOSTCC    ?= $(DEF_CC)
>   HOSTCXX   ?= $(DEF_CXX)
>   endif
>   
> -CPP        = $(CC) -E
> -AR         = $(CROSS_COMPILE)ar
> -RANLIB     = $(CROSS_COMPILE)ranlib
> -NM         = $(CROSS_COMPILE)nm
> -STRIP      = $(CROSS_COMPILE)strip
> -OBJCOPY    = $(CROSS_COMPILE)objcopy
> -OBJDUMP    = $(CROSS_COMPILE)objdump
> -SIZEUTIL   = $(CROSS_COMPILE)size
> +AS        ?= $(CROSS_COMPILE)as
> +LD        ?= $(CROSS_COMPILE)ld
> +CPP       ?= $(CC) -E
> +AR        ?= $(CROSS_COMPILE)ar
> +RANLIB    ?= $(CROSS_COMPILE)ranlib
> +NM        ?= $(CROSS_COMPILE)nm
> +STRIP     ?= $(CROSS_COMPILE)strip
> +OBJCOPY   ?= $(CROSS_COMPILE)objcopy
> +OBJDUMP   ?= $(CROSS_COMPILE)objdump
> +SIZEUTIL  ?= $(CROSS_COMPILE)size

This does affect more than just the toolchain part of the tree,
doesn't it? Irrespective of this my point made for patch 2
applies here as well. Furthermore, if we were to go this route,
then SunOS.mk would want similar massaging.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 1/3] kconfig: include default toolchain values

[Roger Pau Monné]

On Mon, Jul 29, 2019 at 03:31:31PM +0000, Jan Beulich wrote:
> On 26.07.2019 15:33, Roger Pau Monne wrote:
> > Include config/$(OS).mk which contains the default values for the
> > toolchain variables. This removes the need to pass HOST{CC/CXX} as
> > parameters from the high level make target or to default them to
> > gcc/g++ if unset.
> 
>  From this description I thought reviewing would be straightforward.
> However, ...
> 
> > --- a/xen/tools/kconfig/Makefile.kconfig
> > +++ b/xen/tools/kconfig/Makefile.kconfig
> > @@ -35,15 +35,14 @@ KBUILD_DEFCONFIG := $(ARCH)_defconfig
> >   # provide our shell
> >   CONFIG_SHELL := $(SHELL)
> >   
> > -# provide the host compiler
> > -HOSTCC ?= gcc
> > -HOSTCXX ?= g++
> > -
> >   # force target
> >   PHONY += FORCE
> >   
> >   FORCE:
> >   
> > +# Sets toolchain binaries to use
> > +include $(XEN_ROOT)/config/$(shell uname -s).mk
> > +
> >   # include the original Makefile and Makefile.host from Linux
> >   include $(src)/Makefile
> >   include $(src)/Makefile.host
> 
> ... neither the make file here nor the two ones included (in
> context above) include any other file (afaics) that would lead to
> HOSTCC being defined. And nothing under $(XEN_ROOT)/config/ looks
> to define it either. I guess I'm missing something, as I'm sure
> this is working for you.

Oh, that's my fault. There's a pre-patch missing in this series. The
series should have been 4 patches, not 3. Given the comments to the
other patches I will hold off sending a new version until we get
consensus.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Roger Pau Monné]

On Mon, Jul 29, 2019 at 03:35:36PM +0000, Jan Beulich wrote:
> On 26.07.2019 15:33, Roger Pau Monne wrote:
> > Don't force the usage of the hardcoded compiler values if those are
> > already set on the environment. This allows the Xen build system to
> > correctly pick CC/CXX values present on the environment, and fixes the
> > usage of those by the Gitlab CI test system.
> > 
> > Note that without this fix the Xen build system will completely ignore
> > any CC or CXX values set on the environment, and the only way to pass
> > a different CC or CXX is to overwrite it on the make command line.
> 
> Now the question is: Do we possibly want it to be that way? I've always
> been of the opinion that inheriting something that happens to be (left?)
> set in the environment is not a good idea.

Then what's the point in having an environment with stuff anyway if
Xen build is just going to ignore it?

I don't have things 'left' in the environment, neither have most build
systems AFAIK. I do have things set that I want the build to
acknowledge, instead of fight against it.

> Hence I've been welcoming all
> changes that removed dependencies on settings possibly coming from the
> environment. (Exceptions of course are XEN_* environment variables we
> specifically evaluate.)
> 
> As a result I'm inclined to nak this patch, but I'm open to arguments.

IMO doing things like this is going to make Xen harder to package for
everyone, since distro build systems and test systems (like Travis or
Gitlab) expect the build system to pick the relevant
compiler/toolchain variables from the environment. Not doing it this
way means we need to adapt each build system to work with Xen.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Jan Beulich]

On 20.08.2019 09:58, Roger Pau Monné  wrote:
> On Mon, Jul 29, 2019 at 03:35:36PM +0000, Jan Beulich wrote:
>> On 26.07.2019 15:33, Roger Pau Monne wrote:
>>> Don't force the usage of the hardcoded compiler values if those are
>>> already set on the environment. This allows the Xen build system to
>>> correctly pick CC/CXX values present on the environment, and fixes the
>>> usage of those by the Gitlab CI test system.
>>>
>>> Note that without this fix the Xen build system will completely ignore
>>> any CC or CXX values set on the environment, and the only way to pass
>>> a different CC or CXX is to overwrite it on the make command line.
>>
>> Now the question is: Do we possibly want it to be that way? I've always
>> been of the opinion that inheriting something that happens to be (left?)
>> set in the environment is not a good idea.
> 
> Then what's the point in having an environment with stuff anyway if
> Xen build is just going to ignore it?
> 
> I don't have things 'left' in the environment, neither have most build
> systems AFAIK. I do have things set that I want the build to
> acknowledge, instead of fight against it.

My view is this: XEN_* things coming from the environment are fine.
Generic (i.e. project agnostic) variables (like CC) otoh would
better be ignored, as it's not clear for what purpose they've been
set. (Istr a case where some FORTIFY option was set by RPM build
environments, breaking our build.) They may well have been meant
for some other project.

Question is whether to take the above just for the hypervisor part
of the build, or also the tool stack etc ones.

>> Hence I've been welcoming all
>> changes that removed dependencies on settings possibly coming from the
>> environment. (Exceptions of course are XEN_* environment variables we
>> specifically evaluate.)
>>
>> As a result I'm inclined to nak this patch, but I'm open to arguments.
> 
> IMO doing things like this is going to make Xen harder to package for
> everyone, since distro build systems and test systems (like Travis or
> Gitlab) expect the build system to pick the relevant
> compiler/toolchain variables from the environment. Not doing it this
> way means we need to adapt each build system to work with Xen.

Well, what you describe means customizing the environment. What I
suggest means customizing the make command line. But it's customization
in either case. It _may_ happen that customizing the environment for
Xen means doing nothing, and the same settings being useful for other
projects as well. But this doesn't have to be this way and, as said
above, has been causing problems.

Furthermore - what do you do if different parts of the tree (xen/,
tools/, stubdom/) want to have different settings in effect? To me
it's quite a bit more natural to have three different but specific
make invocations, rather than fiddling with various env vars between
any two of them.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Ian Jackson]

Jan Beulich writes ("Re: [PATCH 2/3] build: allow picking the env values for compiler variables"):
> On 20.08.2019 09:58, Roger Pau Monné  wrote:
> > I don't have things 'left' in the environment, neither have most build
> > systems AFAIK. I do have things set that I want the build to
> > acknowledge, instead of fight against it.
> 
> My view is this: XEN_* things coming from the environment are fine.
> Generic (i.e. project agnostic) variables (like CC) otoh would
> better be ignored, as it's not clear for what purpose they've been
> set. (Istr a case where some FORTIFY option was set by RPM build
> environments, breaking our build.) They may well have been meant
> for some other project.

CC is set *specifically to cause build systems's like Xen's to use
that compiler*.  That is its purpose.  It should be honoured, not
ignored.

Likewise FORTIFY, even though it broke something.  FORTIFY_SOURCE was
set specifically to cause the changes it did.  The people who set it
didn't see all the implications, but that change *was* their design
intent and the bugs were real bugs in what they were trying to do.

> Question is whether to take the above just for the hypervisor part
> of the build, or also the tool stack etc ones.

*Definitely* for the toolstack build, we should honour CC et al.

The hypervisor is a more subtle question because people who set these
variables often forget to think about kernel code, embedded code,
etc.  That's what happened with FORTIFY_SOURCE.  However, I would
argue that it is better, in such a situation, to honour the variable
and break the build, than it is to simply ignore it.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/3] build: allow picking the env values for compiler variables

[Roger Pau Monné]

On Tue, Aug 27, 2019 at 11:59:33AM +0100, Ian Jackson wrote:
> Jan Beulich writes ("Re: [PATCH 2/3] build: allow picking the env values for compiler variables"):
> > On 20.08.2019 09:58, Roger Pau Monné  wrote:
> > > I don't have things 'left' in the environment, neither have most build
> > > systems AFAIK. I do have things set that I want the build to
> > > acknowledge, instead of fight against it.
> > 
> > My view is this: XEN_* things coming from the environment are fine.
> > Generic (i.e. project agnostic) variables (like CC) otoh would
> > better be ignored, as it's not clear for what purpose they've been
> > set. (Istr a case where some FORTIFY option was set by RPM build
> > environments, breaking our build.) They may well have been meant
> > for some other project.
> 
> CC is set *specifically to cause build systems's like Xen's to use
> that compiler*.  That is its purpose.  It should be honoured, not
> ignored.
> 
> Likewise FORTIFY, even though it broke something.  FORTIFY_SOURCE was
> set specifically to cause the changes it did.  The people who set it
> didn't see all the implications, but that change *was* their design
> intent and the bugs were real bugs in what they were trying to do.
> 
> > Question is whether to take the above just for the hypervisor part
> > of the build, or also the tool stack etc ones.
> 
> *Definitely* for the toolstack build, we should honour CC et al.
> 
> The hypervisor is a more subtle question because people who set these
> variables often forget to think about kernel code, embedded code,
> etc.  That's what happened with FORTIFY_SOURCE.  However, I would
> argue that it is better, in such a situation, to honour the variable
> and break the build, than it is to simply ignore it.

I fully agree. It's true that some build systems will likely break
bulding Xen, but that's a build system issue, and we shouldn't try to
work around this in Xen. As said before, what build systems (like
travis or gitlab for instance) set in the environment should be
acknowledged by Xen, or else we are forcing everyone to have custom
workarounds for building Xen.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel