Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

From: Pasha Bolokhov <pasha.bolokhov@gmail.com>
Date: May 23, 2014, 22:40 UTC
Message-ID: <CAKpPgvdR5vwSMk0w_uMH8oDKAgH+v1v5JnQOSbaiZt4aNxO0qA@mail.gmail.com>
In-Reply-To: <xmqqha4gi5vg.fsf@gitster.dls.corp.google.com>

Hi,

>
>>     /* "--git-dir" has been given */
>
> ... or it could have come from GIT_DIR environment, no?

    Yes, it does not matter where it came from, but I'll correct the comment

>
> Does this "additional exclude" need to kick in if GIT_DIR is set to
> "/home/pasha/w/.git"?  That is, when gitdir is ".git" or ends with
> "/.git"?

    I don't think it needs to kick in in either of these cases, as
".git" is already handled by "treat_path()". Now, here ".git" is
excluded by "if (strcmp()) {", while the first case needs to be
addressed too. Agree.

Show 31 quoted lines

>> +#
>> +# Create a tree:
>> +#
>> +#    a  b  c  d  subdir/
>> +#
>> +# subdir:
>> +#    e  f  g  h  meta/  ssubdir/
>> +#
>> +# subdir/meta:
>> +#    aa
>> +#
>> +# subdir/ssubdir:
>> +#    meta/
>> +#
>> +# subdir/ssubdir/meta:
>> +#    aaa
>> +#
> It is not quite clear with this large blob of comment what are
> noises and what exactly are being tested.  I think you have two
> directories called "meta", but which one is the repository?  Or do
> you have yet another one next to {a,b,c,d,subdir} called "meta" that
> is not listed above?
>
> Given that the reason why people use --git-dir is so that they can
> put it completely outside the working tree (in which case, the usual
> "start from cwd and go upwards while trying to see if there is .git/
> that governs the working tree" logic would not work), readers would
> not expect to find the directory to be used as GIT_DIR in the
> hierarchy you are creating in the first place.  Because of that, it
> is even more important to clearify which "meta" you mean to use as
> your GIT_DIR if you want to be understood by readers.

I guess I was too simplistic, need to clarify a bit. And indeed, perhaps two distinct subtrees are needed to test the repository that is outside the work-tree, that would just do a slightly cleaner job

Pasha