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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 19, 2024, 19:28 UTC
Message-ID: <xmqqjzo5jf79.fsf@gitster.g>
In-Reply-To: <ae2df6316f79e372c51d59666d685e59981d2f98.1705659748.git.ps@pks.im>

Patrick Steinhardt <ps@pks.im> 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"

Show 22 quoted lines

> @@ -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.