From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 23 Aug 2016 17:25:35 GMT
Subject: Re: [PATCH v10 0/9] submodule inline diff format
Message-ID: <xmqqlgznwg6o.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kbKibe2RpQ9QWN1naY07feZLKbhc+dts3BD-4+y-pUOVw@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

> On Mon, Aug 22, 2016 at 4:43 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
>> From: Jacob Keller <jacob.keller@gmail.com>
>>
>> A few suggestions from Stefan in regards to falling back to
>> .git/modules/<path> being a bad idea. I've chosen I think to avoid using
>> die() as we just stick with the current path if we can't find its name.
>
> Which makes the existing bug more subtle :(
>
>> I think this should be safe since we already do this today.
>
> It's a bug today already. Thanks for spotting!
>
>> The new flow
>> only changes if we are able to lookup the submodule, so I don't think
>> it's worth adding a die() call.
>
> Well this series improves the buggy-ness as it is only buggy when the name
> is not found, and we fall back on the path.

I am not so sure about that.  If there is an existing place that is
buggy, shouldn't we fix that, instead of spreading the same bug
(assuming that it is a bug in the first place, which I do not have a
strong opinion on, at least not yet)?

Can there be .git/modules/<foo>/ repository that is pointed at an
in-tree .git file when there is no "name" defined?  I thought we
errored out in module_name helper function in git-submodule.sh when
we need a name and only have path (I just checked in the maint-2.6
track); did we break it recently? submodule--helper.c::module_name()
seems to error out when submodule_from_path() fails to find one and
will segfault if it does not have name, so it is not likely.