threads / discuss / 22653

git stash pop not reapplying deletions

Subject: git stash pop not reapplying deletions

## tl;dr

8 messages between Feb 14, 2010 and Feb 16, 2010.

replies: 7people: 4as markdown or json

Steve Folly· Feb 14, 2010, 19:45 UTC · lore

Hi,

I'm not sure if I've found a bug in 'git stash' or if I'm using it the wrong way? (This is with git 1.6.6):

$ git init stashtest $ cd stashtest $ mkdir dira $ touch dira/a dira/b dira/c $ git stage dira $ git commit -m "added dira" $ git mv dira dirb $ git status # correctly shows renames

# On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # renamed: dira/a -> dirb/a # renamed: dira/b -> dirb/b # renamed: dira/c -> dirb/c #

$ git stash $ git stash pop

# On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # new file: dirb/a # new file: dirb/b # new file: dirb/c # # Changed but not updated: # (use "git add/rm <file>..." to update what will be committed) # (use "git checkout -- <file>..." to discard changes in working directory) # # deleted: dira/a # deleted: dira/b # deleted: dira/c # Dropped refs/stash@{0} (cf9efdede3a3ee8e078192b574520fd2ed7f3d9b)

It's added the new files in dirb but hasn't deleted the old files in dira. Is this right?

Regards, Steve

Thomas Rast· Feb 14, 2010, 22:08 UTC · re: Steve Folly · lore

Re: git stash pop not reapplying deletions

On Sunday 14 February 2010 20:45:03 Steve Folly wrote:

> 
> I'm not sure if I've found a bug in 'git stash' or if I'm using 
> it the wrong way? (This is with git 1.6.6):
>

[eliding a lot everywhere to make it clearer]

Show 14 quoted lines

> $ git status   # correctly shows renames
> #	renamed:    dira/a -> dirb/a
> #	renamed:    dira/b -> dirb/b
> #	renamed:    dira/c -> dirb/c
> $ git stash
> $ git stash pop
> # Changes to be committed:
> #	new file:   dirb/a
> #	new file:   dirb/b
> #	new file:   dirb/c
> # Changed but not updated:
> #	deleted:    dira/a
> #	deleted:    dira/b
> #	deleted:    dira/c

The problem is that you aren't using --index, but still expecting it to restore your index. If you change it to 'git stash pop --index', everything will work as expected.

Yes, it does stage new files, but that is only to help you: otherwise you could forget them before committing.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Steve Folly· Feb 15, 2010, 14:32 UTC · re: Thomas Rast · lore

Re: git stash pop not reapplying deletions

Thomas Rast <trast <at> student.ethz.ch> writes:

Show 25 quoted lines

> 
> On Sunday 14 February 2010 20:45:03 Steve Folly wrote:
> > 
> > I'm not sure if I've found a bug in 'git stash' or if I'm using 
> > it the wrong way? (This is with git 1.6.6):
> > 
> [eliding a lot everywhere to make it clearer]
> > $ git status   # correctly shows renames
> > #	renamed:    dira/a -> dirb/a
> > #	renamed:    dira/b -> dirb/b
> > #	renamed:    dira/c -> dirb/c
> > $ git stash
> > $ git stash pop
> > # Changes to be committed:
> > #	new file:   dirb/a
> > #	new file:   dirb/b
> > #	new file:   dirb/c
> > # Changed but not updated:
> > #	deleted:    dira/a
> > #	deleted:    dira/b
> > #	deleted:    dira/c
> 
> The problem is that you aren't using --index, but still expecting it
> to restore your index.  If you change it to 'git stash pop --index',
> everything will work as expected.

OK, yep - got it. Thanks.

> Yes, it does stage new files, but that is only to help you: otherwise
> you could forget them before committing.
>

But that's even more confusing - not using --index only restores *some* of the index. To be honest, that's not really helping - I still have to stage deletions manually.

If not using --index isn't supposed to restore the index, then surely it shouldn't be staging the new files?

Cheers Steve

Thomas Rast· Feb 15, 2010, 15:41 UTC · re: Steve Folly · lore

Re: git stash pop not reapplying deletions

Please don't remove the Cc list, it helps us (among other things) flag the mail that is important.

On Monday 15 February 2010 15:32:44 Steve Folly wrote:

Show 12 quoted lines

> Thomas Rast <trast <at> student.ethz.ch> writes:
> > Yes, [git stash without --index] does stage new files, but that is
> > only to help you: otherwise you could forget them before
> > committing.
> 
> But that's even more confusing - not using --index only 
> restores *some* of the index. To be honest, 
> that's not really helping - I still have to stage deletions 
> manually.
> 
> If not using --index isn't supposed to restore the index, 
> then surely it shouldn't be staging the new files?

Not sure. I personally prefer the behaviour as it is now, as I have status.showUntrackedFiles = no and would most likely forget about them. It has been that way since the introduction of git-stash, so maybe Nanako (Cc'd) can enlighten us further.

That being said, it could probably be made configurable along the lines of the patch below, or even with a config option.

diff --git i/git-stash.sh w/git-stash.sh
index 3a0685f..fc56e1b 100755
--- i/git-stash.sh
+++ w/git-stash.sh
@@ -222,12 +222,26 @@ show_stash () {
 
 apply_stash () {
 	unstash_index=
+	dont_touch_index=
 
 	while test $# != 0
 	do
 		case "$1" in
 		--index)
 			unstash_index=t
+			dont_touch_index=
+			;;
+		--index=apply)
+			unstash_index=t
+			dont_touch_index=
+			;;
+		--index=added-only)
+			unstash_index=
+			dont_touch_index=
+			;;
+		--index=none)
+			unstash_index=
+			dont_touch_index=t
 			;;
 		-q|--quiet)
 			GIT_QUIET=t
@@ -293,7 +307,7 @@ apply_stash () {
 			a="$TMP-added" &&
 			git diff-index --cached --name-only --diff-filter=A $c_tree >"$a" &&
 			git read-tree --reset $c_tree &&
-			git update-index --add --stdin <"$a" ||
+			(test -n "$dont_touch_index" || git update-index --add --stdin <"$a") ||
 				die "Cannot unstage modified files"
 			rm -f "$a"
 		fi

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Thomas Rast· Feb 15, 2010, 16:01 UTC · re: Thomas Rast · lore

[PATCH] stash pop: remove 'apply' options during 'drop' invocation

The 'git stash pop' option parsing used to remove the first argument in --index mode. At the time this was implemented, this first argument was always --index. However, since the invention of the -q option in fcdd0e9 (stash: teach quiet option, 2009-06-17) 'git stash pop') you can cause an internal invocation of

  git stash drop --index

by running

  git stash pop -q --index

which then of course fails because drop doesn't know --index.

To handle this, instead let 'git stash apply' decide what the future argument to 'drop' should be.

Warning: this means that 'git stash apply' must parse all options that
'drop' can take, and deal with them in the same way.  This is
currently true for its only option -q.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

Noticed this while whipping up the draft patch. I'm trading one maintainability problem (that bit us) for another (that hasn't... yet), but I'm scared to try properly filtering the arguments in sh.

 git-stash.sh     |    7 +++++--
 t/t3903-stash.sh |    9 +++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 3a0685f..2d69196 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -221,6 +221,7 @@ show_stash () {
 }
 
 apply_stash () {
+	applied_stash=
 	unstash_index=
 
 	while test $# != 0
@@ -242,6 +243,9 @@ apply_stash () {
 	if test $# = 0
 	then
 		have_stash || die 'Nothing to apply'
+		applied_stash="$ref_stash@{0}"
+	else
+		applied_stash="$*"
 	fi
 
 	# stash records the work tree, and is a merge between the
@@ -415,8 +419,7 @@ pop)
 	shift
 	if apply_stash "$@"
 	then
-		test -z "$unstash_index" || shift
-		drop_stash "$@"
+		drop_stash "$applied_stash"
 	fi
 	;;
 branch)
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 5514f74..476e5ec 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -194,6 +194,15 @@ test_expect_success 'pop -q is quiet' '
 	test ! -s output.out
 '
 
+test_expect_success 'pop -q --index works and is quiet' '
+	echo foo > file &&
+	git add file &&
+	git stash save --quiet &&
+	git stash pop -q --index > output.out 2>&1 &&
+	test foo = "$(git show :file)" &&
+	test ! -s output.out
+'
+
 test_expect_success 'drop -q is quiet' '
 	git stash &&
 	git stash drop -q > output.out 2>&1 &&

-- 
1.7.0.225.g2927b

Stephen Boyd· Feb 15, 2010, 18:46 UTC · re: Thomas Rast · lore

Re: [PATCH v2] stash pop: remove 'apply' options during 'drop' invocation

On 02/15/2010 08:05 AM, Thomas Rast wrote:

Show 23 quoted lines

> The 'git stash pop' option parsing used to remove the first argument
> in --index mode.  At the time this was implemented, this first
> argument was always --index.  However, since the invention of the -q
> option in fcdd0e9 (stash: teach quiet option, 2009-06-17) you can
> cause an internal invocation of
>
>   git stash drop --index
>
> by running
>
>   git stash pop -q --index
>
> which then of course fails because drop doesn't know --index.
>
> To handle this, instead let 'git stash apply' decide what the future
> argument to 'drop' should be.
>
> Warning: this means that 'git stash apply' must parse all options that
> 'drop' can take, and deal with them in the same way.  This is
> currently true for its only option -q.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>

Acked-by: Stephen Boyd <bebarino@gmail.com>

Steve Folly· Feb 15, 2010, 21:09 UTC · re: Thomas Rast · lore

Re: git stash pop not reapplying deletions

Thomas Rast <trast <at> student.ethz.ch> writes:

> 
> Please don't remove the Cc list, it helps us (among other things) flag
> the mail that is important.

It's probably happened again, but not my fault - I'm reply via gmane.org, not email.

Steve

Junio C Hamano· Feb 16, 2010, 02:17 UTC · re: Thomas Rast · lore

Re: git stash pop not reapplying deletions

Thomas Rast <trast@student.ethz.ch> writes:

Show 6 quoted lines

> The problem is that you aren't using --index, but still expecting it
> to restore your index.  If you change it to 'git stash pop --index',
> everything will work as expected.
>
> Yes, it does stage new files, but that is only to help you: otherwise
> you could forget them before committing.

I think the reason _new_ files are added is simply because ack then there was no other way to make it tracked.

A sane thing to do these days might be to add a newly created file with the "intent to add" option, to mimic the way other files in the work tree with changes are unstashed. They get the full change in the work tree, without restoring what their corresponding index entry used to have when the stash was made, if you unstash without the --index option.