Subject: Re: [PATCH 1/1] t1403: prefer test_path_exists helper function

From: Junio C Hamano <[email-masked]>
Date: Tue, 04 Mar 2025 09:49:00 -0800

Eric Sunshine <[email-masked]> writes:

> On Tue, Mar 4, 2025 at 7:05 AM Junio C Hamano <[email-masked]> wrote:
>> Mahendra Dani <[email-masked]> writes:
>> >> >       remove_object() {
>> >> >               file=$(sha1_file "$*") &&
>> >> > -             test -e "$file" &&
>> >> > +             test_path_exists "$file" &&
>> >> >               rm -f "$file"
>> >> >       } &&
> That's a good question to ask, but isn't the implied suggestion of
> dropping "-f" going in the wrong direction? If I'm reading
> remove_object() correctly, `test -e` is being used as control flow,
> *not* as an assertion that the file exists.

If sha1_file says the loose object must be at path $file, and the
call to test -e "$file" returns false, two things happen in this
function:

 (1) control stops and "rm -f" does not trigger
 (2) the function returns non-zero status to the caller

If you omit the check and say rm "$file" instead, under the same
scenario, (1) "rm" is attempted, but there is nothing to remove so
the command returns non-zero status, and (2) the function returns
that non-zero status to the caller

If the file does exist, both will remove the file, and give the
caller zero status return.

So?