| Message ID | 20190603104902.23799-1-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kbuild: use more portable 'command -v' for cc-cross-prefix | expand |
Hi Masahiro-san, > -----Original Message----- > From: linux-snps-arc <linux-snps-arc-bounces@lists.infradead.org> On Behalf Of Masahiro Yamada > Sent: Monday, June 3, 2019 1:49 PM > To: linux-kbuild@vger.kernel.org > Cc: Michal Marek <michal.lkml@markovi.net>; Vineet Gupta <vgupta@synopsys.com>; Alexey Brodkin > <abrodkin@synopsys.com>; linux-kernel@vger.kernel.org; linux-stable <stable@vger.kernel.org>; Masahiro > Yamada <yamada.masahiro@socionext.com>; linux-snps-arc@lists.infradead.org > Subject: [PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix [snip] > Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") > Cc: linux-stable <stable@vger.kernel.org> # 5.1 > Reported-by: Alexey Brodkin <abrodkin@synopsys.com> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- Thanks for the prompt fix - it does the trick, now no junk in the console! Tested-by: Alexey Brodkin <abrodkin@synopsys.com>
From: Masahiro Yamada > Sent: 03 June 2019 11:49 > > To print the pathname that will be used by shell in the current > environment, 'command -v' is a standardized way. [1] > > 'which' is also often used in scripting, but it is not portable. > > When I worked on commit bd55f96fa9fc ("kbuild: refactor cc-cross-prefix > implementation"), I was eager to use 'command -v' but it did not work. > (The reason is explained below.) > > I kept 'which' as before but got rid of '> /dev/null 2>&1' as I > thought it was no longer needed. Sorry, I was wrong. > > It works well on my Ubuntu machine, but Alexey Brodkin reports annoying > warnings from the 'which' on CentOS 7 when the given command is not > found in the PATH environment. > > $ which foo > which: no foo in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) > > Given that behavior of 'which' is different on environment, I want > to try 'command -v' again. > > The specification [1] clearly describes the behavior of 'command -v' > when the given command is not found: > > Otherwise, no output shall be written and the exit status shall reflect > that the name was not found. > > However, we need a little magic to use 'command -v' from Make. > > $(shell ...) passes the argument to a subshell for execution, and > returns the standard output of the command. > > Here is a trick. GNU Make may optimize this by executing the command > directly instead of forking a subshell, if no shell special characters > are found in the command line and omitting the subshell will not > change the behavior. > > In this case, no shell special character is used. So, Make will try > to run the command directly. However, 'command' is a shell-builtin > command. In fact, Make has a table of shell-builtin commands because > it must spawn a subshell to execute them. > > Until recently, 'command' was missing in the table. > > This issue was fixed by the following commit: > > | commit 1af314465e5dfe3e8baa839a32a72e83c04f26ef > | Author: Paul Smith <psmith@gnu.org> > | Date: Sun Nov 12 18:10:28 2017 -0500 > | > | * job.c: Add "command" as a known shell built-in. > | > | This is not a POSIX shell built-in but it's common in UNIX shells. > | Reported by Nick Bowler <nbowler@draconx.ca>. > > This is not included in any released versions of Make yet. > (But, some distributions may have back-ported the fix-up.) > > To trick Make and let it fork the subshell, I added a shell special > character '~'. We may be able to get rid of this workaround someday, > but it is very far into the future. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html > > Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") > Cc: linux-stable <stable@vger.kernel.org> # 5.1 > Reported-by: Alexey Brodkin <abrodkin@synopsys.com> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > --- > > scripts/Kbuild.include | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > index 85d758233483..5a32ca80c3f6 100644 > --- a/scripts/Kbuild.include > +++ b/scripts/Kbuild.include > @@ -74,8 +74,11 @@ endef > # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) > # Return first <prefix> where a <prefix>gcc is found in PATH. > # If no gcc found in PATH with listed prefixes return nothing > +# > +# Note: the special character '~' forces Make to invoke a shell. This workaround > +# is needed because this issue was only fixed after GNU Make 4.2.1 release. > cc-cross-prefix = $(firstword $(foreach c, $(filter-out -%, $(1)), \ > - $(if $(shell which $(c)gcc), $(c)))) > + $(if $(shell command -v $(c)gcc ~), $(c)))) I see a problem here: command -v foo bar could be deemed to be an error (extra argument). You could use: $(shell sh -c "command -v $(c)gcc") or maybe: $(shell command$${x:+} -v $(c)gcc) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Masahiro Yamada > Sent: 03 June 2019 11:49 > > To print the pathname that will be used by shell in the current > environment, 'command -v' is a standardized way. [1] > > 'which' is also often used in scripting, but it is not portable. All uses of 'which' should be expunged. It is a bourne shell script that is trying to emulate a csh builtin. It is doomed to fail in corner cases. ISTR it has serious problems with shell functions and aliases. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Hi David, On Mon, Jun 3, 2019 at 8:14 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 03 June 2019 11:49 > > > > To print the pathname that will be used by shell in the current > > environment, 'command -v' is a standardized way. [1] > > > > 'which' is also often used in scripting, but it is not portable. > > > > When I worked on commit bd55f96fa9fc ("kbuild: refactor cc-cross-prefix > > implementation"), I was eager to use 'command -v' but it did not work. > > (The reason is explained below.) > > > > I kept 'which' as before but got rid of '> /dev/null 2>&1' as I > > thought it was no longer needed. Sorry, I was wrong. > > > > It works well on my Ubuntu machine, but Alexey Brodkin reports annoying > > warnings from the 'which' on CentOS 7 when the given command is not > > found in the PATH environment. > > > > $ which foo > > which: no foo in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) > > > > Given that behavior of 'which' is different on environment, I want > > to try 'command -v' again. > > > > The specification [1] clearly describes the behavior of 'command -v' > > when the given command is not found: > > > > Otherwise, no output shall be written and the exit status shall reflect > > that the name was not found. > > > > However, we need a little magic to use 'command -v' from Make. > > > > $(shell ...) passes the argument to a subshell for execution, and > > returns the standard output of the command. > > > > Here is a trick. GNU Make may optimize this by executing the command > > directly instead of forking a subshell, if no shell special characters > > are found in the command line and omitting the subshell will not > > change the behavior. > > > > In this case, no shell special character is used. So, Make will try > > to run the command directly. However, 'command' is a shell-builtin > > command. In fact, Make has a table of shell-builtin commands because > > it must spawn a subshell to execute them. > > > > Until recently, 'command' was missing in the table. > > > > This issue was fixed by the following commit: > > > > | commit 1af314465e5dfe3e8baa839a32a72e83c04f26ef > > | Author: Paul Smith <psmith@gnu.org> > > | Date: Sun Nov 12 18:10:28 2017 -0500 > > | > > | * job.c: Add "command" as a known shell built-in. > > | > > | This is not a POSIX shell built-in but it's common in UNIX shells. > > | Reported by Nick Bowler <nbowler@draconx.ca>. > > > > This is not included in any released versions of Make yet. > > (But, some distributions may have back-ported the fix-up.) > > > > To trick Make and let it fork the subshell, I added a shell special > > character '~'. We may be able to get rid of this workaround someday, > > but it is very far into the future. > > > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html > > > > Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") > > Cc: linux-stable <stable@vger.kernel.org> # 5.1 > > Reported-by: Alexey Brodkin <abrodkin@synopsys.com> > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > --- > > > > scripts/Kbuild.include | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > > index 85d758233483..5a32ca80c3f6 100644 > > --- a/scripts/Kbuild.include > > +++ b/scripts/Kbuild.include > > @@ -74,8 +74,11 @@ endef > > # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) > > # Return first <prefix> where a <prefix>gcc is found in PATH. > > # If no gcc found in PATH with listed prefixes return nothing > > +# > > +# Note: the special character '~' forces Make to invoke a shell. This workaround > > +# is needed because this issue was only fixed after GNU Make 4.2.1 release. > > cc-cross-prefix = $(firstword $(foreach c, $(filter-out -%, $(1)), \ > > - $(if $(shell which $(c)gcc), $(c)))) > > + $(if $(shell command -v $(c)gcc ~), $(c)))) > > I see a problem here: > command -v foo bar > could be deemed to be an error (extra argument). OK, the specification does not allow to pass arguments with -v. > You could use: > $(shell sh -c "command -v $(c)gcc") > or maybe: > $(shell command$${x:+} -v $(c)gcc) How about this? $(shell : ~; command -v $(c)gcc)
On Mon, Jun 3, 2019 at 8:16 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 03 June 2019 11:49 > > > > To print the pathname that will be used by shell in the current > > environment, 'command -v' is a standardized way. [1] > > > > 'which' is also often used in scripting, but it is not portable. > > All uses of 'which' should be expunged. > It is a bourne shell script that is trying to emulate a csh builtin. > It is doomed to fail in corner cases. > ISTR it has serious problems with shell functions and aliases. OK, I do not have time to check it treewide. I expect somebody will contribute to it. BTW, I see yet another way to get the command path. 'type -path' is bash-specific. Maybe, we should do this too: diff --git a/scripts/mkuboot.sh b/scripts/mkuboot.sh index 4b1fe09e9042..77829ee4268e 100755 --- a/scripts/mkuboot.sh +++ b/scripts/mkuboot.sh @@ -1,14 +1,14 @@ -#!/bin/bash +#!/bin/sh # SPDX-License-Identifier: GPL-2.0 # # Build U-Boot image when `mkimage' tool is available. # -MKIMAGE=$(type -path "${CROSS_COMPILE}mkimage") +MKIMAGE=$(command -v "${CROSS_COMPILE}mkimage") if [ -z "${MKIMAGE}" ]; then - MKIMAGE=$(type -path mkimage) + MKIMAGE=$(command -v mkimage) if [ -z "${MKIMAGE}" ]; then # Doesn't exist echo '"mkimage" command not found - U-Boot images will not be built' >&2
From: Masahiro Yamada > Sent: 03 June 2019 12:45 > On Mon, Jun 3, 2019 at 8:16 PM David Laight <David.Laight@aculab.com> wrote: > > > > From: Masahiro Yamada > > > Sent: 03 June 2019 11:49 > > > > > > To print the pathname that will be used by shell in the current > > > environment, 'command -v' is a standardized way. [1] > > > > > > 'which' is also often used in scripting, but it is not portable. > > > > All uses of 'which' should be expunged. > > It is a bourne shell script that is trying to emulate a csh builtin. > > It is doomed to fail in corner cases. > > ISTR it has serious problems with shell functions and aliases. > > OK, I do not have time to check it treewide. > I expect somebody will contribute to it. > > > > BTW, I see yet another way to get the command path. > > 'type -path' is bash-specific. 'type' itself should be supported by all shells, but the output format (esp for errors) probably varies. > Maybe, we should do this too: > > diff --git a/scripts/mkuboot.sh b/scripts/mkuboot.sh > index 4b1fe09e9042..77829ee4268e 100755 > --- a/scripts/mkuboot.sh > +++ b/scripts/mkuboot.sh > @@ -1,14 +1,14 @@ > -#!/bin/bash > +#!/bin/sh /bin/sh might be 'dash' - which is just plain broken in so many ways. Try (IIRC) ${foo%${foo#bar}} It might even be the original SYSV /bin/sh which doesn't support $((expr)) or ${foo#bar} - but that may break too much, but $SHELL might fix it. dash probably has the rather obscure bug in stripping '\n' from $(...) output that I found and fixed in NetBSD's ash may years ago. Try: foo="$(jot -b "" 130)" All 130 '\n' should be deleted. Mostly it fails to delete all the '\n', but it can remove extra ones! David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
From: Masahiro Yamada > Sent: 03 June 2019 12:38 > Hi David, > > On Mon, Jun 3, 2019 at 8:14 PM David Laight <David.Laight@aculab.com> wrote: > > > > From: Masahiro Yamada > > > Sent: 03 June 2019 11:49 > > > > > > To print the pathname that will be used by shell in the current > > > environment, 'command -v' is a standardized way. [1] > > > > > > 'which' is also often used in scripting, but it is not portable. > > > > > > When I worked on commit bd55f96fa9fc ("kbuild: refactor cc-cross-prefix > > > implementation"), I was eager to use 'command -v' but it did not work. > > > (The reason is explained below.) > > > > > > I kept 'which' as before but got rid of '> /dev/null 2>&1' as I > > > thought it was no longer needed. Sorry, I was wrong. > > > > > > It works well on my Ubuntu machine, but Alexey Brodkin reports annoying > > > warnings from the 'which' on CentOS 7 when the given command is not > > > found in the PATH environment. > > > > > > $ which foo > > > which: no foo in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) > > > > > > Given that behavior of 'which' is different on environment, I want > > > to try 'command -v' again. > > > > > > The specification [1] clearly describes the behavior of 'command -v' > > > when the given command is not found: > > > > > > Otherwise, no output shall be written and the exit status shall reflect > > > that the name was not found. > > > > > > However, we need a little magic to use 'command -v' from Make. > > > > > > $(shell ...) passes the argument to a subshell for execution, and > > > returns the standard output of the command. > > > > > > Here is a trick. GNU Make may optimize this by executing the command > > > directly instead of forking a subshell, if no shell special characters > > > are found in the command line and omitting the subshell will not > > > change the behavior. > > > > > > In this case, no shell special character is used. So, Make will try > > > to run the command directly. However, 'command' is a shell-builtin > > > command. In fact, Make has a table of shell-builtin commands because > > > it must spawn a subshell to execute them. > > > > > > Until recently, 'command' was missing in the table. > > > > > > This issue was fixed by the following commit: > > > > > > | commit 1af314465e5dfe3e8baa839a32a72e83c04f26ef > > > | Author: Paul Smith <psmith@gnu.org> > > > | Date: Sun Nov 12 18:10:28 2017 -0500 > > > | > > > | * job.c: Add "command" as a known shell built-in. > > > | > > > | This is not a POSIX shell built-in but it's common in UNIX shells. > > > | Reported by Nick Bowler <nbowler@draconx.ca>. > > > > > > This is not included in any released versions of Make yet. > > > (But, some distributions may have back-ported the fix-up.) > > > > > > To trick Make and let it fork the subshell, I added a shell special > > > character '~'. We may be able to get rid of this workaround someday, > > > but it is very far into the future. > > > > > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html > > > > > > Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") > > > Cc: linux-stable <stable@vger.kernel.org> # 5.1 > > > Reported-by: Alexey Brodkin <abrodkin@synopsys.com> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > > --- > > > > > > scripts/Kbuild.include | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > > > index 85d758233483..5a32ca80c3f6 100644 > > > --- a/scripts/Kbuild.include > > > +++ b/scripts/Kbuild.include > > > @@ -74,8 +74,11 @@ endef > > > # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) > > > # Return first <prefix> where a <prefix>gcc is found in PATH. > > > # If no gcc found in PATH with listed prefixes return nothing > > > +# > > > +# Note: the special character '~' forces Make to invoke a shell. This workaround > > > +# is needed because this issue was only fixed after GNU Make 4.2.1 release. > > > cc-cross-prefix = $(firstword $(foreach c, $(filter-out -%, $(1)), \ > > > - $(if $(shell which $(c)gcc), $(c)))) > > > + $(if $(shell command -v $(c)gcc ~), $(c)))) > > > > I see a problem here: > > command -v foo bar > > could be deemed to be an error (extra argument). > > OK, the specification does not allow to pass arguments > with -v. > > > > You could use: > > $(shell sh -c "command -v $(c)gcc") > > or maybe: > > $(shell command$${x:+} -v $(c)gcc) > > > How about this? > > $(shell : ~; command -v $(c)gcc) Overcomplicated .... I've not looked at the list of 'special characters' in make, but I suspect any variable expansion is enough. Since ${x:+} always expands to the empty string (whether or not 'x' is defined) it can't have any unfortunate side effects. I'd comment as: # Note: ${x:+} always expands to the empty string and forces all # versions of make to actually exec $SHELL rather than try to # directly execute the shell builtin 'command'. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Mon, Jun 3, 2019 at 10:09 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 03 June 2019 12:38 > > Hi David, > > > > On Mon, Jun 3, 2019 at 8:14 PM David Laight <David.Laight@aculab.com> wrote: > > > > > > From: Masahiro Yamada > > > > Sent: 03 June 2019 11:49 > > > > > > > > To print the pathname that will be used by shell in the current > > > > environment, 'command -v' is a standardized way. [1] > > > > > > > > 'which' is also often used in scripting, but it is not portable. > > > > > > > > When I worked on commit bd55f96fa9fc ("kbuild: refactor cc-cross-prefix > > > > implementation"), I was eager to use 'command -v' but it did not work. > > > > (The reason is explained below.) > > > > > > > > I kept 'which' as before but got rid of '> /dev/null 2>&1' as I > > > > thought it was no longer needed. Sorry, I was wrong. > > > > > > > > It works well on my Ubuntu machine, but Alexey Brodkin reports annoying > > > > warnings from the 'which' on CentOS 7 when the given command is not > > > > found in the PATH environment. > > > > > > > > $ which foo > > > > which: no foo in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) > > > > > > > > Given that behavior of 'which' is different on environment, I want > > > > to try 'command -v' again. > > > > > > > > The specification [1] clearly describes the behavior of 'command -v' > > > > when the given command is not found: > > > > > > > > Otherwise, no output shall be written and the exit status shall reflect > > > > that the name was not found. > > > > > > > > However, we need a little magic to use 'command -v' from Make. > > > > > > > > $(shell ...) passes the argument to a subshell for execution, and > > > > returns the standard output of the command. > > > > > > > > Here is a trick. GNU Make may optimize this by executing the command > > > > directly instead of forking a subshell, if no shell special characters > > > > are found in the command line and omitting the subshell will not > > > > change the behavior. > > > > > > > > In this case, no shell special character is used. So, Make will try > > > > to run the command directly. However, 'command' is a shell-builtin > > > > command. In fact, Make has a table of shell-builtin commands because > > > > it must spawn a subshell to execute them. > > > > > > > > Until recently, 'command' was missing in the table. > > > > > > > > This issue was fixed by the following commit: > > > > > > > > | commit 1af314465e5dfe3e8baa839a32a72e83c04f26ef > > > > | Author: Paul Smith <psmith@gnu.org> > > > > | Date: Sun Nov 12 18:10:28 2017 -0500 > > > > | > > > > | * job.c: Add "command" as a known shell built-in. > > > > | > > > > | This is not a POSIX shell built-in but it's common in UNIX shells. > > > > | Reported by Nick Bowler <nbowler@draconx.ca>. > > > > > > > > This is not included in any released versions of Make yet. > > > > (But, some distributions may have back-ported the fix-up.) > > > > > > > > To trick Make and let it fork the subshell, I added a shell special > > > > character '~'. We may be able to get rid of this workaround someday, > > > > but it is very far into the future. > > > > > > > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html > > > > > > > > Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") > > > > Cc: linux-stable <stable@vger.kernel.org> # 5.1 > > > > Reported-by: Alexey Brodkin <abrodkin@synopsys.com> > > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> > > > > --- > > > > > > > > scripts/Kbuild.include | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include > > > > index 85d758233483..5a32ca80c3f6 100644 > > > > --- a/scripts/Kbuild.include > > > > +++ b/scripts/Kbuild.include > > > > @@ -74,8 +74,11 @@ endef > > > > # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) > > > > # Return first <prefix> where a <prefix>gcc is found in PATH. > > > > # If no gcc found in PATH with listed prefixes return nothing > > > > +# > > > > +# Note: the special character '~' forces Make to invoke a shell. This workaround > > > > +# is needed because this issue was only fixed after GNU Make 4.2.1 release. > > > > cc-cross-prefix = $(firstword $(foreach c, $(filter-out -%, $(1)), \ > > > > - $(if $(shell which $(c)gcc), $(c)))) > > > > + $(if $(shell command -v $(c)gcc ~), $(c)))) > > > > > > I see a problem here: > > > command -v foo bar > > > could be deemed to be an error (extra argument). > > > > OK, the specification does not allow to pass arguments > > with -v. > > > > > > > You could use: > > > $(shell sh -c "command -v $(c)gcc") > > > or maybe: > > > $(shell command$${x:+} -v $(c)gcc) > > > > > > How about this? > > > > $(shell : ~; command -v $(c)gcc) > > Overcomplicated .... > > I've not looked at the list of 'special characters' in make, > but I suspect any variable expansion is enough. > Since ${x:+} always expands to the empty string (whether or > not 'x' is defined) it can't have any unfortunate side effects. Probably, my eyes are used to Makefile. ":" is a no-op command, and it is used everywhere in kernel Makefiles in the form of "@:' It depends on people which solution seems simpler. So, this argument tends to end up with bikesheding.
On Mon, Jun 3, 2019 at 9:43 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 03 June 2019 12:45 > > On Mon, Jun 3, 2019 at 8:16 PM David Laight <David.Laight@aculab.com> wrote: > > > > > > From: Masahiro Yamada > > > > Sent: 03 June 2019 11:49 > > > > > > > > To print the pathname that will be used by shell in the current > > > > environment, 'command -v' is a standardized way. [1] > > > > > > > > 'which' is also often used in scripting, but it is not portable. > > > > > > All uses of 'which' should be expunged. > > > It is a bourne shell script that is trying to emulate a csh builtin. > > > It is doomed to fail in corner cases. > > > ISTR it has serious problems with shell functions and aliases. > > > > OK, I do not have time to check it treewide. > > I expect somebody will contribute to it. > > > > > > > > BTW, I see yet another way to get the command path. > > > > 'type -path' is bash-specific. > > 'type' itself should be supported by all shells, but the output > format (esp for errors) probably varies. > > > Maybe, we should do this too: > > > > diff --git a/scripts/mkuboot.sh b/scripts/mkuboot.sh > > index 4b1fe09e9042..77829ee4268e 100755 > > --- a/scripts/mkuboot.sh > > +++ b/scripts/mkuboot.sh > > @@ -1,14 +1,14 @@ > > -#!/bin/bash > > +#!/bin/sh > > /bin/sh might be 'dash' - which is just plain broken in so many ways. > Try (IIRC) ${foo%${foo#bar}} > It might even be the original SYSV /bin/sh which doesn't support $((expr)) > or ${foo#bar} - but that may break too much, but $SHELL might fix it. We cannot use any tool if you start to argue like "Hey, I know ancient implementation that did not work as expected". Nobody can cover all corner-cases. That's why we have standard. I think the reliable source is the Open Group Specification. The behavior of /bin/sh is defined here: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_01 ${parameter%[word]} and ${parameter#[word]} are defined, so we can use them in /bin/sh scripts. > dash probably has the rather obscure bug in stripping '\n' from $(...) > output that I found and fixed in NetBSD's ash may years ago. > Try: foo="$(jot -b "" 130)" > All 130 '\n' should be deleted. > Mostly it fails to delete all the '\n', but it can remove extra ones! > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales)
From: Masahiro Yamada > Sent: 04 June 2019 04:31 ... > > > > You could use: > > > > $(shell sh -c "command -v $(c)gcc") > > > > or maybe: > > > > $(shell command$${x:+} -v $(c)gcc) > > > > > > > > > How about this? > > > > > > $(shell : ~; command -v $(c)gcc) > > > > Overcomplicated .... > > > > I've not looked at the list of 'special characters' in make, > > but I suspect any variable expansion is enough. > > Since ${x:+} always expands to the empty string (whether or > > not 'x' is defined) it can't have any unfortunate side effects. > > > Probably, my eyes are used to Makefile. > ":" is a no-op command, and it is used everywhere in kernel Makefiles > in the form of "@:' > > It depends on people which solution seems simpler. > So, this argument tends to end up with bikesheding. I am fully aware of ':', it is a shell builtin that always return success. Usually used when you want the side-effects of substitutions without executing anything (eg : ${foo:=bar} ), to change the result of a sequence of shell commands or as a dummy (eg while :; do :; done; ) Very annoyingly bash parses !: as something other than 'not true'. $(shell command$${x:+} -v $(c)gcc) will be marginally faster because it is less parsing. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, Jun 4, 2019 at 6:01 PM David Laight <David.Laight@aculab.com> wrote: > > From: Masahiro Yamada > > Sent: 04 June 2019 04:31 > ... > > > > > You could use: > > > > > $(shell sh -c "command -v $(c)gcc") > > > > > or maybe: > > > > > $(shell command$${x:+} -v $(c)gcc) > > > > > > > > > > > > How about this? > > > > > > > > $(shell : ~; command -v $(c)gcc) > > > > > > Overcomplicated .... > > > > > > I've not looked at the list of 'special characters' in make, > > > but I suspect any variable expansion is enough. > > > Since ${x:+} always expands to the empty string (whether or > > > not 'x' is defined) it can't have any unfortunate side effects. > > > > > > Probably, my eyes are used to Makefile. > > ":" is a no-op command, and it is used everywhere in kernel Makefiles > > in the form of "@:' > > > > It depends on people which solution seems simpler. > > So, this argument tends to end up with bikesheding. > > I am fully aware of ':', it is a shell builtin that always return success. > Usually used when you want the side-effects of substitutions without > executing anything (eg : ${foo:=bar} ), to change the result of a > sequence of shell commands or as a dummy (eg while :; do :; done; ) > Very annoyingly bash parses !: as something other than 'not true'. > > $(shell command$${x:+} -v $(c)gcc) will be marginally faster > because it is less parsing. > I will use this: $(shell command -v $(c)gcc 2>/dev/null) Make does not handle redirection by itself. '2>/dev/null' is easy to understand, and might be useful as extra safety.
diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 85d758233483..5a32ca80c3f6 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -74,8 +74,11 @@ endef # Usage: CROSS_COMPILE := $(call cc-cross-prefix, m68k-linux-gnu- m68k-linux-) # Return first <prefix> where a <prefix>gcc is found in PATH. # If no gcc found in PATH with listed prefixes return nothing +# +# Note: the special character '~' forces Make to invoke a shell. This workaround +# is needed because this issue was only fixed after GNU Make 4.2.1 release. cc-cross-prefix = $(firstword $(foreach c, $(filter-out -%, $(1)), \ - $(if $(shell which $(c)gcc), $(c)))) + $(if $(shell command -v $(c)gcc ~), $(c)))) # output directory for tests below TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
To print the pathname that will be used by shell in the current environment, 'command -v' is a standardized way. [1] 'which' is also often used in scripting, but it is not portable. When I worked on commit bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation"), I was eager to use 'command -v' but it did not work. (The reason is explained below.) I kept 'which' as before but got rid of '> /dev/null 2>&1' as I thought it was no longer needed. Sorry, I was wrong. It works well on my Ubuntu machine, but Alexey Brodkin reports annoying warnings from the 'which' on CentOS 7 when the given command is not found in the PATH environment. $ which foo which: no foo in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin) Given that behavior of 'which' is different on environment, I want to try 'command -v' again. The specification [1] clearly describes the behavior of 'command -v' when the given command is not found: Otherwise, no output shall be written and the exit status shall reflect that the name was not found. However, we need a little magic to use 'command -v' from Make. $(shell ...) passes the argument to a subshell for execution, and returns the standard output of the command. Here is a trick. GNU Make may optimize this by executing the command directly instead of forking a subshell, if no shell special characters are found in the command line and omitting the subshell will not change the behavior. In this case, no shell special character is used. So, Make will try to run the command directly. However, 'command' is a shell-builtin command. In fact, Make has a table of shell-builtin commands because it must spawn a subshell to execute them. Until recently, 'command' was missing in the table. This issue was fixed by the following commit: | commit 1af314465e5dfe3e8baa839a32a72e83c04f26ef | Author: Paul Smith <psmith@gnu.org> | Date: Sun Nov 12 18:10:28 2017 -0500 | | * job.c: Add "command" as a known shell built-in. | | This is not a POSIX shell built-in but it's common in UNIX shells. | Reported by Nick Bowler <nbowler@draconx.ca>. This is not included in any released versions of Make yet. (But, some distributions may have back-ported the fix-up.) To trick Make and let it fork the subshell, I added a shell special character '~'. We may be able to get rid of this workaround someday, but it is very far into the future. [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html Fixes: bd55f96fa9fc ("kbuild: refactor cc-cross-prefix implementation") Cc: linux-stable <stable@vger.kernel.org> # 5.1 Reported-by: Alexey Brodkin <abrodkin@synopsys.com> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> --- scripts/Kbuild.include | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)