[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] kbuild: Do not use hyphen in exported variable name



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


Reply to: