On Tue, Mar 04, 2025 at 05:41:08PM -0600, Justin Tobler wrote:
> On 25/02/26 04:24PM, Patrick Steinhardt wrote:
> > diff --git a/reflog.h b/reflog.h
> > index a9d464bbf8c..b08780a30a7 100644
> > --- a/reflog.h
> > +++ b/reflog.h
> > @@ -2,7 +2,15 @@
> > #define REFLOG_H
> > #include "refs.h"
> >
> > +struct reflog_expire_entry_option {
> > + struct reflog_expire_entry_option *next;
> > + timestamp_t expire_total;
> > + timestamp_t expire_unreachable;
> > + char pattern[FLEX_ARRAY];
> > +};
> > +
> > struct reflog_expire_options {
> > + struct reflog_expire_entry_option *entries, **entries_tail;
>
> Now we can also store the configured per-reflog-pattern expiry entries
> in the options type instead of relying on global state.
>
> > int stalefix;
> > int explicit_expiry;
> > timestamp_t default_expire_total;
>
> Now that all the reflog expiry configuration is contained within
> reflog_expire_options, I wonder if it really makes sense to also keep
> the expire_total and expire_unreachable fields.
>
> From my understanding these fields are not really for configuration, but
> hold the reflog expiry configuration for the current active reference
> while iterating. This gets set by set_reflog_expiry_param() prior to
> calling refs_reflog_expire(). It seems like this could be figured out
> during refs_reflog_expire() now.
Yes, these fields hold state indeed, namely the value for a given
refname. These fields thus need to be updated for every refname for
which you want to check expiration, which is done by calling
`reflog_expire_options_set_refname()`. This interfaces is extremely
awkward from my perspective, and it would be very much preferable to
instead have an interface that, given the options and a refname as
parameters, tells you whether the reflog contains entries that should be
pruned.
In fact, I did have a look at fixing this awkward interface, but it
always ended up with way more changes than I felt comfortable with. So I
decided to only go a couple steps into the direction of better
encapsulation, but to not fix all of the design issues with the current
interface.
Patrick
