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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 4, 2025, 17:49 UTC
Message-ID: <xmqqcyewr8j7.fsf@gitster.g>
In-Reply-To: <CAPig+cQ275qBWzaYmMXydiA_8+CTx3FhAAY+o8ro+hD03wzu0g@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 12 quoted lines

> On Tue, Mar 4, 2025 at 7:05 AM Junio C Hamano <gitster@pobox.com> wrote:
>> Mahendra Dani <danimahendra0904@gmail.com> 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?