On Mon, Aug 11, 2025 at 03:04:05PM -0500, Justin Tobler wrote:
> On 25/08/04 11:40AM, Patrick Steinhardt wrote:
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index f77d7f58e8..effa2fc8cb 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -1215,9 +1220,24 @@ static int stack_compact_range(struct reftable_stack *st,
> > goto done;
> > }
> >
> > + /*
> > + * Check whether the stack is up-to-date. We unfortunately cannot
> > + * handle the situation gracefully in case it's _not_ up-to-date
> > + * because the range of tables that the user has requested us to
> > + * compact may have been changed. So instead we abort.
> > + *
> > + * We could in theory improve the situation by having the caller not
> > + * pass in a range, but instead the list of tables to compact. If so,
> > + * we could check that relevant tables still exist. But for now it's
> > + * good enough to just abort.
> > + */
> > err = stack_uptodate(st);
> > - if (err)
> > + if (err < 0)
> > goto done;
> > + if (err > 0) {
> > + err = REFTABLE_OUTDATED_ERROR;
> > + goto done;
> > + }
>
> I was thinking that maybe `stack_uptodate()` could maybe handle
> returning the `REFTABLE_OUTDATED_ERROR` directly so we could avoid
> having to map the error here. This could require callers to check for
> `err == REFTABLE_OUTDATED_ERROR` instead of `err > 0`. Probably not a
> big deal either way though.
We probably could, but I don't see a strong-enough reason to do so now.
Patrick
