stash: make sure we have a valid index before writing it

In 'do_apply_stash()' we refresh the index in the end.  Since
34933d0eff ("stash: make sure to write refreshed cache", 2019-09-11),
we also write that refreshed index when --quiet is given to 'git stash
apply'.

However if '--index' is not given to 'git stash apply', we also
discard the index in the else clause just before.  We need to do so
because we use an external 'git update-index --add --stdin', which
leads to an out of date in-core index.

Later we call 'refresh_and_write_cache', which now leads to writing
the discarded index, which means we essentially write an empty index
file.  This is obviously not correct, or the behaviour the user
wanted.  We should not modify the users index without being asked to
do so.

Make sure to re-read the index after discarding the current in-core
index, to avoid dealing with outdated information.  Instead we could
also drop the 'discard_cache()' + 'read_cache()', however that would
make it easy to fall into the same trap as 34933d0eff did, so it's
better to avoid that.

We can also drop the 'refresh_and_write_cache' completely in the quiet
case.  Previously in legacy stash we relied on 'git status' to refresh
the index after calling 'git read-tree' when '--index' was passed to
'git apply'.  However the 'reset_tree()' call that replaced 'git
read-tree' always passes options that are equivalent to '-m', making
the refresh of the index unnecessary.

Reported-by: Grzegorz Rajchman <rayman17@gmail.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/stash.c b/builtin/stash.c
index ab30d1e9203b42dec154b4b171f74ba7fec1b6b7..372fbdb7ac059cb858ae959266bd1ce0603b0752 100644 (file)
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -481,13 +481,12 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
                if (ret)
                        return -1;
 
+               /* read back the result of update_index() back from the disk */
                discard_cache();
+               read_cache();
        }
 
-       if (quiet) {
-               if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
-                       warning("could not refresh index");
-       } else {
+       if (!quiet) {
                struct child_process cp = CHILD_PROCESS_INIT;
 
                /*

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 392954d6dd9243faa03a9092f21ba9959acdc371..9de1c3616ab3bed69cf3d7012b8866ce57a34965 100755 (executable)
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -232,8 +232,11 @@ test_expect_success 'save -q is quiet' '
        test_must_be_empty output.out
 '
 
-test_expect_success 'pop -q is quiet' '
+test_expect_success 'pop -q works and is quiet' '
        git stash pop -q >output.out 2>&1 &&
+       echo bar >expect &&
+       git show :file >actual &&
+       test_cmp expect actual &&
        test_must_be_empty output.out
 '
 
@@ -242,6 +245,8 @@ test_expect_success 'pop -q --index works and is quiet' '
        git add file &&
        git stash save --quiet &&
        git stash pop -q --index >output.out 2>&1 &&
+       git diff-files file2 >file2.diff &&
+       test_must_be_empty file2.diff &&
        test foo = "$(git show :file)" &&
        test_must_be_empty output.out
 '