Re: [PATCH 15/20] t/t5000-tar-tree.sh: avoid "test <cond> -a/-o <cond>"

From: René Scharfe <l.s.r@web.de>
Date: Jun 10, 2014, 21:47 UTC
Message-ID: <53977D05.5070505@web.de>
In-Reply-To: <20140610200804.GD72977@gmail.com>

Am 10.06.2014 22:08, schrieb David Aguilar:

Show 29 quoted lines

> [Resent using René's correct email address this time, sorry for the noise]
>
> On Fri, Jun 06, 2014 at 07:55:58AM -0700, Elia Pinto wrote:
>> The construct is error-prone; "test" being built-in in most modern
>> shells, the reason to avoid "test <cond> && test <cond>" spawning
>> one extra process by using a single "test <cond> -a <cond>" no
>> longer exists.
>>
>> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
>> ---
>>   t/t5000-tar-tree.sh |    2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
>> index 74fc5a8..ad6fa0d 100755
>> --- a/t/t5000-tar-tree.sh
>> +++ b/t/t5000-tar-tree.sh
>> @@ -72,7 +72,7 @@ check_tar() {
>>   			for header in *.paxheader
>>   			do
>>   				data=${header%.paxheader}.data &&
>> -				if test -h $data -o -e $data
>> +				if test -h $data || test -e $data
>>   				then
>
> This looks okay, but it raises a question for the original author
> (René, I think that's you so I've added you to the To: line).
>
> Should that be "test -f" instead of "test -e"?

With -f instead of -e the function would ignore pax path headers for directories and special files. The latter is not relevant for git at all and we don't currently have a test for long directory names, but why restrict the code to handle only regular files?

A better change would be adding tests for symlinks and directories with long names.

Show 6 quoted lines

>
> This is a very minor note and should not block this patch.
> It's probably a change that's better made in a follow-up patch.
>
>>   					path=$(get_pax_header $header path) &&
>>   					if test -n "$path"