Re: [PATCH v3 7/7] submodule: support reading .gitmodules even when it's not checked out

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 22, 2018, 15:29 UTC
Message-ID: <xmqqd0ua773e.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180822135152.1d40cd05d0b0cadb5eefb31f@ao2.it>

Antonio Ospite <ao2@ao2.it> writes:

> Maybe "submodule--helper config --check-writeable" could be a better
> name to avoid confusion between the boolean return value of the C
> function (0: false, 1: true) and the exit status returned to the shell
> (0: safe to write, !0: unsafe).

Perhaps. The main point was to replace the comment that tells the developers to keep two things stay in sync with an actually shared implementation; as long as that is done, I am not too much worried about the details.

>> > > +		else if (get_oid(GITMODULES_HEAD, &oid) >= 0)
>> > > +			config_source.blob = GITMODULES_HEAD;
>> > 
>> Would using ":.gitmodules" instead of "HEAD:.gitmodules" be enough?

Yeah, either "instead of", or "in addition" (i.e. "try the index version in addition, before falling further back to the HEAD version"), would be more consistent with the remainder of the system (or, at least where the remainder of the system wants to go).

>> If so, what name should I use instead of GITMODULES_HEAD?
>> GITMODULES_BLOB is already taken for something different, maybe
>> GITMODULES_REF or GITMODULES_OBJECT?

I do not know why you want to refrain from spelling them out as "HEAD:.gitmodules" and ":.gitmodules"; at least to me the extra layer of names do not look like they are making the code easier to understand that much.