Re: [PATCH 3/4] Automatically detect a bare git repository.

From: Junio C Hamano <junkio@cox.net>
Date: Dec 31, 2006, 05:46 UTC
Message-ID: <7v8xgok46s.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20061231052606.GA5722@spearce.org>

Shawn Pearce <spearce@spearce.org> writes:

Show 6 quoted lines

> Junio C Hamano <junkio@cox.net> 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.

Show 26 quoted lines

>> >  	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.