Patrick Steinhardt <ps@xxxxxx> writes:
> On Sun, Apr 21, 2024 at 08:50:07AM -0400, Karthik Nayak wrote:
>> Patrick Steinhardt <ps@xxxxxx> writes:
>>
>> > On Fri, Apr 12, 2024 at 11:59:06AM +0200, Karthik Nayak wrote:
>> >> From: Karthik Nayak <karthik.188@xxxxxxxxx>
>> > [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
>
Indeed, this is pretty nice side-effect. Thanks for explaining, I'll
remove the explicit check and allow dangling refs.
Attachment:
signature.asc
Description: PGP signature
