Re: [RFC PATCH 1/6] leak fix: cache_put_path

[Junio C Hamano <gitster@pobox.com>]

Calvin Wan <calvinwan@google.com> writes:

>> Assuming that the "last one wins" is the sensible thing to do, the
>> change proposed by this patch does seem reasonable way to plug the
>> leak.
>
> Swapping this functionality to "first one wins" or erroring out breaks many
> tests that are setup improperly. If we continue with the "last one wins"
> precedence, then a warning and documentation should be added. We
> definitely should not swap it to "first one wins" -- one doesn't make sense
> than the other, but "last one wins" at least has precedence. If we choose
> to error out during config parsing when duplicated submodule paths are
> detected, then those respective tests will also need to be updated.

These tests expect the same submodule to be registered at different
paths?  Is that a set-up that is expected to happen commonly in real
life?  If so, yes, the current behaviour needs to be kept (with some
documentation to explain why it makes sense and how it is usefully
used).  If not, these tests may need to be updated to test scenarios
that are closer to the real life, I guess, plus an additional test
that makes sure such a .gitmodules file is diagnosed as an error
with the code to do that some time in the future.