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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Shawn O. Pearce" <spearce@xxxxxxxxxxx> writes:

>  This is in response to Theodore Tso's email asking why 'git log'
>  doesn't work in a bare repository.  Now it does.  :-)

Does it?

> +static int is_git_directory(const char *suspect)
>  {
> +	char path[PATH_MAX];
> ...
> +
>  	return 1;
>  }

I think this is a good refactoring.

> @@ -160,36 +178,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	 * to do any discovery, but we still do repository
>  	 * validation.
>  	 */
> -	if (getenv(GIT_DIR_ENVIRONMENT)) {
> -		char path[PATH_MAX];
> -		int len = strlen(getenv(GIT_DIR_ENVIRONMENT));
> -		if (sizeof(path) - 40 < len)
> +	gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> +	if (gitdirenv) {
> +		if (PATH_MAX - 40 < strlen(gitdirenv))
>  			die("'$%s' too big", GIT_DIR_ENVIRONMENT);
> -		memcpy(path, getenv(GIT_DIR_ENVIRONMENT), len);
> -		
> -		strcpy(path + len, "/refs");
> -		if (access(path, X_OK))
> -			goto bad_dir_environ;
> -		strcpy(path + len, "/HEAD");
> -		if (validate_symref(path))
> -			goto bad_dir_environ;
> -		if (getenv(DB_ENVIRONMENT)) {
> -			if (access(getenv(DB_ENVIRONMENT), X_OK))
> -				goto bad_dir_environ;
> -		}
> -		else {
> -			strcpy(path + len, "/objects");
> -			if (access(path, X_OK))
> -				goto bad_dir_environ;
> -		}
> -		return NULL;
> -	bad_dir_environ:
> +		if (is_git_directory(gitdirenv))
> +			return NULL;
>  		if (nongit_ok) {
>  			*nongit_ok = 1;
>  			return NULL;
>  		}

I do not think this is correct.

What happens when GIT_DIR is set, and nongit_ok is passed?
Earlier code returned NULL after setting *nongit_ok so that the
caller knows the environment points at a directory that is not
yet a git repository control area.

> @@ -197,11 +196,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  
>  	offset = len = strlen(cwd);
>  	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.

The existing chdir(cwd) is for an error return -- when there was
no directory that has ".git" even when you went all the way up
to the root level, we give up and come back to where we started,
only when the caller suspected that there was no git directory
and is prepared to handle that case, which is signalled by us
storing 1 to *nongit_ok.


-
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]