Re: [GSoC][PATCH v2 3/3] t3600: use helpers to replace test -d/f/e/s <path>

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 11, 2019, 01:54 UTC
Message-ID: <xmqqa7i2mazb.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CAPig+cQFMNFTMfMz5EnMZxnXGLWnKYZ5_=D3eiNsX24hdZSPRw@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 22 quoted lines

> On Fri, Mar 8, 2019 at 12:38 AM Junio C Hamano <gitster@pobox.com> wrote:
>> An unrelated tangent, but what do you think of this patch?  In the
>> context of testing "git rm", if foo is a dangling symbolic link,
>> "git rm foo && test_path_is_missing foo" would need something like
>> this to work correctly, I would think.
>>
>>  test_path_is_missing () {
>> -       if test -e "$1"
>> +       if test -e "$1" || test -L "$1"
>>         then
>>                 echo "Path exists:"
>>                 ls -ld "$1"
>
> Makes sense. Won't we also want:
>
>     test_path_exists () {
>     -    if ! test -e "$1"
>     +   if ! test -e "$1" && ! test -L "$1"
>        then
>             echo "Path $1 doesn't exist. $2"
>
> or something like that?

That would make them symmetric, but what I was driving at with "In the context of testing git rm" was that I highly suspect that among other existing users of test_path_is_missing there are some that want to consider a dangling symbolic link as if it is not there (and vice versa for test_path_exists), and it may not be a good idea to unconditionally declare that, unlike the underlying "test" command that dereferences symlinks for most operations, our wrapper does not dereference symbolic links, which is what the "what do you think?" patch and your addtion do.