git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Thomas Gummerer <t.gummerer@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Joel Teichroeb" <joel@teichroeb.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jeff King" <peff@peff.net>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Thomas Gummerer" <t.gummerer@gmail.com>
Subject: [PATCH v4 3/3] stash: make sure to write refreshed cache
Date: Wed, 11 Sep 2019 19:20:27 +0100	[thread overview]
Message-ID: <20190911182027.41284-4-t.gummerer@gmail.com> (raw)
In-Reply-To: <20190911182027.41284-1-t.gummerer@gmail.com>

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

diff --git a/builtin/stash.c b/builtin/stash.c
index b5a301f24d..ab30d1e920 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -396,7 +396,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	const struct object_id *bases[1];
 
 	read_cache_preload(NULL);
-	if (refresh_cache(REFRESH_QUIET))
+	if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
 		return -1;
 
 	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,
 	}
 
 	if (quiet) {
-		if (refresh_cache(REFRESH_QUIET))
+		if (refresh_and_write_cache(REFRESH_QUIET, 0, 0))
 			warning("could not refresh index");
 	} else {
 		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
 	prepare_fallback_ident("git stash", "git@stash");
 
 	read_cache_preload(NULL);
-	refresh_cache(REFRESH_QUIET);
+	if (refresh_and_write_cache(REFRESH_QUIET, 0, 0) < 0) {
+		ret = -1;
+		goto done;
+	}
 
 	if (get_oid("HEAD", &info->b_commit)) {
 		if (!quiet)
@@ -1290,7 +1293,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		free(ps_matched);
 	}
 
-	if (refresh_cache(REFRESH_QUIET)) {
+	if (refresh_and_write_cache(REFRESH_QUIET, 0, 0)) {
 		ret = -1;
 		goto done;
 	}
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index b8e337893f..392954d6dd 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1241,4 +1241,20 @@ test_expect_success 'stash --keep-index with file deleted in index does not resu
 	test_path_is_missing to-remove
 '
 
+test_expect_success 'stash apply should succeed with unmodified file' '
+	echo base >file &&
+	git add file &&
+	git commit -m base &&
+
+	# now stash a modification
+	echo modified >file &&
+	git stash &&
+
+	# make the file stat dirty
+	cp file other &&
+	mv other file &&
+
+	git stash apply
+'
+
 test_done
-- 
2.23.0.rc2.194.ge5444969c9


      parent reply	other threads:[~2019-09-11 18:21 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-27 10:14 [PATCH 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-27 10:14 ` [PATCH 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-28 15:49   ` Martin Ågren
2019-08-29 17:59     ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-28 15:52   ` Martin Ågren
2019-08-29 18:00     ` Thomas Gummerer
2019-08-27 10:14 ` [PATCH 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-08-29 18:27 ` [PATCH v2 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-08-29 18:27   ` [PATCH v2 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-08-30 15:07     ` Martin Ågren
2019-08-30 17:06       ` Junio C Hamano
2019-09-02 17:15         ` Thomas Gummerer
2019-09-03 17:43           ` Junio C Hamano
2019-08-29 18:27   ` [PATCH v2 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-08-29 18:27   ` [PATCH v2 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-03 19:10   ` [PATCH v3 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-09-03 19:10     ` [PATCH v3 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-05 22:00       ` Junio C Hamano
2019-09-06 14:18         ` Thomas Gummerer
2019-09-11 10:57           ` Johannes Schindelin
2019-09-11 17:52             ` Thomas Gummerer
2019-09-12 16:46               ` Junio C Hamano
2019-09-03 19:10     ` [PATCH v3 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-03 19:10     ` [PATCH v3 3/3] stash: make sure to write refreshed cache Thomas Gummerer
2019-09-11 18:20     ` [PATCH v4 0/3] make sure stash refreshes the index properly Thomas Gummerer
2019-09-11 18:20       ` [PATCH v4 1/3] factor out refresh_and_write_cache function Thomas Gummerer
2019-09-11 18:20       ` [PATCH v4 2/3] merge: use refresh_and_write_cache Thomas Gummerer
2019-09-11 18:20       ` Thomas Gummerer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190911182027.41284-4-t.gummerer@gmail.com \
    --to=t.gummerer@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=joel@teichroeb.net \
    --cc=martin.agren@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).