Re: hitting home directory's parent

From: Jeff King <peff@peff.net>
Date: Aug 25, 2009, 04:42 UTC
Message-ID: <20090825044236.GA13738@coredump.intra.peff.net>
In-Reply-To: <fcaeb9bf0908222107i6d999335r998a304aaa3cd405@mail.gmail.com>

On Sun, Aug 23, 2009 at 11:07:39AM +0700, Nguyen Thai Ngoc Duy wrote:

Show 7 quoted lines

> It is (and should be worked around with GIT_CEILING_DIRECTORIES).
> Unfortunately in my test, it could not chdir() back when it failed to
> find gitdir. chdir() was called with an absolute directory, and one
> directory in that path was inaccesible, leading another die("Cannot
> come back to cwd"). This one is fatal and should not be ignored. I
> don't know whether having an inaccesible parent directory is a real
> scenario, as lots of tools would break.

Hmm, good point. Some of those die()s really can't be removed, because we have munged state in a way that can't be recovered (or is hard to recover).

Also, thinking about it a bit more, I'm not sure it is correct to turn such a die() into a "return -1" without passing more information along to the caller. Because there are really three possible outcomes:

  1. we are in a git repo
  2. we are not in a git repo
  3. some error occurred and we could not determine which
     of (1) and (2) is the case

And the "gently" bit really just refers to (1) versus (2). It is tempting to fold (3) into (2), but I'm not sure that is the right thing to do; some callers (including script callers not under our control) may care about the distinction (though I think we already blur it; for example, a non-existent '.git' regular file is treated the same as one which we couldn't stat for permission problems).

So perhaps the right thing is to either pass back a tri-state status, or to have an extra_gently mode, either of which could be used by "git help". The problem with either is that the current setup process is not amenable to exiting in the middle -- it really may leave us in a weird and buggy state due to the chdir.

So short of major surgery on the setup procedure, I think we may have to leave this bug be for now (especially because I'm not convinced the setup of a non-executable parent directory isn't a little bit crazy).

-Peff