git stash pop not reapplying deletions

git stash pop not reapplying deletions

[Steve Folly]

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

Re: git stash pop not reapplying deletions

[Thomas Rast]

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.

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

Re: git stash pop not reapplying deletions

[Steve Folly]

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

> 
> 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

Re: git stash pop not reapplying deletions

[Thomas Rast]

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:
> 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

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

[Thomas Rast]

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

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

[Stephen Boyd]

On 02/15/2010 08:05 AM, Thomas Rast wrote:
> 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>

Re: git stash pop not reapplying deletions

[Steve Folly]

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

Re: git stash pop not reapplying deletions

[Junio C Hamano]

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

> 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.