Junio C Hamano <junkio@xxxxxxx> wrote:
> "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?
It did in my testing. ;-)
> > @@ -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.
The new code is correct, or at least does what the old code did.
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.
If GIT_DIR is set and we fail is_git_directory() then one of the
checks failed. In this case the old code jumped to bad_dir_env.
In the new code we don't return NULL and fall through into the if
nongit_ok testing, or into the die("Not a git repository").
> > @@ -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.
No, its not.
We only bother to look to see if the original cwd (before we started
chdir("..")'ing up) is a git directory if we did not find one during
that chdir up loop. This means our current working directory is now
"/" but we found a valid repository in cwd.
In this case we chdir back to the repository directory before
returning back to the caller, as what's the point of being in "/"
when running in a bare repository? Probably better to be in the
repository directory itself.
One could argue that maybe we should run in "/", or in "/tmp" in
this case as there is no working directory associated with this
repository, but that argument is about the same as just saying we
go back into the now discovered GIT_DIR.
--
Shawn.
-
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