Re: [PATCH v4 2/8] refs: add `index` field to `struct ref_udpate`

[Toon Claes <toon@iotcl.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> The reftable backend, sorts its updates by refname before applying them,
> this ensures that the references are stored sorted. When migrating
> reflogs from one backend to another, the order of the reflogs must be
> maintained. Add a new `index` field to the `ref_update` struct to
> facilitate this.
>
> This field is used in the reftable backend's sort comparison function
> `transaction_update_cmp`, to ensure that indexed fields maintain their
> order.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  refs/refs-internal.h    |  7 +++++++
>  refs/reftable-backend.c | 13 +++++++++++--
>  2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
> index 0fd95cdacd99e4a728c22f5286f6b3f0f360c110..f5c733d099f0c6f1076a25f4f77d9d5eb345ec87 100644
> --- a/refs/refs-internal.h
> +++ b/refs/refs-internal.h
> @@ -115,6 +115,13 @@ struct ref_update {
>  	char *msg;
>  	char *committer_info;
>  
> +	/*
> +	 * The index overrides the default sort algorithm. This is needed
> +	 * when migrating reflogs and we want to ensure we carry over the
> +	 * same order.
> +	 */
> +	unsigned int index;
> +
>  	/*
>  	 * If this ref_update was split off of a symref update via
>  	 * split_symref_update(), then this member points at that
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index e882602487c66261d586a94101bb1b4e9a2ed60e..c008f20be719fec3af6a8f81c821cb9c263764d7 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -1279,8 +1279,17 @@ static int reftable_be_transaction_abort(struct ref_store *ref_store UNUSED,
>  
>  static int transaction_update_cmp(const void *a, const void *b)
>  {
> -	return strcmp(((struct reftable_transaction_update *)a)->update->refname,
> -		      ((struct reftable_transaction_update *)b)->update->refname);
> +	struct reftable_transaction_update *update_a = (struct reftable_transaction_update *)a;
> +	struct reftable_transaction_update *update_b = (struct reftable_transaction_update *)b;
> +
> +	/*
> +	 * If there is an index set, it should take preference (default is 0).
> +	 * This ensures that updates with indexes are sorted amongst themselves.
> +	 */
> +	if (update_a->update->index || update_b->update->index)

What if one of both simply isn't set, and the other one is? Then we
compare an unset with one that is set? Or am I being too paranoid?

--
Toon