On Fri, Apr 12, 2024 at 11:59 AM Karthik Nayak <karthik.188@xxxxxxxxx> wrote:
>
> From: Karthik Nayak <karthik.188@xxxxxxxxx>
>
> The `ref_transaction[_add]_update` functions obtain ref information and
> flags to create a `ref_update` and add it to the transaction at hand.
>
> To extend symref support in transactions, we need to also accept the
> old and new ref values and process it. In this commit, let's add the
> required paramaters to the function and modify all call sites.
s/paramaters/parameters/
> The two paramaters added are `new_ref` and `old_ref`. The `new_ref` is
s/paramaters/parameters/
> used to denote what the reference should point to when the transaction
> is applied. Some functions allow this parameter to be NULL, meaning that
> the reference is not changed, or `""`, meaning that the reference should
> be deleted.
> The `old_ref` denotes the value of that the reference must have before
s/the value of that the reference/the value the reference/
> the update. Some functions allow this parameter to be NULL, meaning that
> the old value of the reference is not checked, or `""`, meaning that the
> reference must not exist before the update. A copy of this value is made
> in the transaction.
>
> The handling logic of these parameters will be added in consequent
> commits as we implement symref-{create, update, delete, verify}.
>
> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
> diff --git a/refs.h b/refs.h
> index d278775e08..645fe9fdb8 100644
> --- a/refs.h
> +++ b/refs.h
There is the following big comment in this file:
/*
* Reference transaction updates
*
* The following four functions add a reference check or update to a
* ref_transaction. They have some common similar parameters:
*
* transaction -- a pointer to an open ref_transaction, obtained
* from ref_transaction_begin().
*
* refname -- the name of the reference to be affected.
*
* new_oid -- the object ID that should be set to be the new value
* of the reference. Some functions allow this parameter to be
* NULL, meaning that the reference is not changed, or
* null_oid, meaning that the reference should be deleted. A
* copy of this value is made in the transaction.
*
* old_oid -- the object ID that the reference must have before
* the update. Some functions allow this parameter to be NULL,
* meaning that the old value of the reference is not checked,
* or null_oid, meaning that the reference must not exist
* before the update. A copy of this value is made in the
* transaction.
*
* flags -- flags affecting the update, passed to
* update_ref_lock(). Possible flags: REF_NO_DEREF,
* REF_FORCE_CREATE_REFLOG. See those constants for more
* information.
*
* msg -- a message describing the change (for the reflog).
*
* err -- a strbuf for receiving a description of any error that
* might have occurred.
*
* The functions make internal copies of refname and msg, so the
* caller retains ownership of these parameters.
*
* The functions return 0 on success and non-zero on failure. A
* failure means that the transaction as a whole has failed and needs
* to be rolled back.
*/
I would expect the patch to update this comment.
> @@ -722,6 +728,7 @@ int ref_transaction_update(struct ref_transaction *transaction,
> const char *refname,
> const struct object_id *new_oid,
> const struct object_id *old_oid,
> + const char *new_ref, const char *old_ref,
> unsigned int flags, const char *msg,
> struct strbuf *err);
The patch might also want to update the comment just above the
ref_transaction_update() declaration as it is changing what the
function can (or will be able to) do.
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 56641aa57a..4c5fe02687 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -124,6 +124,19 @@ struct ref_update {
> */
> struct object_id old_oid;
>
> + /*
> + * If (flags & REF_SYMREF_UPDATE), set the reference to this
> + * value (or delete it, if `new_ref` is an empty string).
What if this value is NULL?
> + */
> + const char *new_ref;
> +
> + /*
> + * If (type & REF_SYMREF_UPDATE), check that the reference
> + * previously had this value (or didn't previously exist,
> + * if `old_ref` is an empty string).
What if this value is NULL?
> + */
> + const char *old_ref;
> +
> /*
> * One or more of REF_NO_DEREF, REF_FORCE_CREATE_REFLOG,
> * REF_HAVE_NEW, REF_HAVE_OLD, or backend-specific flags.
