[PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

Hi,

this patch series introduces a new "reflog-expire" task to
git-maintenance(1). This task is designed to plug a gap when the "gc"
task is disabled, as there is no way to expire reflog entries in that
case.

This patch series has been inspired by the discussion at [1]. I consider
it to be another step into the direction of replacing git-gc(1) and
allowing for more flexible maintenance strategies overall. Next steps
could be:

  1. Enable the "reflog-expire" task by default when using the
     "incremental" strategy. and then we might eventually switch over
     the

  2. Use "incremental" strategy when "features.experimental" is enabled.

  3. Switch over the default strategy to "incremental" after a couple of
     releases.

Thanks!

Patrick

[1]: <e650f4e4-e267-4f1f-bb3a-c71b1fe0b276@uxp.de>

---
Patrick Steinhardt (6):
      reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`
      builtin/reflog: stop storing default reflog expiry dates globally
      builtin/reflog: stop storing per-reflog expiry dates globally
      builtin/reflog: make functions regarding `reflog_expire_options` public
      builtin/gc: split out function to expire reflog entries
      builtin/maintenance: introduce "reflog-expire" task

 Documentation/config/maintenance.adoc |   9 ++
 Documentation/git-maintenance.adoc    |   4 +
 builtin/gc.c                          |  72 +++++++++++++---
 builtin/reflog.c                      | 153 ++++------------------------------
 reflog.c                              | 137 ++++++++++++++++++++++++++----
 reflog.h                              |  35 +++++++-
 t/t7900-maintenance.sh                |  18 ++++
 7 files changed, 263 insertions(+), 165 deletions(-)


---
base-commit: 5a526e5e18ddb9a7dfc5a2967d21d6154df64a4f
change-id: 20250226-pks-maintenance-reflog-expire-61c61410751a

[PATCH 1/6] reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`

[Patrick Steinhardt]

We're about to expose `struct cmd_reflog_expire_cb` via "reflog.h" so
that we can also use this structure in "builtin/gc.c". Once we make it
accessible to a wider scope though it becomes awkwardly named, as it
isn't only useful in the context of a callback. Instead, the function is
containing all kinds of options relevant to whether or not a reflog
entry should be expired.

Rename the structure to `reflog_expire_options` to prepare for this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 38 +++++++++++++++++++-------------------
 reflog.c         | 30 +++++++++++++++---------------
 reflog.h         |  4 ++--
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 95f264989bb..dee49881d32 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -168,7 +168,7 @@ static int reflog_expire_config(const char *var, const char *value,
 	return 0;
 }
 
-static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, const char *ref)
+static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
 {
 	struct reflog_expire_cfg *ent;
 
@@ -207,15 +207,15 @@ static int expire_unreachable_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
 {
-	struct cmd_reflog_expire_cb *cmd = opt->value;
+	struct reflog_expire_options *opts = opt->value;
 
 	BUG_ON_OPT_NEG(unset);
 
-	if (parse_expiry_date(arg, &cmd->expire_unreachable))
+	if (parse_expiry_date(arg, &opts->expire_unreachable))
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	cmd->explicit_expiry |= EXPIRE_UNREACH;
+	opts->explicit_expiry |= EXPIRE_UNREACH;
 	return 0;
 }
 
@@ -223,15 +223,15 @@ static int expire_total_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
 {
-	struct cmd_reflog_expire_cb *cmd = opt->value;
+	struct reflog_expire_options *opts = opt->value;
 
 	BUG_ON_OPT_NEG(unset);
 
-	if (parse_expiry_date(arg, &cmd->expire_total))
+	if (parse_expiry_date(arg, &opts->expire_total))
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	cmd->explicit_expiry |= EXPIRE_TOTAL;
+	opts->explicit_expiry |= EXPIRE_TOTAL;
 	return 0;
 }
 
@@ -276,7 +276,7 @@ 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 cmd_reflog_expire_cb cmd = { 0 };
+	struct reflog_expire_options opts = { 0 };
 	timestamp_t now = time(NULL);
 	int i, status, do_all, single_worktree = 0;
 	unsigned int flags = 0;
@@ -292,15 +292,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 			N_("update the reference to the value of the top reflog entry"),
 			EXPIRE_REFLOGS_UPDATE_REF),
 		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen")),
-		OPT_CALLBACK_F(0, "expire", &cmd, N_("timestamp"),
+		OPT_CALLBACK_F(0, "expire", &opts, N_("timestamp"),
 			       N_("prune entries older than the specified time"),
 			       PARSE_OPT_NONEG,
 			       expire_total_callback),
-		OPT_CALLBACK_F(0, "expire-unreachable", &cmd, N_("timestamp"),
+		OPT_CALLBACK_F(0, "expire-unreachable", &opts, N_("timestamp"),
 			       N_("prune entries older than <time> that are not reachable from the current tip of the branch"),
 			       PARSE_OPT_NONEG,
 			       expire_unreachable_callback),
-		OPT_BOOL(0, "stale-fix", &cmd.stalefix,
+		OPT_BOOL(0, "stale-fix", &opts.stalefix,
 			 N_("prune any reflog entries that point to broken commits")),
 		OPT_BOOL(0, "all", &do_all, N_("process the reflogs of all references")),
 		OPT_BOOL(0, "single-worktree", &single_worktree,
@@ -315,9 +315,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 	save_commit_buffer = 0;
 	do_all = status = 0;
 
-	cmd.explicit_expiry = 0;
-	cmd.expire_total = default_reflog_expire;
-	cmd.expire_unreachable = default_reflog_expire_unreachable;
+	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);
 
@@ -329,7 +329,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 	 * even in older repository.  We cannot trust what's reachable
 	 * from reflog if the repository was pruned with older git.
 	 */
-	if (cmd.stalefix) {
+	if (opts.stalefix) {
 		struct rev_info revs;
 
 		repo_init_revisions(the_repository, &revs, prefix);
@@ -363,11 +363,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 
 		for_each_string_list_item(item, &collected.reflogs) {
 			struct expire_reflog_policy_cb cb = {
-				.cmd = cmd,
+				.opts = opts,
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 			};
 
-			set_reflog_expiry_param(&cb.cmd,  item->string);
+			set_reflog_expiry_param(&cb.opts,  item->string);
 			status |= refs_reflog_expire(get_main_ref_store(the_repository),
 						     item->string, flags,
 						     reflog_expiry_prepare,
@@ -380,13 +380,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 
 	for (i = 0; i < argc; i++) {
 		char *ref;
-		struct expire_reflog_policy_cb cb = { .cmd = cmd };
+		struct expire_reflog_policy_cb cb = { .opts = opts };
 
 		if (!repo_dwim_log(the_repository, argv[i], strlen(argv[i]), NULL, &ref)) {
 			status |= error(_("%s points nowhere!"), argv[i]);
 			continue;
 		}
-		set_reflog_expiry_param(&cb.cmd, ref);
+		set_reflog_expiry_param(&cb.opts, ref);
 		status |= refs_reflog_expire(get_main_ref_store(the_repository),
 					     ref, flags,
 					     reflog_expiry_prepare,
diff --git a/reflog.c b/reflog.c
index 1b5f031f6d7..bcdb75514d0 100644
--- a/reflog.c
+++ b/reflog.c
@@ -252,15 +252,15 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	struct expire_reflog_policy_cb *cb = cb_data;
 	struct commit *old_commit, *new_commit;
 
-	if (timestamp < cb->cmd.expire_total)
+	if (timestamp < cb->opts.expire_total)
 		return 1;
 
 	old_commit = new_commit = NULL;
-	if (cb->cmd.stalefix &&
+	if (cb->opts.stalefix &&
 	    (!keep_entry(&old_commit, ooid) || !keep_entry(&new_commit, noid)))
 		return 1;
 
-	if (timestamp < cb->cmd.expire_unreachable) {
+	if (timestamp < cb->opts.expire_unreachable) {
 		switch (cb->unreachable_expire_kind) {
 		case UE_ALWAYS:
 			return 1;
@@ -272,7 +272,7 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		}
 	}
 
-	if (cb->cmd.recno && --(cb->cmd.recno) == 0)
+	if (cb->opts.recno && --(cb->opts.recno) == 0)
 		return 1;
 
 	return 0;
@@ -331,7 +331,7 @@ void reflog_expiry_prepare(const char *refname,
 	struct commit_list *elem;
 	struct commit *commit = NULL;
 
-	if (!cb->cmd.expire_unreachable || is_head(refname)) {
+	if (!cb->opts.expire_unreachable || is_head(refname)) {
 		cb->unreachable_expire_kind = UE_HEAD;
 	} else {
 		commit = lookup_commit_reference_gently(the_repository,
@@ -341,7 +341,7 @@ void reflog_expiry_prepare(const char *refname,
 		cb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;
 	}
 
-	if (cb->cmd.expire_unreachable <= cb->cmd.expire_total)
+	if (cb->opts.expire_unreachable <= cb->opts.expire_total)
 		cb->unreachable_expire_kind = UE_ALWAYS;
 
 	switch (cb->unreachable_expire_kind) {
@@ -358,7 +358,7 @@ void reflog_expiry_prepare(const char *refname,
 		/* For reflog_expiry_cleanup() below */
 		cb->tip_commit = commit;
 	}
-	cb->mark_limit = cb->cmd.expire_total;
+	cb->mark_limit = cb->opts.expire_total;
 	mark_reachable(cb);
 }
 
@@ -390,7 +390,7 @@ int count_reflog_ent(struct object_id *ooid UNUSED,
 		     timestamp_t timestamp, int tz UNUSED,
 		     const char *message UNUSED, void *cb_data)
 {
-	struct cmd_reflog_expire_cb *cb = cb_data;
+	struct reflog_expire_options *cb = cb_data;
 	if (!cb->expire_total || timestamp < cb->expire_total)
 		cb->recno++;
 	return 0;
@@ -398,7 +398,7 @@ int count_reflog_ent(struct object_id *ooid UNUSED,
 
 int reflog_delete(const char *rev, enum expire_reflog_flags flags, int verbose)
 {
-	struct cmd_reflog_expire_cb cmd = { 0 };
+	struct reflog_expire_options opts = { 0 };
 	int status = 0;
 	reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
 	const char *spec = strstr(rev, "@{");
@@ -421,17 +421,17 @@ int reflog_delete(const char *rev, enum expire_reflog_flags flags, int verbose)
 
 	recno = strtoul(spec + 2, &ep, 10);
 	if (*ep == '}') {
-		cmd.recno = -recno;
+		opts.recno = -recno;
 		refs_for_each_reflog_ent(get_main_ref_store(the_repository),
-					 ref, count_reflog_ent, &cmd);
+					 ref, count_reflog_ent, &opts);
 	} else {
-		cmd.expire_total = approxidate(spec + 2);
+		opts.expire_total = approxidate(spec + 2);
 		refs_for_each_reflog_ent(get_main_ref_store(the_repository),
-					 ref, count_reflog_ent, &cmd);
-		cmd.expire_total = 0;
+					 ref, count_reflog_ent, &opts);
+		opts.expire_total = 0;
 	}
 
-	cb.cmd = cmd;
+	cb.opts = opts;
 	status |= refs_reflog_expire(get_main_ref_store(the_repository), ref,
 				     flags,
 				     reflog_expiry_prepare,
diff --git a/reflog.h b/reflog.h
index d2906fb9f8d..eb948119e53 100644
--- a/reflog.h
+++ b/reflog.h
@@ -2,7 +2,7 @@
 #define REFLOG_H
 #include "refs.h"
 
-struct cmd_reflog_expire_cb {
+struct reflog_expire_options {
 	int stalefix;
 	int explicit_expiry;
 	timestamp_t expire_total;
@@ -18,7 +18,7 @@ struct expire_reflog_policy_cb {
 	} unreachable_expire_kind;
 	struct commit_list *mark_list;
 	unsigned long mark_limit;
-	struct cmd_reflog_expire_cb cmd;
+	struct reflog_expire_options opts;
 	struct commit *tip_commit;
 	struct commit_list *tips;
 	unsigned int dry_run:1;

-- 
2.48.1.741.g8a9f3a5cdc.dirty

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

[Patrick Steinhardt]

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
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;
 			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;
 }
 
 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);
 	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, \
+}
 
 struct expire_reflog_policy_cb {
 	enum {

-- 
2.48.1.741.g8a9f3a5cdc.dirty

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

[Justin Tobler]

On 25/02/26 04:24PM, Patrick Steinhardt wrote:
> 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/

> 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.

>  			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.

>  }
>  
>  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 :)

>  	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

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

[PATCH 3/6] builtin/reflog: stop storing per-reflog expiry dates globally

[Patrick Steinhardt]

As described in the preceding commit, the per-reflog expiry dates are
stored in a global pair of variables. Refactor the code so that they are
contained in `sturct reflog_expire_options` to make the structure useful
in other contexts.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 30 ++++++++++++------------------
 reflog.h         |  8 ++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 0910a4e25dc..a231cf4b857 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -88,27 +88,21 @@ static int collect_reflog(const char *ref, void *cb_data)
 	return 0;
 }
 
-static struct reflog_expire_cfg {
-	struct reflog_expire_cfg *next;
-	timestamp_t expire_total;
-	timestamp_t expire_unreachable;
-	char pattern[FLEX_ARRAY];
-} *reflog_expire_cfg, **reflog_expire_cfg_tail;
-
-static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
+static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
+						       const char *pattern, size_t len)
 {
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
-	if (!reflog_expire_cfg_tail)
-		reflog_expire_cfg_tail = &reflog_expire_cfg;
+	if (!opts->entries_tail)
+		opts->entries_tail = &opts->entries;
 
-	for (ent = reflog_expire_cfg; ent; ent = ent->next)
+	for (ent = opts->entries; ent; ent = ent->next)
 		if (!xstrncmpz(ent->pattern, pattern, len))
 			return ent;
 
 	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
-	*reflog_expire_cfg_tail = ent;
-	reflog_expire_cfg_tail = &(ent->next);
+	*opts->entries_tail = ent;
+	opts->entries_tail = &(ent->next);
 	return ent;
 }
 
@@ -124,7 +118,7 @@ static int reflog_expire_config(const char *var, const char *value,
 	size_t pattern_len;
 	timestamp_t expire;
 	int slot;
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
 	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
 		return git_default_config(var, value, ctx, cb);
@@ -152,7 +146,7 @@ static int reflog_expire_config(const char *var, const char *value,
 		return 0;
 	}
 
-	ent = find_cfg_ent(pattern, pattern_len);
+	ent = find_cfg_ent(opts, pattern, pattern_len);
 	if (!ent)
 		return -1;
 	switch (slot) {
@@ -168,12 +162,12 @@ static int reflog_expire_config(const char *var, const char *value,
 
 static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
 {
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
 	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
 		return; /* both given explicitly -- nothing to tweak */
 
-	for (ent = reflog_expire_cfg; ent; ent = ent->next) {
+	for (ent = cb->entries; ent; ent = ent->next) {
 		if (!wildmatch(ent->pattern, ref, 0)) {
 			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
 				cb->expire_total = ent->expire_total;
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;
 	int stalefix;
 	int explicit_expiry;
 	timestamp_t default_expire_total;

-- 
2.48.1.741.g8a9f3a5cdc.dirty

Re: [PATCH 3/6] builtin/reflog: stop storing per-reflog expiry dates globally

[Justin Tobler]

On 25/02/26 04:24PM, Patrick Steinhardt wrote:
> As described in the preceding commit, the per-reflog expiry dates are
> stored in a global pair of variables. Refactor the code so that they are
> contained in `sturct reflog_expire_options` to make the structure useful

s/sturct/struct/

> in other contexts.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/reflog.c | 30 ++++++++++++------------------
>  reflog.h         |  8 ++++++++
>  2 files changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 0910a4e25dc..a231cf4b857 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -88,27 +88,21 @@ static int collect_reflog(const char *ref, void *cb_data)
>  	return 0;
>  }
>  
> -static struct reflog_expire_cfg {
> -	struct reflog_expire_cfg *next;
> -	timestamp_t expire_total;
> -	timestamp_t expire_unreachable;
> -	char pattern[FLEX_ARRAY];
> -} *reflog_expire_cfg, **reflog_expire_cfg_tail;
> -
> -static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
> +static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
> +						       const char *pattern, size_t len)
>  {
> -	struct reflog_expire_cfg *ent;
> +	struct reflog_expire_entry_option *ent;
>  
> -	if (!reflog_expire_cfg_tail)
> -		reflog_expire_cfg_tail = &reflog_expire_cfg;
> +	if (!opts->entries_tail)
> +		opts->entries_tail = &opts->entries;
>  
> -	for (ent = reflog_expire_cfg; ent; ent = ent->next)
> +	for (ent = opts->entries; ent; ent = ent->next)
>  		if (!xstrncmpz(ent->pattern, pattern, len))
>  			return ent;
>  
>  	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
> -	*reflog_expire_cfg_tail = ent;
> -	reflog_expire_cfg_tail = &(ent->next);
> +	*opts->entries_tail = ent;
> +	opts->entries_tail = &(ent->next);
>  	return ent;
>  }
>  
> @@ -124,7 +118,7 @@ static int reflog_expire_config(const char *var, const char *value,
>  	size_t pattern_len;
>  	timestamp_t expire;
>  	int slot;
> -	struct reflog_expire_cfg *ent;
> +	struct reflog_expire_entry_option *ent;
>  
>  	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
>  		return git_default_config(var, value, ctx, cb);
> @@ -152,7 +146,7 @@ static int reflog_expire_config(const char *var, const char *value,
>  		return 0;
>  	}
>  
> -	ent = find_cfg_ent(pattern, pattern_len);
> +	ent = find_cfg_ent(opts, pattern, pattern_len);
>  	if (!ent)
>  		return -1;
>  	switch (slot) {
> @@ -168,12 +162,12 @@ static int reflog_expire_config(const char *var, const char *value,
>  
>  static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
>  {
> -	struct reflog_expire_cfg *ent;
> +	struct reflog_expire_entry_option *ent;
>  
>  	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
>  		return; /* both given explicitly -- nothing to tweak */
>  
> -	for (ent = reflog_expire_cfg; ent; ent = ent->next) {
> +	for (ent = cb->entries; ent; ent = ent->next) {
>  		if (!wildmatch(ent->pattern, ref, 0)) {
>  			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
>  				cb->expire_total = ent->expire_total;
> 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.

-Justin

Re: [PATCH 3/6] builtin/reflog: stop storing per-reflog expiry dates globally

[Patrick Steinhardt]

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

Re: [PATCH 3/6] builtin/reflog: stop storing per-reflog expiry dates globally

[Justin Tobler]

On 25/03/06 11:37AM, Patrick Steinhardt wrote:
> > 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.

That's fair. This interface certainly feels awkward and exposing it
further doesn't seem ideal. At the same time, maybe its not too big of a
deal that it should be dealt with now.

-Justin

[PATCH 4/6] builtin/reflog: make functions regarding `reflog_expire_options` public

[Patrick Steinhardt]

Make functions that are required to manage `reflog_expire_options`
available elsewhere by moving them into "reflog.c" and exposing them in
the corresponding header. The functions will be used in a subsequent
commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 115 ++-----------------------------------------------------
 reflog.c         | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
 reflog.h         |  17 ++++++++
 3 files changed, 128 insertions(+), 111 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index a231cf4b857..5fea31f9c3c 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -88,113 +88,6 @@ static int collect_reflog(const char *ref, void *cb_data)
 	return 0;
 }
 
-static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
-						       const char *pattern, size_t len)
-{
-	struct reflog_expire_entry_option *ent;
-
-	if (!opts->entries_tail)
-		opts->entries_tail = &opts->entries;
-
-	for (ent = opts->entries; ent; ent = ent->next)
-		if (!xstrncmpz(ent->pattern, pattern, len))
-			return ent;
-
-	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
-	*opts->entries_tail = ent;
-	opts->entries_tail = &(ent->next);
-	return ent;
-}
-
-/* expiry timer slot */
-#define EXPIRE_TOTAL   01
-#define EXPIRE_UNREACH 02
-
-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;
-	int slot;
-	struct reflog_expire_entry_option *ent;
-
-	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
-		return git_default_config(var, value, ctx, cb);
-
-	if (!strcmp(key, "reflogexpire")) {
-		slot = EXPIRE_TOTAL;
-		if (git_config_expiry_date(&expire, var, value))
-			return -1;
-	} else if (!strcmp(key, "reflogexpireunreachable")) {
-		slot = EXPIRE_UNREACH;
-		if (git_config_expiry_date(&expire, var, value))
-			return -1;
-	} else
-		return git_default_config(var, value, ctx, cb);
-
-	if (!pattern) {
-		switch (slot) {
-		case EXPIRE_TOTAL:
-			opts->default_expire_total = expire;
-			break;
-		case EXPIRE_UNREACH:
-			opts->default_expire_unreachable = expire;
-			break;
-		}
-		return 0;
-	}
-
-	ent = find_cfg_ent(opts, pattern, pattern_len);
-	if (!ent)
-		return -1;
-	switch (slot) {
-	case EXPIRE_TOTAL:
-		ent->expire_total = expire;
-		break;
-	case EXPIRE_UNREACH:
-		ent->expire_unreachable = expire;
-		break;
-	}
-	return 0;
-}
-
-static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
-{
-	struct reflog_expire_entry_option *ent;
-
-	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
-		return; /* both given explicitly -- nothing to tweak */
-
-	for (ent = cb->entries; ent; ent = ent->next) {
-		if (!wildmatch(ent->pattern, ref, 0)) {
-			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-				cb->expire_total = ent->expire_total;
-			if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-				cb->expire_unreachable = ent->expire_unreachable;
-			return;
-		}
-	}
-
-	/*
-	 * If unconfigured, make stash never expire
-	 */
-	if (!strcmp(ref, "refs/stash")) {
-		if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-			cb->expire_total = 0;
-		if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-			cb->expire_unreachable = 0;
-		return;
-	}
-
-	/* Nothing matched -- use the default value */
-	if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-		cb->expire_total = cb->default_expire_total;
-	if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-		cb->expire_unreachable = cb->default_expire_unreachable;
-}
-
 static int expire_unreachable_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
@@ -207,7 +100,7 @@ static int expire_unreachable_callback(const struct option *opt,
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	opts->explicit_expiry |= EXPIRE_UNREACH;
+	opts->explicit_expiry |= REFLOG_EXPIRE_UNREACH;
 	return 0;
 }
 
@@ -223,7 +116,7 @@ static int expire_total_callback(const struct option *opt,
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	opts->explicit_expiry |= EXPIRE_TOTAL;
+	opts->explicit_expiry |= REFLOG_EXPIRE_TOTAL;
 	return 0;
 }
 
@@ -353,7 +246,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 			};
 
-			set_reflog_expiry_param(&cb.opts,  item->string);
+			reflog_expire_options_set_refname(&cb.opts,  item->string);
 			status |= refs_reflog_expire(get_main_ref_store(the_repository),
 						     item->string, flags,
 						     reflog_expiry_prepare,
@@ -372,7 +265,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 			status |= error(_("%s points nowhere!"), argv[i]);
 			continue;
 		}
-		set_reflog_expiry_param(&cb.opts, ref);
+		reflog_expire_options_set_refname(&cb.opts, ref);
 		status |= refs_reflog_expire(get_main_ref_store(the_repository),
 					     ref, flags,
 					     reflog_expiry_prepare,
diff --git a/reflog.c b/reflog.c
index bcdb75514d0..642b162ef70 100644
--- a/reflog.c
+++ b/reflog.c
@@ -2,13 +2,120 @@
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
+#include "config.h"
 #include "gettext.h"
 #include "object-store-ll.h"
+#include "parse-options.h"
 #include "reflog.h"
 #include "refs.h"
 #include "revision.h"
 #include "tree.h"
 #include "tree-walk.h"
+#include "wildmatch.h"
+
+static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
+						       const char *pattern, size_t len)
+{
+	struct reflog_expire_entry_option *ent;
+
+	if (!opts->entries_tail)
+		opts->entries_tail = &opts->entries;
+
+	for (ent = opts->entries; ent; ent = ent->next)
+		if (!xstrncmpz(ent->pattern, pattern, len))
+			return ent;
+
+	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
+	*opts->entries_tail = ent;
+	opts->entries_tail = &(ent->next);
+	return ent;
+}
+
+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;
+	int slot;
+	struct reflog_expire_entry_option *ent;
+
+	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
+		return git_default_config(var, value, ctx, cb);
+
+	if (!strcmp(key, "reflogexpire")) {
+		slot = REFLOG_EXPIRE_TOTAL;
+		if (git_config_expiry_date(&expire, var, value))
+			return -1;
+	} else if (!strcmp(key, "reflogexpireunreachable")) {
+		slot = REFLOG_EXPIRE_UNREACH;
+		if (git_config_expiry_date(&expire, var, value))
+			return -1;
+	} else
+		return git_default_config(var, value, ctx, cb);
+
+	if (!pattern) {
+		switch (slot) {
+		case REFLOG_EXPIRE_TOTAL:
+			opts->default_expire_total = expire;
+			break;
+		case REFLOG_EXPIRE_UNREACH:
+			opts->default_expire_unreachable = expire;
+			break;
+		}
+		return 0;
+	}
+
+	ent = find_cfg_ent(opts, pattern, pattern_len);
+	if (!ent)
+		return -1;
+	switch (slot) {
+	case REFLOG_EXPIRE_TOTAL:
+		ent->expire_total = expire;
+		break;
+	case REFLOG_EXPIRE_UNREACH:
+		ent->expire_unreachable = expire;
+		break;
+	}
+	return 0;
+}
+
+void reflog_expire_options_set_refname(struct reflog_expire_options *cb,
+				       const char *ref)
+{
+	struct reflog_expire_entry_option *ent;
+
+	if (cb->explicit_expiry == (REFLOG_EXPIRE_TOTAL|REFLOG_EXPIRE_UNREACH))
+		return; /* both given explicitly -- nothing to tweak */
+
+	for (ent = cb->entries; ent; ent = ent->next) {
+		if (!wildmatch(ent->pattern, ref, 0)) {
+			if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+				cb->expire_total = ent->expire_total;
+			if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+				cb->expire_unreachable = ent->expire_unreachable;
+			return;
+		}
+	}
+
+	/*
+	 * If unconfigured, make stash never expire
+	 */
+	if (!strcmp(ref, "refs/stash")) {
+		if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+			cb->expire_total = 0;
+		if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+			cb->expire_unreachable = 0;
+		return;
+	}
+
+	/* Nothing matched -- use the default value */
+	if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+		cb->expire_total = cb->default_expire_total;
+	if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+		cb->expire_unreachable = cb->default_expire_unreachable;
+}
 
 /* Remember to update object flag allocation in object.h */
 #define INCOMPLETE	(1u<<10)
diff --git a/reflog.h b/reflog.h
index b08780a30a7..63bb56280f4 100644
--- a/reflog.h
+++ b/reflog.h
@@ -2,6 +2,9 @@
 #define REFLOG_H
 #include "refs.h"
 
+#define REFLOG_EXPIRE_TOTAL   (1 << 0)
+#define REFLOG_EXPIRE_UNREACH (1 << 1)
+
 struct reflog_expire_entry_option {
 	struct reflog_expire_entry_option *next;
 	timestamp_t expire_total;
@@ -24,6 +27,20 @@ struct reflog_expire_options {
 	.default_expire_unreachable = now - 90 * 24 * 3600, \
 }
 
+/*
+ * Parse the reflog expire configuration. This should be used with
+ * `repo_config()`.
+ */
+int reflog_expire_config(const char *var, const char *value,
+			 const struct config_context *ctx, void *cb);
+
+/*
+ * Adapt the options so that they apply to the given refname. This applies any
+ * per-reference reflog expiry configuration that may exist to the options.
+ */
+void reflog_expire_options_set_refname(struct reflog_expire_options *cb,
+				       const char *refname);
+
 struct expire_reflog_policy_cb {
 	enum {
 		UE_NORMAL,

-- 
2.48.1.741.g8a9f3a5cdc.dirty

[PATCH 5/6] builtin/gc: split out function to expire reflog entries

[Patrick Steinhardt]

We're about to introduce a new task for git-maintenance(1) that knows to
expire reflog entries. The logic will be shared with git-gc(1), which
already knows how to do this.

Pull out the common logic into a separate function so that we can share
the implementation between both builtins.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 409d454a4b7..e8f5705dc59 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -53,7 +53,6 @@ static const char * const builtin_gc_usage[] = {
 
 static timestamp_t gc_log_expire_time;
 
-static struct strvec reflog = STRVEC_INIT;
 static struct strvec repack = STRVEC_INIT;
 static struct strvec prune = STRVEC_INIT;
 static struct strvec prune_worktrees = STRVEC_INIT;
@@ -286,6 +285,15 @@ static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
 	return run_command(&cmd);
 }
 
+static int maintenance_task_reflog_expire(struct maintenance_run_opts *opts UNUSED,
+					  struct gc_config *cfg UNUSED)
+{
+	struct child_process cmd = CHILD_PROCESS_INIT;
+	cmd.git_cmd = 1;
+	strvec_pushl(&cmd.args, "reflog", "expire", "--all", NULL);
+	return run_command(&cmd);
+}
+
 static int too_many_loose_objects(struct gc_config *cfg)
 {
 	/*
@@ -662,15 +670,8 @@ static void gc_before_repack(struct maintenance_run_opts *opts,
 
 	if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg))
 		die(FAILED_RUN, "pack-refs");
-
-	if (cfg->prune_reflogs) {
-		struct child_process cmd = CHILD_PROCESS_INIT;
-
-		cmd.git_cmd = 1;
-		strvec_pushv(&cmd.args, reflog.v);
-		if (run_command(&cmd))
-			die(FAILED_RUN, reflog.v[0]);
-	}
+	if (cfg->prune_reflogs && maintenance_task_reflog_expire(opts, cfg))
+		die(FAILED_RUN, "reflog");
 }
 
 int cmd_gc(int argc,
@@ -718,7 +719,6 @@ struct repository *repo UNUSED)
 	show_usage_with_options_if_asked(argc, argv,
 					 builtin_gc_usage, builtin_gc_options);
 
-	strvec_pushl(&reflog, "reflog", "expire", "--all", NULL);
 	strvec_pushl(&repack, "repack", "-d", "-l", NULL);
 	strvec_pushl(&prune, "prune", "--expire", NULL);
 	strvec_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL);

-- 
2.48.1.741.g8a9f3a5cdc.dirty

[PATCH 6/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

By default, git-maintenance(1) uses the "gc" task to ensure that the
repository is well-maintained. This can be changed, for example by
either explicitly configuring which tasks should be enabled or by using
the "incremental" maintenance strategy. If so, git-maintenance(1) does
not know to expire reflog entries, which is a subtask that git-gc(1)
knows to perform for the user. Consequently, the reflog will grow
indefinitely unless the user manually trims it.

Introduce a new "reflog-expire" task that plugs this gap:

  - When running the task directly, then we simply execute `git reflog
    expire --all`, which is the same as git-gc(1).

  - When running git-maintenance(1) with the `--auto` flag, then we only
    run the task in case the "HEAD" reflog has at least N reflog entries
    that would be discarded. By default, N is set to 100, but this can
    be configured via "maintenance.reflog-expire.auto". When a negative
    integer has been provided we always expire entries, zero causes us
    to never expire entries, and a positive value specifies how many
    entries need to exist before we consider pruning the entries.

Note that the condition for the `--auto` flags is merely a heuristic and
optimized for being fast. This is because `git maintenance run --auto`
will be executed quite regularly, so scanning through all reflogs would
likely be too expensive in many repositories.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/maintenance.adoc |  9 +++++++
 Documentation/git-maintenance.adoc    |  4 +++
 builtin/gc.c                          | 50 +++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh                | 18 +++++++++++++
 4 files changed, 81 insertions(+)

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index 72a9d6cf816..e57f346a067 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -69,3 +69,12 @@ maintenance.incremental-repack.auto::
 	Otherwise, a positive value implies the command should run when the
 	number of pack-files not in the multi-pack-index is at least the value
 	of `maintenance.incremental-repack.auto`. The default value is 10.
+
+maintenance.reflog-expire.auto::
+	This integer config option controls how often the `reflog-expire` task
+	should be run as part of `git maintenance run --auto`. If zero, then
+	the `reflog-expire` task will not run with the `--auto` option. A
+	negative value will force the task to run every time. Otherwise, a
+	positive value implies the command should run when the number of
+	expired reflog entries in the "HEAD" reflog is at least the value of
+	`maintenance.loose-objects.auto`. The default value is 100.
diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index 0450d74aff1..8bc94a6d4ff 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -158,6 +158,10 @@ pack-refs::
 	need to iterate across many references. See linkgit:git-pack-refs[1]
 	for more information.
 
+reflog-expire::
+	The `reflog-expire` task deletes any entries in the reflog older than the
+	expiry threshold. See linkgit:git-reflog[1] for more information.
+
 OPTIONS
 -------
 --auto::
diff --git a/builtin/gc.c b/builtin/gc.c
index e8f5705dc59..ce5bb2630f8 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,6 +33,7 @@
 #include "pack.h"
 #include "pack-objects.h"
 #include "path.h"
+#include "reflog.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,6 +286,49 @@ static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
 	return run_command(&cmd);
 }
 
+struct count_reflog_entries_data {
+	struct expire_reflog_policy_cb policy;
+	size_t count;
+	size_t limit;
+};
+
+static int count_reflog_entries(struct object_id *old_oid, struct object_id *new_oid,
+				const char *committer, timestamp_t timestamp,
+				int tz, const char *msg, void *cb_data)
+{
+	struct count_reflog_entries_data *data = cb_data;
+	if (should_expire_reflog_ent(old_oid, new_oid, committer, timestamp, tz, msg, &data->policy))
+		data->count++;
+	return data->count >= data->limit;
+}
+
+static int reflog_expire_condition(struct gc_config *cfg UNUSED)
+{
+	timestamp_t now = time(NULL);
+	struct count_reflog_entries_data data = {
+		.policy = {
+			.opts = REFLOG_EXPIRE_OPTIONS_INIT(now),
+		},
+	};
+	int limit = 100;
+
+	git_config_get_int("maintenance.reflog-expire.auto", &limit);
+	if (!limit)
+		return 0;
+	if (limit < 0)
+		return 1;
+	data.limit = limit;
+
+	repo_config(the_repository, reflog_expire_config, &data.policy.opts);
+
+	reflog_expire_options_set_refname(&data.policy.opts, "HEAD");
+	refs_for_each_reflog_ent(get_main_ref_store(the_repository), "HEAD",
+				 count_reflog_entries, &data);
+
+	reflog_expiry_cleanup(&data.policy);
+	return data.count >= data.limit;
+}
+
 static int maintenance_task_reflog_expire(struct maintenance_run_opts *opts UNUSED,
 					  struct gc_config *cfg UNUSED)
 {
@@ -1383,6 +1427,7 @@ enum maintenance_task_label {
 	TASK_GC,
 	TASK_COMMIT_GRAPH,
 	TASK_PACK_REFS,
+	TASK_REFLOG_EXPIRE,
 
 	/* Leave as final value */
 	TASK__COUNT
@@ -1419,6 +1464,11 @@ static struct maintenance_task tasks[] = {
 		maintenance_task_pack_refs,
 		pack_refs_condition,
 	},
+	[TASK_REFLOG_EXPIRE] = {
+		"reflog-expire",
+		maintenance_task_reflog_expire,
+		reflog_expire_condition,
+	},
 };
 
 static int compare_tasks_by_selection(const void *a_, const void *b_)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1909aed95e0..ff98cde92c0 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -447,6 +447,24 @@ test_expect_success 'pack-refs task' '
 	test_subcommand git pack-refs --all --prune <pack-refs.txt
 '
 
+test_expect_success 'reflog-expire task' '
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire.txt" \
+		git maintenance run --task=reflog-expire &&
+	test_subcommand git reflog expire --all <reflog-expire.txt
+'
+
+test_expect_success 'reflog-expire task --auto only packs when exceeding limits' '
+	git reflog expire --all --expire=now &&
+	test_commit reflog-one &&
+	test_commit reflog-two &&
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
+		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
+	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
+		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
+	test_subcommand git reflog expire --all <reflog-expire-auto.txt
+'
+
 test_expect_success '--auto and --schedule incompatible' '
 	test_must_fail git maintenance run --auto --schedule=daily 2>err &&
 	test_grep "at most one" err

-- 
2.48.1.741.g8a9f3a5cdc.dirty

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Ramsay Jones]

On 26/02/2025 15:24, Patrick Steinhardt wrote:
> Hi,
> 
> this patch series introduces a new "reflog-expire" task to
> git-maintenance(1). This task is designed to plug a gap when the "gc"
> task is disabled, as there is no way to expire reflog entries in that
> case.
> 
> This patch series has been inspired by the discussion at [1]. I consider
> it to be another step into the direction of replacing git-gc(1) and
> allowing for more flexible maintenance strategies overall. Next steps

Hmm, I don't know what you have in mind, but just as a data-point, I have
never used, and have no inclination to use, git-maintenance. However, I do
use git-gc extensively: at least once (times the number of repos fetched
which have changes) per day, pretty much every day! :)

ATB,
Ramsay Jones

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Junio C Hamano]

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

> Hmm, I don't know what you have in mind, but just as a data-point, I have
> never used, and have no inclination to use, git-maintenance. However, I do
> use git-gc extensively: at least once (times the number of repos fetched
> which have changes) per day, pretty much every day! :)

That makes two of us, but everybody knows that we are old fashioned ;-)

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Ramsay Jones]

On 26/02/2025 18:40, Junio C Hamano wrote:
> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>> Hmm, I don't know what you have in mind, but just as a data-point, I have
>> never used, and have no inclination to use, git-maintenance. However, I do
>> use git-gc extensively: at least once (times the number of repos fetched
>> which have changes) per day, pretty much every day! :)
> 
> That makes two of us, but everybody knows that we are old fashioned ;-)

true, very true. :)

ATB,
Ramsay Jones

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

On Wed, Feb 26, 2025 at 06:54:48PM +0000, Ramsay Jones wrote:
> On 26/02/2025 18:40, Junio C Hamano wrote:
> > Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> > 
> >> Hmm, I don't know what you have in mind, but just as a data-point, I have
> >> never used, and have no inclination to use, git-maintenance. However, I do
> >> use git-gc extensively: at least once (times the number of repos fetched
> >> which have changes) per day, pretty much every day! :)
> > 
> > That makes two of us, but everybody knows that we are old fashioned ;-)
> 
> true, very true. :)

Well, it depends on what you mean by "use". In fact, both of you use it
implicitly assuming that you use a recent version of Git because that is
what Git nowadays spawns automatically: we don't use `git gc --auto`
anymore, but instead use `git maintenance run --auto`. It _does_ still
use git-gc(1) under the hood by default, but that is something we can
change going forward.

The opportunity here is to have a more fine-grained strategy to perform
maintenance, both when run explicitly but also when run automatically by
Git. git-maintenance(1) is written in a way that makes it significantly
more flexible overall, so we can iterate on how exactly it performs the
maintenance for the user. Different strategies may make sense in some
contexts, but not in others, and that is something we can account for
here.

It also allows us to bring newer features to the masses that have a
chance to improve performance or reduce the time spent maintaining
repositories for everyone: multi-pack indices, split commit graphs,
geometric repacking, incremental bitmaps.

While we could move them into git-gc(1), I think that this tool is just
not well-suited for such changes as it simply doesn't provide a good
foundation for tweakable behaviour.

Patrick

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> this patch series introduces a new "reflog-expire" task to
> git-maintenance(1). This task is designed to plug a gap when the "gc"
> task is disabled, as there is no way to expire reflog entries in that
> case.

I think in the longer run, "maintenance" users should be able to
treat the single ball of wax "gc" task as a mere short-hand to
invoke a set of often used maintenance tasks, and we would want to
break down the component tasks grouped in it and make them
independently available.  This is a good step along that journey.

Are there other things that the "gc" task covers that are not
available elsewhere?  "git gc --help" suggests there are things
related to pruning (unused?) worktrees and stale rerere database
entries.

Another thing, how much control do we want to cede to the end users
the choice of tasks and order of running them?  When you are
expiring stale reflog entries and repacking the object database to
discard unreachable objects, it would only make sense to do them in
the order I just said.  We could leave it up to the end users, but
that may be doing disservice to them.

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

On Wed, Feb 26, 2025 at 05:23:10PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > this patch series introduces a new "reflog-expire" task to
> > git-maintenance(1). This task is designed to plug a gap when the "gc"
> > task is disabled, as there is no way to expire reflog entries in that
> > case.
> 
> I think in the longer run, "maintenance" users should be able to
> treat the single ball of wax "gc" task as a mere short-hand to
> invoke a set of often used maintenance tasks, and we would want to
> break down the component tasks grouped in it and make them
> independently available.  This is a good step along that journey.
> 
> Are there other things that the "gc" task covers that are not
> available elsewhere?  "git gc --help" suggests there are things
> related to pruning (unused?) worktrees and stale rerere database
> entries.

These are more gaps indeed. I'm happy to work on them once this patch
series has landed. I don't know about any other gaps.

> Another thing, how much control do we want to cede to the end users
> the choice of tasks and order of running them?  When you are
> expiring stale reflog entries and repacking the object database to
> discard unreachable objects, it would only make sense to do them in
> the order I just said.  We could leave it up to the end users, but
> that may be doing disservice to them.

This is a good question. From my perspective, there are three classes of
users here:

  - Those that don't care and don't have special needs. This class of
    users is unlikely to tweak things anyway.

  - Those that aren't deeply familiar with how Git works, but who do
    have special needs e.g. because they have huge repositories. This
    class of users may need to tweak configuration, but we should give
    them an _easy_ way to do so. Configuring individual tasks ain't that
    from my perspective.

  - Power users that are deeply familiar with how Git works. This class
    of users may even want to tweak the order in which specific tasks
    run.

"maintenance.strategy" exists to cater to the second class of users and
allows them to configure the high-level strategy used to maintain repos.
I don't know whether it's honored by `git maintenance run`, but I think
it is (and if it's not it should be).

That to me means that the configuration for individual tasks for power
users can be as flexible as possible, including configuring the order in
which tasks are run.

Patrick

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Feb 26, 2025 at 05:23:10PM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@pks.im> writes:
>> 
>> > this patch series introduces a new "reflog-expire" task to
>> > git-maintenance(1). This task is designed to plug a gap when the "gc"
>> > task is disabled, as there is no way to expire reflog entries in that
>> > case.
>> 
>> I think in the longer run, "maintenance" users should be able to
>> treat the single ball of wax "gc" task as a mere short-hand to
>> invoke a set of often used maintenance tasks, and we would want to
>> break down the component tasks grouped in it and make them
>> independently available.  This is a good step along that journey.
>> 
>> Are there other things that the "gc" task covers that are not
>> available elsewhere?  "git gc --help" suggests there are things
>> related to pruning (unused?) worktrees and stale rerere database
>> entries.
>
> These are more gaps indeed. I'm happy to work on them once this patch
> series has landed. I don't know about any other gaps.

Or maybe leave breadcrumbs and invite others to help advance the
cause?  If we know we have achieved consensus that it is a good
direction to go in, that is (we already saw a mention that indicates
that there are populations of us who do not care too much about
extending maintenance but are familiar with gc).

Re: [PATCH 0/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

On Thu, Feb 27, 2025 at 09:01:49AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Wed, Feb 26, 2025 at 05:23:10PM -0800, Junio C Hamano wrote:
> >> Patrick Steinhardt <ps@pks.im> writes:
> >> 
> >> > this patch series introduces a new "reflog-expire" task to
> >> > git-maintenance(1). This task is designed to plug a gap when the "gc"
> >> > task is disabled, as there is no way to expire reflog entries in that
> >> > case.
> >> 
> >> I think in the longer run, "maintenance" users should be able to
> >> treat the single ball of wax "gc" task as a mere short-hand to
> >> invoke a set of often used maintenance tasks, and we would want to
> >> break down the component tasks grouped in it and make them
> >> independently available.  This is a good step along that journey.
> >> 
> >> Are there other things that the "gc" task covers that are not
> >> available elsewhere?  "git gc --help" suggests there are things
> >> related to pruning (unused?) worktrees and stale rerere database
> >> entries.
> >
> > These are more gaps indeed. I'm happy to work on them once this patch
> > series has landed. I don't know about any other gaps.
> 
> Or maybe leave breadcrumbs and invite others to help advance the
> cause?  If we know we have achieved consensus that it is a good
> direction to go in, that is (we already saw a mention that indicates
> that there are populations of us who do not care too much about
> extending maintenance but are familiar with gc).

Oh, sure, I wouldn't mind at all if somebody else picked this up. The
question to me is where to leave the breadcrumb, other than having it in
this thread.

Patrick

[PATCH v2 0/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

Hi,

this patch series introduces a new "reflog-expire" task to
git-maintenance(1). This task is designed to plug a gap when the "gc"
task is disabled, as there is no way to expire reflog entries in that
case.

This patch series has been inspired by the discussion at [1]. I consider
it to be another step into the direction of replacing git-gc(1) and
allowing for more flexible maintenance strategies overall. Next steps
could be:

  1. Enable the "reflog-expire" task by default when using the
     "incremental" strategy. and then we might eventually switch over
     the

  2. Use "incremental" strategy when "features.experimental" is enabled.

  3. Switch over the default strategy to "incremental" after a couple of
     releases.

Changes in v2:
  - Two commit message typos.
  - Link to v1: https://lore.kernel.org/r/20250226-pks-maintenance-reflog-expire-v1-0-a1204a814952@pks.im

Thanks!

Patrick

[1]: <e650f4e4-e267-4f1f-bb3a-c71b1fe0b276@uxp.de>

---
Patrick Steinhardt (6):
      reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`
      builtin/reflog: stop storing default reflog expiry dates globally
      builtin/reflog: stop storing per-reflog expiry dates globally
      builtin/reflog: make functions regarding `reflog_expire_options` public
      builtin/gc: split out function to expire reflog entries
      builtin/maintenance: introduce "reflog-expire" task

 Documentation/config/maintenance.adoc |   9 ++
 Documentation/git-maintenance.adoc    |   4 +
 builtin/gc.c                          |  72 +++++++++++++---
 builtin/reflog.c                      | 153 ++++------------------------------
 reflog.c                              | 137 ++++++++++++++++++++++++++----
 reflog.h                              |  35 +++++++-
 t/t7900-maintenance.sh                |  18 ++++
 7 files changed, 263 insertions(+), 165 deletions(-)

Range-diff versus v1:

1:  62ed4d1dcb6 = 1:  92d20e9403b reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`
2:  c9da014e85c ! 2:  5625b7472f8 builtin/reflog: stop storing default reflog expiry dates globally
    @@ Commit message
     
           - The currently active set of expiry dates for a given reference.
     
    -    While the last item is stored in `struct reflog_expiry_options`, the
    +    While the last item is stored in `struct reflog_expire_options`, the
         other items aren't, which makes it hard to reuse the structure in other
         places.
     
3:  bd06fee3d23 ! 3:  146080fe970 builtin/reflog: stop storing per-reflog expiry dates globally
    @@ Commit message
     
         As described in the preceding commit, the per-reflog expiry dates are
         stored in a global pair of variables. Refactor the code so that they are
    -    contained in `sturct reflog_expire_options` to make the structure useful
    +    contained in `struct reflog_expire_options` to make the structure useful
         in other contexts.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
4:  90ec3a8ffc8 = 4:  0f40bf3f45a builtin/reflog: make functions regarding `reflog_expire_options` public
5:  17d0e940266 = 5:  3072a421957 builtin/gc: split out function to expire reflog entries
6:  5799eb30843 = 6:  ca81a7e5686 builtin/maintenance: introduce "reflog-expire" task

---
base-commit: 5a526e5e18ddb9a7dfc5a2967d21d6154df64a4f
change-id: 20250226-pks-maintenance-reflog-expire-61c61410751a

[PATCH v2 1/6] reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`

[Patrick Steinhardt]

We're about to expose `struct cmd_reflog_expire_cb` via "reflog.h" so
that we can also use this structure in "builtin/gc.c". Once we make it
accessible to a wider scope though it becomes awkwardly named, as it
isn't only useful in the context of a callback. Instead, the function is
containing all kinds of options relevant to whether or not a reflog
entry should be expired.

Rename the structure to `reflog_expire_options` to prepare for this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 38 +++++++++++++++++++-------------------
 reflog.c         | 30 +++++++++++++++---------------
 reflog.h         |  4 ++--
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 95f264989bb..dee49881d32 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -168,7 +168,7 @@ static int reflog_expire_config(const char *var, const char *value,
 	return 0;
 }
 
-static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, const char *ref)
+static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
 {
 	struct reflog_expire_cfg *ent;
 
@@ -207,15 +207,15 @@ static int expire_unreachable_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
 {
-	struct cmd_reflog_expire_cb *cmd = opt->value;
+	struct reflog_expire_options *opts = opt->value;
 
 	BUG_ON_OPT_NEG(unset);
 
-	if (parse_expiry_date(arg, &cmd->expire_unreachable))
+	if (parse_expiry_date(arg, &opts->expire_unreachable))
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	cmd->explicit_expiry |= EXPIRE_UNREACH;
+	opts->explicit_expiry |= EXPIRE_UNREACH;
 	return 0;
 }
 
@@ -223,15 +223,15 @@ static int expire_total_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
 {
-	struct cmd_reflog_expire_cb *cmd = opt->value;
+	struct reflog_expire_options *opts = opt->value;
 
 	BUG_ON_OPT_NEG(unset);
 
-	if (parse_expiry_date(arg, &cmd->expire_total))
+	if (parse_expiry_date(arg, &opts->expire_total))
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	cmd->explicit_expiry |= EXPIRE_TOTAL;
+	opts->explicit_expiry |= EXPIRE_TOTAL;
 	return 0;
 }
 
@@ -276,7 +276,7 @@ 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 cmd_reflog_expire_cb cmd = { 0 };
+	struct reflog_expire_options opts = { 0 };
 	timestamp_t now = time(NULL);
 	int i, status, do_all, single_worktree = 0;
 	unsigned int flags = 0;
@@ -292,15 +292,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 			N_("update the reference to the value of the top reflog entry"),
 			EXPIRE_REFLOGS_UPDATE_REF),
 		OPT_BOOL(0, "verbose", &verbose, N_("print extra information on screen")),
-		OPT_CALLBACK_F(0, "expire", &cmd, N_("timestamp"),
+		OPT_CALLBACK_F(0, "expire", &opts, N_("timestamp"),
 			       N_("prune entries older than the specified time"),
 			       PARSE_OPT_NONEG,
 			       expire_total_callback),
-		OPT_CALLBACK_F(0, "expire-unreachable", &cmd, N_("timestamp"),
+		OPT_CALLBACK_F(0, "expire-unreachable", &opts, N_("timestamp"),
 			       N_("prune entries older than <time> that are not reachable from the current tip of the branch"),
 			       PARSE_OPT_NONEG,
 			       expire_unreachable_callback),
-		OPT_BOOL(0, "stale-fix", &cmd.stalefix,
+		OPT_BOOL(0, "stale-fix", &opts.stalefix,
 			 N_("prune any reflog entries that point to broken commits")),
 		OPT_BOOL(0, "all", &do_all, N_("process the reflogs of all references")),
 		OPT_BOOL(0, "single-worktree", &single_worktree,
@@ -315,9 +315,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 	save_commit_buffer = 0;
 	do_all = status = 0;
 
-	cmd.explicit_expiry = 0;
-	cmd.expire_total = default_reflog_expire;
-	cmd.expire_unreachable = default_reflog_expire_unreachable;
+	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);
 
@@ -329,7 +329,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 	 * even in older repository.  We cannot trust what's reachable
 	 * from reflog if the repository was pruned with older git.
 	 */
-	if (cmd.stalefix) {
+	if (opts.stalefix) {
 		struct rev_info revs;
 
 		repo_init_revisions(the_repository, &revs, prefix);
@@ -363,11 +363,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 
 		for_each_string_list_item(item, &collected.reflogs) {
 			struct expire_reflog_policy_cb cb = {
-				.cmd = cmd,
+				.opts = opts,
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 			};
 
-			set_reflog_expiry_param(&cb.cmd,  item->string);
+			set_reflog_expiry_param(&cb.opts,  item->string);
 			status |= refs_reflog_expire(get_main_ref_store(the_repository),
 						     item->string, flags,
 						     reflog_expiry_prepare,
@@ -380,13 +380,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 
 	for (i = 0; i < argc; i++) {
 		char *ref;
-		struct expire_reflog_policy_cb cb = { .cmd = cmd };
+		struct expire_reflog_policy_cb cb = { .opts = opts };
 
 		if (!repo_dwim_log(the_repository, argv[i], strlen(argv[i]), NULL, &ref)) {
 			status |= error(_("%s points nowhere!"), argv[i]);
 			continue;
 		}
-		set_reflog_expiry_param(&cb.cmd, ref);
+		set_reflog_expiry_param(&cb.opts, ref);
 		status |= refs_reflog_expire(get_main_ref_store(the_repository),
 					     ref, flags,
 					     reflog_expiry_prepare,
diff --git a/reflog.c b/reflog.c
index 1b5f031f6d7..bcdb75514d0 100644
--- a/reflog.c
+++ b/reflog.c
@@ -252,15 +252,15 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	struct expire_reflog_policy_cb *cb = cb_data;
 	struct commit *old_commit, *new_commit;
 
-	if (timestamp < cb->cmd.expire_total)
+	if (timestamp < cb->opts.expire_total)
 		return 1;
 
 	old_commit = new_commit = NULL;
-	if (cb->cmd.stalefix &&
+	if (cb->opts.stalefix &&
 	    (!keep_entry(&old_commit, ooid) || !keep_entry(&new_commit, noid)))
 		return 1;
 
-	if (timestamp < cb->cmd.expire_unreachable) {
+	if (timestamp < cb->opts.expire_unreachable) {
 		switch (cb->unreachable_expire_kind) {
 		case UE_ALWAYS:
 			return 1;
@@ -272,7 +272,7 @@ int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		}
 	}
 
-	if (cb->cmd.recno && --(cb->cmd.recno) == 0)
+	if (cb->opts.recno && --(cb->opts.recno) == 0)
 		return 1;
 
 	return 0;
@@ -331,7 +331,7 @@ void reflog_expiry_prepare(const char *refname,
 	struct commit_list *elem;
 	struct commit *commit = NULL;
 
-	if (!cb->cmd.expire_unreachable || is_head(refname)) {
+	if (!cb->opts.expire_unreachable || is_head(refname)) {
 		cb->unreachable_expire_kind = UE_HEAD;
 	} else {
 		commit = lookup_commit_reference_gently(the_repository,
@@ -341,7 +341,7 @@ void reflog_expiry_prepare(const char *refname,
 		cb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;
 	}
 
-	if (cb->cmd.expire_unreachable <= cb->cmd.expire_total)
+	if (cb->opts.expire_unreachable <= cb->opts.expire_total)
 		cb->unreachable_expire_kind = UE_ALWAYS;
 
 	switch (cb->unreachable_expire_kind) {
@@ -358,7 +358,7 @@ void reflog_expiry_prepare(const char *refname,
 		/* For reflog_expiry_cleanup() below */
 		cb->tip_commit = commit;
 	}
-	cb->mark_limit = cb->cmd.expire_total;
+	cb->mark_limit = cb->opts.expire_total;
 	mark_reachable(cb);
 }
 
@@ -390,7 +390,7 @@ int count_reflog_ent(struct object_id *ooid UNUSED,
 		     timestamp_t timestamp, int tz UNUSED,
 		     const char *message UNUSED, void *cb_data)
 {
-	struct cmd_reflog_expire_cb *cb = cb_data;
+	struct reflog_expire_options *cb = cb_data;
 	if (!cb->expire_total || timestamp < cb->expire_total)
 		cb->recno++;
 	return 0;
@@ -398,7 +398,7 @@ int count_reflog_ent(struct object_id *ooid UNUSED,
 
 int reflog_delete(const char *rev, enum expire_reflog_flags flags, int verbose)
 {
-	struct cmd_reflog_expire_cb cmd = { 0 };
+	struct reflog_expire_options opts = { 0 };
 	int status = 0;
 	reflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;
 	const char *spec = strstr(rev, "@{");
@@ -421,17 +421,17 @@ int reflog_delete(const char *rev, enum expire_reflog_flags flags, int verbose)
 
 	recno = strtoul(spec + 2, &ep, 10);
 	if (*ep == '}') {
-		cmd.recno = -recno;
+		opts.recno = -recno;
 		refs_for_each_reflog_ent(get_main_ref_store(the_repository),
-					 ref, count_reflog_ent, &cmd);
+					 ref, count_reflog_ent, &opts);
 	} else {
-		cmd.expire_total = approxidate(spec + 2);
+		opts.expire_total = approxidate(spec + 2);
 		refs_for_each_reflog_ent(get_main_ref_store(the_repository),
-					 ref, count_reflog_ent, &cmd);
-		cmd.expire_total = 0;
+					 ref, count_reflog_ent, &opts);
+		opts.expire_total = 0;
 	}
 
-	cb.cmd = cmd;
+	cb.opts = opts;
 	status |= refs_reflog_expire(get_main_ref_store(the_repository), ref,
 				     flags,
 				     reflog_expiry_prepare,
diff --git a/reflog.h b/reflog.h
index d2906fb9f8d..eb948119e53 100644
--- a/reflog.h
+++ b/reflog.h
@@ -2,7 +2,7 @@
 #define REFLOG_H
 #include "refs.h"
 
-struct cmd_reflog_expire_cb {
+struct reflog_expire_options {
 	int stalefix;
 	int explicit_expiry;
 	timestamp_t expire_total;
@@ -18,7 +18,7 @@ struct expire_reflog_policy_cb {
 	} unreachable_expire_kind;
 	struct commit_list *mark_list;
 	unsigned long mark_limit;
-	struct cmd_reflog_expire_cb cmd;
+	struct reflog_expire_options opts;
 	struct commit *tip_commit;
 	struct commit_list *tips;
 	unsigned int dry_run:1;

-- 
2.49.0.682.gc9b6a7b2b0.dirty

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

[Patrick Steinhardt]

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_expire_options`, the
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;
 			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;
 }
 
 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);
 	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, \
+}
 
 struct expire_reflog_policy_cb {
 	enum {

-- 
2.49.0.682.gc9b6a7b2b0.dirty

[PATCH v2 3/6] builtin/reflog: stop storing per-reflog expiry dates globally

[Patrick Steinhardt]

As described in the preceding commit, the per-reflog expiry dates are
stored in a global pair of variables. Refactor the code so that they are
contained in `struct reflog_expire_options` to make the structure useful
in other contexts.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 30 ++++++++++++------------------
 reflog.h         |  8 ++++++++
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 0910a4e25dc..a231cf4b857 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -88,27 +88,21 @@ static int collect_reflog(const char *ref, void *cb_data)
 	return 0;
 }
 
-static struct reflog_expire_cfg {
-	struct reflog_expire_cfg *next;
-	timestamp_t expire_total;
-	timestamp_t expire_unreachable;
-	char pattern[FLEX_ARRAY];
-} *reflog_expire_cfg, **reflog_expire_cfg_tail;
-
-static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
+static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
+						       const char *pattern, size_t len)
 {
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
-	if (!reflog_expire_cfg_tail)
-		reflog_expire_cfg_tail = &reflog_expire_cfg;
+	if (!opts->entries_tail)
+		opts->entries_tail = &opts->entries;
 
-	for (ent = reflog_expire_cfg; ent; ent = ent->next)
+	for (ent = opts->entries; ent; ent = ent->next)
 		if (!xstrncmpz(ent->pattern, pattern, len))
 			return ent;
 
 	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
-	*reflog_expire_cfg_tail = ent;
-	reflog_expire_cfg_tail = &(ent->next);
+	*opts->entries_tail = ent;
+	opts->entries_tail = &(ent->next);
 	return ent;
 }
 
@@ -124,7 +118,7 @@ static int reflog_expire_config(const char *var, const char *value,
 	size_t pattern_len;
 	timestamp_t expire;
 	int slot;
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
 	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
 		return git_default_config(var, value, ctx, cb);
@@ -152,7 +146,7 @@ static int reflog_expire_config(const char *var, const char *value,
 		return 0;
 	}
 
-	ent = find_cfg_ent(pattern, pattern_len);
+	ent = find_cfg_ent(opts, pattern, pattern_len);
 	if (!ent)
 		return -1;
 	switch (slot) {
@@ -168,12 +162,12 @@ static int reflog_expire_config(const char *var, const char *value,
 
 static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
 {
-	struct reflog_expire_cfg *ent;
+	struct reflog_expire_entry_option *ent;
 
 	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
 		return; /* both given explicitly -- nothing to tweak */
 
-	for (ent = reflog_expire_cfg; ent; ent = ent->next) {
+	for (ent = cb->entries; ent; ent = ent->next) {
 		if (!wildmatch(ent->pattern, ref, 0)) {
 			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
 				cb->expire_total = ent->expire_total;
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;
 	int stalefix;
 	int explicit_expiry;
 	timestamp_t default_expire_total;

-- 
2.49.0.682.gc9b6a7b2b0.dirty

[PATCH v2 4/6] builtin/reflog: make functions regarding `reflog_expire_options` public

[Patrick Steinhardt]

Make functions that are required to manage `reflog_expire_options`
available elsewhere by moving them into "reflog.c" and exposing them in
the corresponding header. The functions will be used in a subsequent
commit.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/reflog.c | 115 ++-----------------------------------------------------
 reflog.c         | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++
 reflog.h         |  17 ++++++++
 3 files changed, 128 insertions(+), 111 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index a231cf4b857..5fea31f9c3c 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -88,113 +88,6 @@ static int collect_reflog(const char *ref, void *cb_data)
 	return 0;
 }
 
-static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
-						       const char *pattern, size_t len)
-{
-	struct reflog_expire_entry_option *ent;
-
-	if (!opts->entries_tail)
-		opts->entries_tail = &opts->entries;
-
-	for (ent = opts->entries; ent; ent = ent->next)
-		if (!xstrncmpz(ent->pattern, pattern, len))
-			return ent;
-
-	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
-	*opts->entries_tail = ent;
-	opts->entries_tail = &(ent->next);
-	return ent;
-}
-
-/* expiry timer slot */
-#define EXPIRE_TOTAL   01
-#define EXPIRE_UNREACH 02
-
-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;
-	int slot;
-	struct reflog_expire_entry_option *ent;
-
-	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
-		return git_default_config(var, value, ctx, cb);
-
-	if (!strcmp(key, "reflogexpire")) {
-		slot = EXPIRE_TOTAL;
-		if (git_config_expiry_date(&expire, var, value))
-			return -1;
-	} else if (!strcmp(key, "reflogexpireunreachable")) {
-		slot = EXPIRE_UNREACH;
-		if (git_config_expiry_date(&expire, var, value))
-			return -1;
-	} else
-		return git_default_config(var, value, ctx, cb);
-
-	if (!pattern) {
-		switch (slot) {
-		case EXPIRE_TOTAL:
-			opts->default_expire_total = expire;
-			break;
-		case EXPIRE_UNREACH:
-			opts->default_expire_unreachable = expire;
-			break;
-		}
-		return 0;
-	}
-
-	ent = find_cfg_ent(opts, pattern, pattern_len);
-	if (!ent)
-		return -1;
-	switch (slot) {
-	case EXPIRE_TOTAL:
-		ent->expire_total = expire;
-		break;
-	case EXPIRE_UNREACH:
-		ent->expire_unreachable = expire;
-		break;
-	}
-	return 0;
-}
-
-static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char *ref)
-{
-	struct reflog_expire_entry_option *ent;
-
-	if (cb->explicit_expiry == (EXPIRE_TOTAL|EXPIRE_UNREACH))
-		return; /* both given explicitly -- nothing to tweak */
-
-	for (ent = cb->entries; ent; ent = ent->next) {
-		if (!wildmatch(ent->pattern, ref, 0)) {
-			if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-				cb->expire_total = ent->expire_total;
-			if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-				cb->expire_unreachable = ent->expire_unreachable;
-			return;
-		}
-	}
-
-	/*
-	 * If unconfigured, make stash never expire
-	 */
-	if (!strcmp(ref, "refs/stash")) {
-		if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-			cb->expire_total = 0;
-		if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-			cb->expire_unreachable = 0;
-		return;
-	}
-
-	/* Nothing matched -- use the default value */
-	if (!(cb->explicit_expiry & EXPIRE_TOTAL))
-		cb->expire_total = cb->default_expire_total;
-	if (!(cb->explicit_expiry & EXPIRE_UNREACH))
-		cb->expire_unreachable = cb->default_expire_unreachable;
-}
-
 static int expire_unreachable_callback(const struct option *opt,
 				 const char *arg,
 				 int unset)
@@ -207,7 +100,7 @@ static int expire_unreachable_callback(const struct option *opt,
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	opts->explicit_expiry |= EXPIRE_UNREACH;
+	opts->explicit_expiry |= REFLOG_EXPIRE_UNREACH;
 	return 0;
 }
 
@@ -223,7 +116,7 @@ static int expire_total_callback(const struct option *opt,
 		die(_("invalid timestamp '%s' given to '--%s'"),
 		    arg, opt->long_name);
 
-	opts->explicit_expiry |= EXPIRE_TOTAL;
+	opts->explicit_expiry |= REFLOG_EXPIRE_TOTAL;
 	return 0;
 }
 
@@ -353,7 +246,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 				.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),
 			};
 
-			set_reflog_expiry_param(&cb.opts,  item->string);
+			reflog_expire_options_set_refname(&cb.opts,  item->string);
 			status |= refs_reflog_expire(get_main_ref_store(the_repository),
 						     item->string, flags,
 						     reflog_expiry_prepare,
@@ -372,7 +265,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
 			status |= error(_("%s points nowhere!"), argv[i]);
 			continue;
 		}
-		set_reflog_expiry_param(&cb.opts, ref);
+		reflog_expire_options_set_refname(&cb.opts, ref);
 		status |= refs_reflog_expire(get_main_ref_store(the_repository),
 					     ref, flags,
 					     reflog_expiry_prepare,
diff --git a/reflog.c b/reflog.c
index bcdb75514d0..642b162ef70 100644
--- a/reflog.c
+++ b/reflog.c
@@ -2,13 +2,120 @@
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
+#include "config.h"
 #include "gettext.h"
 #include "object-store-ll.h"
+#include "parse-options.h"
 #include "reflog.h"
 #include "refs.h"
 #include "revision.h"
 #include "tree.h"
 #include "tree-walk.h"
+#include "wildmatch.h"
+
+static struct reflog_expire_entry_option *find_cfg_ent(struct reflog_expire_options *opts,
+						       const char *pattern, size_t len)
+{
+	struct reflog_expire_entry_option *ent;
+
+	if (!opts->entries_tail)
+		opts->entries_tail = &opts->entries;
+
+	for (ent = opts->entries; ent; ent = ent->next)
+		if (!xstrncmpz(ent->pattern, pattern, len))
+			return ent;
+
+	FLEX_ALLOC_MEM(ent, pattern, pattern, len);
+	*opts->entries_tail = ent;
+	opts->entries_tail = &(ent->next);
+	return ent;
+}
+
+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;
+	int slot;
+	struct reflog_expire_entry_option *ent;
+
+	if (parse_config_key(var, "gc", &pattern, &pattern_len, &key) < 0)
+		return git_default_config(var, value, ctx, cb);
+
+	if (!strcmp(key, "reflogexpire")) {
+		slot = REFLOG_EXPIRE_TOTAL;
+		if (git_config_expiry_date(&expire, var, value))
+			return -1;
+	} else if (!strcmp(key, "reflogexpireunreachable")) {
+		slot = REFLOG_EXPIRE_UNREACH;
+		if (git_config_expiry_date(&expire, var, value))
+			return -1;
+	} else
+		return git_default_config(var, value, ctx, cb);
+
+	if (!pattern) {
+		switch (slot) {
+		case REFLOG_EXPIRE_TOTAL:
+			opts->default_expire_total = expire;
+			break;
+		case REFLOG_EXPIRE_UNREACH:
+			opts->default_expire_unreachable = expire;
+			break;
+		}
+		return 0;
+	}
+
+	ent = find_cfg_ent(opts, pattern, pattern_len);
+	if (!ent)
+		return -1;
+	switch (slot) {
+	case REFLOG_EXPIRE_TOTAL:
+		ent->expire_total = expire;
+		break;
+	case REFLOG_EXPIRE_UNREACH:
+		ent->expire_unreachable = expire;
+		break;
+	}
+	return 0;
+}
+
+void reflog_expire_options_set_refname(struct reflog_expire_options *cb,
+				       const char *ref)
+{
+	struct reflog_expire_entry_option *ent;
+
+	if (cb->explicit_expiry == (REFLOG_EXPIRE_TOTAL|REFLOG_EXPIRE_UNREACH))
+		return; /* both given explicitly -- nothing to tweak */
+
+	for (ent = cb->entries; ent; ent = ent->next) {
+		if (!wildmatch(ent->pattern, ref, 0)) {
+			if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+				cb->expire_total = ent->expire_total;
+			if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+				cb->expire_unreachable = ent->expire_unreachable;
+			return;
+		}
+	}
+
+	/*
+	 * If unconfigured, make stash never expire
+	 */
+	if (!strcmp(ref, "refs/stash")) {
+		if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+			cb->expire_total = 0;
+		if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+			cb->expire_unreachable = 0;
+		return;
+	}
+
+	/* Nothing matched -- use the default value */
+	if (!(cb->explicit_expiry & REFLOG_EXPIRE_TOTAL))
+		cb->expire_total = cb->default_expire_total;
+	if (!(cb->explicit_expiry & REFLOG_EXPIRE_UNREACH))
+		cb->expire_unreachable = cb->default_expire_unreachable;
+}
 
 /* Remember to update object flag allocation in object.h */
 #define INCOMPLETE	(1u<<10)
diff --git a/reflog.h b/reflog.h
index b08780a30a7..63bb56280f4 100644
--- a/reflog.h
+++ b/reflog.h
@@ -2,6 +2,9 @@
 #define REFLOG_H
 #include "refs.h"
 
+#define REFLOG_EXPIRE_TOTAL   (1 << 0)
+#define REFLOG_EXPIRE_UNREACH (1 << 1)
+
 struct reflog_expire_entry_option {
 	struct reflog_expire_entry_option *next;
 	timestamp_t expire_total;
@@ -24,6 +27,20 @@ struct reflog_expire_options {
 	.default_expire_unreachable = now - 90 * 24 * 3600, \
 }
 
+/*
+ * Parse the reflog expire configuration. This should be used with
+ * `repo_config()`.
+ */
+int reflog_expire_config(const char *var, const char *value,
+			 const struct config_context *ctx, void *cb);
+
+/*
+ * Adapt the options so that they apply to the given refname. This applies any
+ * per-reference reflog expiry configuration that may exist to the options.
+ */
+void reflog_expire_options_set_refname(struct reflog_expire_options *cb,
+				       const char *refname);
+
 struct expire_reflog_policy_cb {
 	enum {
 		UE_NORMAL,

-- 
2.49.0.682.gc9b6a7b2b0.dirty

[PATCH v2 5/6] builtin/gc: split out function to expire reflog entries

[Patrick Steinhardt]

We're about to introduce a new task for git-maintenance(1) that knows to
expire reflog entries. The logic will be shared with git-gc(1), which
already knows how to do this.

Pull out the common logic into a separate function so that we can share
the implementation between both builtins.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 409d454a4b7..e8f5705dc59 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -53,7 +53,6 @@ static const char * const builtin_gc_usage[] = {
 
 static timestamp_t gc_log_expire_time;
 
-static struct strvec reflog = STRVEC_INIT;
 static struct strvec repack = STRVEC_INIT;
 static struct strvec prune = STRVEC_INIT;
 static struct strvec prune_worktrees = STRVEC_INIT;
@@ -286,6 +285,15 @@ static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
 	return run_command(&cmd);
 }
 
+static int maintenance_task_reflog_expire(struct maintenance_run_opts *opts UNUSED,
+					  struct gc_config *cfg UNUSED)
+{
+	struct child_process cmd = CHILD_PROCESS_INIT;
+	cmd.git_cmd = 1;
+	strvec_pushl(&cmd.args, "reflog", "expire", "--all", NULL);
+	return run_command(&cmd);
+}
+
 static int too_many_loose_objects(struct gc_config *cfg)
 {
 	/*
@@ -662,15 +670,8 @@ static void gc_before_repack(struct maintenance_run_opts *opts,
 
 	if (cfg->pack_refs && maintenance_task_pack_refs(opts, cfg))
 		die(FAILED_RUN, "pack-refs");
-
-	if (cfg->prune_reflogs) {
-		struct child_process cmd = CHILD_PROCESS_INIT;
-
-		cmd.git_cmd = 1;
-		strvec_pushv(&cmd.args, reflog.v);
-		if (run_command(&cmd))
-			die(FAILED_RUN, reflog.v[0]);
-	}
+	if (cfg->prune_reflogs && maintenance_task_reflog_expire(opts, cfg))
+		die(FAILED_RUN, "reflog");
 }
 
 int cmd_gc(int argc,
@@ -718,7 +719,6 @@ struct repository *repo UNUSED)
 	show_usage_with_options_if_asked(argc, argv,
 					 builtin_gc_usage, builtin_gc_options);
 
-	strvec_pushl(&reflog, "reflog", "expire", "--all", NULL);
 	strvec_pushl(&repack, "repack", "-d", "-l", NULL);
 	strvec_pushl(&prune, "prune", "--expire", NULL);
 	strvec_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL);

-- 
2.49.0.682.gc9b6a7b2b0.dirty

[PATCH v2 6/6] builtin/maintenance: introduce "reflog-expire" task

[Patrick Steinhardt]

By default, git-maintenance(1) uses the "gc" task to ensure that the
repository is well-maintained. This can be changed, for example by
either explicitly configuring which tasks should be enabled or by using
the "incremental" maintenance strategy. If so, git-maintenance(1) does
not know to expire reflog entries, which is a subtask that git-gc(1)
knows to perform for the user. Consequently, the reflog will grow
indefinitely unless the user manually trims it.

Introduce a new "reflog-expire" task that plugs this gap:

  - When running the task directly, then we simply execute `git reflog
    expire --all`, which is the same as git-gc(1).

  - When running git-maintenance(1) with the `--auto` flag, then we only
    run the task in case the "HEAD" reflog has at least N reflog entries
    that would be discarded. By default, N is set to 100, but this can
    be configured via "maintenance.reflog-expire.auto". When a negative
    integer has been provided we always expire entries, zero causes us
    to never expire entries, and a positive value specifies how many
    entries need to exist before we consider pruning the entries.

Note that the condition for the `--auto` flags is merely a heuristic and
optimized for being fast. This is because `git maintenance run --auto`
will be executed quite regularly, so scanning through all reflogs would
likely be too expensive in many repositories.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 Documentation/config/maintenance.adoc |  9 +++++++
 Documentation/git-maintenance.adoc    |  4 +++
 builtin/gc.c                          | 50 +++++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh                | 18 +++++++++++++
 4 files changed, 81 insertions(+)

diff --git a/Documentation/config/maintenance.adoc b/Documentation/config/maintenance.adoc
index 72a9d6cf816..e57f346a067 100644
--- a/Documentation/config/maintenance.adoc
+++ b/Documentation/config/maintenance.adoc
@@ -69,3 +69,12 @@ maintenance.incremental-repack.auto::
 	Otherwise, a positive value implies the command should run when the
 	number of pack-files not in the multi-pack-index is at least the value
 	of `maintenance.incremental-repack.auto`. The default value is 10.
+
+maintenance.reflog-expire.auto::
+	This integer config option controls how often the `reflog-expire` task
+	should be run as part of `git maintenance run --auto`. If zero, then
+	the `reflog-expire` task will not run with the `--auto` option. A
+	negative value will force the task to run every time. Otherwise, a
+	positive value implies the command should run when the number of
+	expired reflog entries in the "HEAD" reflog is at least the value of
+	`maintenance.loose-objects.auto`. The default value is 100.
diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc
index 0450d74aff1..8bc94a6d4ff 100644
--- a/Documentation/git-maintenance.adoc
+++ b/Documentation/git-maintenance.adoc
@@ -158,6 +158,10 @@ pack-refs::
 	need to iterate across many references. See linkgit:git-pack-refs[1]
 	for more information.
 
+reflog-expire::
+	The `reflog-expire` task deletes any entries in the reflog older than the
+	expiry threshold. See linkgit:git-reflog[1] for more information.
+
 OPTIONS
 -------
 --auto::
diff --git a/builtin/gc.c b/builtin/gc.c
index e8f5705dc59..ce5bb2630f8 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -33,6 +33,7 @@
 #include "pack.h"
 #include "pack-objects.h"
 #include "path.h"
+#include "reflog.h"
 #include "blob.h"
 #include "tree.h"
 #include "promisor-remote.h"
@@ -285,6 +286,49 @@ static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,
 	return run_command(&cmd);
 }
 
+struct count_reflog_entries_data {
+	struct expire_reflog_policy_cb policy;
+	size_t count;
+	size_t limit;
+};
+
+static int count_reflog_entries(struct object_id *old_oid, struct object_id *new_oid,
+				const char *committer, timestamp_t timestamp,
+				int tz, const char *msg, void *cb_data)
+{
+	struct count_reflog_entries_data *data = cb_data;
+	if (should_expire_reflog_ent(old_oid, new_oid, committer, timestamp, tz, msg, &data->policy))
+		data->count++;
+	return data->count >= data->limit;
+}
+
+static int reflog_expire_condition(struct gc_config *cfg UNUSED)
+{
+	timestamp_t now = time(NULL);
+	struct count_reflog_entries_data data = {
+		.policy = {
+			.opts = REFLOG_EXPIRE_OPTIONS_INIT(now),
+		},
+	};
+	int limit = 100;
+
+	git_config_get_int("maintenance.reflog-expire.auto", &limit);
+	if (!limit)
+		return 0;
+	if (limit < 0)
+		return 1;
+	data.limit = limit;
+
+	repo_config(the_repository, reflog_expire_config, &data.policy.opts);
+
+	reflog_expire_options_set_refname(&data.policy.opts, "HEAD");
+	refs_for_each_reflog_ent(get_main_ref_store(the_repository), "HEAD",
+				 count_reflog_entries, &data);
+
+	reflog_expiry_cleanup(&data.policy);
+	return data.count >= data.limit;
+}
+
 static int maintenance_task_reflog_expire(struct maintenance_run_opts *opts UNUSED,
 					  struct gc_config *cfg UNUSED)
 {
@@ -1383,6 +1427,7 @@ enum maintenance_task_label {
 	TASK_GC,
 	TASK_COMMIT_GRAPH,
 	TASK_PACK_REFS,
+	TASK_REFLOG_EXPIRE,
 
 	/* Leave as final value */
 	TASK__COUNT
@@ -1419,6 +1464,11 @@ static struct maintenance_task tasks[] = {
 		maintenance_task_pack_refs,
 		pack_refs_condition,
 	},
+	[TASK_REFLOG_EXPIRE] = {
+		"reflog-expire",
+		maintenance_task_reflog_expire,
+		reflog_expire_condition,
+	},
 };
 
 static int compare_tasks_by_selection(const void *a_, const void *b_)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1909aed95e0..ff98cde92c0 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -447,6 +447,24 @@ test_expect_success 'pack-refs task' '
 	test_subcommand git pack-refs --all --prune <pack-refs.txt
 '
 
+test_expect_success 'reflog-expire task' '
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire.txt" \
+		git maintenance run --task=reflog-expire &&
+	test_subcommand git reflog expire --all <reflog-expire.txt
+'
+
+test_expect_success 'reflog-expire task --auto only packs when exceeding limits' '
+	git reflog expire --all --expire=now &&
+	test_commit reflog-one &&
+	test_commit reflog-two &&
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
+		git -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&
+	test_subcommand ! git reflog expire --all <reflog-expire-auto.txt &&
+	GIT_TRACE2_EVENT="$(pwd)/reflog-expire-auto.txt" \
+		git -c maintenance.reflog-expire.auto=2 maintenance run --auto --task=reflog-expire &&
+	test_subcommand git reflog expire --all <reflog-expire-auto.txt
+'
+
 test_expect_success '--auto and --schedule incompatible' '
 	test_must_fail git maintenance run --auto --schedule=daily 2>err &&
 	test_grep "at most one" err

-- 
2.49.0.682.gc9b6a7b2b0.dirty