[PATCH v2 2/2] stash: make sure we have a valid index before writing it

[Thomas Gummerer <t.gummerer@gmail.com>]

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>
---
 builtin/stash.c  | 7 +++----
 t/t3903-stash.sh | 7 ++++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index ab30d1e920..372fbdb7ac 100644
--- 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 db7cc6e664..0157771e24 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -263,8 +263,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
 '
 
@@ -273,6 +276,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 &&
 	echo foo >expect &&
 	git show :file >actual &&
 	test_cmp expect actual &&
-- 
2.24.0.155.gd9f6f3b619