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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 14, 2023, 21:08 UTC
Message-ID: <xmqqa61g550j.fsf@gitster.g>
In-Reply-To: <CAFySSZBAXCGTEhTK+rpLaZz4_RhdEDV5e5QewUwN-LHgSOTe2g@mail.gmail.com>

Calvin Wan <calvinwan@google.com> writes:

Show 11 quoted lines

>> 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.