Re: [PATCH 3/4] Automatically detect a bare git repository.
- From
Junio C Hamano <junkio@cox.net>
- Date
- Dec 31, 2006, 05:09 UTC
- Message-ID
- <7vy7ook5xj.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <20061231043019.GC5823@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> 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?
Show 7 quoted lines
> +static int is_git_directory(const char *suspect)
> {
> + char path[PATH_MAX];
> ...
> +
> return 1;
> }I think this is a good refactoring.
Show 37 quoted lines
> @@ -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.
Show 19 quoted lines
> @@ -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.