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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 21, 2025, 11:45 UTC
Message-ID: <CAOLa=ZT_PVwD0=hFEEqxutFyrQRiz3f4iB8019Sax7_nEwrJfw@mail.gmail.com>
In-Reply-To: <Z6YxB6qL8WQFZyWW@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 14 quoted lines

> On Fri, Feb 07, 2025 at 08:34:41AM +0100, Karthik Nayak wrote:
>> diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
>> index 9e6935d38d031b4890135e0cce36fffcc349ac1d..529d3c15404cdc13216219fba6f56dde91f4909c 100644
>> --- a/Documentation/git-update-ref.txt
>> +++ b/Documentation/git-update-ref.txt
>> @@ -8,7 +8,7 @@ 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])
>> +'git update-ref' [-m <reason>] [--no-deref] (-d <ref> [<old-oid>] | [--create-reflog] <ref> <new-oid> [<old-oid>] | --stdin [-z] [--allow-partial])
>
> I think it's time that we start to split this line into multiple lines :)
>

Yes, indeed, will do.

Show 26 quoted lines

>> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
>> index 4d35bdc4b4b57937112e6c4c9740420b1f1771e5..83dcb7d8d73f423226c36b61374c86c6b29ec756 100644
>> --- a/builtin/update-ref.c
>> +++ b/builtin/update-ref.c
>> @@ -562,6 +563,30 @@ 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,
>> +				const struct strbuf *reason,
>> +				void *cb_data UNUSED)
>> +{
>> +	struct strbuf sb = STRBUF_INIT;
>> +	char space = ' ';
>> +
>> +	if (!line_termination)
>> +		space = line_termination;
>> +
>> +	strbuf_addf(&sb, "rejected%c%s%c%s%c%c%s%c%s%c", space,
>
> Whew, that's a lot of placeholders.
>

True. More prone to errors too.

Show 18 quoted lines

>> @@ -723,7 +754,8 @@ int cmd_update_ref(int argc,
>>  	const char *refname, *oldval;
>>  	struct object_id oid, oldoid;
>>  	int delete = 0, no_deref = 0, read_stdin = 0, end_null = 0;
>> -	int create_reflog = 0;
>> +	int create_reflog = 0, allow_partial = 0;
>> +
>>  	struct option options[] = {
>>  		OPT_STRING( 'm', NULL, &msg, N_("reason"), N_("reason of the update")),
>>  		OPT_BOOL('d', NULL, &delete, N_("delete the reference")),
>> @@ -732,6 +764,7 @@ 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_BOOL('0', "allow-partial", &allow_partial, N_("allow partial transactions")),
>
> You can use `OPT_BIT()` to set a specific bit in a flags field..
>

That would be cleaner, will fix.

Show 26 quoted lines

>> @@ -749,13 +782,19 @@ int cmd_update_ref(int argc,
>>  	}
>>
>>  	if (read_stdin) {
>> +		unsigned int flags = 0;
>> +
>> +		if (allow_partial)
>> +			flags |= REF_TRANSACTION_ALLOW_PARTIAL;
>> +
>>  		if (delete || argc > 0)
>>  			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 (allow_partial)
>> +		die("--allow-partial can only be used with --stdin");
>>
>>  	if (end_null)
>>  		usage_with_options(git_update_ref_usage, options);
>
> The implementation is quite simple, nice.
>
> Patrick