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

[Masahiro Yamada]

On Mon, Jun 3, 2019 at 10:09 PM David Laight <David.Laight@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxx>
> > > > | 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@xxxxxxxxxx>.
> > > >
> > > > 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@xxxxxxxxxxxxxxx> # 5.1
> > > > Reported-by: Alexey Brodkin <abrodkin@xxxxxxxxxxxx>
> > > > Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> > > > ---
> > > >
> > > >  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