Web lists-archives.com

Re: [PATCH] test-lib: try harder to ensure a working jgit




Hi,

Jeff King wrote:
> On Tue, May 14, 2019 at 02:14:19AM +0000, brian m. carlson wrote:
> 
>> On Mon, May 13, 2019 at 10:05:20PM -0400, Todd Zullinger wrote:
>>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>>> index 908ddb9c46..599fd70e14 100644
>>> --- a/t/test-lib.sh
>>> +++ b/t/test-lib.sh
>>> @@ -1522,7 +1522,7 @@ test_lazy_prereq NOT_ROOT '
>>>  '
>>>  
>>>  test_lazy_prereq JGIT '
>>> -	type jgit
>>> +	jgit --version
>>>  '
>> 
>> I think this is an improvement, not only because of the reasons you
>> mentioned, but because we remove the use of "type", which is not
>> guaranteed to be present in a POSIX shell.
>
> Isn't it?

I wondered the same thing, but I know I am not nearly as
familiar with the POSIX rules as any of you.

> I have always treated it as the most-portable option for this
> (compared to, say, `which`).  It is in POSIX as a utility (albeit marked
> with XSI), which even says (in APPLICATION USAGE):
> 
>   Since type must be aware of the contents of the current shell
>   execution environment (such as the lists of commands, functions, and
>   built-ins processed by hash), it is always provided as a shell regular
>   built-in.
> 
> All that said, I think Todd's patch makes perfect sense even without
> wanting to avoid "type".

Yeah, `which` surely isn't a portable option.  I presumed
`type` must be fairly widely available since it was in the
test suite since you added it way back in 212f2ffbf0 ("t:
add basic bitmap functionality tests", 2013-12-21).

I usually make use of `command -p -v $foo` in scripts that
need to be portable across systems.  But I don't have access
to many esoteric systems.

Based on Junio's follow-up, I think we can avoid adding
anything to the commit message about the use of `type` here.
That way no one will take it as a sign that we should remove
other uses of it just for conformance.  (I will send a
follow-up with an update based on Jonathan and Ævar's
comments.)

Thanks to all of you.

-- 
Todd