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. > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html