Re: [PATCH 1/1] t1403: prefer test_path_exists helper function
- From
Mahendra Dani <danimahendra0904@gmail.com>
- Date
- Mar 4, 2025, 17:35 UTC
- Message-ID
- <CABGrwBBaHzM9tE6vWkuMSZuUbHVWwCZVTgkvMb04C=NYyGCLAw@mail.gmail.com>
- In-Reply-To
- <xmqqwmd4r9jx.fsf@gitster.g>
On Tue, Mar 4, 2025 at 10:57 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
>
> Mahendra Dani <danimahendra0904@gmail.com> writes:
>
> > On Tue, Mar 4, 2025 at 5:35 PM 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"
> >> >> > } &&
> >>
> >> You may want to think about why there is "-f" there. If we remove
> >> it, do we still need to have any check there?
> >
> > Here, the "-f" flag in `rm -f "$file"` does not produce an error message even
> > if the file does not exist [1], thus the `test -e "$file"` check was redundant,
> > as pointed out by Patrick in [2].
>
> So what happens if you dropped "-f" as I hinted? We'll notice the
> lack of file and the command exits with non-zero status. So "test -e"
> was not necessary in the first place, was it?
>Yes, due to the use of the "-f" flag, it's not necessary to explicitly check the lack of file using `test -e`. But if we drop the "-f" flag, we will have to check the lack of file using `test -e` or `test_path_is_file()`.
Thanks, Mahendra