Shawn Pearce <spearce@xxxxxxxxxxx> writes:
> Junio C Hamano <junkio@xxxxxxx> wrote:
> ...
>
> If GIT_DIR is set we call is_git_directory(); if that returns 1
> then the checks passed. In this case the old code returned NULL
> and ignored nongit_ok. We do the same in the new code.
Ok.
>> > for (;;) {
>> > - if (is_toplevel_directory())
>> > + if (is_git_directory(".git"))
>> > break;
>> > chdir("..");
>> > do {
>> > if (!offset) {
>> > + if (is_git_directory(cwd)) {
>> > + if (chdir(cwd))
>> > + die("Cannot come back to cwd");
>> > + setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
>> > + return NULL;
>> > + }
>> > if (nongit_ok) {
>> > if (chdir(cwd))
>> > die("Cannot come back to cwd");
>>
>> I do not know what the new behaviour of this part of the code is
>> trying to do. This is supposed to see if "." is the toplevel
>> (equivalently, ".git" is the git_dir, in your implementation),
>> otherwise chdir("..") repeatedly until it finds one, and the
>> normal return condition is for the working directory of the
>> process to be at the toplevel. So chdir(cwd) you introduced is
>> obviously changing the behaviour.
>
> No, its not.
Ah, "changing the behaviour" is the correct thing to do, because
the code is now allowing a new fallback to allow the original
directory to be a bare git repository, and the new chdir/setenv
makes perfect sense. Thanks for clarification.
For a short time I wondered if this should be a fallback
position, or we might want to make this the very first thing to
check, but I think adding this as the last ditch fallback
position as you did is much safer than making it the first thing
to check.
Thanks.
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html