`git rev-parse --is-inside-work-tree` and $GIT_WORK_TREE

So, I find this behaviour a little strange; I can't determine if it's a subtle bug, or intentionally undefined/‘fuzzy’ behaviour:

    $ cd a-repo/.git/
    $ pwd
    /path/to/a-repo/.git
    $ git rev-parse --is-inside-work-tree
    false
    $ export GIT_WORK_TREE=/path/to/a-repo
    $ git rev-parse --is-inside-work-tree
    true

i.e. when within the repository (the `.git` directory), and when that directory is a sub-directory of the working-tree, `rev-parse --is-inside-work-tree` reports *false* (reasonable enough, I suppose); but then if `$GIT_WORK_TREE` is set to precisely the directory that git was *already* assuming was the working-directory, then the same command, in the same location, reports *true*.

This should probably be made consistent: either `rev-parse --is-inside-work-tree` should report “true”, even inside the `.git` dir, as long as that directory is a sub-directory of the working-tree … or repository-directories / `$GIT_DIR` / `.git` directories should be excluded from truthy responses to `rev-parse --is-inside-work-tree`.

⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

On Tue, Mar 29, 2016 at 6:42 AM, Elliott Cable <me@ell.io> wrote:

Oh lord, it gets worse ...

$ cd a-repo $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir true false $ cd .git $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir false true $ export GIT_WORK_TREE="$(git rev-parse --show-toplevel)" # !!! $ git rev-parse --is-inside-work-tree; git rev-parse --is-inside-git-dir true false $ # !!?!?

So, basically, if `$GIT_WORK_TREE` is set at all, it appears that the `rev-parse --is-inside...` flags don't function reliably at all.

⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt

On Tue, Mar 29, 2016 at 06:53:35AM -0500, Elliott Cable wrote:

I believe these are working correctly, the .git directory is not part of the working tree.

Did you check the value of GIT_WORK_TREE here? When I try it's the empty string.

If I set the core.worktree config variable to ".." then rev-parse does find the working tree correctly. I recall some previous discussion about this but I can't find it in the list archives from a quick search.

If you set GIT_WORK_TREE you're telling Git to override all of the normal detection logic. What version of Git are you using? When I try this it says:

	fatal: The empty string is not a valid path

If I set GIT_WORK_TREE to the correct value for this repository then it behaves the same as with the auto-detection logic.

John Keeping <john@keeping.me.uk> writes:

I didn't carefully read the other parts of the discussion, but this is not entirely correct.

The actual/intended rules are fairly simple.

 * With GIT_DIR (without GIT_WORK_TREE), you specify where the .git
   directory is, and refuse the "normal detection logic".  As you
   refuse the normal "where is the root of the working tree and its
   .git directory?" logic, Git uses the current directory as the
   root of the working tree.

 * But that means if you set GIT_DIR, you cannot work from a
   subdirectory of your working tree--you always have to run Git
   from the root level of your working tree.  This may be
   inconvenient.

 * That is why GIT_WORK_TREE exists.  It allows you to say "No, I am
   not at the root level but am in a subdirectory somewhere inside
   the working tree. The root level of the working tree is there
   above, not here".

So it is a misconfiguration if you only set GIT_WORK_TREE without setting GIT_DIR.

Also, if you set both and run Git from outside $GIT_WORK_TREE, even though Git may try to do its best to give you a reasonable behaviour [*1*], it is working by accident not by design (see the statement you are making by setting GIT_WORK_TREE in the third bullet above).

IOW, running from outside GIT_WORK_TREE is a misconfiguration.

[Footnote]

*1* Think what should happen when you are outside GIT_WORK_TREE and
    say this:

	$ git grep foo

    As you are not even inside the working tree, the command would
    not know in which subdirectory you want to find the string foo;
    the "reasonable behaviour" is to work on the whole working tree
    in this case.

On Tue, Mar 29, 2016 at 08:08:29AM -0700, Junio C Hamano wrote:

Hmm. I have frequently done this when my cwd is a git repository (e.g., a bare one), and it works as you'd expect (find the git-dir in the current path, then the working tree via $GIT_WORK_TREE).

I always assumed that was the intended behavior, as it is the only one that makes sense. I suspect I am not alone in having relied on this.

Likewise, I always assumed this "reasonable behavior" was intended. When we setup_git_directory(), we end up in the root of the working tree as usual. The "prefix" must be empty, as we were not in the work tree at all, and we do a whole-tree operation.

Those behaviors may not have been fully designed, but as they do the only reasonable thing (besides dying with an error), and people may have baked that assumption into their scripts, I think we should avoid changing them unless there is a compelling reason.

-Peff

Jeff King <peff@peff.net> writes:

Hmm, does what is done by "git add HEAD" in such a situation match what you'd expect?

        git init work
        cd work; date >HEAD; git commit -m initial
        git push ../bare master:master
	date >>HEAD
        export GIT_WORK_TREE=$(pwd)
	cd ..
	git --bare init bare
	cd bare
	git add HEAD

I'd have to say that this invites unnecessary confusion, even though I agree that "go to the GIT_WORK_TREE and take pathspecs relative to that directory" is the only sensible thing for us to be doing.

But that is not an issue about "set only work-tree" (it is about "run from outside the work-tree").

On Tue, Mar 29, 2016 at 12:56:41PM -0700, Junio C Hamano wrote:

I had to tweak your commands a little, but I assume the part you are interested in is the end, when git-add finds HEAD in $GIT_WORK_TREE and not the bare repository.

And yes, that is exactly what I'd expect, and why it is useful (if you wanted to add arbitrary cruft from the bare repo, you'd set $GIT_WORK_TREE to point there).

Yeah, there are two things going on:

  1. Without $GIT_DIR but with $GIT_WORK_TREE, we find $GIT_DIR via the
     usual discovery path.

  2. When outside $GIT_WORK_TREE, any work-tree operations work as if
     they were started from $GIT_WORK_TREE.

And relying on (1) almost always relies on (2), unless your work-tree happens to be inside the discovery path for your $GIT_DIR. So you could do:

  git init repo
  mkdir repo/subdir
  echo content >file
  GIT_WORK_TREE=$(pwd) git add .

which adds "file" at the top-level. And we used only rule (1), not rule (2). I don't know whether people actually do that or not (I guess it could be useful for tricky subtree things).

-Peff

On Tue, Mar 29, 2016 at 06:42:44AM -0500, Elliott Cable wrote:

Yeah, I think this is a bug. Presumably what is happening is that we are too eager to "cd $GIT_WORK_TREE" inside git-rev-parse, and by the time we ask "are we in a work tree", the answer has become yes. But the caller really wants to know "am _I_ inside the work tree".

Unfortunately, I think the fix is likely to be rather tricky, as the work-tree stuff is happening deep inside setup_git_directory().

-Peff

On Tue, Mar 29, 2016 at 04:34:25PM -0400, Jeff King wrote:

I don't think that's what's happening.  Try:

	$ cd .git/
	$ GIT_WORK_TREE=.. git rev-parse --is-inside-work-tree
	true

so I think it's that we refuse to assume that the directory above a Git directory is a working tree (something similar happens when the "core.worktree" config variable is set). I'm not convinced that's unreasonable.

However, the case above also gives:

	$ GIT_WORK_TREE=.. git rev-parse --is-inside-git-dir
	false
	$ test $(pwd) = $(GIT_WORK_TREE=.. git rev-parse --git-dir); echo $?
	0

so even though $PWD *is* the Git directory, we're not in the Git directory! Setting GIT_DIR=$(pwd) makes no different to that.

On Tue, Mar 29, 2016 at 09:52:08PM +0100, John Keeping wrote:

Yeah, you're right, but I'm not sure how your example shows that, (isn't it basically the same as Elliott's original, except using a relative path?). A more compelling counter-example to my hypothesis is:

  $ cd .git
  $ GIT_WORK_TREE=/tmp git rev-parse --is-inside-work-tree
  false

So it is not that we chdir too early, but just that we blindly check "is $(pwd) inside $GIT_WORK_TREE". And it does not create a problem for the normal discovered-path cases, because either:

  - we discovered .git by walking up the directory tree, which means we
    must be in a work-tree

  - we discovered that we are inside a .git directory, and therefore
    take it to be bare (and thus there is no work tree, and we cannot be
    inside it). This is what happens in Elliott's original example that
    behaves differently than the $GIT_WORK_TREE case.

I'd be tempted to say that "inside the work tree" is further clarified to "not inside the $GIT_DIR". But as you note:

We seem to get that wrong. I'm also not sure if it would make sense if you explicitly set the two to be equal, like:

  # checking in your own refs?
  GIT_WORK_TREE=$(pwd) GIT_DIR=$(pwd) git add refs packed-refs

So the current behavior may just be weird-but-true.

-Peff

On Tue, Mar 29, 2016 at 05:21:43PM -0400, Jeff King wrote:

Yes, I think that's reasonable.  But...

This case definitely feels wrong:

	$ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir
	false

Shouldn't that be the same as if GIT_WORK_TREE and GIT_DIR aren't set? (It's also potentially surprising since "git rev-parse --git-dir" does give the right answer in this case.)

If GIT_WORK_TREE points somewhere unrelated then it is correct:

	$ GIT_WORK_TREE=/tmp GIT_DIR=$(pwd) git rev-parse --is-inside-git-dir
	true

On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:

It seems that this is a result of changing the working directory to the root of the working tree if we're inside it. is_inside_dir() doesn't take account of startup_info->prefix and changing to:

	real_path(startup_info->prefix)

instead of xgetcwd() means that these tests are less surprising.

But I haven't run the test suite or thought about what else this could break.

On Tue, Mar 29, 2016 at 11:00:03PM +0100, John Keeping wrote:

Yeah, and not just the is-inside-git-dir test:

  $ echo content >../file
  $ GIT_WORK_TREE=$(cd ..; pwd) GIT_DIR=$(pwd) git add file
  fatal: pathspec 'file' did not match any files

I'd expect that to work, and it doesn't, because we pass ".git/" as the "prefix" to cmd_add(). Which I guess is true, but it feels kind of weird (I think most people who set both variables like that would generally point to some other directory entirely, and we would have a NULL prefix).

The --is-inside-git-dir thing is related, but a different problem. I just got your follow-up mentioning that it doesn't take the prefix into account, which I agree it probably should.

-Peff

Jeff King <peff@peff.net> writes:

That reminds me of a related tangent. If we really want to properly support running from outside the working tree (or from inside .git for that matter), I suspect we need two separate "prefix" for two different uses. The "we would have a NULL prefix" is what was considered the true "prefix" traditionally, i.e. it is the directory to which any pathspecs and relative paths that name paths in the history are taken relative to. E.g. if you run "git add HEAD" from inside your GIT_DIR but you have GIT_WORK_TREE set up correctly, you would want to add HEAD from the root of the working tree.

Another is the base directory for a relative filename that names a file that does not have anything to do with the paths in the history. E.g. if you run "git grep --file patterns" from outside the working tree but with GIT_DIR/GIT_WORK_TREE correctly set up, you would still want to read the "patterns" file from the current directory.

The former can be done by using prefix=NULL to say "we may or may not have come from outside a working tree but we no longer care after we chdir(2) to the root of the working tree. Any path is relative to the root of the working tree." But then we may lose the clue to read from the latter (the OPT_FILENAME option is handled by prefix_filename() using the prefix).

The distinction between the two does not exist as long as you start inside GIT_WORK_TREE and outside GIT_DIR.

On Wed, Mar 30, 2016 at 3:34 AM, Jeff King <peff@peff.net> wrote:

No. Once you set GIT_WORK_TREE you tell git that worktree exists. That overrides the "bare repo" attribute (i.e. no worktree) that's automatically set when we try to find .git directory.

On relative GIT_WORK_TREE some mail down this thread, there's this note in t1510 that you might find interesting

5. GIT_WORK_TREE/core.worktree was originally meant to work only if
   GIT_DIR is set, but earlier git didn't enforce it, and some scripts
   depend on the implementation that happened to first discover .git by
   going up from the users $cwd and then using the specified working tree
   that may or may not have any relation to where .git was found in.  This
   historical behaviour must be kept.

Basically if you set GIT_WORK_TREE you better set GIT_DIR too to keep things sane.

oh, wow, this got over my head *real* fast. Okay,

1. Yeah, my `$GIT_WORK_TREE` was def. an absolute path; I typed that
example code without running it *precisely* that way (entirely my
mistake! I'm so sorry for the confusion it caused, and all that typing
you did!); if I remember correctly (not at the machine right now), I
had run `git rev-parse --show-toplevel` from a different directory,
with `$GIT_DIR` set, while trying to narrow down this bug, so it gave
me an absolute path … and then copy-pasted that path, and then
replaced my copy-paste with the original command to make the
bug-report example as concise as possible? oops. But, yeah, it fails
in the manner described above with an absolute path. (Which it looks
like you two figured out above.)

2. Re: intentions, again, it seems like you've changed your mind, but …

   >  So it is a misconfiguration if you only set GIT_WORK_TREE
without setting GIT_DIR.

   I really, really hope not! Half the usefulness of `$GIT_WORK_TREE`
existing is in that mode. In fact, that's how I found `$GIT_WORK_TREE`
documented and explained, in [this blog
post](https://git-scm.com/blog/2010/04/11/environment.html) on the Git
site. That usage seems pretty damn useful, so I do hope it's
eventually explicitly supported … (and if that's *not* going to be the
case, it should be explicitly documented in the `GIT(1)` manpage,
alongside the other documentation of the environment-variables, that
the behaviour is undefined is `$GIT_DIR` isn't set first. =)

⁓ ELLIOTTCABLE — fly safe.
  http://ell.io/tt