On Mon, Oct 5, 2015 at 12:36 AM, Ray Donnelly <mingw.android@xxxxxxxxx> wrote: > On Sun, Oct 4, 2015 at 6:21 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Ray Donnelly <mingw.android@xxxxxxxxx> writes: >> >>>> 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? >> >> I agree with you that "normalize" should "normalize". Making sure >> that all the callers expect the same kind of normalization would be >> a lot of work but I do think that is the best approach in the long >> run. Thanks for the ls-tree example, by the way, did you find it by >> code inspection? I do not think it is reasonable to expect the test >> coverage for this to be 100%, so the "try to fix them up" would have >> to involve a lot of manual work both in fixing and reviewing, >> unfortunately. > > For the ls-tree failure, I ran "make test" to see how much fell out. > I'm not familiar with the code-base yet, so I figured that at least > investigating the changes needed to make the test-suite pass would be > a good entry point to reading the code; I will study it at the same > time to try and get my bearings. > >> >> The first step of the "best approach" would be to make a note on >> normalize_path_copy() by adding a NEEDSWORK: comment to describe the >> situation. I hope this is acceptable for the first part of this task. >> >> Thanks.
Attachment:
0001-normalize_path_copy-NEEDSWORK-for-trailing.patch
Description: Binary data