Re: [PATCH v3 15/25] setup.c: detect $GIT_COMMON_DIR in is_git_directory()

From: Duy Nguyen <pclouds@gmail.com>
Date: Mar 1, 2014, 03:33 UTC
Message-ID: <CACsJy8BQxZOQOHEuw9w2-gCqiFyP=YZ_PQzHkydnSM1j3hUK-Q@mail.gmail.com>
In-Reply-To: <xmqqr46p1kbo.fsf@gitster.dls.corp.google.com>

On Thu, Feb 27, 2014 at 7:16 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

>> +     if (file_exists(path.buf)) {
>> +             if (strbuf_read_file(&data, path.buf, 0) <= 0)
>> +                     die_errno(_("failed to read %s"), path.buf);
>
> Do we care about the case where we cannot tell if the file exists
> (e.g. stat() fails due to EPERM or something), or would it be not
> worth worrying about?

In that case we assume (incorrectly) that the repository is complete. Following operations would fail. So not too bad, I think.

Show 13 quoted lines

>> @@ -188,14 +212,20 @@ int is_git_directory(const char *suspect)
>>       int ret = 0;
>>       size_t len;
>>
>> -     strbuf_addstr(&path, suspect);
>> +     strbuf_addf(&path, "%s/HEAD", suspect);
>
>> +     if (validate_headref(path.buf))
>> +             goto done;
>
> Is there a reason why we want to check HEAD before other stuff?
> Just being curious, as I do not think of any (I am not saying that
> we shouldn't change the order).

Yes, it's reordered so that worktree signature (e.g. HEAD) is checked first, against $GIT_DIR. Then non-worktree signatures ("refs" and "objects") are checked against $GIT_COMMON_DIR (or $GIT_DIR still if $GIT_DIR/commondir does not exist). Notice "path" is reset to $GIT_COMMON_DIR just after checking HEAD. I should probably add a comment about this separation.

-- 
Duy