Re: [PATCH v2 7/7] update-ref: add --allow-partial flag for stdin mode

[Patrick Steinhardt <ps@pks.im>]

On Tue, Feb 25, 2025 at 10:29:10AM +0100, Karthik Nayak wrote:
> diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
> index 9e6935d38d..fc73f1d8aa 100644
> --- a/Documentation/git-update-ref.adoc
> +++ b/Documentation/git-update-ref.adoc
> @@ -7,8 +7,10 @@ git-update-ref - Update the object name stored in a ref safely
>  
>  SYNOPSIS
>  --------
> -[verse]
> -'git update-ref' [-m <reason>] [--no-deref] (-d <ref> [<old-oid>] | [--create-reflog] <ref> <new-oid> [<old-oid>] | --stdin [-z])
> +[synopsis]
> +git update-ref [-m <reason>] [--no-deref] -d <ref> [<old-oid>]
> +	       [-m <reason>] [--no-deref] [--create-reflog] <ref> <new-oid> [<old-oid>]
> +               [-m <reason>] [--no-deref] --stdin [-z] [--allow-partial]
>  
>  DESCRIPTION
>  -----------
> @@ -57,6 +59,17 @@ performs all modifications together.  Specify commands of the form:
>  With `--create-reflog`, update-ref will create a reflog for each ref
>  even if one would not ordinarily be created.
>  
> +With `--allow-partial`, update-ref continues executing the transaction even if
> +some updates fail due to invalid or incorrect user input, applying only the
> +successful updates. Errors resulting from user-provided input are treated as
> +non-system-related and do not cause the entire transaction to be aborted.
> +However, system-related errors—such as I/O failures or memory issues—will still
> +result in a full failure. Additionally, errors like F/D conflicts are batched
> +for performance optimization and will also cause a full failure. Any failed
> +updates will be reported in the following format:

Shouldn't it be possible to detect F/D conflicts though and not abort
the transaction? If we want to make use of partial transactions in the
context of git-fetch(1) and/or git-receive-pack(1) we would have to
handle them.

> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 1d541e13ad..b03b40eacb 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -565,6 +566,54 @@ static void parse_cmd_abort(struct ref_transaction *transaction,
>  	report_ok("abort");
>  }
>  
> +static void print_rejected_refs(const char *refname,
> +				const struct object_id *old_oid,
> +				const struct object_id *new_oid,
> +				const char *old_target,
> +				const char *new_target,
> +				enum transaction_error err,
> +				void *cb_data UNUSED)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	char space = ' ';
> +	const char *reason = "";
> +
> +	switch (err) {
> +	case TRANSACTION_NAME_CONFLICT:
> +		reason = _("refname conflict");
> +		break;
> +	case TRANSACTION_CREATE_EXISTS:
> +		reason = _("reference already exists");
> +		break;
> +	case TRANSACTION_NONEXISTENT_REF:
> +		reason = _("reference does not exist");
> +		break;
> +	case TRANSACTION_INCORRECT_OLD_VALUE:
> +		reason = _("incorrect old value provided");
> +		break;
> +	case TRANSACTION_INVALID_NEW_VALUE:
> +		reason = _("invalid new value provided");
> +		break;
> +	case TRANSACTION_EXPECTED_SYMREF:
> +		reason = _("expected symref but found regular ref");
> +		break;
> +	default:
> +		reason = _("unkown failure");
> +	}

As git-update-ref(1) is part of plumbing we don't want to translate
those messages.

Patrick