[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

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 at 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 at 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 at vger.kernel.org> # 5.1
Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
Signed-off-by: Masahiro Yamada <yamada.masahiro at 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))))
 
 # output directory for tests below
 TMPOUT := $(if $(KBUILD_EXTMOD),$(firstword $(KBUILD_EXTMOD))/)
-- 
2.17.1

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Alexey Brodkin]

Hi Masahiro-san,

> -----Original Message-----
> From: linux-snps-arc <linux-snps-arc-bounces at lists.infradead.org> On Behalf Of Masahiro Yamada
> Sent: Monday, June 3, 2019 1:49 PM
> To: linux-kbuild at vger.kernel.org
> Cc: Michal Marek <michal.lkml at markovi.net>; Vineet Gupta <vgupta at synopsys.com>; Alexey Brodkin
> <abrodkin at synopsys.com>; linux-kernel at vger.kernel.org; linux-stable <stable at vger.kernel.org>; Masahiro
> Yamada <yamada.masahiro at socionext.com>; linux-snps-arc at 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 at vger.kernel.org> # 5.1
> Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
> Signed-off-by: Masahiro Yamada <yamada.masahiro at socionext.com>
> ---

Thanks for the prompt fix - it does the trick, now no junk in the console!

Tested-by: Alexey Brodkin <abrodkin at synopsys.com>

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[David Laight]

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 at 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 at 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 at vger.kernel.org> # 5.1
> Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
> Signed-off-by: Masahiro Yamada <yamada.masahiro at 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)

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[David Laight]

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)

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

Hi David,

On Mon, Jun 3, 2019@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 at 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 at 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 at vger.kernel.org> # 5.1
> > Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
> > Signed-off-by: Masahiro Yamada <yamada.masahiro at 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)



-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

On Mon, Jun 3, 2019@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


-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[David Laight]

From: Masahiro Yamada
> Sent: 03 June 2019 12:45
> On Mon, Jun 3, 2019@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)

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[David Laight]

From: Masahiro Yamada
> Sent: 03 June 2019 12:38
> Hi David,
> 
> On Mon, Jun 3, 2019@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 at 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 at 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 at vger.kernel.org> # 5.1
> > > Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
> > > Signed-off-by: Masahiro Yamada <yamada.masahiro at 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)

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

On Mon, Jun 3, 2019@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@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 at 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 at 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 at vger.kernel.org> # 5.1
> > > > Reported-by: Alexey Brodkin <abrodkin at synopsys.com>
> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro at 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.




-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

On Mon, Jun 3, 2019@9:43 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Masahiro Yamada
> > Sent: 03 June 2019 12:45
> > On Mon, Jun 3, 2019@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)



-- 
Best Regards
Masahiro Yamada

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[David Laight]

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)

[PATCH] kbuild: use more portable 'command -v' for cc-cross-prefix

[Masahiro Yamada]

On Tue, Jun 4, 2019@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.




-- 
Best Regards
Masahiro Yamada