diff mbox

Kconfig: fix environment variable handling

Message ID 5696393302000078000C636E@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Jan. 13, 2016, 10:46 a.m. UTC 
With xen/Makefile including include/config/auto.conf.cmd, environment
variables checked in the latter must be available at the time of
inclusion of that file, and hence must be populated in xen/Makefile
rather than by passing to or inside xen/tools/kconfig/Makefile.kconfig.
Otherwise incremental re-builds will always be full re-builds, which is
not only annoying but actively problematic when building as non-root
and only running "install-xen" as root.

Also take the opportunity and remove stray $(Q) uses.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Kconfig: fix environment variable handling

With xen/Makefile including include/config/auto.conf.cmd, environment
variables checked in the latter must be available at the time of
inclusion of that file, and hence must be populated in xen/Makefile
rather than by passing to or inside xen/tools/kconfig/Makefile.kconfig.
Otherwise incremental re-builds will always be full re-builds, which is
not only annoying but actively problematic when building as non-root
and only running "install-xen" as root.

Also take the opportunity and remove stray $(Q) uses.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -20,6 +20,9 @@ MAKEFLAGS += -rR
 
 EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
 
+ARCH=$(XEN_TARGET_ARCH)
+SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+
 # Don't break if the build process wasn't called from the top level
 # we need XEN_TARGET_ARCH to generate the proper config
 include $(XEN_ROOT)/Config.mk
@@ -101,7 +104,7 @@ _clean: delete-unfresh-files
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig clean
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
 	find . \( -name "*.o" -o -name ".*.d" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET)-syms *~ core
 	rm -f include/asm-*/asm-offsets.h
@@ -236,14 +239,14 @@ kconfig := silentoldconfig oldconfig con
 	nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig
 .PHONY: $(kconfig)
 $(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) $@
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) $@
 
 include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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):
-	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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: ;
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -44,10 +44,6 @@ PHONY += FORCE
 
 FORCE:
 
-SRCARCH = $(shell echo $(ARCH) | \
-	sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
-export SRCARCH
-
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host

Comments

Douglas Goldstein Jan. 13, 2016, 3:50 p.m. UTC | #1 
On 1/13/16 4:46 AM, Jan Beulich wrote:
> With xen/Makefile including include/config/auto.conf.cmd, environment
> variables checked in the latter must be available at the time of
> inclusion of that file, and hence must be populated in xen/Makefile
> rather than by passing to or inside xen/tools/kconfig/Makefile.kconfig.
> Otherwise incremental re-builds will always be full re-builds, which is
> not only annoying but actively problematic when building as non-root
> and only running "install-xen" as root.
> 
> Also take the opportunity and remove stray $(Q) uses.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -20,6 +20,9 @@ MAKEFLAGS += -rR
>  
>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>  
> +ARCH=$(XEN_TARGET_ARCH)
> +SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
> +
>  # Don't break if the build process wasn't called from the top level
>  # we need XEN_TARGET_ARCH to generate the proper config
>  include $(XEN_ROOT)/Config.mk
> @@ -101,7 +104,7 @@ _clean: delete-unfresh-files
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
>  	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
> -	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig clean
> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
>  	find . \( -name "*.o" -o -name ".*.d" \) -exec rm -f {} \;
>  	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET)-syms *~ core
>  	rm -f include/asm-*/asm-offsets.h
> @@ -236,14 +239,14 @@ kconfig := silentoldconfig oldconfig con
>  	nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig
>  .PHONY: $(kconfig)
>  $(kconfig):
> -	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) $@
> +	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) $@
>  
>  include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
> -	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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):
> -	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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: ;
> --- a/xen/tools/kconfig/Makefile.kconfig
> +++ b/xen/tools/kconfig/Makefile.kconfig
> @@ -44,10 +44,6 @@ PHONY += FORCE
>  
>  FORCE:
>  
> -SRCARCH = $(shell echo $(ARCH) | \
> -	sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
> -export SRCARCH
> -
>  # include the original Makefile and Makefile.host from Linux
>  include $(src)/Makefile
>  include $(src)/Makefile.host
> 
> 
> 

Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
Ian Campbell Jan. 15, 2016, 3:44 p.m. UTC | #2 
On Wed, 2016-01-13 at 03:46 -0700, Jan Beulich wrote:
> With xen/Makefile including include/config/auto.conf.cmd, environment
> variables checked in the latter must be available at the time of
> inclusion of that file, and hence must be populated in xen/Makefile
> rather than by passing to or inside xen/tools/kconfig/Makefile.kconfig.
> Otherwise incremental re-builds will always be full re-builds, which is
> not only annoying but actively problematic when building as non-root
> and only running "install-xen" as root.
> 
> Also take the opportunity and remove stray $(Q) uses.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -20,6 +20,9 @@ MAKEFLAGS += -rR
>  
>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>  
> +ARCH=$(XEN_TARGET_ARCH)
> +SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')

Why not just use XEN_TARGET_ARCH and TARGET_ARCH as defined in
xen/Rules.mk, at the point where recursing into kconfig/Makefile.kconfig?

Otherwise wouldn't we risk SRCARCH and TARGET_ARCH getting out of sync?
Jan Beulich Jan. 15, 2016, 3:54 p.m. UTC | #3 
>>> On 15.01.16 at 16:44, <ian.campbell@citrix.com> wrote:
> On Wed, 2016-01-13 at 03:46 -0700, Jan Beulich wrote:
>> With xen/Makefile including include/config/auto.conf.cmd, environment
>> variables checked in the latter must be available at the time of
>> inclusion of that file, and hence must be populated in xen/Makefile
>> rather than by passing to or inside xen/tools/kconfig/Makefile.kconfig.
>> Otherwise incremental re-builds will always be full re-builds, which is
>> not only annoying but actively problematic when building as non-root
>> and only running "install-xen" as root.
>> 
>> Also take the opportunity and remove stray $(Q) uses.
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> --- a/xen/Makefile
>> +++ b/xen/Makefile
>> @@ -20,6 +20,9 @@ MAKEFLAGS += -rR
>>  
>>  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>>  
>> +ARCH=$(XEN_TARGET_ARCH)
>> +SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
> 
> Why not just use XEN_TARGET_ARCH and TARGET_ARCH as defined in
> xen/Rules.mk, at the point where recursing into kconfig/Makefile.kconfig?

xen/Makefile doesn't include xen/Rules.mk afaics. And the recursion
into kconfig/Makefile.kconfig doesn't use it either (and probably
shouldn't, as we want to keep the two environments distinct, in
order to not risk collisions).

> Otherwise wouldn't we risk SRCARCH and TARGET_ARCH getting out of sync?

That would be bad indeed, but no worse than before this patch
(which just moves it). I vaguely recall even maybe having
commented on that duplication in the context of the Kconfig series.

Jan
Ian Campbell Jan. 15, 2016, 4:01 p.m. UTC | #4 
On Fri, 2016-01-15 at 08:54 -0700, Jan Beulich wrote:
> > > > On 15.01.16 at 16:44, <ian.campbell@citrix.com> wrote:
> > On Wed, 2016-01-13 at 03:46 -0700, Jan Beulich wrote:
> > > With xen/Makefile including include/config/auto.conf.cmd, environment
> > > variables checked in the latter must be available at the time of
> > > inclusion of that file, and hence must be populated in xen/Makefile
> > > rather than by passing to or inside
> > > xen/tools/kconfig/Makefile.kconfig.
> > > Otherwise incremental re-builds will always be full re-builds, which
> > > is
> > > not only annoying but actively problematic when building as non-root
> > > and only running "install-xen" as root.
> > > 
> > > Also take the opportunity and remove stray $(Q) uses.
> > > 
> > > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > > 
> > > --- a/xen/Makefile
> > > +++ b/xen/Makefile
> > > @@ -20,6 +20,9 @@ MAKEFLAGS += -rR
> > >  
> > >  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
> > >  
> > > +ARCH=$(XEN_TARGET_ARCH)
> > > +SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e
> > > s'/arm\(32\|64\)/arm/g')
> > 
> > Why not just use XEN_TARGET_ARCH and TARGET_ARCH as defined in
> > xen/Rules.mk, at the point where recursing into
> > kconfig/Makefile.kconfig?
> 
> xen/Makefile doesn't include xen/Rules.mk afaics. And the recursion
> into kconfig/Makefile.kconfig doesn't use it either (and probably
> shouldn't, as we want to keep the two environments distinct, in
> order to not risk collisions).

Hrm, true. In which case:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> > Otherwise wouldn't we risk SRCARCH and TARGET_ARCH getting out of sync?
> 
> That would be bad indeed, but no worse than before this patch
> (which just moves it). I vaguely recall even maybe having
> commented on that duplication in the context of the Kconfig series.

Perhaps the answer is another file alongside Rules.mk which is more
constrained and can therefore be included from more places (including
xen/Makefile)?

Ian.
Jan Beulich Jan. 15, 2016, 4:17 p.m. UTC | #5 
>>> On 15.01.16 at 17:01, <ian.campbell@citrix.com> wrote:
> On Fri, 2016-01-15 at 08:54 -0700, Jan Beulich wrote:
>> > > > On 15.01.16 at 16:44, <ian.campbell@citrix.com> wrote:
>> > On Wed, 2016-01-13 at 03:46 -0700, Jan Beulich wrote:
>> > > With xen/Makefile including include/config/auto.conf.cmd, environment
>> > > variables checked in the latter must be available at the time of
>> > > inclusion of that file, and hence must be populated in xen/Makefile
>> > > rather than by passing to or inside
>> > > xen/tools/kconfig/Makefile.kconfig.
>> > > Otherwise incremental re-builds will always be full re-builds, which
>> > > is
>> > > not only annoying but actively problematic when building as non-root
>> > > and only running "install-xen" as root.
>> > > 
>> > > Also take the opportunity and remove stray $(Q) uses.
>> > > 
>> > > Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> > > 
>> > > --- a/xen/Makefile
>> > > +++ b/xen/Makefile
>> > > @@ -20,6 +20,9 @@ MAKEFLAGS += -rR
>> > >  
>> > >  EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
>> > >  
>> > > +ARCH=$(XEN_TARGET_ARCH)
>> > > +SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e
>> > > s'/arm\(32\|64\)/arm/g')
>> > 
>> > Why not just use XEN_TARGET_ARCH and TARGET_ARCH as defined in
>> > xen/Rules.mk, at the point where recursing into
>> > kconfig/Makefile.kconfig?
>> 
>> xen/Makefile doesn't include xen/Rules.mk afaics. And the recursion
>> into kconfig/Makefile.kconfig doesn't use it either (and probably
>> shouldn't, as we want to keep the two environments distinct, in
>> order to not risk collisions).
> 
> Hrm, true. In which case:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.

>> > Otherwise wouldn't we risk SRCARCH and TARGET_ARCH getting out of sync?
>> 
>> That would be bad indeed, but no worse than before this patch
>> (which just moves it). I vaguely recall even maybe having
>> commented on that duplication in the context of the Kconfig series.
> 
> Perhaps the answer is another file alongside Rules.mk which is more
> constrained and can therefore be included from more places (including
> xen/Makefile)?

Indeed I was thinking along those same lines.

Jan
diff mbox

Patch

--- a/xen/Makefile
+++ b/xen/Makefile
@@ -20,6 +20,9 @@  MAKEFLAGS += -rR
 
 EFI_MOUNTPOINT ?= $(BOOT_DIR)/efi
 
+ARCH=$(XEN_TARGET_ARCH)
+SRCARCH=$(shell echo $(ARCH) | sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
+
 # Don't break if the build process wasn't called from the top level
 # we need XEN_TARGET_ARCH to generate the proper config
 include $(XEN_ROOT)/Config.mk
@@ -101,7 +104,7 @@  _clean: delete-unfresh-files
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig clean
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
 	find . \( -name "*.o" -o -name ".*.d" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET)-syms *~ core
 	rm -f include/asm-*/asm-offsets.h
@@ -236,14 +239,14 @@  kconfig := silentoldconfig oldconfig con
 	nconfig xconfig gconfig savedefconfig listnewconfig olddefconfig
 .PHONY: $(kconfig)
 $(kconfig):
-	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) $@
+	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) $@
 
 include/config/%.conf: include/config/auto.conf.cmd $(KCONFIG_CONFIG)
-	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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):
-	$(Q)$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(XEN_TARGET_ARCH) 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: ;
--- a/xen/tools/kconfig/Makefile.kconfig
+++ b/xen/tools/kconfig/Makefile.kconfig
@@ -44,10 +44,6 @@  PHONY += FORCE
 
 FORCE:
 
-SRCARCH = $(shell echo $(ARCH) | \
-	sed -e 's/x86.*/x86/' -e s'/arm\(32\|64\)/arm/g')
-export SRCARCH
-
 # include the original Makefile and Makefile.host from Linux
 include $(src)/Makefile
 include $(src)/Makefile.host