Re: [PATCH 2/6] builtin/reflog: stop storing default reflog expiry dates globally

From: Justin Tobler <jltobler@gmail.com>
Date: Mar 4, 2025, 23:23 UTC
Message-ID: <ammsrvhyzo6f7hfcizlh42e3e5cznfb2j37tcicliqqpqaaqgs@4mniagc5ejzz>
In-Reply-To: <20250226-pks-maintenance-reflog-expire-v1-2-a1204a814952@pks.im>

On 25/02/26 04:24PM, Patrick Steinhardt wrote:

Show 12 quoted lines

> When expiring reflog entries, it is possible to configure expiry dates
> that depend on the name of the reflog. This requires us to store a
> couple of different expiry dates:
> 
>   - The default expiry date for reflog entries that aren't otherwise
>     specified.
> 
>   - The per-reflog expiry date.
> 
>   - The currently active set of expiry dates for a given reference.
> 
> While the last item is stored in `struct reflog_expiry_options`, the

s/reflog_expiry_options/reflog_expire_options/

Show 45 quoted lines

> other items aren't, which makes it hard to reuse the structure in other
> places.
> 
> Refactor the code so that the default expiry date is stored as part of
> the structure. The per-reflog expiry dates will be adapted accordingly
> in the subsequent commit.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/reflog.c | 22 +++++++---------------
>  reflog.h         |  6 ++++++
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index dee49881d32..0910a4e25dc 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -63,9 +63,6 @@ static const char *const reflog_usage[] = {
>  	NULL
>  };
>  
> -static timestamp_t default_reflog_expire;
> -static timestamp_t default_reflog_expire_unreachable;
> -
>  struct worktree_reflogs {
>  	struct worktree *worktree;
>  	struct string_list reflogs;
> @@ -122,6 +119,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
>  static int reflog_expire_config(const char *var, const char *value,
>  				const struct config_context *ctx, void *cb)
>  {
> +	struct reflog_expire_options *opts = cb;
>  	const char *pattern, *key;
>  	size_t pattern_len;
>  	timestamp_t expire;
> @@ -145,10 +143,10 @@ static int reflog_expire_config(const char *var, const char *value,
>  	if (!pattern) {
>  		switch (slot) {
>  		case EXPIRE_TOTAL:
> -			default_reflog_expire = expire;
> +			opts->default_expire_total = expire;
>  			break;
>  		case EXPIRE_UNREACH:
> -			default_reflog_expire_unreachable = expire;
> +			opts->default_expire_unreachable = expire;

Ok, instead of setting the default fallback expiry values globably, it is set in the options type.

Show 12 quoted lines

>  			break;
>  		}
>  		return 0;
> @@ -198,9 +196,9 @@ static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char
>  
>  	/* Nothing matched -- use the default value */
>  	if (!(cb->explicit_expiry & EXPIRE_TOTAL))
> -		cb->expire_total = default_reflog_expire;
> +		cb->expire_total = cb->default_expire_total;
>  	if (!(cb->explicit_expiry & EXPIRE_UNREACH))
> -		cb->expire_unreachable = default_reflog_expire_unreachable;
> +		cb->expire_unreachable = cb->default_expire_unreachable;

Here we update where the defaults are used.

Show 10 quoted lines

>  }
>  
>  static int expire_unreachable_callback(const struct option *opt,
> @@ -276,8 +274,8 @@ static int cmd_reflog_list(int argc, const char **argv, const char *prefix,
>  static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
>  			     struct repository *repo UNUSED)
>  {
> -	struct reflog_expire_options opts = { 0 };
>  	timestamp_t now = time(NULL);
> +	struct reflog_expire_options opts = REFLOG_EXPIRE_OPTIONS_INIT(now);

The options type is initialized with the default defaults :)

Show 40 quoted lines

>  	int i, status, do_all, single_worktree = 0;
>  	unsigned int flags = 0;
>  	int verbose = 0;
> @@ -308,17 +306,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
>  		OPT_END()
>  	};
>  
> -	default_reflog_expire_unreachable = now - 30 * 24 * 3600;
> -	default_reflog_expire = now - 90 * 24 * 3600;
> -	git_config(reflog_expire_config, NULL);
> +	git_config(reflog_expire_config, &opts);
>  
>  	save_commit_buffer = 0;
>  	do_all = status = 0;
>  
> -	opts.explicit_expiry = 0;
> -	opts.expire_total = default_reflog_expire;
> -	opts.expire_unreachable = default_reflog_expire_unreachable;
> -
>  	argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0);
>  
>  	if (verbose)
> diff --git a/reflog.h b/reflog.h
> index eb948119e53..a9d464bbf8c 100644
> --- a/reflog.h
> +++ b/reflog.h
> @@ -5,10 +5,16 @@
>  struct reflog_expire_options {
>  	int stalefix;
>  	int explicit_expiry;
> +	timestamp_t default_expire_total;
>  	timestamp_t expire_total;
> +	timestamp_t default_expire_unreachable;
>  	timestamp_t expire_unreachable;
>  	int recno;
>  };
> +#define REFLOG_EXPIRE_OPTIONS_INIT(now) { \
> +	.default_expire_total = now - 30 * 24 * 3600, \
> +	.default_expire_unreachable = now - 90 * 24 * 3600, \
> +}

Looking good

Show 8 quoted lines

>  
>  struct expire_reflog_policy_cb {
>  	enum {
> 
> -- 
> 2.48.1.741.g8a9f3a5cdc.dirty
> 
>