Re: [PATCH v12 18/26] stash: convert push to builtin

>> > diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
>> > index c77f62c895..3dab488bd6 100644
>> > --- a/builtin/stash--helper.c
>> > +++ b/builtin/stash--helper.c
>> > @@ -231,6 +231,7 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
>> >  	struct tree *tree;
>> >  	struct lock_file lock_file = LOCK_INIT;
>> >  
>> > +	discard_cache();
>> >  	read_cache_preload(NULL);
>> >  	if (refresh_cache(REFRESH_QUIET))
>> >  		return -1;
>> > 
>
> So this is working, but it is not the correct spot for that
> `discard_cache()`, as it forces unnecessary cycles on code paths calling
> `reset_tree()` (which corresponds to `git read-tree`, admittedly a bit
> confusing) with a fully up to date index.
>
> The real fix, I believe, is this:
>
> -- snip --
> diff --git a/builtin/stash.c b/builtin/stash.c
> index 2d6dfce883..516dee0fa4 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -1372,6 +1372,7 @@ static int do_push_stash(struct pathspec ps, const char *stash_msg, int quiet,
>  			}
>  		} else {
>  			struct child_process cp = CHILD_PROCESS_INIT;
> +			discard_cache();
>  			cp.git_cmd = 1;
>  			argv_array_pushl(&cp.args, "reset", "--hard", "-q",
>  					 NULL);
> -- snap --
>
> And the reason this is needed: we spawn a `git reset --hard` here, which
> will change the index, but outside of the current process. So the
> in-process copy is stale. And when the index' mtime does not help us
> detect that, we run into that test breakage.