git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Justin Tobler <jltobler@gmail.com>
Date
Mar 4, 2025, 23:41 UTC
Message-ID
<usf4pzrp322xvfrvsasgy22rnw7d2a6krzuctwpp23ppdgh6ji@ram73wv5dn54>
In-Reply-To
<20250226-pks-maintenance-reflog-expire-v1-3-a1204a814952@pks.im>
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
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 6 of 26 in “builtin/maintenance: introduce "reflog-expire" task”
  1. 0/6 builtin/maintenance: introduce "reflog-expire" taskPatrick Steinhardt, Feb 26, 2025
  2. 1/6 reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`Patrick Steinhardt, Feb 26, 2025
  3. 2/6 builtin/reflog: stop storing default reflog expiry dates globallyPatrick Steinhardt, Feb 26, 2025
  4. Justin ToblerMar 4, 2025
  5. 3/6 builtin/reflog: stop storing per-reflog expiry dates globallyPatrick Steinhardt, Feb 26, 2025
  6. Justin ToblerMar 4, 2025
  7. Patrick SteinhardtMar 6, 2025
  8. Justin ToblerMar 6, 2025
  9. 4/6 builtin/reflog: make functions regarding `reflog_expire_options` publicPatrick Steinhardt, Feb 26, 2025
  10. 5/6 builtin/gc: split out function to expire reflog entriesPatrick Steinhardt, Feb 26, 2025
  11. 6/6 builtin/maintenance: introduce "reflog-expire" taskPatrick Steinhardt, Feb 26, 2025
  12. Ramsay JonesFeb 26, 2025
  13. Junio C HamanoFeb 26, 2025
  14. Ramsay JonesFeb 26, 2025
  15. Patrick SteinhardtFeb 27, 2025
  16. Junio C HamanoFeb 27, 2025
  17. Patrick SteinhardtFeb 27, 2025
  18. Junio C HamanoFeb 27, 2025
  19. Patrick SteinhardtFeb 28, 2025
  20. 0/6 builtin/maintenance: introduce "reflog-expire" taskPatrick Steinhardt, Apr 8, 2025
  21. 2/6 builtin/reflog: stop storing default reflog expiry dates globallyPatrick Steinhardt, Apr 8, 2025
  22. 1/6 reflog: rename `cmd_reflog_expire_cb` to `reflog_expire_options`Patrick Steinhardt, Apr 8, 2025
  23. 3/6 builtin/reflog: stop storing per-reflog expiry dates globallyPatrick Steinhardt, Apr 8, 2025
  24. 4/6 builtin/reflog: make functions regarding `reflog_expire_options` publicPatrick Steinhardt, Apr 8, 2025
  25. 5/6 builtin/gc: split out function to expire reflog entriesPatrick Steinhardt, Apr 8, 2025
  26. 6/6 builtin/maintenance: introduce "reflog-expire" taskPatrick Steinhardt, Apr 8, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.