git stash doesn't honor --work-tree or GIT_WORK_TREE

git stash doesn't honor --work-tree or GIT_WORK_TREE

[Aaron Brooks]

Unlike other commands, git stash doesn't work outside of the worktree,
even when --work-tree is specified:

abrooks@host:~/tmp$ mkdir test-repo
abrooks@host:~/tmp$ cd !$
cd test-repo
abrooks@host:~/tmp/test-repo$ git init
Initialized empty Git repository in /home/abrooks/tmp/test-repo/.git/
abrooks@host:~/tmp/test-repo$ echo "foo" > foo.txt
abrooks@host:~/tmp/test-repo$ git add foo.txt
abrooks@host:~/tmp/test-repo$ git commit -m "adding foo"
[master (root-commit) 9d0705e] adding foo
 1 file changed, 1 insertion(+)
 create mode 100644 foo.txt
abrooks@host:~/tmp/test-repo$ echo "bar" >> foo.txt
abrooks@host:~/tmp/test-repo$ cd ..
abrooks@host:~/tmp$ git --git-dir=./test-repo/.git --work-tree=./test-repo stash
fatal: /usr/lib/git-core/git-stash cannot be used without a working tree.
abrooks@host:~/tmp$ cd test-repo/
abrooks@host:~/tmp/test-repo$ git stash
Saved working directory and index state WIP on master: 9d0705e adding foo
HEAD is now at 9d0705e adding foo
abrooks@host:~/tmp/test-repo$ cd ../
abrooks@host:~/tmp$ git --git-dir=./test-repo/.git
--work-tree=./test-repo stash list
fatal: /usr/lib/git-core/git-stash cannot be used without a working tree.

The same also does not work when setting GIT_DIR and GIT_WORK_TREE.

As a user, this seems wrong.

It looks like the "require_work_tree" function should check the
environment variables in addition to the status of the PWD (via
git-rev-parse).

Having looked through several of the other git-*.sh scripts, I think
other shell based git commands will have similar problems.

Thanks,

Aaron

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Øystein Walle]

Aaron Brooks <aaron <at> brooks1.net> writes:

> 
> Unlike other commands, git stash doesn't work outside of the worktree,
> even when --work-tree is specified:
> 
> (...)
> 
> It looks like the "require_work_tree" function should check the
> environment variables in addition to the status of the PWD (via
> git-rev-parse).
> 
> Having looked through several of the other git-*.sh scripts, I think
> other shell based git commands will have similar problems.
> 
> Thanks,
> 
> Aaron
> 

The environment variables are properly exported. I verified this by
adding 'echo $GIT_WORK_TREE; echo $GIT_DIR' at the top of git-stash.sh.
So these should propagate to "child gits" just fine, and so it shouldn't
be necessary to test them explicitly.

The problem seems to be that git rev-parse --is-inside-work-tree does
not honor these. In fact it doesn't even honor --git-dir or --work-tree.
Judging by the name this may be intentional.

In the mean time, if you are able to use Git 1.8.5, `git -C test-repo
stash` will work just fine.

Øsse

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Thomas Rast]

Øystein Walle <oystwa@gmail.com> writes:

> Aaron Brooks <aaron <at> brooks1.net> writes:
>
>> Unlike other commands, git stash doesn't work outside of the worktree,
>> even when --work-tree is specified:
[...]
> The environment variables are properly exported. I verified this by
> adding 'echo $GIT_WORK_TREE; echo $GIT_DIR' at the top of git-stash.sh.
> So these should propagate to "child gits" just fine, and so it shouldn't
> be necessary to test them explicitly.
>
> The problem seems to be that git rev-parse --is-inside-work-tree does
> not honor these. In fact it doesn't even honor --git-dir or --work-tree.
> Judging by the name this may be intentional.

Thanks for investigating this.

Duy, you are the expert on the worktree detection logic.  Do you know if
there is a reason for --is-inside-work-tree to not honor the
GIT_WORK_TREE / GIT_DIR overrides?

-- 
Thomas Rast
tr@thomasrast.ch

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Duy Nguyen]

On Sun, Dec 1, 2013 at 6:12 PM, Thomas Rast <tr@thomasrast.ch> wrote:
> Øystein Walle <oystwa@gmail.com> writes:
>> The problem seems to be that git rev-parse --is-inside-work-tree does
>> not honor these. In fact it doesn't even honor --git-dir or --work-tree.
>> Judging by the name this may be intentional.
>
> Thanks for investigating this.
>
> Duy, you are the expert on the worktree detection logic.  Do you know if
> there is a reason for --is-inside-work-tree to not honor the
> GIT_WORK_TREE / GIT_DIR overrides?

It should. At the beginning of cmd_rev_parse(), setup_git_directory()
is called, which will check and follow all GIT_* or their command line
equivalent. I'll look into this some time later.
-- 
Duy

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Thomas Rast]

Duy Nguyen <pclouds@gmail.com> writes:

> On Sun, Dec 1, 2013 at 6:12 PM, Thomas Rast <tr@thomasrast.ch> wrote:
>> Øystein Walle <oystwa@gmail.com> writes:
>>> The problem seems to be that git rev-parse --is-inside-work-tree does
>>> not honor these. In fact it doesn't even honor --git-dir or --work-tree.
>>> Judging by the name this may be intentional.
>>
>> Thanks for investigating this.
>>
>> Duy, you are the expert on the worktree detection logic.  Do you know if
>> there is a reason for --is-inside-work-tree to not honor the
>> GIT_WORK_TREE / GIT_DIR overrides?
>
> It should. At the beginning of cmd_rev_parse(), setup_git_directory()
> is called, which will check and follow all GIT_* or their command line
> equivalent. I'll look into this some time later.

Hrm, I'm wrong, and it's not clear what to actually do in this case.
--is-inside-work-tree works as claimed: if we can detect a git
repository and your current directory is inside it, you are "inside a
worktree".

The problem here is that shell scripts that want to do something with a
worktree tend to call require_work_tree in git-sh-setup:

require_work_tree () {
	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
	die "fatal: $0 cannot be used without a working tree."
}

However, when an explicit GIT_WORK_TREE is in effect, that seems a bit
silly.  The _intent_ of that command is "I need a worktree to work
with".  But what it currently checks is something completely different,
namely "am I _inside_ the worktree".

--show-cdup might be a better candidate, because it actually (and
despite what the docs suggest, sigh) shows the path to the worktree as
with --show-toplevel for the case where you are not --is-inside-work-tree.

Which means that the output --show-cdup is in fact not necessarily "up"
from the worktree, but something you could cd to to get to its top.

Long story short, this might work (and it passes tests):


diff --git i/git-sh-setup.sh w/git-sh-setup.sh
index 190a539..47c7f1d 100644
--- i/git-sh-setup.sh
+++ w/git-sh-setup.sh
@@ -192,8 +192,12 @@ require_work_tree_exists () {
 }
 
 require_work_tree () {
-	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
+	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true &&
+	return 0
+	cdup="$(git rev-parse --show-cdup 2>/dev/null)"
+	test -z "$cdup" &&
 	die "fatal: $0 cannot be used without a working tree."
+	cd "$cdup"
 }
 
 require_clean_work_tree () {


But it would give require_work_tree somewhat interesting and unintuitive
side effects.

-- 
Thomas Rast
tr@thomasrast.ch

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Øystein Walle]

Duy Nguyen <pclouds <at> gmail.com> writes:

> It should. At the beginning of cmd_rev_parse(), setup_git_directory()
> is called, which will check and follow all GIT_* or their command line
> equivalent. I'll look into this some time later.

I think I was wrong and rev-parse --is-inside-work-tree *does* honor
them. It prints 'false'. If it hadn't honored them it would have printed
"fatal: Not a git repository (...)".

Øsse

Re: git stash doesn't honor --work-tree or GIT_WORK_TREE

[Junio C Hamano]

Thomas Rast <tr@thomasrast.ch> writes:

> The problem here is that shell scripts that want to do something with a
> worktree tend to call require_work_tree in git-sh-setup:
>
> require_work_tree () {
> 	test "$(git rev-parse --is-inside-work-tree 2>/dev/null)" = true ||
> 	die "fatal: $0 cannot be used without a working tree."
> }
>
> However, when an explicit GIT_WORK_TREE is in effect, that seems a bit
> silly.  The _intent_ of that command is "I need a worktree to work
> with".  But what it currently checks is something completely different,
> namely "am I _inside_ the worktree".

Correct.

I have a few issues with the proposed "solution", though.

 - require_work_tree has always meant that "This command has to be
   run inside a working tree".  That automatically implies that you
   cannot be working with a bare repository but it is much stronger
   than that.  You actually have to be inside one.

 - $GIT_WORK_TREE (and core.worktree) never meant "I do not bother
   to chdir there myself".  More specifically, "GIT_WORK_TREE=$there
   git foo" is not "cd $there && git foo".  It only means "Because
   the working tree may not have .git directory embedded in it, even
   though you may be able to know where the .git repository is
   (perhaps because I am telling you with $GIT_DIR), you may not be
   able to tell where the top level of the working tree is---hence I
   am telling you where it is".

 - "I do not bother to chdir there myself" has long been treated as
   a non-issue desire, but recently we added "git -C $there". We
   should not conflate GIT_WORK_TREE, which is a discovery mechanism
   for where the working tree is, with that.

 - Some command "git foo" may want to affect and/or look at only a
   part of the working tree, and the $cwd is one way of specifying
   that, e.g. "cd doc && git grep foo".  "If a command that needs to
   be run from somewhere in a working tree was run outside, error
   out." has been the general design principle for commands that
   interact with files in the working tree.

   It may be OK to propose changing it to "Instead of erroring out,
   pretend as if the command were run from the top-level of the
   working tree", but there is an issue of what to do with the
   command line arguments that are path-like.  For example,
   should this command sequence:

	cd /tmp
	GIT_WORK_TREE=/var/tmp/work git foo -f inputFile

   read from /tmp/inputFile, or /var/tmp/work/inputFile?  There may
   be other "unintuitive interactions with silent chdir"; with "git
   -C $there", we are very clear that chdir is the first thing that
   happens and everything path-like are relative to $there, which is
   similar to what "make" does, but doing the same for core.worktree
   or $GIT_WORK_TREE may be surprising, given that environment is
   "set and forget" (the examples in this message explicitly spell
   it out with one-shot export syntax, but that is only for
   illustration) and not visible during usual use of the commands.

> But it would give require_work_tree somewhat interesting and unintuitive
> side effects.

Exactly. My knee-jerk reaction is that I do not think we would want
to go there, but I haven't thought things through.