Subject: Re: [PATCH v2 3/6] merge: fix save_state() to work when there are racy-dirty files

From: Junio C Hamano <[email-masked]>
Date: Tue, 19 Jul 2022 15:43:10 -0700

"Elijah Newren via GitGitGadget" <[email-masked]> writes:

> From: Elijah Newren <[email-masked]>
>
> When there are racy-dirty files, but no files are modified,
> `git stash create` exits with unsuccessful status.  This causes merge
> to fail.  Refresh the index first to avoid this problem.
>
> Signed-off-by: Elijah Newren <[email-masked]>
> ---
>  builtin/merge.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 00de224a2da..8ce4336dd3f 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -313,8 +313,16 @@ static int save_state(struct object_id *stash)
>  	int len;
>  	struct child_process cp = CHILD_PROCESS_INIT;
>  	struct strbuf buffer = STRBUF_INIT;
> +	struct lock_file lock_file = LOCK_INIT;
> +	int fd;
>  	int rc = -1;
>  
> +	fd = repo_hold_locked_index(the_repository, &lock_file, 0);
> +	refresh_cache(REFRESH_QUIET);
> +	if (0 <= fd)
> +		repo_update_index_if_able(the_repository, &lock_file);
> +	rollback_lock_file(&lock_file);

I might have added "else" but rolling back a lock file that was
already committed or rolled back is a safe no-op, so this is OK.
The pattern already appears elsewhere twice, anyway.

Is it sufficient to be opportunistic?  IOW, if we fail to refresh
the index or write the refreshed result to disk, can we be silent
here and rely on "stash create" and things that follow to safely
fail as necessary, or should we also be detecting errors?

>  	strvec_pushl(&cp.args, "stash", "create", NULL);
>  	cp.out = -1;
>  	cp.git_cmd = 1;