Re: [PATCH 2/2] stash push: remove duplicate changes detection
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 7, 2026, 17:15 UTC
- Message-ID
- <xmqqa4opa0up.fsf@gitster.g>
- In-Reply-To
- <725487d3-5a07-40c6-a603-990a661e0193@gmail.com>
Phillip Wood <phillip.wood123@gmail.com> writes:
Show 23 quoted lines
>> 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.
>>
>>> @@ -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"?
>
> We've always been doing that when there is something to stash (which is
> the normal case).So when there is nothing to stash, we only refreshed once but now refreshing twice is not a regression?
Show 6 quoted lines
> To avoid that we need to make the callers of do_create_stash() > responsible for refreshing the index. At the moment > create_stash() calls do_create_stash() without evening reading the > index, but do_push_stash() needs to read the index to check if it > the pathspec contains paths that don't match the index. That would > also avoid calling preload_index() multiple times.
Yes, that exactly is the right direction. Then we can reduce the number of check_changes() without increasing the number of refreshes, right?
Thanks.