From: Karthik Nayak <karthik.188@gmail.com>
Date: Mon, 24 Mar 2025 17:51:20 GMT
Subject: Re: [PATCH v4 8/8] update-ref: add --batch-updates flag for stdin mode
Message-ID: <CAOLa=ZSjETz71mmeULYy74djOLZWfsixNCcAfONrAteHjdDdVA@mail.gmail.com>
In-Reply-To: <Z-FZQOYGPIuB2h-t@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> On Thu, Mar 20, 2025 at 12:44:03PM +0100, Karthik Nayak wrote:
>> diff --git a/Documentation/git-update-ref.adoc b/Documentation/git-update-ref.adoc
>> index 9e6935d38d..5be2c16776 100644
>> --- a/Documentation/git-update-ref.adoc
>> +++ b/Documentation/git-update-ref.adoc
>> @@ -57,6 +59,14 @@ 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 `--batch-updates`, update-ref executes the updates in a batch but allows
>> +individual updates to fail due to invalid or incorrect user input, applying only
>> +the successful updates. However, system-related errors—such as I/O failures or
>> +memory issues—will result in a full failure of all batched updates. Any failed
>> +updates will be reported in the following format:
>> +
>> +	rejected SP (<old-oid> | <old-target>) SP (<new-oid> | <new-target>) SP <rejection-reason> LF
>> +
>
> Does this support NUL-terminated mode? It probably should, and if it
> does we should also document the format.
>

It only does for inputs. So there is nothing to be done for outputs.
I actually added support for '-z' mode here, but there was an assumption
on my part that this is for both input/output.

Phillip corrected my assumption in the first version of this series [1].

[1]: https://lore.kernel.org/all/ceda422e-8c8e-4a1d-aaab-9a7a2fc009dd@gmail.com/

>> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
>> index 1d541e13ad..97e14b279e 100644
>> --- a/builtin/update-ref.c
>> +++ b/builtin/update-ref.c
>> @@ -735,6 +787,8 @@ int cmd_update_ref(int argc,
>>  		OPT_BOOL('z', NULL, &end_null, N_("stdin has NUL-terminated arguments")),
>>  		OPT_BOOL( 0 , "stdin", &read_stdin, N_("read updates from stdin")),
>>  		OPT_BOOL( 0 , "create-reflog", &create_reflog, N_("create a reflog")),
>> +		OPT_BIT('0', "batch-updates", &flags, N_("batch reference updates"),
>> +			REF_TRANSACTION_ALLOW_FAILURE),
>>  		OPT_END(),
>>  	};
>>
>> @@ -756,9 +810,10 @@ int cmd_update_ref(int argc,
>>  			usage_with_options(git_update_ref_usage, options);
>>  		if (end_null)
>>  			line_termination = '\0';
>> -		update_refs_stdin();
>> +		update_refs_stdin(flags);
>>  		return 0;
>> -	}
>> +	} else if (flags & REF_TRANSACTION_ALLOW_FAILURE)
>> +		die("--batch-updates can only be used with --stdin");
>
> Nit: formatting, the `else if` branch should have curly braces.

Ah! Thanks, will change!

> Patrick