Re: [Buildroot] [RFC PATCH 0/2] use `command -v' instead of `which'

[Petr Vorel <petr.vorel@gmail.com>]

Hi Arnout, Yann, all,

<snip>

> > > which we have in package/Makefile.in@240. In this case, make will try to
> > NOTE for myself (when debugging this later): package/Makefile.in (@240 is probably garbage)

>  It *is* on line 240 in current master, and package/Makefile.in hasn't
> changed since July 4 (except for the command -v replacement, of course).

+1, thanks.

> > > run the command (split à-la python), as can be seen with strace:

> > >      ['command', '-v', 'perl']

> > > Second, the issue was invisible to Arnout, because the distribution he
> > > uses, Fedora, provides /usr/bin/command, which is a simple shell script
> > > that just basically does:

> > >      #!/bin/sh
> > >      builtin command "${@}"

> > Hm, wrapping shell builtin into script is really strange.

>  POSIX doesn't say anything about `command` etc. being shell builtins. Thus,
> a program written for POSIX may assume that it can execvp("command", ...)
> (i.e. without going through the shell).
Good point. Obviously POSIX didn't realize problems we encountered. IMHO it'd be
great to specify this precisely in POSIX.

>  I assume that someone in RedHat encountered such an issue and that that was
> their solution.
I wonder if they tried to contribute it to coreutils. It'd be great to have
upstream solution.

> > BTW I was already thinking to add similar script to buildroot,

>  Right - instead of providing our on `which`, we can also provide our own
> `command`!
Yes (fairly smaller amount of code, which likely solve all our problems).
I'll also test "builtin command" in Makefile (I suppose it will be the same:
"Command not found").

>  One small caveat though: we currently don't add anything to PATH. Adding
> our own thing (whether it's which or command) will require us to add
> something to PATH, which by itself can have surprising effects as well.
Good point. IMHO we'll have to go this way (and solve any problem with PATH)
even if we wan to take 'which'. Sooner or later which version disappear from
Debian. GNU which is discontinued, but I suppose distros which ship it now (RHEL/Fedora and SLES/Tumbleweed and probably many others) will keep it for long time.

> > but I'm really surprised that Fedora added that to distro.

> > > However, this is provided by no package in the distribution I use,
> > > Ubuntu 20.04.1 LTS (filtering to ignore /usr/bin/commander et al.):

> > >      $ apt-file search bin/command |grep -E 'bin/command$'
> > >      [nothing]

> > This is probably Fedora/RHEL specific. I'll investigate which package it belongs

>  Yes it is. It's provided by the bash package.
+1

> > and ask Fedora maintainer for a reason (unfortunately Fedora does not have any
> > search like Debian https://packages.debian.org/ [1]).

> > > So, probably you did not see the error either, because your distribution
> > > also provides command as an actual executable. Could you check that by
> > > running:    which command     (Ahaha! :-])

> > > Note: if we change the line above to

> > >      export PERL=$(shell command -v perl 2>/dev/null)

> > > then make no longer believes this is a simple command, and will execute
> > > with system() and the warning goes away.

>  However, a future version of GNU make may very well decide that
> '2>/dev/null' is something it can handle itself, and bypass the shell as
> well.
Maybe it'd be good to teach make always run 'command' via system().

> > Hm, definitely worth to more investigate which make releases are affected and
> > report if not already fixed.

> > > So, bottom line, there are more impacts than previously expected, and we
> > > need to think the transition more carefully.
> > +1

> > > And to be extra clear: I am OK with transitionning away from which, or
> > > at least from relying on which being provided by the distro.
> > Thanks!

> > Also going back to the issue with bash 'command -v' implementation. First is it
> > also relevant to this issue or not?

>  It's an issue if we pass several parameters to `command -v`. We probably
> should simply never do that - the case that Markus found is a bug anyway.
+1

> > You mentioned Ubuntu with bash as /bin/sh as a default shell in previous mail.
> > Testing 'make defconfig && make help 2>&1 |grep -i found' on bash as /bin/sh
> > does not trigger the error (make[1]: command: Command not found). Thus bash it's
> > not the problem (at least less problematic than make you mentioned).

> > Also, does anybody understand POSIX spec [2] whether only single command_name
> > can be used as Markus reported [3]? Should we report it to bash?

>  The POSIX spec says:

> command [-p][-v|-V] command_name

> So it's only specified with a single argument. The POSIX spec is pretty
Yep, I understood it that way.
> vague about what to do with stuff that is not specified, like a second
> argument. So I guess implementations can do what they want...
+1

>  By the way, the differences between which and command -v are a bit larger
> than this. command -v also works for shell builtins, while which doesn't -
> unless which is defined as a function, which is the case for the `which`
> package in Fedora...

> $ command -v ls
> alias ls='ls -AFh'
> $ which ls
> alias ls='ls -AFh'
>         /usr/bin/ls
> $ /bin/which ls
> /usr/bin/ls
> $ type which
> which is a function
> which ()
> {
>     ( alias;
>     eval ${which_declare} ) | /usr/bin/which --tty-only --read-alias
> --read-functions --show-tilde --show-dot "$@"
> }

Yes, 'type' is quite different than which/command -v. More verbose, thus I
understand why 'type' is POSIX extension and 'command -v' mandatory.

>  I think we can safely say: this is a mess :-)


>  From this, I actually conclude that it is indeed safer to go to command -v,
> with a wrapper script that we add to $PATH to handle the case where make
> tries to bypass the shell.
+1. I'll try to have look into it.

Thanks a lot to all for your comments.

Kind regards,
Petr

>  Regards,
>  Arnout

<snip>
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot