Thread (26 messages) flat view 26 messages, 5 authors, 13d ago

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

From: Patrick Steinhardt <hidden>
Date: 2026-08-20 05:51:23

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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help