Re: [PATCH v2 6/6] reftable/stack: handle outdated stacks when compacting

[Patrick Steinhardt <ps@pks.im>]

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