kbuild: Do not use hyphen in exported variable name
Subsystems (2)
PATCH 1/1 kbuild: Do not use hyphen in exported variable name
This definition in Makefile.dtbinst:
export dtbinst-root ?= $(obj)
should define and export dtbinst-root when handling the root dts directory, and do nothing in the subdirectories. However, the variable does not reliably get exported to the environment, perhaps because its name contains a hyphen.
Rename the variable to dtbinst_root.
References: https://bugs.debian.org/833561
---
scripts/Makefile.dtbinst +3 -3
@@ -14,7 +14,7 @@ src := $(obj) PHONY := __dtbs_install __dtbs_install: -export dtbinst-root ?= $(obj) +export dtbinst_root ?= $(obj) include include/config/auto.conf include scripts/Kbuild.include
@@ -22,7 +22,7 @@ include $(src)/Makefile PHONY += __dtbs_install_prep __dtbs_install_prep: -ifeq ("$(dtbinst-root)", "$(obj)") +ifeq ("$(dtbinst_root)", "$(obj)") $(Q)mkdir -p $(INSTALL_DTBS_PATH) endif
@@ -33,7 +33,7 @@ dtbinst-dirs := $(dts-dirs) quiet_cmd_dtb_install = INSTALL $< cmd_dtb_install = mkdir -p $(2); cp $< $(2) -install-dir = $(patsubst $(dtbinst-root)%,$(INSTALL_DTBS_PATH)%,$(obj)) +install-dir = $(patsubst $(dtbinst_root)%,$(INSTALL_DTBS_PATH)%,$(obj)) $(dtbinst-files) $(dtbinst-dirs): | __dtbs_install_prep
Quoted 10 lines · Ben Hutchings
This definition in Makefile.dtbinst:
export dtbinst-root ?= $(obj)should define and export dtbinst-root when handling the root dts directory, and do nothing in the subdirectories. However, the variable does not reliably get exported to the environment, perhaps because its name contains a hyphen.
Rename the variable to
dtbinst_root.
Exported environment variables are usually in UPPPER CASE. And also listed in: Documentation/kbuild/kbuild.txt
There are deviations from this already, so not something that is followed by all of kbuild.
Sam
Hi Ben,
2017-04-18 10:00 GMT+09:00 Ben Hutchings <[email protected]>:
Quoted 36 lines
This definition in Makefile.dtbinst:
export dtbinst-root ?= $(obj)should define and export dtbinst-root when handling the root dts directory, and do nothing in the subdirectories. However, the variable does not reliably get exported to the environment, perhaps because its name contains a hyphen.
Rename the variable to
dtbinst_root.References: https://bugs.debian.org/833561
Fixes 323a028d39cdi ("dts, kbuild: Implement support for dtb vendor subdirs")Signed-off-by Ben Hutchings <[email protected]>---
scripts/Makefile.dtbinst +2
-2@@ -14,7 +14,7 @@ src := $(obj) PHONY := __dtbs_install __dtbs_install: -export dtbinst-root ?= $(obj) +export dtbinst_root ?= $(obj) include include/config/auto.conf include scripts/Kbuild.include@@ -22,7 +22,7 @@ include $(src)/Makefile PHONY += __dtbs_install_prep __dtbs_install_prep: -ifeq ("$(dtbinst-root)", "$(obj)") +ifeq ("$(dtbinst_root)", "$(obj)") $(Q)mkdir -p $(INSTALL_DTBS_PATH) endif
scripts/Makefile.dtbinst dtbinst-dirs := $(dts-dirs) · line 39
Quoted 8 lines
scripts/Makefile.dtbinst +1
-1@@ -33,7 +33,7 @@ dtbinst-dirs := $(dts-dirs) quiet_cmd_dtb_install = INSTALL $< cmd_dtb_install = mkdir -p $(2); cp $< $(2) -install-dir = $(patsubst $(dtbinst-root)%,$(INSTALL_DTBS_PATH)%,$(obj)) +install-dir = $(patsubst $(dtbinst_root)%,$(INSTALL_DTBS_PATH)%,$(obj)) $(dtbinst-files) $(dtbinst-dirs): | __dtbs_install_prep
I tested dtbs_install once again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.
I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."
If so, why is this patch here?
How is the dtbs_install procedure different in the Debian package?
Signature · 2 lines
Best Regards Masahiro Yamada
On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]
Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."
Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.
I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."
I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?
This is the patch I applied to the package.
Ben.
Signature · 2 lines
Ben Hutchings I haven't lost my mind; it's backed up on tape somewhere.
Hi Ben,
2017-04-23 16:23 GMT+09:00 Ben Hutchings <[email protected]>:
Quoted 32 lines · Masahiro Yamada
[...]
Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."
Quoted 8 lines
I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?This is the patch I applied to the package.
Do you still need this patch for Debian?
If so, I can pick it up because it is apparently harmless (maybe safer as you said).
However, I may add the following excuse in the git-log:
[masahiro
I cound not reproduce the issue, but at least this patch is harmless.]
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 39 lines · Masahiro Yamada
Hi Ben,
Quoted 36 lines
2017-04-23 16:23 GMT+09:00 Ben Hutchings <[email protected]>:
On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?This is the patch I applied to the package.
Do you still need this patch for Debian?
[...]
I don't think so. I just don't know for sure.
Ben.
Signature · 2 lines
Ben Hutchings This sentence contradicts itself - no actually it doesn't.
Hi Ben,
2017-04-30 23:49 GMT+09:00 Ben Hutchings <[email protected]>:
Quoted 46 lines · Masahiro Yamada
Quoted 45 lines
Hi Ben,
Quoted 41 lines
2017-04-23 16:23 GMT+09:00 Ben Hutchings <[email protected]>:
On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?This is the patch I applied to the package.
Quoted 6 lines
Quoted 1 line
Do you still need this patch for Debian?
[...]
I don't think so. I just don't know for sure.
Ben.
I postpone this patch for now.
Please re-post if this patch turns out to be really needed.
Signature · 2 lines
Best Regards Masahiro Yamada
Quoted 43 lines · Ben Hutchings
Quoted 42 lines · Masahiro Yamada
Hi Ben,
Quoted 39 lines
2017-04-23 16:23 GMT+09:00 Ben Hutchings <[email protected]>:
On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?
Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
This is the patch I applied to the package.
Do you still need this patch for Debian?
[...]
I don't think so. I just don't know for sure.
I've now seen a similar problem with the suffix-y variable exported from arch/sh/boot/Makefile to arch/sh/boot/compressed/Makefile:
https://buildd.debian.org/status/fetch.php?pkg=linux&arch=sh4&ver=4.13%7Erc5-1%7Eexp1&stamp=1502943967&raw=0
Those Makefiles haven't changed recently, so there must be some difference in the build environment. Here's a Makefile that demonstrates the difference in behaviour between bash and dash:
--- BEGIN ---
ifdef WANTED_SHELL
SHELL := $(WANTED_SHELL)
endif
test:
@WANTED_SHELL=/bin/bash $(MAKE) test2 @WANTED_SHELL=/bin/dash $(MAKE) test2 @WANTED_SHELL= $(MAKE) test2
test2: export foo_bar := foo_bar
test2: export foo-bar := foo-bar
test2:
@echo SHELL=$(SHELL) @$(MAKE) test3
test3:
@echo foo_bar=$(foo_bar) @echo foo-bar=$(foo-bar)
.PHONY: test test2 test3
--- END ---
For me, this produces:
$ make -s SHELL=/bin/bash foo_bar=foo_bar foo-bar=foo-bar SHELL=/bin/dash foo_bar=foo_bar foo-bar= SHELL=/bin/sh foo_bar=foo_bar foo-bar=foo-bar
Note that the last two cases are different even though /bin/sh is dash here. It turns out that make avoids using the shell for simple commands, unless it has been changed from the default:
https://sources.debian.net/src/make-dfsg/4.1-9.1/job.c/#L2575
Ben.
Signature · 3 lines
Ben Hutchings The generation of random numbers is too important to be left to chance. - Robert Coveyou
Hi Ben,
Thanks for useful info.
2017-08-19 10:13 GMT+09:00 Ben Hutchings <[email protected]>:
Quoted 96 lines · Ben Hutchings
Quoted 43 lines · Masahiro Yamada
Quoted 42 lines
Hi Ben,
Quoted 39 lines
2017-04-23 16:23 GMT+09:00 Ben Hutchings <[email protected]>:
On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]Quoted 13 lines
I tested
dtbs_installonce again by myself, but
dtbinst-root is exported to the sub make
and the vendor directories are created correctly.I checked the debian's forum you gave
Quoted 1 line
References: https://bugs.debian.org/833561
In there, you mentioned:
"This looks like a bug in make, but we can at least work around it by using a non-hyphenated variable name."Quoted 4 lines
Does this issue happen on a specific Make version?
I tested GNU make 3.81, 3.82, 4.0, 4.1, 4.2,
but I was not hit by the problem.I don't think this is make version dependent. I can't reproduce the issue today with make 4.1. But I would have been using the same version in August when I wrote that.
What more can I say? Clearly the hyphenated variable gets passed to the sub-make in most cases. But it's not totally reliable because last year it wasn't working for us.
Quoted 3 lines
In the last post in the thread, you concluded:
"We believe that the bug you reported is fixed in the latest version of linux, which is due to be installed in the Debian FTP archive."I didn't write that, it's a standard message generated for bugs marked as closed in a package changelog. :-)
Quoted 2 lines
If so, why is this patch here?
How is thedtbs_installprocedure different in the Debian package?Quoted 7 lines
Quoted 4 lines
Quoted 2 lines
This is the patch I applied to the package.
Do you still need this patch for Debian?
[...]
I don't think so. I just don't know for sure.
I've now seen a similar problem with the suffix-y variable exported from
arch/sh/boot/Makefiletoarch/sh/boot/compressed/Makefile:
https://buildd.debian.org/status/fetch.php?pkg=linux&arch=sh4&ver=4.13%7Erc5-1%7Eexp1&stamp=1502943967&raw=0Those Makefiles haven't changed recently, so there must be some difference in the build environment. Here's a
Makefilethat demonstrates the difference in behaviour between bash and dash:--- BEGIN ---
ifdefWANTED_SHELL
SHELL := $(WANTED_SHELL)
endiftest:
@WANTED_SHELL=/bin/bash $(MAKE) test2 @WANTED_SHELL=/bin/dash $(MAKE) test2 @WANTED_SHELL= $(MAKE) test2test2: export
foo_bar:=foo_bar
test2: export foo-bar := foo-bar
test2:@echo SHELL=$(SHELL) @$(MAKE) test3test3:
@echo foo_bar=$(foo_bar) @echo foo-bar=$(foo-bar).PHONY: test test2 test3
--- END ---For me, this produces:
$ make -s SHELL=/bin/bash foo_bar=foo_bar foo-bar=foo-bar SHELL=/bin/dash foo_bar=foo_bar foo-bar= SHELL=/bin/sh foo_bar=foo_bar foo-bar=foo-bar
Quoted 4 lines
Note that the last two cases are different even though
/bin/shis dash here. It turns out that make avoids using the shell for simple commands, unless it has been changed from the default:
https://sources.debian.net/src/make-dfsg/4.1-9.1/job.c/#L2575
Yes. This is also explained in O'Reilly GNU Make:
http://www.oreilly.com/openbook/make3/book/ch05.pdf
------------------->8------------------------------ Commands are essentially one-line shell scripts. In effect, make grabs each line and passes it to a subshell for execution. In fact, make can optimize this (relatively) expensive fork/exec algorithm if it can guarantee that omitting the shell will not change the behavior of the program. It checks this by scanning each command line for shell special characters, such as wildcard characters and i/o redirection. If none are found, make directly executes the command without passing it to a subshell. By default, /bin/sh is used for the shell. This shell is controlled by the make variable SHELL but it is not inherited from the environment. When make starts, it imports all the variables from the user’s environment as make variables, except SHELL. This is because the user’s choice of shell should not cause a makefile (possibly included in some downloaded software package) to fail. If a user really wants to change the default shell used by make, he can set the SHELL variable explicitly in the makefile. We will discuss this issue in the section “Which Shell to Use” later in this chapter ------------------->8------------------------------
From your test case (the third one),
if SHELL is unset, it should work
whether /bin/sh is dash, bash, or whatever. (I use Ubuntu, and /bin/sh is dash. I see no problem for installing dtbs.)
We see the problem only when SHELL is set to /bin/dash.
foo-bar becomes empty if SHELL is set to /bin/dash in a Makefile that exports it.
SHELL in a Makefile that references $(foo-bar) is don't care.
The following is the test scripts.
---------------- Makefile ----------------
ifneq ($(WANTED_SHELL),)
SHELL := $(WANTED_SHELL)
endif
export foo_bar := foo_bar
export foo-bar := foo-bar
test:
@WANTED_SHELL= $(MAKE) test2
@WANTED_SHELL=/bin/dash $(MAKE) test2test2:
@echo SHELL of parent-Makefile is $(SHELL)
$(MAKE) -C sub CHILD_SHELL=/bin/sh
$(MAKE) -C sub CHILD_SHELL=/bin/dash-------------------------------------------
------------------ sub/Makefile -----------
SHELL := $(CHILD_SHELL)
sub:
@echo SHELL of sub-Makefile is $(SHELL)
@echo foo_bar $(foo_bar)
@echo foo-bar $(foo-bar).PHONY: sub
-------------------------------------------
My test result was as follows:
$ make --no-print-directory SHELL of parent-Makefile is /bin/sh make -C sub CHILD_SHELL=/bin/sh SHELL of sub-Makefile is /bin/sh foo_bar foo_bar foo-bar foo-bar make -C sub CHILD_SHELL=/bin/dash SHELL of sub-Makefile is /bin/dash foo_bar foo_bar foo-bar foo-bar SHELL of parent-Makefile is /bin/dash make -C sub CHILD_SHELL=/bin/sh SHELL of sub-Makefile is /bin/sh foo_bar foo_bar foo-bar make -C sub CHILD_SHELL=/bin/dash SHELL of sub-Makefile is /bin/dash foo_bar foo_bar foo-bar
I did "grep SHELL" in kernel sources, but I could not find suspicious line.
Is there anyone that sets SHELL
in debian specific patches?
Signature · 2 lines
Best Regards Masahiro Yamada
On Sun, 2017-08-20 at 02:37 +0900, Masahiro Yamada wrote:
[...]
Quoted 6 lines
I did "grep SHELL" in kernel sources, but I could not find suspicious line.
Is there anyone that sets SHELL
in debian specific patches?
No, there isn't. But I finally worked out the trigger conditions:
1. Source directory name includes a shell special character. This is true in Debian whenever we build a release candidate, because the source directory is named after the package version, and we use ~ to represent a pre-release version (e.g. 4.13~rc5 sorts before 4.13).
2. Object directory is outside the source directory, or at least two levels below it (see the definition of srctree in the top level Makefile). This is always true in Debian package builds.
Shell comamnds to reproduce this:
rsync --archive --exclude /.git/ . /tmp/linux~/
cd /tmp/linux~
make mrproper
make O=one/two ARCH=arm64 defconfig
make -C one/two ARCH=arm64 dtbs
make -C one/two ARCH=arm64 INSTALL_DTBS_PATH=dtbs~ dtbs_install
Ben.
Signature · 3 lines
Ben Hutchings The generation of random numbers is too important to be left to chance. - Robert Coveyou