All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kbuild: Do not use hyphen in exported variable name
@ 2017-04-18  1:00 Ben Hutchings
  2017-04-18  4:44 ` Sam Ravnborg
  2017-04-23  6:47 ` Masahiro Yamada
  0 siblings, 2 replies; 10+ messages in thread
From: Ben Hutchings @ 2017-04-18  1:00 UTC (permalink / raw)
  To: linux-kbuild; +Cc: debian-kernel

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

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 <ben@decadent.org.uk>
---
--- a/scripts/Makefile.dtbinst
+++ b/scripts/Makefile.dtbinst
@@ -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
 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-18  1:00 [PATCH] kbuild: Do not use hyphen in exported variable name Ben Hutchings
@ 2017-04-18  4:44 ` Sam Ravnborg
  2017-04-23  6:47 ` Masahiro Yamada
  1 sibling, 0 replies; 10+ messages in thread
From: Sam Ravnborg @ 2017-04-18  4:44 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: linux-kbuild, debian-kernel

On Tue, Apr 18, 2017 at 02:00:11AM +0100, Ben Hutchings wrote:
> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-18  1:00 [PATCH] kbuild: Do not use hyphen in exported variable name Ben Hutchings
  2017-04-18  4:44 ` Sam Ravnborg
@ 2017-04-23  6:47 ` Masahiro Yamada
  2017-04-23  7:23   ` Ben Hutchings
  1 sibling, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2017-04-23  6:47 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel

Hi Ben,


2017-04-18 10:00 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> 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 <ben@decadent.org.uk>
> ---
> --- a/scripts/Makefile.dtbinst
> +++ b/scripts/Makefile.dtbinst
> @@ -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
>



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
> 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?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-23  6:47 ` Masahiro Yamada
@ 2017-04-23  7:23   ` Ben Hutchings
  2017-04-30 14:14     ` Masahiro Yamada
  0 siblings, 1 reply; 10+ messages in thread
From: Ben Hutchings @ 2017-04-23  7:23 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, debian-kernel

[-- Attachment #1: Type: text/plain, Size: 1511 bytes --]

On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
[...]
> 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
> > 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.

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.

> 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. :-)

> If so, why is this patch here?
> How is the dtbs_install procedure different in the Debian package?

This is the patch I applied to the package.

Ben.

-- 
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-23  7:23   ` Ben Hutchings
@ 2017-04-30 14:14     ` Masahiro Yamada
  2017-04-30 14:49       ` Ben Hutchings
  0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2017-04-30 14:14 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel

Hi Ben,


2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
> [...]
>> 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
>> > 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.
>
> 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.
>
>> 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. :-)
>
>> If so, why is this patch here?
>> How is the dtbs_install procedure 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.]




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-30 14:14     ` Masahiro Yamada
@ 2017-04-30 14:49       ` Ben Hutchings
  2017-05-03  4:47         ` Masahiro Yamada
  2017-08-19  1:13         ` Ben Hutchings
  0 siblings, 2 replies; 10+ messages in thread
From: Ben Hutchings @ 2017-04-30 14:49 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, debian-kernel

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
> Hi Ben,
> 
> 
> > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
> > [...]
> > > 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
> > > > 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.
> > 
> > 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.
> > 
> > > 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. :-)
> > 
> > > If so, why is this patch here?
> > > How is the dtbs_install procedure 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.

-- 
Ben Hutchings
This sentence contradicts itself - no actually it doesn't.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-30 14:49       ` Ben Hutchings
@ 2017-05-03  4:47         ` Masahiro Yamada
  2017-08-19  1:13         ` Ben Hutchings
  1 sibling, 0 replies; 10+ messages in thread
From: Masahiro Yamada @ 2017-05-03  4:47 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel

Hi Ben,


2017-04-30 23:49 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
>> Hi Ben,
>>
>>
>> > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
>> > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
>> > [...]
>> > > 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
>> > > > 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.
>> >
>> > 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.
>> >
>> > > 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. :-)
>> >
>> > > If so, why is this patch here?
>> > > How is the dtbs_install procedure 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.


I postpone this patch for now.
Please re-post if this patch turns out to be really needed.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-04-30 14:49       ` Ben Hutchings
  2017-05-03  4:47         ` Masahiro Yamada
@ 2017-08-19  1:13         ` Ben Hutchings
  2017-08-19 17:37           ` Masahiro Yamada
  1 sibling, 1 reply; 10+ messages in thread
From: Ben Hutchings @ 2017-08-19  1:13 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, debian-kernel

[-- Attachment #1: Type: text/plain, Size: 3528 bytes --]

On Sun, 2017-04-30 at 15:49 +0100, Ben Hutchings wrote:
> On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
> > Hi Ben,
> > 
> > 
> > > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> > > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
> > > [...]
> > > > 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
> > > > > 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.
> > > 
> > > 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.
> > > 
> > > > 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. :-)
> > > 
> > > > If so, why is this patch here?
> > > > How is the dtbs_install procedure 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.

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.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                       - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-08-19  1:13         ` Ben Hutchings
@ 2017-08-19 17:37           ` Masahiro Yamada
  2017-08-19 20:14             ` Ben Hutchings
  0 siblings, 1 reply; 10+ messages in thread
From: Masahiro Yamada @ 2017-08-19 17:37 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: Linux Kbuild mailing list, debian-kernel

Hi Ben,

Thanks for useful info.


2017-08-19 10:13 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
> On Sun, 2017-04-30 at 15:49 +0100, Ben Hutchings wrote:
>> On Sun, 2017-04-30 at 23:14 +0900, Masahiro Yamada wrote:
>> > Hi Ben,
>> >
>> >
>> > > 2017-04-23 16:23 GMT+09:00 Ben Hutchings <ben@decadent.org.uk>:
>> > > On Sun, 2017-04-23 at 15:47 +0900, Masahiro Yamada wrote:
>> > > [...]
>> > > > 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
>> > > > > 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.
>> > >
>> > > 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.
>> > >
>> > > > 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. :-)
>> > >
>> > > > If so, why is this patch here?
>> > > > How is the dtbs_install procedure 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.
>
> 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


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) test2

test2:
        @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?


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] kbuild: Do not use hyphen in exported variable name
  2017-08-19 17:37           ` Masahiro Yamada
@ 2017-08-19 20:14             ` Ben Hutchings
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Hutchings @ 2017-08-19 20:14 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, debian-kernel

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]

On Sun, 2017-08-20 at 02:37 +0900, Masahiro Yamada wrote:
[...]
> 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.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                       - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-08-19 20:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18  1:00 [PATCH] kbuild: Do not use hyphen in exported variable name Ben Hutchings
2017-04-18  4:44 ` Sam Ravnborg
2017-04-23  6:47 ` Masahiro Yamada
2017-04-23  7:23   ` Ben Hutchings
2017-04-30 14:14     ` Masahiro Yamada
2017-04-30 14:49       ` Ben Hutchings
2017-05-03  4:47         ` Masahiro Yamada
2017-08-19  1:13         ` Ben Hutchings
2017-08-19 17:37           ` Masahiro Yamada
2017-08-19 20:14             ` Ben Hutchings

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.