Subject: Re: [PATCH 3/7] refs: convert AUTO_MERGE to become a normal pseudo-ref

From: Junio C Hamano <[email-masked]>
Date: Fri, 19 Jan 2024 11:28:10 -0800

Patrick Steinhardt <[email-masked]> writes:

> In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have
> inrtoduced a new `is_special_ref()` function that classifies some refs

"introduced"

> @@ -4687,10 +4685,17 @@ void merge_switch_to_result(struct merge_options *opt,
>  		trace2_region_leave("merge", "record_conflicted", opt->repo);
>  
>  		trace2_region_enter("merge", "write_auto_merge", opt->repo);
> -		filename = git_path_auto_merge(opt->repo);
> -		fp = xfopen(filename, "w");
> -		fprintf(fp, "%s\n", oid_to_hex(&result->tree->object.oid));
> -		fclose(fp);
> +		if (refs_update_ref(get_main_ref_store(opt->repo), "", "AUTO_MERGE",
> +				    &result->tree->object.oid, NULL, REF_NO_DEREF,
> +				    UPDATE_REFS_MSG_ON_ERR)) {
> +			/* failure to function */
> +			opt->priv = NULL;
> +			result->clean = -1;
> +			merge_finalize(opt, result);
> +			trace2_region_leave("merge", "write_auto_merge",
> +					    opt->repo);
> +			return;
> +		}
>  		trace2_region_leave("merge", "write_auto_merge", opt->repo);
>  	}
>  	if (display_update_msgs)

We used to ignore errors while updating AUTO_MERGE, implying that it
is an optional nicety that does not have to block the merge.  Now we
explicitly mark the resulting index unclean.  While my gut feeling
says that it should not matter all that much (as such a failure
would be rare enough that the user may want to inspect and double
check the situation before going forward), I am not 100% sure if the
change is behaviour is acceptable by everybody (cc'ed Elijah for
second opinion).

Everything else in this patch looked good.

Thanks.