threads / patch / 23465

patch, 2 parts(resend) stash bug: stash can lose data in a file removed from the index

Subject: [PATCH 1/2] (resend) stash bug: stash can lose data in a file removed from the index

## tl;dr

4 messages between Apr 15, 2010 and Apr 15, 2010. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Charles Bailey· Apr 15, 2010, 00:24 UTC · lore

If a file is removed from the index and then modified in the working tree then stash will discard the working tree file with no way to recover the changes.

This can might be done in one of a number of ways.

git rm file vi file # edit a new version git stash

or with git mv

git mv file newfile vi file # make a new file with the old name git stash

Signed-off-by: Charles Bailey <charles@hashpling.org>
---
 t/t3903-stash.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

Show changes to t/t3903-stash.sh +9 −0

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 476e5ec..c85bf44 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -228,4 +228,13 @@ test_expect_success 'stash --invalid-option' '
 	test bar,bar2 = $(cat file),$(cat file2)
 '
 
+test_expect_failure 'stash git rm' '
+    git reset --hard &&
+	git rm file &&
+	echo bar7 > file &&
+	git stash &&
+	git stash apply &&
+	test bar7 = $(cat file)
+'
+
 test_done

-- 
1.7.1.rc1.241.g4e72f

Charles Bailey· Apr 15, 2010, 00:24 UTC · re: Charles Bailey · lore

[PATCH 2/2] (resend) stash: Don't overwrite files that have gone from the index

---

This patch is deliberately not signed off. It feels kludgy and uses a non-portable xargs invocation.

It is designed as a starting point for discussion, that is all.

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

Show changes to git-stash.sh +2 −0

diff --git a/git-stash.sh b/git-stash.sh
index 908aab2..9efc544 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -87,6 +87,8 @@ create_stash () {
 			export GIT_INDEX_FILE &&
 			git read-tree -m $i_tree &&
 			git add -u &&
+			{ git diff --quiet --diff-filter=D --cached ||
+				git diff -z --name-only --diff-filter=D --cached | xargs -0 git add --ignore-errors; } &&
 			git write-tree &&
 			rm -f "$TMP-index"
 		) ) ||

-- 
1.7.1.rc1.241.g4e72f

Thomas Rast· Apr 15, 2010, 07:33 UTC · re: Charles Bailey · lore

Re: [PATCH 2/2] (resend) stash: Don't overwrite files that have gone from the index

Charles Bailey wrote:

Show 10 quoted lines

> diff --git a/git-stash.sh b/git-stash.sh
> index 908aab2..9efc544 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -87,6 +87,8 @@ create_stash () {
>  			export GIT_INDEX_FILE &&
>  			git read-tree -m $i_tree &&
>  			git add -u &&
> +			{ git diff --quiet --diff-filter=D --cached ||
> +				git diff -z --name-only --diff-filter=D --cached | xargs -0 git add --ignore-errors; } &&

I think you'll also have to turn it into an 'add -f' invocation since the file in question could conceivably have been ignored-but-tracked, and straight 'add' would refuse to re-track it.

(No, I don't have any good ideas on how to get rid of xargs short of some shell loop...)

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

Jeff King· Apr 15, 2010, 07:38 UTC · re: Thomas Rast · lore

Re: [PATCH 2/2] (resend) stash: Don't overwrite files that have gone from the index

On Thu, Apr 15, 2010 at 09:33:40AM +0200, Thomas Rast wrote:

Show 18 quoted lines

> Charles Bailey wrote:
> > diff --git a/git-stash.sh b/git-stash.sh
> > index 908aab2..9efc544 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -87,6 +87,8 @@ create_stash () {
> >  			export GIT_INDEX_FILE &&
> >  			git read-tree -m $i_tree &&
> >  			git add -u &&
> > +			{ git diff --quiet --diff-filter=D --cached ||
> > +				git diff -z --name-only --diff-filter=D --cached | xargs -0 git add --ignore-errors; } &&
> 
> I think you'll also have to turn it into an 'add -f' invocation since
> the file in question could conceivably have been ignored-but-tracked,
> and straight 'add' would refuse to re-track it.
> 
> (No, I don't have any good ideas on how to get rid of xargs short of
> some shell loop...)

It should probably use update-index, which has a --stdin mode, and which doesn't care about ignores.

-Peff

PS I have not looked carefully into this issue yet, so I don't know if
   the patch actually does the right thing or not.