Re: [PATCH 1/2] test-path-utils.c: remove incorrect assumption

From: Ray Donnelly <mingw.android@gmail.com>
Date: Oct 4, 2015, 14:51 UTC
Message-ID: <CAOYw7dv4iPQ4cq4Ab1ZeThrp=u51T5v387a1Y8QPO-yj=fyMcg@mail.gmail.com>
In-Reply-To: <xmqqlhbj3mfo.fsf@gitster.mtv.corp.google.com>

On Sat, Oct 3, 2015 at 6:13 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

> Ray Donnelly <mingw.android@gmail.com> writes:
>
>> In normalize_ceiling_entry(), we test that normalized paths end with
>> slash, *unless* the path to be normalized was already the root
>> directory.
>>
>> However, normalize_path_copy() does not even enforce this condition.
>
> Perhaps the real issue to be addressed is the above, and your patch
> is killing a coalmine canary?
>
> Some callers of this function in real code (i.e. not the one you are
> removing the check) do seem to depend on that condition, e.g. the
> codepath in clone that leads to add_to_alternates_file() wants to
> make sure it does not add an duplicate, so it may end up not noticing
> /foo/bar and /foo/bar/ are the same thing, no?  There may be others.
>
>

Enforcing that normalize_path_copy() removes any trailing '/' (apart from the root directory) breaks other things that assume it doesn't mess with trailing '/'s, for example filtering in ls-tree. Any suggestions for what to do about this? Would a flag be appropriate as to whether to do this part or not? Though I'll admit I don't like the idea of adding flags to modify the behavior of something that's meant to "normalize" something. Alternatively, I could go through all the breakages and try to fix them up?