Re: [PATCH 1/3] reftable/stack: remove `REFTABLE_STACK_NEW_ADDITION_RELOAD`

[Patrick Steinhardt <ps@pks.im>]

On Wed, Aug 19, 2026 at 03:19:37PM +0200, Karthik Nayak wrote:
> In 80e7342ea8 (reftable/stack: allow locking of outdated stacks,
> 2024-09-24), the `REFTABLE_STACK_NEW_ADDITION_RELOAD` was introduced so
> that callers of `reftable_stack_init_addition()` can also reload the
> stack if there was a concurrent update made before the lock was
> obtained.
> 
> Then 16684b6fae (refs/reftable: always reload stacks when creating
> lock, 2025-08-12) updated all of the remaining call-sites to propagate
> this flag to ensure that we always reload the stack whenever there was a
> concurrent update.
> 
> As all calls to `reftable_stack_init_addition()` inevitably propagate
> the flag, it is safe to remove the flag and its associated code and make
> the reloading of the stack the default flow. This makes it easier to
> follow the flow and simplifies the logic.
> 
> The only exceptions are:
> 
>   1. Unit tests, where we explicitly do not propagate the flag. These
>      tests are now modified with the new status quo.
> 
>   2. `reftable_stack_clean_locked()`, which was propagating 0 to
>      `reftable_stack_new_addition()` but was then manually reloading the
>      stack after. Here the new flow will achieve the same, while also
>      allowing us to remove the manual reload.

libgit2 uses this flag though, so we'd have to adapt it, too. As far as
I can see though all of the calls to `reftable_stack_add()` it has pass
this flag.

> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
> index 5d22d84e80..5d224f8079 100644
> --- a/reftable/reftable-stack.h
> +++ b/reftable/reftable-stack.h
> @@ -58,22 +58,13 @@ uint64_t reftable_stack_next_update_index(struct reftable_stack *st);
>  /* holds a transaction to add tables at the top of a stack. */
>  struct reftable_addition;
>  
> -enum {
> -	/*
> -	 * Reload the stack when the stack is out-of-date after locking it.
> -	 */
> -	REFTABLE_STACK_NEW_ADDITION_RELOAD = (1 << 0),
> -};
> -
>  /*
>   * returns a new transaction to add reftables to the given stack. As a side
> - * effect, the ref database is locked. Accepts REFTABLE_STACK_NEW_ADDITION_*
> - * flags.
> + * effect, the ref database is locked.
>   */
>  int reftable_stack_new_addition(struct reftable_addition **dest,
>  				struct reftable_stack *st,
> -				const struct reftable_write_options *opts,
> -				unsigned int flags);
> +				const struct reftable_write_options *opts);
>  
>  /* Adds a reftable to transaction. */
>  int reftable_addition_add(struct reftable_addition *add,

We're already busy adapting this function anyway, so do we maybe want to
fix its name to `reftable_stack_addition_new` while at it?

Patrick