patch, 2 partsstash: stop checking for changes twice

3 messages between Oct 5, 2026 and Oct 5, 2026, from Phillip Wood.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Phillip WoodOct 5, 2026, 16:35 UTC on lore

"git stash push" and "git stash create" check if there are any unstaged or uncommitted changes at startup, and then again when they try to create the stash. This short series removes that duplication of effort which I spotted while looking at <20260929074222.11942-1-kazumasa.shigeta@kanamei.com>.

base-commit: c46c1e37724f0478939de636ab8ea5a89086d532
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fstash-optimize-check_changes-calls%2Fv1
View-Changes-At: https://github.com/phillipwood/git/compare/c46c1e377...95b7d582a
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/stash-optimize-check_changes-calls/v1

Phillip Wood (2):
  stash create: remove duplicate changes detection
  stash push: remove duplicate changes detection

 builtin/stash.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

-- 
2.56.0.134.g299a3c16181

Phillip WoodOct 5, 2026, 16:35 UTC in reply to Phillip Wood on lore

[PATCH 1/2] stash create: remove duplicate changes detection

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Before it creates a stash, git checks if there are any unstaged, or uncommitted changes. If there isn't anything to stash it bails out. Since ef0f0b4509 (stash: optimize `get_untracked_files()` and `check_changes()`, 2019-02-25) "git stash store" has checked for changes twice, once in create_stash() before we refresh the index and then again in do_create_stash() after the index has been refreshed. That commit claims it is an optimization but it is not clear what it is trying to optimize by checking for changes twice, especially as checking for changes before refreshing the index is unreliable (the scripted version of "git stash store", called "git update-index -q --refresh" before looking for any changes).

Avoid checking for changes twice by removing the call to check_changes_tracked_files() from store_stash() and restore the return code handling in store_stash() that was removed by ef0f0b4509 so that we continue to exit 0 when there are no changes to stash. In principle we could remove the call to check_changes() from do_store_stash() instead, but then we'd need to pass in the list of untracked files.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/stash.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Show changes to builtin/stash.c +5 −3

diff --git a/builtin/stash.c b/builtin/stash.c
index 7a9843413b1..9a5006e3d92 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1655,8 +1655,6 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
 	strbuf_join_argv(&stash_msg_buf, argc - 1, ++argv, ' ');
 
 	memset(&ps, 0, sizeof(ps));
-	if (!check_changes_tracked_files(&ps))
-		return 0;
 
 	ret = do_create_stash(&ps, &stash_msg_buf, 0, 0, NULL, 0, &info,
 			      NULL, 0);
@@ -1665,7 +1663,11 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
 
 	free_stash_info(&info);
 	strbuf_release(&stash_msg_buf);
-	return ret;
+	/*
+	 * ret is 1 if there were no changes. In this case, we should
+	 * not error out.
+	 */
+	return ret < 0;
 }
 
 static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int quiet,

-- 
2.56.0.134.g299a3c16181

Phillip WoodOct 5, 2026, 16:35 UTC in reply to Phillip Wood on lore

[PATCH 2/2] stash push: remove duplicate changes detection

From: Phillip Wood <phillip.wood@dunelm.org.uk>

Before it creates a stash, git checks if there are any unstaged, or uncommitted changes. If there isn't anything to stash it bails out. "git stash push" checks for changes twice, once in do_push_stash() and then again in do_create_stash(). Avoid that by removing the call to check_changes() from do_push_stash() and checking the return value of do_create_stash() to see if there were any changes to stash. If check_changes() finds there are no changes do_create_stash() now returns 2 rather than 1. This enables us to distinguish between there being no changes and there being nothing stashed so that "git stash push --patch" when nothing is select, and "git stash push --staged" when the index matches HEAD still exit 1 rather than 0.

There is still one small change in behavior as, if there is nothing to stash, we'll try now to create the reflog for stashes before we realize that there is nothing to stash. I don't think that should matter in practice.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 builtin/stash.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

Show changes to builtin/stash.c +12 −11

diff --git a/builtin/stash.c b/builtin/stash.c
index 9a5006e3d92..79fdfff09a2 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1538,7 +1538,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	}
 
 	if (!check_changes(ps, include_untracked, &untracked_files)) {
-		ret = 1;
+		ret = 2;
 		goto done;
 	}
 
@@ -1664,8 +1664,8 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED,
 	free_stash_info(&info);
 	strbuf_release(&stash_msg_buf);
 	/*
-	 * ret is 1 if there were no changes. In this case, we should
-	 * not error out.
+	 * ret is greater than zero if there were no changes. In this case,
+	 * we should not error out.
 	 */
 	return ret < 0;
 }
@@ -1728,12 +1728,6 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 		goto done;
 	}
 
-	if (!check_changes(ps, include_untracked, &untracked_files)) {
-		if (!quiet)
-			printf_ln(_("No local changes to save"));
-		goto done;
-	}
-
 	if (!refs_reflog_exists(get_main_ref_store(the_repository), ref_stash) && do_clear_stash()) {
 		ret = -1;
 		if (!quiet)
@@ -1743,8 +1737,15 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 
 	if (stash_msg)
 		strbuf_addstr(&stash_msg_buf, stash_msg);
-	if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode,
-			    interactive_opts, only_staged, &info, &patch, quiet)) {
+	ret =  do_create_stash(ps, &stash_msg_buf, include_untracked,
+			       patch_mode, interactive_opts, only_staged, &info,
+			       &patch, quiet);
+	if (ret == 2) {
+		if (!quiet)
+			printf_ln(_("No local changes to save"));
+		ret = 0;
+		goto done;
+	} else if (ret) {
 		ret = -1;
 		goto done;
 	}

-- 
2.56.0.134.g299a3c16181