Re: git submodules implementation question

[Uma Srinivasan <usrinivasan@twitter.com>]

Yes, is_git_directory() is much better. Thanks for the pointer.

I will submit a patch unless I hear more suggestions from others.

Uma



On Mon, Aug 29, 2016 at 4:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Uma Srinivasan <usrinivasan@twitter.com> writes:
>
>> On Mon, Aug 29, 2016 at 2:13 PM, Uma Srinivasan <usrinivasan@twitter.com> wrote:
>>> Ok that makes sense. Thanks much.
>>>
>>> Uma
>>>
>> With respect to my original problem with a corrupted .git directory
>> under the submodule directory, I am thinking of adding the following 4
>> lines marked with ### to is_submodule_modified() to detect the
>> corrupted dir and die quickly instead of forking several child
>> processes:
>>
>>                strbuf_addf(&buf, "%s/.git", path);
>>                git_dir = read_gitfile(buf.buf);
>>                if (!git_dir) {
>>                  ### strbuf_addf(&head_ref, "%s/HEAD",buf.buf);
>>                  ### if (strbuf_read_file(&temp_ref, head_ref.buf,0) < 0) {
>>                           ### die("Corrupted .git dir in submodule %s", path);
>>                  ###}
>>                          git_dir = buf.buf;
>>               }
>>
>> This fixes my issue but what do you think? Is this the right way to
>> fix it? Is there a better way?
>
> I think we already have a helper function that does a lot better
> than "does it have a file called HEAD" to ask "is this a git
> directory?" and its name I think is "is_git_directory" (I know, we
> are not imaginative when naming our functions).
>
> As to the check makes sense in the context of this function, I am
> not an expert to judge.  I'd expect Jens, Heiko and/or Stefan to
> know better than I do.