Subject: Re: [PATCH v3 1/2] update_symref: add REF_CREATE_ONLY option

From: Junio C Hamano <[email-masked]>
Date: Thu, 19 Sep 2024 15:46:00 -0700

Bence Ferdinandy <[email-masked]> writes:

> Add a new REF_CREATE_ONLY flag for use by the files backend which will
> only update the symref if it doesn't already exist. Add the possibility
> to pass extra flags to refs_update_symref so that it can utilize this
> new flag.

If I wanted to create a symref that points at A, there are three cases:

 (1) the symref does not exist.  
 (2) the symref exists and points at A.
 (3) the symref exists and points at B.

I'll see a symref that points at A at the end in the first two
cases, and my request is silently ignored in the third case.

I'd expect that the caller can tell the failing case apart from the
successful case with the return value or something.  The caller
might want to tell between the first two cases for reporting
purposes, but I do not care as much as I would care about detecting
true failures.

Nobody actually passes the flag yet, so we would not be able to tell
if any of the added code is buggy from this step alone.  Let's see
what happens in the next patch ;-).

> diff --git a/refs.c b/refs.c
> index ceb72d4bd7..7afe46cadc 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2085,8 +2085,9 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct
>  	return peel_object(r, base, peeled) ? -1 : 0;
>  }
>  
> +
>  int refs_update_symref(struct ref_store *refs, const char *ref,
> -		       const char *target, const char *logmsg)
> +		       const char *target, const unsigned int extra_flags, const char *logmsg)

While it is not _wrong_ per-se to mark an "unsigned int" parameter
as "const", it is a bit unusual in this code base.  The only thing
it prevents us from doing is to mutate it until this function
returns, which does not help all that much in making the code safer,
as opposed to marking a parameter of a pointer type as a const
pointer.