RFR: 8243656: Shell built-in test in configure depends on help
Severin Gehwolf
sgehwolf at redhat.com
Wed May 6 08:52:38 UTC 2020
On Wed, 2020-05-06 at 10:16 +0200, Magnus Ihse Bursie wrote:
> On 2020-05-05 17:15, Severin Gehwolf wrote:
> > Hi,
> >
> > Could I please get a review of this trivial change? Apparently using
> > the help builtin for determining whether or not a builtin is available
> > is not a good idea. A more portable way to do this is to use "command
> > -v" or "type". Thanks to Michael Zucchi for contributing this fix.
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8243656
> > webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8243656/02/webrev/
> Hi Severin,
>
> I missed commenting on this before you pushed it, but does this really
> work?! Did you try it on your system?
I believe so.
> I just tested the following:
> $ command -V ps
> ps is /bin/ps
> $ command -V fg
> fg is a shell builtin
> $ command -v ps
> /bin/ps
> $ echo $?
> 0
> $ command -v fg
> fg
> $ echo $?
> 0
>
> That is, it does not seem like "command -v" is making any difference in
> return value between builtins and external commands!
Sure, but is that a problem? The configure check is there as a fallback
if the tool is not found via AC_PATH_PROGS it tries this fallback (i.e.
it must be a built-in if "command -v <tool>" works, no?).
$ command -v help
help
$ echo $?
0
$ command -v foo
$ echo $?
1
> Also, even if it should do, this is not documented and I don't think it
> should be relied on -- as evident, it does not work on my system.
I'm confused, what's not documented? Everything you said seems to be
documented.
Also from 'man bash' for 'command' I see:
"""
If the -V or -v option is supplied, the exit status is 0 if command
was found, and 1 if not.
"""
> In contrast, the (builtin) command "type" seems to work fine, and is
> documented to work:
>
> $ type -t ps
> file
> $ type -t fg
> builtin
>
> I recommend that use $(type -t) = builtin instead.
I agree type would work too. If you insist, I can change the fallback
from 'command -v' to 'type -t'.
Thoughts?
Thanks,
Severin
More information about the build-dev
mailing list