Re: [PATCH v2 5/7] update-ref: add support for symref-create

> On Sun, Apr 21, 2024 at 08:50:07AM -0400, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>>
>> > On Fri, Apr 12, 2024 at 11:59:06AM +0200, Karthik Nayak wrote:
>> >> From: Karthik Nayak <karthik.188@gmail.com>
>> > [snip]
>> >> @@ -268,6 +268,39 @@ static void parse_cmd_create(struct ref_transaction *transaction,
>> >>  	strbuf_release(&err);
>> >>  }
>> >>
>> >> +static void parse_cmd_symref_create(struct ref_transaction *transaction,
>> >> +				    const char *next, const char *end)
>> >> +{
>> >> +	struct strbuf err = STRBUF_INIT;
>> >> +	char *refname, *new_ref;
>> >> +
>> >> +	if (!(update_flags & REF_NO_DEREF))
>> >> +                die("symref-create: cannot operate with deref mode");
>> >> +
>> >> +	refname = parse_refname(&next);
>> >> +	if (!refname)
>> >> +		die("symref-create: missing <ref>");
>> >> +
>> >> +	new_ref = parse_next_refname(&next);
>> >> +	if (!new_ref)
>> >> +		die("symref-create %s: missing <new-ref>", refname);
>> >> +	if (read_ref(new_ref, NULL))
>> >> +		die("symref-create %s: invalid <new-ref>", refname);
>> >
>> > This restricts the creation of dangling symrefs, right? I think we might
>> > want to lift this restriction, in which case we can eventually get rid
>> > of the `create_symref` callback in ref backends completely.
>> >
>>
>> Yes it does. I thought it'd be more consistent with what update-ref does
>> with regular ref updates. We verify that the object exists. Also this
>> could be an added option 'allow-dangling'.
>>
>> I'm not sure I understand what you mean 'the `create_symref` callback in
>> ref backends completely.'.
>
> Well, once normal reference transactions learn to update symrefs we can
> do the following:
>
>     diff --git a/refs/refs-internal.h b/refs/refs-internal.h
>     index 56641aa57a..2302311282 100644
>     --- a/refs/refs-internal.h
>     +++ b/refs/refs-internal.h
>     @@ -676,7 +676,6 @@ struct ref_storage_be {
>         ref_transaction_commit_fn *initial_transaction_commit;
>
>         pack_refs_fn *pack_refs;
>     -	create_symref_fn *create_symref;
>         rename_ref_fn *rename_ref;
>         copy_ref_fn *copy_ref;
>
> There would be no need to specifically have this as a separate callback
> anymore as we can now provide a generic wrapper `refs_create_symref()`
> (in pseudo-code):
>
> ```
> int refs_create_symref(struct ref_store *refs, const char *refname,
>                        const char *target)
> {
>     tx = ref_store_transaction_begin(refs);
>     ref_transaction_create_symref(tx, refname, target);
>     ref_transaction_commit(tx);
> }
> ```
>
> Patrick
>