git stash doesn't always save work dir as-is: bug?

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

git stash doesn't always save work dir as-is: bug?

Chris Torek
When "git stash" does its work, if the index and the work
directory are out of sync, but the work directory is in sync with
the HEAD commit, the work directory commit does not contain the
file in its work-directory state, but rather in its index-state.

This seems wrong.  For instance, "git stash branch recover"
gets you the wrong work-directory state in the new branch.

Here's a simple test-case, its output (before and after ?fix?),
and a possible fix, although I'm not at all sure this doesn't
break other cases; I don't properly understand the code here.
(I did not touch the "$patch_mode" logic either as it's entirely
different, and looks correct at first blush.)

(Also, not that I have a patch for this, the man page DISCUSSION
section needs to add that there's a third parent holding unstaged
files when doing "git stash save -u".)

Chris

----snip---- test-stash.sh
#! /bin/sh

fatal()
{
        echo "$@" >&2
        exit 1
}

cd /tmp || fatal "can't cd to /tmp"
rm -rf test_stash || fatal "can't remove old test_stash"
mkdir test_stash &&
cd test_stash &&
git init -q || fatal "can't make test_stash"

echo base > basefile &&
git add basefile &&
git commit -q -m initial || fatal "can't create initial commit"

echo add to basefile >> basefile &&
git add basefile &&
sed -i .bak -e '2d' basefile || fatal "can't set up the problem"
rm -f basefile.bak

echo "status before stash:"
git status --short

git stash save || fatal "stash failed"

echo "stash created"
echo "in the index, basefile contains:"
git show stash^2:basefile | cat -n
echo
echo "in the WIP, basefile contains:"
git show stash:basefile | cat -n
echo
echo "in the actual basefile:"
cat -n basefile
----snip---- ./test-stash.sh (with original stash script)
status before stash:
MM basefile
Saved working directory and index state WIP on master: c334d9e initial
HEAD is now at c334d9e initial
stash created
in the index, basefile contains:
     1 base
     2 add to basefile

in the WIP, basefile contains:
     1 base
     2 add to basefile

in the actual basefile:
     1 base
----snip---- ./test-stash.sh (after ?fix?)
status before stash:
MM basefile
Saved working directory and index state WIP on master: 3097b8b initial
HEAD is now at 3097b8b initial
stash created
in the index, basefile contains:
     1 base
     2 add to basefile

in the WIP, basefile contains:
     1 base

in the actual basefile:
     1 base
----snip---- possible fix
commit 5288ca30b9425a8c3fd1eb179706275cda3eb717
Author: Chris Torek <[hidden email]>
Date:   Sat Sep 7 18:13:31 2013 -0600

    stash: diff work dir against index
   
    When saving a full stash, compare the work directory against the
    already-saved index tree, rather than the HEAD commit, in case
    the work tree reverts things back to the HEAD.

diff --git a/git-stash.sh b/git-stash.sh
index 1e541a2..02818ae 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -115,7 +115,7 @@ create_stash () {
  git read-tree --index-output="$TMPindex" -m $i_tree &&
  GIT_INDEX_FILE="$TMPindex" &&
  export GIT_INDEX_FILE &&
- git diff --name-only -z HEAD -- >"$TMP-stagenames" &&
+ git diff --name-only -z $i_tree -- >"$TMP-stagenames" &&
  git update-index -z --add --remove --stdin <"$TMP-stagenames" &&
  git write-tree &&
  rm -f "$TMPindex"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: git stash doesn't always save work dir as-is: bug?

Dan Lenski
Chris Torek <chris.torek <at> gmail.com> writes:

>
> When "git stash" does its work, if the index and the work
> directory are out of sync, but the work directory is in sync with
> the HEAD commit, the work directory commit does not contain the
> file in its work-directory state, but rather in its index-state.

I too encountered this bug while trying to design a custom pre-commit hook
that would speculatively change the working tree and then "stash pop" its
changes. Chris Torek alerted me to it on StackOverflow:
http://stackoverflow.com/questions/25536034/modifying-working-directory-and-
staging-area-temporarily-in-git-pre-commit-hook

Another small issue I had in solving the same problem: there's no way to
tell if "git stash save" has created a new stash or not, so it's hard to
figure out whether one should be popped later.

It would be helpful if there were a specific non-zero return code assigned
when no stash was generated due to no changes requiring it. Here's a simple
patch for that:

--- a/git-stash
+++ b/git-stash
@@ -220,8 +220,7 @@ save_stash () {
        git update-index -q --refresh
        if no_changes
        then
-               say "$(gettext "No local changes to save")"
-               exit 0
+               die_with_status 9 "$(gettext "No local changes to save")"
        fi
        test -f "$GIT_DIR/logs/$ref_stash" ||
                clear_stash || die "$(gettext "Cannot initialize stash")"

Thanks,
Dan

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html