From: Patrick Steinhardt Date: Mon, 26 Oct 2026 07:28:21 GMT Subject: Re: [PATCH 7/7] rerere: get rid of global rerere cache state Message-ID: In-Reply-To: On Fri, Oct 23, 2026 at 10:04:57AM -0700, Junio C Hamano wrote: > Patrick Steinhardt writes: > > > The rerere cache state is tracked via a global variable. On the one hand > > this ties it to one specific repository. On the other hand it has the > > consequence that the cache may be leaking state across multiple calls > > into the rerere subsystem. > > > > Refactor the subsystem to instead pass around the rerere cache state > > explicitly and make callers responsible for setting it up and then > > clearing it again. > > This step is a logical conclusion of what was done in the previous > step. I very much like it. > > I was somehow hoping to see that an earlier addition of "enabled" > bit to the "struct repository" would be undone and replaced with > this structure, but that may not be feasible, I guess. I guess you could in theory, but it felt a bit cleaner to me to cache this value in the repository itself. Caching whether or not there even is a rerere cache in the `struct rerere_cache` itself just didn't feel right to me. We could also just not bother caching this value at all. As I mention in the commit message, we don't really have any code paths where this logic gets called in a hot loop or something like that. So we could instead just look up "rerere.enabled" every single time you call `rerere_cache_setup()`, and nobody would really care. But we also check for the existence of ".git/rr-cache", adding another stat(3p) call every time this function is called, and that just feels wrong to me. It's probably premature optimization though. So if you agree with that conclusion we could adapt similar to the below patch. Patrick diff --git a/repository.c b/repository.c index f51699a6dd9..e584d6c4054 100644 --- a/repository.c +++ b/repository.c @@ -76,7 +76,6 @@ void initialize_repository(struct repository *repo) repo->bare_cfg = -1; repo->fetch_if_missing = 1; repo_config_values_init(&repo->config_values_private_); - repo->rerere_cache_enabled = -1; /* * When a command runs inside a repository, it learns what diff --git a/repository.h b/repository.h index 86795b6946c..91b1f57db7b 100644 --- a/repository.h +++ b/repository.h @@ -185,12 +185,6 @@ struct repository { /* True if commit-graph has been disabled within this process. */ int commit_graph_disabled; - /* - * Indicates whether the rerere-cache has been enabled. -1 (the default - * value) causes us to perform auto-detection via ".git/rr-cache". - */ - int rerere_cache_enabled; - /* * Controls whether the repository should lazily fetch missing * objects from promisor remotes. Defaults to 1. diff --git a/rerere.c b/rerere.c index e0897612085..58bf298ae00 100644 --- a/rerere.c +++ b/rerere.c @@ -878,31 +878,29 @@ static int do_plain_rerere(struct rerere_cache *cache, static int is_rerere_enabled(struct repository *r) { int rr_cache_exists; + int enabled = -1; char *dir; - if (r->rerere_cache_enabled != -1) - return r->rerere_cache_enabled; - - repo_config_get_bool(r, "rerere.enabled", &r->rerere_cache_enabled); - if (!r->rerere_cache_enabled) + repo_config_get_bool(r, "rerere.enabled", &enabled); + if (!enabled) return 0; dir = repo_git_path(r, "rr-cache"); rr_cache_exists = is_directory(dir); - if (r->rerere_cache_enabled < 0) { - r->rerere_cache_enabled = rr_cache_exists; + if (enabled < 0) { + enabled = rr_cache_exists; goto out; } if (!rr_cache_exists && safe_create_dir_in_gitdir(r, dir)) die(_("could not create directory '%s'"), dir); - r->rerere_cache_enabled = 1; + enabled = 1; out: free(dir); - return r->rerere_cache_enabled; + return enabled; } int rerere_cache_setup(struct rerere_cache *cache,