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@xxxxxxxxx> wrote:
> Uma Srinivasan <usrinivasan@xxxxxxxxxxx> writes:
>
>> On Mon, Aug 29, 2016 at 2:13 PM, Uma Srinivasan <usrinivasan@xxxxxxxxxxx> 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.