git stash: status from current dir not top dir?

Hi,

git status shows paths relative to current directory, so it's possible to copy&paste them directly, even if you're in a subdirectory.

But "git stash apply" shows status from root of git repository. This is misleading because you can't copy and paste the paths.

I wonder if it's possible/better to change it so "git stash apply" shows status relative to directory from which it was executed?

OTOH "git commit" also shows paths relative to root. So maybe git status should be changed (hopefully no, because the relative paths are quite useful IMO).

This patch tries to fix git-stash.sh to show status relative to current directory. I can resend the patch with better commit message.

Example:

$ mkdir d && cd d && echo a > ../topfile && echo b > subfile && git add ..
$ git commit -m 1
[master (root-commit) 6935958] 1
 2 files changed, 2 insertions(+), 0 deletions(-)
 create mode 100644 d/subfile
 create mode 100644 topfile
$ echo x > ../topfile ; echo y > subfile 
$ git status
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#	modified:   subfile
#	modified:   ../topfile
#
no changes added to commit (use "git add" and/or "git commit -a")
$ git stash
Saved working directory and index state WIP on master: 6935958 1
HEAD is now at 6935958 1
$ git stash pop
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#	modified:   d/subfile
#	modified:   topfile
#
no changes added to commit (use "git add" and/or "git commit -a")
Dropped refs/stash@{0} (3c7c82dba01feed37c725a795116354f6e229d76)
$ git checkout -- topfile
error: pathspec 'd/topfile' did not match any file(s) known to git.

With the patch:

$ git stash pop
# On branch master
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working directory)
#
#	modified:   subfile
#	modified:   ../topfile
#
no changes added to commit (use "git add" and/or "git commit -a")
Dropped refs/stash@{0} (4a0f7bb0a3e2902ba9046686d457b1b8f1ade04c)

---8<---
From: Piotr Krukowiecki <[email protected]>
Date: Fri, 11 Mar 2011 20:50:49 +0100
Subject: [PATCH] git stash: show status relative to currect directory

---
 git-stash.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

index 7561b37..586c12f 100755
@@ -12,6 +12,7 @@ USAGE="list [<options>]
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+START_DIR=`pwd`
 . git-sh-setup
 require_work_tree
 cd_to_toplevel

@@ -394,7 +395,7 @@ apply_stash () {
 		then
 			squelch='>/dev/null 2>&1'
 		fi
-		eval "git status $squelch" || :
+		(cd "$START_DIR" && eval "git status $squelch") || :
 	else
 		# Merge conflict; keep the exit status from merge-recursive
 		status=$?

1.7.1

Yeah, I am inclined to call it a bug. git-status will show the status of the whole tree from wherever you are, and people who want full paths will have status.relativePaths turned off, anyway. So I think your proposed semantics are more natural.

Yes, please. There is lots of nice discussion in your email but none of it in the commit message. :)

Thanks for review,
attaching patch with updated commit message.

W dniu 11.03.2011 23:32, Jeff King pisze:

I think it's better this way too, one thing that made me doubt if this is intended or not is fact that git-commit also shows paths relative to root dir...

Didn't want to duplicate ;)

---8<---
From: Piotr Krukowiecki <[email protected]>
Date: Fri, 11 Mar 2011 20:50:49 +0100
Subject: [PATCH] git stash: show status relative to current directory

git status shows modified paths relative to current directory, so it's possible to copy&paste them directly, even if you're in a subdirectory.

But "git stash apply" always shows status from root of git repository. This is misleading because you can't use the paths without modifications.

This is caused by changing directory to root of repository at the beginning of git stash.

This patch makes git stash show status relative to current directory. Instead of removing the "cd to toplevel", which would affect whole script and might have other side-effects, the fix is to change directory temporarily back to original dir just before displaying status.

---
 git-stash.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

index 7561b37..b59c201 100755
@@ -12,6 +12,7 @@ USAGE="list [<options>]
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_SPEC=
+START_DIR=`pwd`
 . git-sh-setup
 require_work_tree
 cd_to_toplevel

@@ -394,7 +395,7 @@ apply_stash () {
 		then
 			squelch='>/dev/null 2>&1'
 		fi
-		eval "git status $squelch" || :
+		(cd "$START_DIR" && eval "git status $squelch") || :
 	else
 		# Merge conflict; keep the exit status from merge-recursive
 		status=$?

1.7.4.1.228.g9e388

Sensible. Thanks.

Don't we want to protect this output with some tests?

W dniu 14.03.2011 08:29, Junio C Hamano pisze:

Right. Wrote a test but it fails mysteriously. Looks like a debug output is added when test is run as "sh t3903-stash.sh" (the "Merging Version" etc). No such output when "git apply" is run by hand.

Not sure what to do with it?

With --verbose I see:

[...]
[master b27a2bc] subdir
Author: A U Thor <[email protected]>

 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 subdir/subfile1

Saved working directory and index state WIP on master: b27a2bc subdir HEAD is now at b27a2bc subdir

@@ -1,9 +1,3 @@
-Merging Version stash was based on with Stashed changes
-Merging:
-virtual Version stash was based on
-virtual Stashed changes
-found 1 common ancestor(s):
-virtual 13419d0b4f5b097f61dde4c911de99a154f8286f
 # On branch master
 # Changes not staged for commit:
 #   (use "git add <file>..." to update what will be committed)

not ok - 41 stash apply shows status same as git status (relative to current directory)

---8<---
From: Piotr Krukowiecki <[email protected]>
Date: Mon, 14 Mar 2011 20:19:36 +0100
Subject: [PATCH] Add test: git stash shows status relative to current dir

---
 t/t3903-stash.sh |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

index 6fd560c..3682f1c 100755
@@ -556,4 +556,19 @@ test_expect_success 'stash branch should not drop the stash if the branch exists
 	git rev-parse stash@{0} --
 '
 
+test_expect_success 'stash apply shows status same as git status (relative to current directory)' '
+	git stash clear &&
+	echo 1 > subdir/subfile1 &&
+	echo 2 > subdir/subfile2 &&
+	git add subdir/subfile1 &&
+	git commit -m subdir &&
+	cd subdir &&
+	echo x > subfile1 &&
+	echo x > ../file &&
+	git stash &&
+	git stash apply > ../output &&
+	git status > ../expect &&
+	test_cmp ../output ../expect
+'
+
 test_done

1.7.4.1.228.g9e388



-- 
Piotr Krukowiecki

W dniu 14.03.2011 20:45, Piotr Krukowiecki pisze:

To recap the problem was that "git stash apply" run during tests printed some debug messages as shown below. In result I could not compare its output with expected "git status" output.

I found out it was caused by
GIT_MERGE_VERBOSITY=5
in test-lib.sh

It was introduced in 8d0fc48f2730 with comment:
Its really nice to be able to run a test with -v and automatically see the "debugging" dump from merge-recursive, especially if we are actually trying to debug merge-recursive.

Now I don't know how should I handle this:

1. unset it just before "git stash apply" in my test A safe, local change

2. remove it from test-lib.sh
The variable changed git behaviour - might impact tests, should it be set by default?

3. add new option in test-lib.sh to set it (--merge-verbosity?) Also looks safe, but still some tests would fail with it (which would be mentioned in the option documentation)

4. change test-lib.sh to set it only when --verbose/--debug is passed This seems to be the intention, but test results would be different with those options (some tests would fail)!

First three choices look more or less sensible.

Piotr Krukowiecki

This is the preferred method; in addition to "a safe local", more importantly, at that point you are testing what you _want_ to be testing, namely, how the output appears to the _real_ end users who do not use verbose message. So for that purpose, mucking locally with MERGE_VERBOSITY is perfectly acceptable.

You would not just "unset it just before", but "unset around it" in a subshell like this:

	git stash &&
	(
		sane_unset GIT_MERGE_VERBOSITY &&
	        git stash apply
	) >../actual &&
	git status >../expect &&
        test_cmp ../expect ../actual

so that if somebody adds new tests later in the script, they are not affected by this change.

Write your test_cmp always to compare expected with actual, not the other way around, so that the diff output you see when the test is run under -v option shows the changes from what is expected.

Thanks.

W dniu 17.03.2011 20:30, Junio C Hamano pisze:

Thanks, updated according to your suggestions.

I've also added a check to see if the output contains a relative path (so we really test not only that git-stash shows the same status as git-status, but that the paths are relative).

I'm not resending the original patch for git-stash.sh - I don't know if it's expected to always send full set of patches?

---8<---
From: Piotr Krukowiecki <[email protected]>
Date: Mon, 14 Mar 2011 20:19:36 +0100
Subject: [PATCH 2/2] Add test: git stash shows status relative to current dir

---
 t/t3903-stash.sh |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

index 6fd560c..13f9ae8 100755
@@ -556,4 +556,23 @@ test_expect_success 'stash branch should not drop the stash if the branch exists
 	git rev-parse stash@{0} --
 '
 
+test_expect_success 'stash apply shows status same as git status (relative to current directory)' '
+	git stash clear &&
+	echo 1 > subdir/subfile1 &&
+	echo 2 > subdir/subfile2 &&
+	git add subdir/subfile1 &&
+	git commit -m subdir &&
+	cd subdir &&
+	echo x > subfile1 &&
+	echo x > ../file &&
+	git stash &&
+	(
+		sane_unset GIT_MERGE_VERBOSITY &&
+		git stash apply
+	) > ../actual &&
+	git status > ../expect &&
+	test_cmp ../expect ../actual &&
+	grep "[.][.]/actual" ../actual 
+'
+
 test_done

1.7.4.1.296.gca6da


-- 
Piotr Krukowiecki