Re: [GSoC][PATCH] t/t5000-tar-tree: add helper function

[Kostya Farber <kostya.farber@gmail.com>]

On Thu, Feb 2, 2023 at 10:36 PM Junio C Hamano <gitster@pobox.com> wrote:

> Kostya Farber <kostya.farber@gmail.com> writes:
>
> > Add the helper function test_file_path_exists to the
> > interpret pax header test. This change makes it clearer
> > as to what the test is trying to check, in this case whether
> > a file path exists.
>
> Really?
>
> The code with "test -e" is already clear that it is checking if the
> path $data exists.  This change does not make it any clearer.  What
> it helps is that it gives a message upon failure, when the test is
> run with the "-v" option.

Okay, noted. I will look into the helper functions more closely to
understand why and how they useful compared to other methods (i.e test
-e in this instance)

> > diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> > index d473048138..19d5bd0c04 100755
> > --- a/t/t5000-tar-tree.sh
> > +++ b/t/t5000-tar-tree.sh
> > @@ -73,7 +73,7 @@ check_tar() {
> >                       for header in *.paxheader
> >                       do
> >                               data=${header%.paxheader}.data &&
> > -                             if test -h $data || test -e $data
> > +                             if test -h $data || test_file_path_exists $data
> >                               then
> >                                       path=$(get_pax_header $header path) &&
> >                                       if test -n "$path"
>
> Nothing seems to be adding a new helper whose name is
> test_file_path_exists; the patch expects such a helper already
> exists and uses it in place for existing "test -e".
>
> Perhaps you meant to say "use test_path_exists" not "add helper" on
> the title, and use that function in the patch instead?

Yes you are right. I made a mistake by using the wrong function name
and I think "use test_path_exists" was my intention as a title name.

> Thanks.