"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