stash: make sure to write refreshed cache

When converting stash into C, calls to 'git update-index --refresh' were replaced with the 'refresh_cache()' function. That is fine as long as the index is only needed in-core, and not re-read from disk. However in many cases we do actually need the refreshed index to be written to disk, for example 'merge_recursive_generic()' discards the in-core index before re-reading it from disk, and in the case of 'apply --quiet', the 'refresh_cache()' we currently have is pointless without writing the index to disk. Always write the index after refreshing it to ensure there are no regressions in this compared to the scripted stash. In the future we can consider avoiding the write where possible after making sure none of the subsequent calls actually need the refreshed cache, and it is not expected to be refreshed after stash exits or it is written somewhere else already. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Thomas Gummerer committed Sep 11, 2019 at 19:20 UTC 34933d0eff5d4c91fae6ad6f71a6e6a69a496ced
2 files changed +23 -4
builtin/stash.c
+7 -4
@@ -396,7 +396,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
396 const struct object_id *bases[1];
397
398 read_cache_preload(NULL);
399 - if (refresh_cache(REFRESH_QUIET))
399 + if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
400 return -1;
401
402 if (write_cache_as_tree(&c_tree, 0, NULL))
@@ -485,7 +485,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
485 }
486
487 if (quiet) {
488 - if (refresh_cache(REFRESH_QUIET))
488 + if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
489 warning("could not refresh index");
490 } else {
491 struct child_process cp = CHILD_PROCESS_INIT;
@@ -1129,7 +1129,10 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
1129 prepare_fallback_ident("git stash", "git@stash");
1130
1131 read_cache_preload(NULL);
1132 - refresh_cache(REFRESH_QUIET);
1132 + if (refresh_and_write_cache(REFRESH_QUIET, 0, 0) < 0) {
1133 + ret = -1;
1134 + goto done;
1135 + }
1136
1137 if (get_oid("HEAD", &info->b_commit)) {
1138 if (!quiet)
@@ -1290,7 +1293,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
1293 free(ps_matched);
1294 }
1295
1293 - if (refresh_cache(REFRESH_QUIET)) {
1296 + if (refresh_and_write_cache(REFRESH_QUIET, 0, 0)) {
1297 ret = -1;
1298 goto done;
1299 }
t/t3903-stash.sh
+16
@@ -1241,4 +1241,20 @@ test_expect_success 'stash --keep-index with file deleted in index does not resu
1241 test_path_is_missing to-remove
1242 '
1243
1244 +test_expect_success 'stash apply should succeed with unmodified file' '
1245 + echo base >file &&
1246 + git add file &&
1247 + git commit -m base &&
1248 +
1249 + # now stash a modification
1250 + echo modified >file &&
1251 + git stash &&
1252 +
1253 + # make the file stat dirty
1254 + cp file other &&
1255 + mv other file &&
1256 +
1257 + git stash apply
1258 +'
1259 +
1260 test_done