threads / patch / 63007

patch, 6 partsbuiltin/maintenance: introduce "reflog-expire" task

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

## tl;dr

26 messages between Feb 26, 2025 and Apr 8, 2025. Diffs are folded; open one to read it.

replies: 25people: 4as markdown or json

Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · lore
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

Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 3 files +36 −36

builtin/reflog.c, reflog.c, reflog.h

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
Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 2 files +13 −15

builtin/reflog.c, reflog.h

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
Justin Tobler· Mar 4, 2025, 23:23 UTC · re: Patrick Steinhardt · lore

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

On 25/02/26 04:24PM, Patrick Steinhardt wrote:
Show 12 quoted lines
> When expiring reflog entries, it is possible to configure expiry dates
> that depend on the name of the reflog. This requires us to store a
> couple of different expiry dates:
> 
>   - The default expiry date for reflog entries that aren't otherwise
>     specified.
> 
>   - The per-reflog expiry date.
> 
>   - The currently active set of expiry dates for a given reference.
> 
> While the last item is stored in `struct reflog_expiry_options`, the
s/reflog_expiry_options/reflog_expire_options/
Show 45 quoted lines
> other items aren't, which makes it hard to reuse the structure in other
> places.
> 
> Refactor the code so that the default expiry date is stored as part of
> the structure. The per-reflog expiry dates will be adapted accordingly
> in the subsequent commit.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/reflog.c | 22 +++++++---------------
>  reflog.h         |  6 ++++++
>  2 files changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index dee49881d32..0910a4e25dc 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -63,9 +63,6 @@ static const char *const reflog_usage[] = {
>  	NULL
>  };
>  
> -static timestamp_t default_reflog_expire;
> -static timestamp_t default_reflog_expire_unreachable;
> -
>  struct worktree_reflogs {
>  	struct worktree *worktree;
>  	struct string_list reflogs;
> @@ -122,6 +119,7 @@ static struct reflog_expire_cfg *find_cfg_ent(const char *pattern, size_t len)
>  static int reflog_expire_config(const char *var, const char *value,
>  				const struct config_context *ctx, void *cb)
>  {
> +	struct reflog_expire_options *opts = cb;
>  	const char *pattern, *key;
>  	size_t pattern_len;
>  	timestamp_t expire;
> @@ -145,10 +143,10 @@ static int reflog_expire_config(const char *var, const char *value,
>  	if (!pattern) {
>  		switch (slot) {
>  		case EXPIRE_TOTAL:
> -			default_reflog_expire = expire;
> +			opts->default_expire_total = expire;
>  			break;
>  		case EXPIRE_UNREACH:
> -			default_reflog_expire_unreachable = expire;
> +			opts->default_expire_unreachable = expire;

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

Show 12 quoted lines
>  			break;
>  		}
>  		return 0;
> @@ -198,9 +196,9 @@ static void set_reflog_expiry_param(struct reflog_expire_options *cb, const char
>  
>  	/* Nothing matched -- use the default value */
>  	if (!(cb->explicit_expiry & EXPIRE_TOTAL))
> -		cb->expire_total = default_reflog_expire;
> +		cb->expire_total = cb->default_expire_total;
>  	if (!(cb->explicit_expiry & EXPIRE_UNREACH))
> -		cb->expire_unreachable = default_reflog_expire_unreachable;
> +		cb->expire_unreachable = cb->default_expire_unreachable;
Here we update where the defaults are used.
Show 10 quoted lines
>  }
>  
>  static int expire_unreachable_callback(const struct option *opt,
> @@ -276,8 +274,8 @@ static int cmd_reflog_list(int argc, const char **argv, const char *prefix,
>  static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
>  			     struct repository *repo UNUSED)
>  {
> -	struct reflog_expire_options opts = { 0 };
>  	timestamp_t now = time(NULL);
> +	struct reflog_expire_options opts = REFLOG_EXPIRE_OPTIONS_INIT(now);
The options type is initialized with the default defaults :)
Show 40 quoted lines
>  	int i, status, do_all, single_worktree = 0;
>  	unsigned int flags = 0;
>  	int verbose = 0;
> @@ -308,17 +306,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
>  		OPT_END()
>  	};
>  
> -	default_reflog_expire_unreachable = now - 30 * 24 * 3600;
> -	default_reflog_expire = now - 90 * 24 * 3600;
> -	git_config(reflog_expire_config, NULL);
> +	git_config(reflog_expire_config, &opts);
>  
>  	save_commit_buffer = 0;
>  	do_all = status = 0;
>  
> -	opts.explicit_expiry = 0;
> -	opts.expire_total = default_reflog_expire;
> -	opts.expire_unreachable = default_reflog_expire_unreachable;
> -
>  	argc = parse_options(argc, argv, prefix, options, reflog_expire_usage, 0);
>  
>  	if (verbose)
> diff --git a/reflog.h b/reflog.h
> index eb948119e53..a9d464bbf8c 100644
> --- a/reflog.h
> +++ b/reflog.h
> @@ -5,10 +5,16 @@
>  struct reflog_expire_options {
>  	int stalefix;
>  	int explicit_expiry;
> +	timestamp_t default_expire_total;
>  	timestamp_t expire_total;
> +	timestamp_t default_expire_unreachable;
>  	timestamp_t expire_unreachable;
>  	int recno;
>  };
> +#define REFLOG_EXPIRE_OPTIONS_INIT(now) { \
> +	.default_expire_total = now - 30 * 24 * 3600, \
> +	.default_expire_unreachable = now - 90 * 24 * 3600, \
> +}
Looking good
Show 8 quoted lines
>  
>  struct expire_reflog_policy_cb {
>  	enum {
> 
> -- 
> 2.48.1.741.g8a9f3a5cdc.dirty
> 
> 
Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 2 files +20 −18

builtin/reflog.c, reflog.h

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
Justin Tobler· Mar 4, 2025, 23:41 UTC · re: Patrick Steinhardt · lore

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

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/
Show 98 quoted lines
> 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
Patrick Steinhardt· Mar 6, 2025, 10:37 UTC · re: Justin Tobler · lore

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

On Tue, Mar 04, 2025 at 05:41:08PM -0600, Justin Tobler wrote:
Show 35 quoted lines
> 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
Justin Tobler· Mar 6, 2025, 23:17 UTC · re: Patrick Steinhardt · lore

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

On 25/03/06 11:37AM, Patrick Steinhardt wrote:
Show 24 quoted lines
> > 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
Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 3 files +128 −111

builtin/reflog.c, reflog.c, reflog.h

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
Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to builtin/gc.c +11 −11
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
Patrick Steinhardt· Feb 26, 2025, 15:24 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 4 files +81 −0

Documentation/config/maintenance.adoc, Documentation/git-maintenance.adoc, builtin/gc.c, t/t7900-maintenance.sh

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
Ramsay Jones· Feb 26, 2025, 17:50 UTC · re: Patrick Steinhardt · lore

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

On 26/02/2025 15:24, Patrick Steinhardt wrote:
Show 10 quoted lines
> 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

Junio C Hamano· Feb 26, 2025, 18:40 UTC · re: Ramsay Jones · lore

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

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 ;-)
Ramsay Jones· Feb 26, 2025, 18:54 UTC · re: Junio C Hamano · lore

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

On 26/02/2025 18:40, Junio C Hamano wrote:
Show 8 quoted lines
> 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

Patrick Steinhardt· Feb 27, 2025, 09:10 UTC · re: Ramsay Jones · lore

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

On Wed, Feb 26, 2025 at 06:54:48PM +0000, Ramsay Jones wrote:
Show 11 quoted lines
> 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
Junio C Hamano· Feb 27, 2025, 01:23 UTC · re: Patrick Steinhardt · lore

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

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.

Patrick Steinhardt· Feb 27, 2025, 09:22 UTC · re: Junio C Hamano · lore

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

On Wed, Feb 26, 2025 at 05:23:10PM -0800, Junio C Hamano wrote:
Show 17 quoted lines
> 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.

Show 6 quoted lines
> 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
Junio C Hamano· Feb 27, 2025, 17:01 UTC · re: Patrick Steinhardt · lore

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

Patrick Steinhardt <ps@pks.im> writes:
Show 21 quoted lines
> 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).

Patrick Steinhardt· Feb 28, 2025, 08:35 UTC · re: Junio C Hamano · lore

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

On Thu, Feb 27, 2025 at 09:01:49AM -0800, Junio C Hamano wrote:
Show 29 quoted lines
> 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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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

Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 2 files +13 −15

builtin/reflog.c, reflog.h

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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 3 files +36 −36

builtin/reflog.c, reflog.c, reflog.h

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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 2 files +20 −18

builtin/reflog.c, reflog.h

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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to 3 files +128 −111

builtin/reflog.c, reflog.c, reflog.h

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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(-)
Show changes to builtin/gc.c +11 −11
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
Patrick Steinhardt· Apr 8, 2025, 06:22 UTC · re: Patrick Steinhardt · lore

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

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(+)
Show changes to 4 files +81 −0

Documentation/config/maintenance.adoc, Documentation/git-maintenance.adoc, builtin/gc.c, t/t7900-maintenance.sh

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

← back to recent threads