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