lkml.org 
[lkml]   [2022]   [Aug]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] init/Kconfig: fix CC_HAS_ASM_GOTO_TIED_OUTPUT test with dash
On Thu, Aug 18, 2022 at 6:14 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Wed, 2022-08-17 at 16:52 -0700, Randy Dunlap wrote:
> >
> > On 8/17/22 16:46, Nick Desaulniers wrote:
> > > On Thu, Aug 4, 2022 at 12:03 PM <alexandre.belloni@bootlin.com> wrote:
> > > >
> > > > From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > > >
> > > > When using dash as /bin/sh, the CC_HAS_ASM_GOTO_TIED_OUTPUT test fails
> > > > with a syntax error which is not the one we are looking for:
> > >
> > > Thanks for the patch, though I think I'd rather see `/bin/bash`
> > > hardcoded. Bash is a non-optional requirement as of
> > > commit da4288b95baa ("scripts/check-local-export: avoid 'wait $!' for
> > > process substitution")
> > > scripts/ is kind of a mess...
> > >
> >
> > Well, once upon a time, we took patches to remove bash-isms (convert to
> > standard shell)...
> > No longer, AFAICT.
>
> This problem is a little more subtle.
>
> As far as I could work out, exec() is used on entries like this in
> kConfig. exec() falls back to /bin/sh so it is hard to see where this
> would be changed to be /bin/bash.



Kconfig uses popen() to execute a shell command.

See do_shell() in scripts/kconfig/preprocess.c



popen(3) says that
"the command is passed to /bin/sh using the -c flag.
interpretation, if any, is performed by the shell."



GNU Make is the same.
Make uses /bin/sh to execute commands in
recipe lines and $(shell ...) functions.
You can change the default shell via 'SHELL' variable.





>
> I have no issue with bash being required and used and if someone can
> work out how to make that happen for the exec() calls, I'm fine with
> that. It would probably require some parsing of the "code" being passed
> to kConfig to decide how to call exec().
>
> What worries me is seeing the kernel behaviour changing depending on
> whether /bin/sh is dash or bash and I think that should be fixed as a
> more urgent matter.
>
> I'd hope Alexandre's patch could be taken in the meantime as it doesn't
> really hurt anything and does fix a very unexpected behaviour change
> depending on the host system setup.



Agree.

We should apply a simple patch to fix this particular case
(I suggest to drop '\n' unless there is a reason to have it)
but discussion is needed to avoid portability issues like this.






One way is to encourage writing really portable code.
We used to strive for this (so we avoided bashism where possible)
because we believed sticking to POSIX was always good.

Some people make an effort in this direction [1], stating that
bash may not be installed, and bash is slower than dash.

In shell commands, we can use only commands/options defined in POSIX.
This is fragile because we do not have real /bin/sh,
and it is difficult to know what is POSIX-compliant.

People submit a script with #!/bin/sh but only tested on
environments where /bin/sh is a symlink to bash.








Another (and the opposite) way is to force users to use a particular
program like bash.

Actually, Googlers suggest this way.

Shell Style Guide [2] says:
"Bash is the only shell scripting language permitted for executables."


If we are forced to use bash, it should work in the same way for everyone.
We do not need to know what is POSIX-defined, and what is bashism.

I was thinking this way for a long time.
I wrote some patches to switch the default shell for Kbuild and Kconfig to bash
some time ago, but I did not submit them after some consideration.
I have patches in my local repository, I can share them.







BTW, Richard is here, so let me ask about BitBake.

The manual [3] clearly says:

"When you create these types of functions in your recipe or class files,
you need to follow the shell programming rules. The scripts are
executed by /bin/sh,
which may not be a bash shell but might be something such as dash.
You should not use Bash-specific script (bashisms)"

I just thought BitBake ran shell code in bash before,
but I might have misunderstood.
Do OE/Yocto allow only POSIX shell code?




[1]: https://lore.kernel.org/linux-kbuild/CAK7LNAT+4fOkJ5WDb9t5qXCqS+GhnbnG8wBffxNa1ZJ3=4Ps3Q@mail.gmail.com/T/#m74e382837a8a47a2278d892bc5d7f8bdbb86dba4
[2]: https://google.github.io/styleguide/shellguide.html
[3]: https://docs.yoctoproject.org/1.6/bitbake-user-manual/bitbake-user-manual.html




--
Best Regards
Masahiro Yamada

\
 
 \ /
  Last update: 2022-08-18 21:34    [W:0.138 / U:0.448 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site