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

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