Re: [PATCH 2/2] stash push: remove duplicate changes detection
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 6, 2026, 14:27 UTC
- Message-ID
- <xmqqpkxmgb00.fsf@gitster.g>
- In-Reply-To
- <95b7d582a2f86a3db4a9e182e482e9eb904ddeee.1791218125.git.phillip.wood@dunelm.org.uk>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 12 quoted lines
> 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;
> }It may be time for us to introduce symbolic constants once we have three choices instead of two.
Show 24 quoted lines
> @@ -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)Before the precontext of this hunk, repo_refresh_and_write_index() is called to refresh the index. We used to leave early when check_changes() saw no need to save. We no longer do so, and instead keep going.
Show 9 quoted lines
> @@ -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);And we call do_create_stash(). The first thing it does is to call repo_read_index_preload() and repo_refresh_and_write_index().
Are we refreshing the index twice now, even though we know nothing has changed in between, when we run "git stash push"?
do_create_stash() does call check_changes() to return early without creating stash, so we did save the cost of check_changes() with this patch, though.
Show 9 quoted lines
> + if (ret == 2) {
> + if (!quiet)
> + printf_ln(_("No local changes to save"));
> + ret = 0;
> + goto done;
> + } else if (ret) {
> ret = -1;
> goto done;
> }