[PATCH] kbuild: document KBUILD_SHELL

[PATCH] kbuild: document KBUILD_SHELL

[Masahiro Yamada]

CONFIG_SHELL was renamed to KBUILD_SHELL.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Michal Marek <mmarek@suse.cz>
---
 Documentation/kbuild/kbuild.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
index 6466704..425cd2e 100644
--- a/Documentation/kbuild/kbuild.txt
+++ b/Documentation/kbuild/kbuild.txt
@@ -233,3 +233,8 @@ KBUILD_VMLINUX_MAIN
 All object files for the main part of vmlinux.
 KBUILD_VMLINUX_INIT and KBUILD_VMLINUX_MAIN together specify
 all the object files used to link vmlinux.
+
+KBUILD_SHELL
+--------------------------------------------------
+Specify which shell to use to run shell scripts in Kbuild.
+Assigned by the top-level Makefile.
-- 
1.9.1

Re: [PATCH] kbuild: document KBUILD_SHELL

[Michal Marek]

On 2014-06-10 04:51, Masahiro Yamada wrote:
> CONFIG_SHELL was renamed to KBUILD_SHELL.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Michal Marek <mmarek@suse.cz>
> ---
>  Documentation/kbuild/kbuild.txt | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/kbuild/kbuild.txt b/Documentation/kbuild/kbuild.txt
> index 6466704..425cd2e 100644
> --- a/Documentation/kbuild/kbuild.txt
> +++ b/Documentation/kbuild/kbuild.txt
> @@ -233,3 +233,8 @@ KBUILD_VMLINUX_MAIN
>  All object files for the main part of vmlinux.
>  KBUILD_VMLINUX_INIT and KBUILD_VMLINUX_MAIN together specify
>  all the object files used to link vmlinux.
> +
> +KBUILD_SHELL
> +--------------------------------------------------
> +Specify which shell to use to run shell scripts in Kbuild.
> +Assigned by the top-level Makefile.

The variable is assinged with a ":=", not a "?=", so you can't change it
from outside, can you? IMO it should not be documented so as not to
confuse users.

Michal

Re: [PATCH] kbuild: document KBUILD_SHELL

[Sam Ravnborg]

> 
> The variable is assinged with a ":=", not a "?=", so you can't change it
> from outside, can you?

Variables assigned with ":=" can be changed like this:

$ cat Makefile
FOO := fisk
$(info FOO=$(FOO))
all:
	@:

$ FOO=bar make
FOO=fisk

$ make FOO=bar
FOO=bar


The first is the same as using an environment variable.
The latter is a special make syntax.

	Sam

Re: [PATCH] kbuild: document KBUILD_SHELL

[Michal Marek]

On 2014-06-10 11:50, Sam Ravnborg wrote:
>>
>> The variable is assinged with a ":=", not a "?=", so you can't change it
>> from outside, can you?
> 
> Variables assigned with ":=" can be changed like this:
> 
> $ cat Makefile
> FOO := fisk
> $(info FOO=$(FOO))
> all:
> 	@:
> 
> $ FOO=bar make
> FOO=fisk
> 
> $ make FOO=bar
> FOO=bar
> 
> 
> The first is the same as using an environment variable.
> The latter is a special make syntax.

I see. But do we want to encourage people to change the value? IMO There
should be some "change this variable only if you know what you are
doing" warning.

Michal

Re: [PATCH] kbuild: document KBUILD_SHELL

[Masahiro Yamada]

Hi Michal, Sam,

On Tue, 10 Jun 2014 13:00:45 +0200
Michal Marek <mmarek@suse.cz> wrote:

> On 2014-06-10 11:50, Sam Ravnborg wrote:
> >>
> >> The variable is assinged with a ":=", not a "?=", so you can't change it
> >> from outside, can you?
> > 
> > Variables assigned with ":=" can be changed like this:
> > 
> > $ cat Makefile
> > FOO := fisk
> > $(info FOO=$(FOO))
> > all:
> > 	@:
> > 
> > $ FOO=bar make
> > FOO=fisk
> > 
> > $ make FOO=bar
> > FOO=bar
> > 
> > 
> > The first is the same as using an environment variable.
> > The latter is a special make syntax.
> 
> I see. But do we want to encourage people to change the value? IMO There
> should be some "change this variable only if you know what you are
> doing" warning.
> 
> Michal

IMHO:
If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
 "KBUILD_SHELL" should always be set to "/bin/sh" and
users should not change it.

I still don't understand why bash is preferable for KBUILD_SHELL.


Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: document KBUILD_SHELL

[Michal Marek]

On 2014-06-10 13:17, Masahiro Yamada wrote:
> IMHO:
> If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
>  "KBUILD_SHELL" should always be set to "/bin/sh" and
> users should not change it.
> 
> I still don't understand why bash is preferable for KBUILD_SHELL.

I'm just speculating, but the reason might have been that if you are
compiling Linux on some oddball UNIX system, the POSIX shell might not
be "/bin/sh", but some other path, who knows which. But if $BASH is
defined or if there is /bin/bash, then it's very likely the familiar GNU
Bash. Hence the preference. Of course, the side effect is that it makes
it easy to introduce bash-only constructs into the scripts :-/.

Michal

Re: [PATCH] kbuild: document KBUILD_SHELL

[Masahiro Yamada]

Hi Michal,

On Tue, 10 Jun 2014 13:36:48 +0200
Michal Marek <mmarek@suse.cz> wrote:

> On 2014-06-10 13:17, Masahiro Yamada wrote:
> > IMHO:
> > If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
> >  "KBUILD_SHELL" should always be set to "/bin/sh" and
> > users should not change it.
> > 
> > I still don't understand why bash is preferable for KBUILD_SHELL.
> 
> I'm just speculating, but the reason might have been that if you are
> compiling Linux on some oddball UNIX system, the POSIX shell might not
> be "/bin/sh", but some other path, who knows which. But if $BASH is
> defined or if there is /bin/bash, then it's very likely the familiar GNU
> Bash. Hence the preference. Of course, the side effect is that it makes
> it easy to introduce bash-only constructs into the scripts :-/.

Hmm, 
We set the default value to /bin/sh  (KBUILD_SHELL ?= /bin/sh)
but allowing oddball system users to override it like,
export  KBUILD_SHELL=/bin/bash; make

Does this sounds reasonable?

I am not sure how much impact it is...

Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: document KBUILD_SHELL

[Michal Marek]

On 2014-06-10 14:02, Masahiro Yamada wrote:
> Hi Michal,
> 
> On Tue, 10 Jun 2014 13:36:48 +0200
> Michal Marek <mmarek@suse.cz> wrote:
> 
>> On 2014-06-10 13:17, Masahiro Yamada wrote:
>>> IMHO:
>>> If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
>>>  "KBUILD_SHELL" should always be set to "/bin/sh" and
>>> users should not change it.
>>>
>>> I still don't understand why bash is preferable for KBUILD_SHELL.
>>
>> I'm just speculating, but the reason might have been that if you are
>> compiling Linux on some oddball UNIX system, the POSIX shell might not
>> be "/bin/sh", but some other path, who knows which. But if $BASH is
>> defined or if there is /bin/bash, then it's very likely the familiar GNU
>> Bash. Hence the preference. Of course, the side effect is that it makes
>> it easy to introduce bash-only constructs into the scripts :-/.
> 
> Hmm, 
> We set the default value to /bin/sh  (KBUILD_SHELL ?= /bin/sh)
> but allowing oddball system users to override it like,
> export  KBUILD_SHELL=/bin/bash; make
> 
> Does this sounds reasonable?

I'm not against it in principle, but it will have to wait for the next
merge window, so that it sees more testing in linux-next. I'd like to
push the current set of changes to Linus and time is getting tight.

Michal

Re: [PATCH] kbuild: document KBUILD_SHELL

[Masahiro Yamada]

Hi Michal,


On Tue, 10 Jun 2014 14:22:00 +0200
Michal Marek <mmarek@suse.cz> wrote:

> On 2014-06-10 14:02, Masahiro Yamada wrote:
> > Hi Michal,
> > 
> > On Tue, 10 Jun 2014 13:36:48 +0200
> > Michal Marek <mmarek@suse.cz> wrote:
> > 
> >> On 2014-06-10 13:17, Masahiro Yamada wrote:
> >>> IMHO:
> >>> If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
> >>>  "KBUILD_SHELL" should always be set to "/bin/sh" and
> >>> users should not change it.
> >>>
> >>> I still don't understand why bash is preferable for KBUILD_SHELL.
> >>
> >> I'm just speculating, but the reason might have been that if you are
> >> compiling Linux on some oddball UNIX system, the POSIX shell might not
> >> be "/bin/sh", but some other path, who knows which. But if $BASH is
> >> defined or if there is /bin/bash, then it's very likely the familiar GNU
> >> Bash. Hence the preference. Of course, the side effect is that it makes
> >> it easy to introduce bash-only constructs into the scripts :-/.
> > 
> > Hmm, 
> > We set the default value to /bin/sh  (KBUILD_SHELL ?= /bin/sh)
> > but allowing oddball system users to override it like,
> > export  KBUILD_SHELL=/bin/bash; make
> > 
> > Does this sounds reasonable?
> 
> I'm not against it in principle, but it will have to wait for the next
> merge window, so that it sees more testing in linux-next. I'd like to
> push the current set of changes to Linus and time is getting tight.
> 

Right.

We do not need to rush to apply this patch now.


Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: document KBUILD_SHELL

[Sam Ravnborg]

On Tue, Jun 10, 2014 at 01:36:48PM +0200, Michal Marek wrote:
> On 2014-06-10 13:17, Masahiro Yamada wrote:
> > IMHO:
> > If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
> >  "KBUILD_SHELL" should always be set to "/bin/sh" and
> > users should not change it.
> > 
> > I still don't understand why bash is preferable for KBUILD_SHELL.
> 
> I'm just speculating, but the reason might have been that if you are
> compiling Linux on some oddball UNIX system, the POSIX shell might not
> be "/bin/sh", but some other path, who knows which. But if $BASH is
> defined or if there is /bin/bash, then it's very likely the familiar GNU
> Bash. Hence the preference. Of course, the side effect is that it makes
> it easy to introduce bash-only constructs into the scripts :-/.

It has been like this forever...
The reasons for having CONFIG_SHELL are:
- with a distribution where /bin/sh does not work with the shells provided
  by the kernel - the user can override this to select another shell.
- avoid hardcoding /bin/sh in all the places where we run a script

I recall we had problems with bash only stuff when ubuntu changed to dash.
But back then the fix was always to remove the bashism from the scripts.

IMO we should document that CONFG_SHELL/KBUILD_SHELL exists.
But that the user needs to modify the top-level Makefiel to change it
is IMO OK as this is seldomly if ever required.

	Sam

Re: [PATCH] kbuild: document KBUILD_SHELL

[Masahiro Yamada]

Hi Michal, Sam,


On Tue, 10 Jun 2014 14:22:00 +0200
Michal Marek <mmarek@suse.cz> wrote:

> On 2014-06-10 14:02, Masahiro Yamada wrote:
> > Hi Michal,
> > 
> > On Tue, 10 Jun 2014 13:36:48 +0200
> > Michal Marek <mmarek@suse.cz> wrote:
> > 
> >> On 2014-06-10 13:17, Masahiro Yamada wrote:
> >>> IMHO:
> >>> If all shell scripts invoked by $KBUILD_SHELL should be sh-compatible,
> >>>  "KBUILD_SHELL" should always be set to "/bin/sh" and
> >>> users should not change it.
> >>>
> >>> I still don't understand why bash is preferable for KBUILD_SHELL.
> >>
> >> I'm just speculating, but the reason might have been that if you are
> >> compiling Linux on some oddball UNIX system, the POSIX shell might not
> >> be "/bin/sh", but some other path, who knows which. But if $BASH is
> >> defined or if there is /bin/bash, then it's very likely the familiar GNU
> >> Bash. Hence the preference. Of course, the side effect is that it makes
> >> it easy to introduce bash-only constructs into the scripts :-/.
> > 
> > Hmm, 
> > We set the default value to /bin/sh  (KBUILD_SHELL ?= /bin/sh)
> > but allowing oddball system users to override it like,
> > export  KBUILD_SHELL=/bin/bash; make
> > 
> > Does this sounds reasonable?
> 
> I'm not against it in principle, but it will have to wait for the next
> merge window, so that it sees more testing in linux-next. I'd like to
> push the current set of changes to Linus and time is getting tight.
> 
> Michal


Now the merge window is closed, so I'd like to resume this topic.

Before that, I have a question.
Sam mentioned as follows:

On Mon, 9 Jun 2014 13:40:10 +0200
Sam Ravnborg <sam@ravnborg.org> wrote:
> All shell scripts that are invoked with $(CONFIG_SHELL) must be sh-compatible,
> or in practice dash compatible as well as bash compatible.
> The preference is bash as expressed with the above code.


I notice at least two files (scripts/coccicheck and scripts/mkuboot.sh)
have shebang "#!/bin/bash" even though they are invoked with $(CONFIG_SHELL).

Should we change them to "#!/bin/sh" ?

In that case, we also have to modify some lines where bash-extension
is used.

COCCIINCLUDE=${LINUXINCLUDE//-I/-I }
COCCIINCLUDE=${COCCIINCLUDE//-include/-I}

I think fixing them is not difficult.



Best Regards
Masahiro Yamada

Re: [PATCH] kbuild: document KBUILD_SHELL

[Michal Marek]

Dne 26.6.2014 04:02, Masahiro Yamada napsal(a):
> On Mon, 9 Jun 2014 13:40:10 +0200
> Sam Ravnborg <sam@ravnborg.org> wrote:
>> All shell scripts that are invoked with $(CONFIG_SHELL) must be sh-compatible,
>> or in practice dash compatible as well as bash compatible.
>> The preference is bash as expressed with the above code.
> 
> 
> I notice at least two files (scripts/coccicheck and scripts/mkuboot.sh)
> have shebang "#!/bin/bash" even though they are invoked with $(CONFIG_SHELL).
> 
> Should we change them to "#!/bin/sh" ?

Sorry for the late reply.

If we are going to change the CONFIG_SHELL (KBUILD_SHELL) variable to
/bin/sh, then yes, please change them to /bin/sh and make them dash
compatible.


> In that case, we also have to modify some lines where bash-extension
> is used.
> 
> COCCIINCLUDE=${LINUXINCLUDE//-I/-I }
> COCCIINCLUDE=${COCCIINCLUDE//-include/-I}
> 
> I think fixing them is not difficult.

Right, nothing a simple sed call wouldn't fix.

Thanks,
Michal