* [PATCH 1/7] rerere: stop using `the_repository` in `rerere_path()`
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 2/7] rerere: stop using `the_repository` in internal helpers Patrick Steinhardt
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
The function `rerere_path()` computes paths of rerere cache entries
contained in the "rr-cache" directory of `the_repository`. This makes us
implicitly depend on global state.
Add a repository parameter so that callers can specify which repository
to compute the path for. Adjust the internal helpers that call it to
pass down the repository they already have available in their contexts.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rerere.c | 6 +++--
rerere.c | 82 +++++++++++++++++++++++++++++---------------------------
rerere.h | 3 ++-
3 files changed, 48 insertions(+), 43 deletions(-)
diff --git a/builtin/rerere.c b/builtin/rerere.c
index ca69505283..e900e302c3 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -129,8 +129,10 @@ int cmd_rerere(int argc,
for (size_t i = 0; i < merge_rr.nr; i++) {
const char *path = merge_rr.items[i].string;
const struct rerere_id *id = merge_rr.items[i].util;
- if (diff_two(rerere_path(&buf, id, "preimage"), path, path, path))
- die(_("unable to generate diff for '%s'"), rerere_path(&buf, id, NULL));
+ if (diff_two(rerere_path(the_repository, &buf, id, "preimage"),
+ path, path, path))
+ die(_("unable to generate diff for '%s'"),
+ rerere_path(the_repository, &buf, id, NULL));
}
strbuf_release(&buf);
diff --git a/rerere.c b/rerere.c
index 7fc32f2cf0..356111838c 100644
--- a/rerere.c
+++ b/rerere.c
@@ -95,17 +95,18 @@ static void assign_variant(struct rerere_id *id)
id->variant = variant;
}
-const char *rerere_path(struct strbuf *buf, const struct rerere_id *id, const char *file)
+const char *rerere_path(struct repository *repo,
+ struct strbuf *buf, const struct rerere_id *id, const char *file)
{
if (!file)
- return repo_git_path_replace(the_repository, buf, "rr-cache/%s",
+ return repo_git_path_replace(repo, buf, "rr-cache/%s",
rerere_id_hex(id));
if (id->variant <= 0)
- return repo_git_path_replace(the_repository, buf, "rr-cache/%s/%s",
+ return repo_git_path_replace(repo, buf, "rr-cache/%s/%s",
rerere_id_hex(id), file);
- return repo_git_path_replace(the_repository, buf, "rr-cache/%s/%s.%d",
+ return repo_git_path_replace(repo, buf, "rr-cache/%s/%s.%d",
rerere_id_hex(id), file, id->variant);
}
@@ -609,8 +610,8 @@ static int try_merge(struct index_state *istate,
mmfile_t base = {NULL, 0}, other = {NULL, 0};
struct strbuf buf = STRBUF_INIT;
- if (read_mmfile(&base, rerere_path(&buf, id, "preimage")) ||
- read_mmfile(&other, rerere_path(&buf, id, "postimage"))) {
+ if (read_mmfile(&base, rerere_path(istate->repo, &buf, id, "preimage")) ||
+ read_mmfile(&other, rerere_path(istate->repo, &buf, id, "postimage"))) {
ret = LL_MERGE_CONFLICT;
} else {
/*
@@ -650,8 +651,8 @@ static int merge(struct index_state *istate, const struct rerere_id *id, const c
* Normalize the conflicts in path and write it out to
* "thisimage" temporary file.
*/
- if ((handle_file(istate, path, NULL, rerere_path(&buf, id, "thisimage")) < 0) ||
- read_mmfile(&cur, rerere_path(&buf, id, "thisimage"))) {
+ if ((handle_file(istate, path, NULL, rerere_path(istate->repo, &buf, id, "thisimage")) < 0) ||
+ read_mmfile(&cur, rerere_path(istate->repo, &buf, id, "thisimage"))) {
ret = 1;
goto out;
}
@@ -664,9 +665,9 @@ static int merge(struct index_state *istate, const struct rerere_id *id, const c
* A successful replay of recorded resolution.
* Mark that "postimage" was used to help gc.
*/
- if (utime(rerere_path(&buf, id, "postimage"), NULL) < 0)
+ if (utime(rerere_path(istate->repo, &buf, id, "postimage"), NULL) < 0)
warning_errno(_("failed utime() on '%s'"),
- rerere_path(&buf, id, "postimage"));
+ rerere_path(istate->repo, &buf, id, "postimage"));
/* Update "path" with the resolution */
f = fopen(path, "w");
@@ -705,11 +706,11 @@ static void update_paths(struct repository *r, struct string_list *update)
die(_("unable to write new index file"));
}
-static void remove_variant(struct rerere_id *id)
+static void remove_variant(struct repository *r, struct rerere_id *id)
{
struct strbuf buf = STRBUF_INIT;
- unlink_or_warn(rerere_path(&buf, id, "postimage"));
- unlink_or_warn(rerere_path(&buf, id, "preimage"));
+ unlink_or_warn(rerere_path(r, &buf, id, "postimage"));
+ unlink_or_warn(rerere_path(r, &buf, id, "preimage"));
id->collection->status[id->variant] = 0;
strbuf_release(&buf);
}
@@ -743,7 +744,7 @@ static void do_rerere_one_path(struct index_state *istate,
const int had_postimage =
id->collection->status[variant] & RR_HAS_POSTIMAGE;
const char *postimage =
- rerere_path(&buf, id, "postimage");
+ rerere_path(istate->repo, &buf, id, "postimage");
if (copy_file(the_repository,
postimage,
@@ -784,7 +785,7 @@ static void do_rerere_one_path(struct index_state *istate,
* cleanly, there is no point maintaining our own variant.
*/
if (0 <= id->variant && id->variant != variant)
- remove_variant(id);
+ remove_variant(istate->repo, id);
if (rerere_autoupdate)
string_list_insert(update, path);
@@ -802,10 +803,10 @@ static void do_rerere_one_path(struct index_state *istate,
variant = id->variant;
if (handle_file(istate, path, NULL,
- rerere_path(&buf, id, "preimage")) < 0)
+ rerere_path(istate->repo, &buf, id, "preimage")) < 0)
goto out;
if (id->collection->status[variant] & RR_HAS_POSTIMAGE) {
- const char *path = rerere_path(&buf, id, "postimage");
+ const char *path = rerere_path(istate->repo, &buf, id, "postimage");
if (unlink(path))
die_errno(_("cannot unlink stray '%s'"), path);
id->collection->status[variant] &= ~RR_HAS_POSTIMAGE;
@@ -846,7 +847,7 @@ static int do_plain_rerere(struct repository *r,
*/
ret = handle_file(r->index, path, hash, NULL);
if (ret != 0 && string_list_has_string(rr, path)) {
- remove_variant(string_list_lookup(rr, path)->util);
+ remove_variant(r, string_list_lookup(rr, path)->util);
string_list_remove(rr, path, 1);
}
if (ret < 1)
@@ -856,7 +857,7 @@ static int do_plain_rerere(struct repository *r,
string_list_insert(rr, path)->util = id;
/* Ensure that the directory exists. */
- safe_create_dir_in_gitdir(the_repository, rerere_path(&buf, id, NULL));
+ safe_create_dir_in_gitdir(the_repository, rerere_path(r, &buf, id, NULL));
}
for (i = 0; i < rr->nr; i++)
@@ -1091,8 +1092,8 @@ static int rerere_forget_one_path(struct index_state *istate,
if (!has_rerere_resolution(id))
continue;
- handle_cache(istate, path, hash, rerere_path(&buf, id, "thisimage"));
- if (read_mmfile(&cur, rerere_path(&buf, id, "thisimage"))) {
+ handle_cache(istate, path, hash, rerere_path(istate->repo, &buf, id, "thisimage"));
+ if (read_mmfile(&cur, rerere_path(istate->repo, &buf, id, "thisimage"))) {
error(_("failed to update conflicted state in '%s'"), path);
goto fail_exit;
}
@@ -1108,7 +1109,7 @@ static int rerere_forget_one_path(struct index_state *istate,
goto fail_exit;
}
- filename = rerere_path(&buf, id, "postimage");
+ filename = rerere_path(istate->repo, &buf, id, "postimage");
if (unlink(filename)) {
if (errno == ENOENT)
error(_("no remembered resolution for '%s'"), path);
@@ -1122,7 +1123,7 @@ static int rerere_forget_one_path(struct index_state *istate,
* conflict in the working tree, run us again to record
* the postimage.
*/
- handle_cache(istate, path, hash, rerere_path(&buf, id, "preimage"));
+ handle_cache(istate, path, hash, rerere_path(istate->repo, &buf, id, "preimage"));
fprintf_ln(stderr, _("Updated preimage for '%s'"), path);
/*
@@ -1181,25 +1182,25 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)
* Garbage collection support
*/
-static timestamp_t rerere_created_at(struct rerere_id *id)
+static timestamp_t rerere_created_at(struct repository *r, struct rerere_id *id)
{
struct strbuf buf = STRBUF_INIT;
struct stat st;
timestamp_t ret;
- ret = stat(rerere_path(&buf, id, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
+ ret = stat(rerere_path(r, &buf, id, "preimage"), &st) ? (time_t) 0 : st.st_mtime;
strbuf_release(&buf);
return ret;
}
-static timestamp_t rerere_last_used_at(struct rerere_id *id)
+static timestamp_t rerere_last_used_at(struct repository *r, struct rerere_id *id)
{
struct strbuf buf = STRBUF_INIT;
struct stat st;
timestamp_t ret;
- ret = stat(rerere_path(&buf, id, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
+ ret = stat(rerere_path(r, &buf, id, "postimage"), &st) ? (time_t) 0 : st.st_mtime;
strbuf_release(&buf);
return ret;
@@ -1208,11 +1209,11 @@ static timestamp_t rerere_last_used_at(struct rerere_id *id)
/*
* Remove the recorded resolution for a given conflict ID
*/
-static void unlink_rr_item(struct rerere_id *id)
+static void unlink_rr_item(struct repository *r, struct rerere_id *id)
{
struct strbuf buf = STRBUF_INIT;
- unlink_or_warn(rerere_path(&buf, id, "thisimage"));
- remove_variant(id);
+ unlink_or_warn(rerere_path(r, &buf, id, "thisimage"));
+ remove_variant(r, id);
id->collection->status[id->variant] = 0;
strbuf_release(&buf);
}
@@ -1231,18 +1232,19 @@ static void rerere_gc_cutoffs(struct repository *r,
*cutoff_noresolve = now - 15 * 86400;
}
-static bool rerere_id_is_stale(struct rerere_id *id,
+static bool rerere_id_is_stale(struct repository *r,
+ struct rerere_id *id,
timestamp_t cutoff_resolve,
timestamp_t cutoff_noresolve)
{
timestamp_t then;
timestamp_t cutoff;
- then = rerere_last_used_at(id);
+ then = rerere_last_used_at(r, id);
if (then) {
cutoff = cutoff_resolve;
} else {
- then = rerere_created_at(id);
+ then = rerere_created_at(r, id);
if (!then)
return false;
cutoff = cutoff_noresolve;
@@ -1251,11 +1253,11 @@ static bool rerere_id_is_stale(struct rerere_id *id,
return then < cutoff;
}
-static void prune_one(struct rerere_id *id,
+static void prune_one(struct repository *r, struct rerere_id *id,
timestamp_t cutoff_resolve, timestamp_t cutoff_noresolve)
{
- if (rerere_id_is_stale(id, cutoff_resolve, cutoff_noresolve))
- unlink_rr_item(id);
+ if (rerere_id_is_stale(r, id, cutoff_resolve, cutoff_noresolve))
+ unlink_rr_item(r, id);
}
/* Does the basename in "path" look plausibly like an rr-cache entry? */
@@ -1297,7 +1299,7 @@ bool rerere_gc_needed(struct repository *r, size_t limit)
for (id.variant = 0;
id.variant < id.collection->status_nr;
id.variant++) {
- if (rerere_id_is_stale(&id, cutoff_resolve,
+ if (rerere_id_is_stale(r, &id, cutoff_resolve,
cutoff_noresolve)) {
count += 256;
if (count >= limit) {
@@ -1349,7 +1351,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
for (id.variant = 0, id.collection = rr_dir;
id.variant < id.collection->status_nr;
id.variant++) {
- prune_one(&id, cutoff_resolve, cutoff_noresolve);
+ prune_one(r, &id, cutoff_resolve, cutoff_noresolve);
if (id.collection->status[id.variant])
now_empty = 0;
}
@@ -1387,8 +1389,8 @@ void rerere_clear(struct repository *r, struct string_list *merge_rr)
struct strbuf buf = STRBUF_INIT;
if (!has_rerere_resolution(id)) {
- unlink_rr_item(id);
- rmdir(rerere_path(&buf, id, NULL));
+ unlink_rr_item(r, id);
+ rmdir(rerere_path(r, &buf, id, NULL));
}
strbuf_release(&buf);
diff --git a/rerere.h b/rerere.h
index 1182ee3fb1..ee8d97917d 100644
--- a/rerere.h
+++ b/rerere.h
@@ -34,7 +34,8 @@ int repo_rerere(struct repository *, int);
* path to that filesystem entity. With "file" specified with NULL,
* return the path to the directory that houses these files.
*/
-const char *rerere_path(struct strbuf *buf, const struct rerere_id *,
+const char *rerere_path(struct repository *,
+ struct strbuf *buf, const struct rerere_id *,
const char *file);
int rerere_forget(struct repository *, struct pathspec *);
int rerere_remaining(struct repository *, struct string_list *);
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/7] rerere: stop using `the_repository` in internal helpers
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 1/7] rerere: stop using `the_repository` in `rerere_path()` Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 3/7] rerere: remove redundant calls to `repo_config()` Patrick Steinhardt
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
Most of the static helpers in "rerere.c" still rely on `the_repository`
even though their callers already have a `struct repository` available.
Thread the repository through those helpers so that we can get rid of
references to that global variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
rerere.c | 69 ++++++++++++++++++++++++++++++++--------------------------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/rerere.c b/rerere.c
index 356111838c..c568bd5e89 100644
--- a/rerere.c
+++ b/rerere.c
@@ -129,13 +129,13 @@ static int is_rr_file(const char *name, const char *filename, int *variant)
return 1;
}
-static void scan_rerere_dir(struct rerere_dir *rr_dir)
+static void scan_rerere_dir(struct repository *r, struct rerere_dir *rr_dir)
{
struct dirent *de;
char *path;
DIR *dir;
- path = repo_git_path(the_repository, "rr-cache/%s", rr_dir->name);
+ path = repo_git_path(r, "rr-cache/%s", rr_dir->name);
dir = opendir(path);
free(path);
if (!dir)
@@ -154,7 +154,7 @@ static void scan_rerere_dir(struct rerere_dir *rr_dir)
closedir(dir);
}
-static struct rerere_dir *find_rerere_dir(const char *hex)
+static struct rerere_dir *find_rerere_dir(struct repository *r, const char *hex)
{
struct rerere_dir *rr_dir;
@@ -166,7 +166,7 @@ static struct rerere_dir *find_rerere_dir(const char *hex)
rr_dir->status_alloc = 0;
strmap_put(&rerere_dirs, hex, rr_dir);
- scan_rerere_dir(rr_dir);
+ scan_rerere_dir(r, rr_dir);
}
return rr_dir;
}
@@ -181,17 +181,17 @@ static int has_rerere_resolution(const struct rerere_id *id)
return ((id->collection->status[variant] & both) == both);
}
-static struct rerere_id *new_rerere_id_hex(char *hex)
+static struct rerere_id *new_rerere_id_hex(struct repository *r, char *hex)
{
struct rerere_id *id = xmalloc(sizeof(*id));
- id->collection = find_rerere_dir(hex);
+ id->collection = find_rerere_dir(r, hex);
id->variant = -1; /* not known yet */
return id;
}
-static struct rerere_id *new_rerere_id(unsigned char *hash)
+static struct rerere_id *new_rerere_id(struct repository *r, unsigned char *hash)
{
- return new_rerere_id_hex(hash_to_hex(hash));
+ return new_rerere_id_hex(r, hash_to_hex(hash));
}
/*
@@ -213,7 +213,7 @@ static void read_rr(struct repository *r, struct string_list *rr)
unsigned char hash[GIT_MAX_RAWSZ];
struct rerere_id *id;
int variant;
- const unsigned hexsz = the_hash_algo->hexsz;
+ const unsigned hexsz = r->hash_algo->hexsz;
/* There has to be the hash, tab, path and then NUL */
if (buf.len < hexsz + 2 || get_hash_hex(buf.buf, hash))
@@ -231,7 +231,7 @@ static void read_rr(struct repository *r, struct string_list *rr)
if (*(path++) != '\t')
die(_("corrupt MERGE_RR"));
buf.buf[hexsz] = '\0';
- id = new_rerere_id_hex(buf.buf);
+ id = new_rerere_id_hex(r, buf.buf);
id->variant = variant;
/*
* make sure id->collection->status has enough space
@@ -411,13 +411,14 @@ static int handle_conflict(struct strbuf *out, struct rerere_io *io,
* Return 1 if conflict hunks are found, 0 if there are no conflict
* hunks and -1 if an error occurred.
*/
-static int handle_path(unsigned char *hash, struct rerere_io *io, int marker_size)
+static int handle_path(struct repository *r,
+ unsigned char *hash, struct rerere_io *io, int marker_size)
{
struct git_hash_ctx ctx;
struct strbuf buf = STRBUF_INIT, out = STRBUF_INIT;
int has_conflicts = 0;
if (hash)
- git_hash_init(&ctx, the_hash_algo);
+ git_hash_init(&ctx, r->hash_algo);
while (!io->getline(&buf, io)) {
if (is_conflict_marker_line(buf.buf, buf.len, marker_size) == '<') {
@@ -466,7 +467,7 @@ static int handle_file(struct index_state *istate,
}
}
- has_conflicts = handle_path(hash, (struct rerere_io *)&io, marker_size);
+ has_conflicts = handle_path(istate->repo, hash, (struct rerere_io *)&io, marker_size);
fclose(io.input);
if (io.io.wrerror)
@@ -746,7 +747,7 @@ static void do_rerere_one_path(struct index_state *istate,
const char *postimage =
rerere_path(istate->repo, &buf, id, "postimage");
- if (copy_file(the_repository,
+ if (copy_file(istate->repo,
postimage,
path, 0666)) {
if (!had_postimage)
@@ -853,11 +854,11 @@ static int do_plain_rerere(struct repository *r,
if (ret < 1)
continue;
- id = new_rerere_id(hash);
+ id = new_rerere_id(r, hash);
string_list_insert(rr, path)->util = id;
/* Ensure that the directory exists. */
- safe_create_dir_in_gitdir(the_repository, rerere_path(r, &buf, id, NULL));
+ safe_create_dir_in_gitdir(r, rerere_path(r, &buf, id, NULL));
}
for (i = 0; i < rr->nr; i++)
@@ -872,18 +873,17 @@ static int do_plain_rerere(struct repository *r,
return write_rr(rr, fd);
}
-static void git_rerere_config(void)
+static void git_rerere_config(struct repository *r)
{
- repo_config_get_bool(the_repository, "rerere.enabled", &rerere_enabled);
- repo_config_get_bool(the_repository, "rerere.autoupdate", &rerere_autoupdate);
- repo_config_get_int(the_repository, "rerere.locktimeout",
- &rerere_lock_timeout_ms);
- repo_config(the_repository, git_default_config, NULL);
+ repo_config_get_bool(r, "rerere.enabled", &rerere_enabled);
+ repo_config_get_bool(r, "rerere.autoupdate", &rerere_autoupdate);
+ repo_config_get_int(r, "rerere.locktimeout", &rerere_lock_timeout_ms);
+ repo_config(r, git_default_config, NULL);
}
static GIT_PATH_FUNC(git_path_rr_cache, "rr-cache")
-static int is_rerere_enabled(void)
+static int is_rerere_enabled(struct repository *r)
{
int rr_cache_exists;
@@ -895,7 +895,7 @@ static int is_rerere_enabled(void)
return rr_cache_exists;
if (!rr_cache_exists &&
- safe_create_dir_in_gitdir(the_repository, git_path_rr_cache()))
+ safe_create_dir_in_gitdir(r, git_path_rr_cache()))
die(_("could not create directory '%s'"), git_path_rr_cache());
return 1;
}
@@ -904,8 +904,8 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
{
int fd;
- git_rerere_config();
- if (!is_rerere_enabled())
+ git_rerere_config(r);
+ if (!is_rerere_enabled(r))
return -1;
if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
@@ -1017,7 +1017,7 @@ static int handle_cache(struct index_state *istate,
break;
i = ce_stage(ce) - 1;
if (!mmfile[i].ptr) {
- mmfile[i].ptr = odb_read_object(the_repository->objects,
+ mmfile[i].ptr = odb_read_object(istate->repo->objects,
&ce->oid, &type, &size);
if (!mmfile[i].ptr)
die(_("unable to read %s"),
@@ -1053,7 +1053,8 @@ static int handle_cache(struct index_state *istate,
* Grab the conflict ID and optionally write the original
* contents with conflict markers out.
*/
- has_conflicts = handle_path(hash, (struct rerere_io *)&io, marker_size);
+ has_conflicts = handle_path(istate->repo, hash,
+ (struct rerere_io *)&io, marker_size);
strbuf_release(&io.input);
if (io.io.output)
fclose(io.io.output);
@@ -1080,7 +1081,7 @@ static int rerere_forget_one_path(struct index_state *istate,
return error(_("could not parse conflict hunks in '%s'"), path);
/* Nuke the recorded resolution for the conflict */
- id = new_rerere_id(hash);
+ id = new_rerere_id(istate->repo, hash);
for (id->variant = 0;
id->variant < id->collection->status_nr;
@@ -1295,7 +1296,7 @@ bool rerere_gc_needed(struct repository *r, size_t limit)
!is_rr_cache_dirname(e->d_name))
continue;
- id.collection = find_rerere_dir(e->d_name);
+ id.collection = find_rerere_dir(r, e->d_name);
for (id.variant = 0;
id.variant < id.collection->status_nr;
id.variant++) {
@@ -1332,8 +1333,8 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
return;
rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
- repo_config(the_repository, git_default_config, NULL);
- dir = opendir(repo_git_path_replace(the_repository, &buf, "rr-cache"));
+ repo_config(r, git_default_config, NULL);
+ dir = opendir(repo_git_path_replace(r, &buf, "rr-cache"));
if (!dir)
die_errno(_("unable to open rr-cache directory"));
/* Collect stale conflict IDs ... */
@@ -1345,7 +1346,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
if (!is_rr_cache_dirname(e->d_name))
continue; /* or should we remove e->d_name? */
- rr_dir = find_rerere_dir(e->d_name);
+ rr_dir = find_rerere_dir(r, e->d_name);
now_empty = 1;
for (id.variant = 0, id.collection = rr_dir;
@@ -1362,7 +1363,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
/* ... and then remove the empty directories */
for (i = 0; i < to_remove.nr; i++)
- rmdir(repo_git_path_replace(the_repository, &buf,
+ rmdir(repo_git_path_replace(r, &buf,
"rr-cache/%s", to_remove.items[i].string));
string_list_clear(&to_remove, 0);
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/7] rerere: remove redundant calls to `repo_config()`
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 1/7] rerere: stop using `the_repository` in `rerere_path()` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 2/7] rerere: stop using `the_repository` in internal helpers Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 4/7] rerere: cache result of `is_rerere_enabled()` Patrick Steinhardt
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
When setting up the rerere cache we eventually end up calling
`repo_config()`. This call is redundant though, as all callers of
`setup_rerere()` already called `repo_config()` beforehand.
Next to being redundant though it also feels like a layering violation.
The intent of that function is to set up state for the rerere cache, but
we potentially end up modifying other global state, too. This is usually
fine, but it is surprising to say the very least. If a caller for
example were to call `repo_config()` and then modifies the parsed config
manually then those modifications would be lost when calling
`setup_rerere()`.
Despite the layering violation though it will also create problems with
a subsequent commit, where we are about to cache whether or not the
rerere cache is enabled. As part of that change we would stop calling
`repo_config()` when the rerere cache is disabled, but continue to call
it when it's enabled. So the above layering violation is essentially
becoming even worse, because we may or may not clobber config values.
Drop the call, as well as another redundant call in `rerere_gc()`.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
rerere.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/rerere.c b/rerere.c
index c568bd5e89..56b6ef547b 100644
--- a/rerere.c
+++ b/rerere.c
@@ -878,7 +878,6 @@ static void git_rerere_config(struct repository *r)
repo_config_get_bool(r, "rerere.enabled", &rerere_enabled);
repo_config_get_bool(r, "rerere.autoupdate", &rerere_autoupdate);
repo_config_get_int(r, "rerere.locktimeout", &rerere_lock_timeout_ms);
- repo_config(r, git_default_config, NULL);
}
static GIT_PATH_FUNC(git_path_rr_cache, "rr-cache")
@@ -1333,7 +1332,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
return;
rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
- repo_config(r, git_default_config, NULL);
+
dir = opendir(repo_git_path_replace(r, &buf, "rr-cache"));
if (!dir)
die_errno(_("unable to open rr-cache directory"));
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/7] rerere: cache result of `is_rerere_enabled()`
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
` (2 preceding siblings ...)
2026-10-23 12:50 ` [PATCH 3/7] rerere: remove redundant calls to `repo_config()` Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 5/7] rerere: stop using `the_repository` Patrick Steinhardt
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
The function `setup_rerere()` reads the rerere configuration and then
figures out whether the rerere cache is enabled after all, either
because it has been explicitly enabled or because it's implicitly
enabled and ".git/rr-cache" exists. Whether the cache is enabled will
not change in a single process though, so it does not make sense to
reevaluate these conditions every single time `setup_rerere()` is
called.
Adapt the function to instead cache the status of the rerere cache via
a new `struct repository::rerere_cache_enabled` member. This allows us
to get rid of the `GIT_PATH_FUNC()`, which caches the path to the rerere
cache directory. But now that we already cache the enabled state in the
first place it becomes moot to cache the path, as well.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
repository.c | 1 +
repository.h | 6 ++++++
rerere.c | 43 +++++++++++++++++++++++--------------------
3 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/repository.c b/repository.c
index e584d6c405..f51699a6dd 100644
--- a/repository.c
+++ b/repository.c
@@ -76,6 +76,7 @@ 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 91b1f57db7..86795b6946 100644
--- a/repository.h
+++ b/repository.h
@@ -185,6 +185,12 @@ 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 56b6ef547b..8fa921fc8f 100644
--- a/rerere.c
+++ b/rerere.c
@@ -27,9 +27,6 @@
#define THREE_STAGED 2
void *RERERE_RESOLVED = &RERERE_RESOLVED;
-/* if rerere_enabled == -1, fall back to detection of .git/rr-cache */
-static int rerere_enabled = -1;
-
/* automatically update cleanly resolved paths to the index */
static int rerere_autoupdate;
@@ -873,40 +870,46 @@ static int do_plain_rerere(struct repository *r,
return write_rr(rr, fd);
}
-static void git_rerere_config(struct repository *r)
-{
- repo_config_get_bool(r, "rerere.enabled", &rerere_enabled);
- repo_config_get_bool(r, "rerere.autoupdate", &rerere_autoupdate);
- repo_config_get_int(r, "rerere.locktimeout", &rerere_lock_timeout_ms);
-}
-
-static GIT_PATH_FUNC(git_path_rr_cache, "rr-cache")
-
static int is_rerere_enabled(struct repository *r)
{
int rr_cache_exists;
+ char *dir;
- if (!rerere_enabled)
+ 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)
return 0;
- rr_cache_exists = is_directory(git_path_rr_cache());
- if (rerere_enabled < 0)
- return rr_cache_exists;
+ 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;
+ goto out;
+ }
if (!rr_cache_exists &&
- safe_create_dir_in_gitdir(r, git_path_rr_cache()))
- die(_("could not create directory '%s'"), git_path_rr_cache());
- return 1;
+ safe_create_dir_in_gitdir(r, dir))
+ die(_("could not create directory '%s'"), dir);
+ r->rerere_cache_enabled = 1;
+
+out:
+ free(dir);
+ return r->rerere_cache_enabled;
}
int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
{
int fd;
- git_rerere_config(r);
if (!is_rerere_enabled(r))
return -1;
+ repo_config_get_bool(r, "rerere.autoupdate", &rerere_autoupdate);
+ repo_config_get_int(r, "rerere.locktimeout", &rerere_lock_timeout_ms);
+
if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);
if ((flags & RERERE_READONLY) && (flags & RERERE_NOWAIT))
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/7] rerere: stop using `the_repository`
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
` (3 preceding siblings ...)
2026-10-23 12:50 ` [PATCH 4/7] rerere: cache result of `is_rerere_enabled()` Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 6/7] rerere: bundle global state into a struct Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 7/7] rerere: get rid of global rerere cache state Patrick Steinhardt
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
There's only a couple of trivial function calls left that implicitly
make us depend on `the_repository`. Refactor those to instead use their
variants that allow us to make the dependency explicit.
Note that this requires us to make `get_hash_hex_algop()` public.
With these changes, "rerere.c" no longer depends on `the_repository`, so
drop `USE_THE_REPOSITORY_VARIABLE`. Despite that, we still carry some
global state in this file that will tie a specific rerere cache to the
first repository used to populate it. This will be fixed in subsequent
commits.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
hex.c | 4 ++--
hex.h | 3 +++
rerere.c | 14 +++++++-------
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/hex.c b/hex.c
index f02832140d..4b7793bb8a 100644
--- a/hex.c
+++ b/hex.c
@@ -5,8 +5,8 @@
#include "hex.h"
#include "strbuf.h"
-static int get_hash_hex_algop(const char *hex, unsigned char *hash,
- const struct git_hash_algo *algop)
+int get_hash_hex_algop(const char *hex, unsigned char *hash,
+ const struct git_hash_algo *algop)
{
for (size_t i = 0; i < algop->rawsz; i++) {
int val = hex2chr(hex);
diff --git a/hex.h b/hex.h
index f15c7e2220..50a0088065 100644
--- a/hex.h
+++ b/hex.h
@@ -59,6 +59,9 @@ int parse_oid_hex_algop_impl(const char *hex, struct object_id *oid, const char
int get_oid_hex_any(const char *hex, struct object_id *oid);
int parse_oid_hex_any(const char *hex, struct object_id *oid, const char **end);
+int get_hash_hex_algop(const char *hex, unsigned char *hash,
+ const struct git_hash_algo *algop);
+
#ifdef USE_THE_REPOSITORY_VARIABLE
/* Like get_oid_hex_algop, but for `the_hash_algo`. */
diff --git a/rerere.c b/rerere.c
index 8fa921fc8f..366797ab7e 100644
--- a/rerere.c
+++ b/rerere.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "git-compat-util.h"
@@ -188,7 +187,7 @@ static struct rerere_id *new_rerere_id_hex(struct repository *r, char *hex)
static struct rerere_id *new_rerere_id(struct repository *r, unsigned char *hash)
{
- return new_rerere_id_hex(r, hash_to_hex(hash));
+ return new_rerere_id_hex(r, hash_to_hex_algop(hash, r->hash_algo));
}
/*
@@ -213,7 +212,8 @@ static void read_rr(struct repository *r, struct string_list *rr)
const unsigned hexsz = r->hash_algo->hexsz;
/* There has to be the hash, tab, path and then NUL */
- if (buf.len < hexsz + 2 || get_hash_hex(buf.buf, hash))
+ if (buf.len < hexsz + 2 ||
+ get_hash_hex_algop(buf.buf, hash, r->hash_algo))
die(_("corrupt MERGE_RR"));
if (buf.buf[hexsz] != '.') {
@@ -1264,11 +1264,11 @@ static void prune_one(struct repository *r, struct rerere_id *id,
}
/* Does the basename in "path" look plausibly like an rr-cache entry? */
-static int is_rr_cache_dirname(const char *path)
+static int is_rr_cache_dirname(struct repository *r, const char *path)
{
struct object_id oid;
const char *end;
- return !parse_oid_hex(path, &oid, &end) && !*end;
+ return !parse_oid_hex_algop(path, &oid, &end, r->hash_algo) && !*end;
}
bool rerere_gc_needed(struct repository *r, size_t limit)
@@ -1295,7 +1295,7 @@ bool rerere_gc_needed(struct repository *r, size_t limit)
* use for estimating the number of loose objects.
*/
if (!starts_with(e->d_name, "17") ||
- !is_rr_cache_dirname(e->d_name))
+ !is_rr_cache_dirname(r, e->d_name))
continue;
id.collection = find_rerere_dir(r, e->d_name);
@@ -1345,7 +1345,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
struct rerere_id id;
int now_empty;
- if (!is_rr_cache_dirname(e->d_name))
+ if (!is_rr_cache_dirname(r, e->d_name))
continue; /* or should we remove e->d_name? */
rr_dir = find_rerere_dir(r, e->d_name);
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/7] rerere: bundle global state into a struct
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
` (4 preceding siblings ...)
2026-10-23 12:50 ` [PATCH 5/7] rerere: stop using `the_repository` Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 12:50 ` [PATCH 7/7] rerere: get rid of global rerere cache state Patrick Steinhardt
6 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
The state of the rerere subsystem is tracked across several global
variables. Move them into a struct so that we can easily de-globalize
the state.
Note that the lock timeout is only ever used in `setup_rerere()`. So
instead of moving it into the new struct, we just keep it as a local
variable.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
rerere.c | 44 +++++++++++++++++++-------------------------
rerere.h | 11 +++++++++++
2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/rerere.c b/rerere.c
index 366797ab7e..23bcb8bc31 100644
--- a/rerere.c
+++ b/rerere.c
@@ -21,17 +21,15 @@
#include "odb.h"
#include "strmap.h"
+static struct rerere_cache cache = {
+ .dirs = STRMAP_INIT,
+};
+
#define RESOLVED 0
#define PUNTED 1
#define THREE_STAGED 2
void *RERERE_RESOLVED = &RERERE_RESOLVED;
-/* automatically update cleanly resolved paths to the index */
-static int rerere_autoupdate;
-
-/* how long to wait for MERGE_RR.lock, in milliseconds */
-static int rerere_lock_timeout_ms = 1000;
-
#define RR_HAS_POSTIMAGE 1
#define RR_HAS_PREIMAGE 2
struct rerere_dir {
@@ -40,19 +38,17 @@ struct rerere_dir {
char name[FLEX_ARRAY];
};
-static struct strmap rerere_dirs = STRMAP_INIT;
-
static void free_rerere_dirs(void)
{
struct hashmap_iter iter;
struct strmap_entry *ent;
- strmap_for_each_entry(&rerere_dirs, &iter, ent) {
+ strmap_for_each_entry(&cache.dirs, &iter, ent) {
struct rerere_dir *rr_dir = ent->value;
free(rr_dir->status);
free(rr_dir);
}
- strmap_clear(&rerere_dirs, 0);
+ strmap_clear(&cache.dirs, 0);
}
static void free_rerere_id(struct string_list_item *item)
@@ -154,13 +150,13 @@ static struct rerere_dir *find_rerere_dir(struct repository *r, const char *hex)
{
struct rerere_dir *rr_dir;
- rr_dir = strmap_get(&rerere_dirs, hex);
+ rr_dir = strmap_get(&cache.dirs, hex);
if (!rr_dir) {
FLEX_ALLOC_STR(rr_dir, name, hex);
rr_dir->status = NULL;
rr_dir->status_nr = 0;
rr_dir->status_alloc = 0;
- strmap_put(&rerere_dirs, hex, rr_dir);
+ strmap_put(&cache.dirs, hex, rr_dir);
scan_rerere_dir(r, rr_dir);
}
@@ -241,8 +237,6 @@ static void read_rr(struct repository *r, struct string_list *rr)
fclose(in);
}
-static struct lock_file write_lock;
-
static int write_rr(struct string_list *rr, int out_fd)
{
int i;
@@ -270,7 +264,7 @@ static int write_rr(struct string_list *rr, int out_fd)
strbuf_release(&buf);
}
- if (commit_lock_file(&write_lock) != 0)
+ if (commit_lock_file(&cache.write_lock) != 0)
die(_("unable to write rerere record"));
return 0;
}
@@ -785,7 +779,7 @@ static void do_rerere_one_path(struct index_state *istate,
if (0 <= id->variant && id->variant != variant)
remove_variant(istate->repo, id);
- if (rerere_autoupdate)
+ if (cache.autoupdate)
string_list_insert(update, path);
else
fprintf_ln(stderr,
@@ -902,27 +896,27 @@ static int is_rerere_enabled(struct repository *r)
int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
{
+ int lock_timeout_ms = 1000;
int fd;
if (!is_rerere_enabled(r))
return -1;
- repo_config_get_bool(r, "rerere.autoupdate", &rerere_autoupdate);
- repo_config_get_int(r, "rerere.locktimeout", &rerere_lock_timeout_ms);
+ repo_config_get_bool(r, "rerere.autoupdate", &cache.autoupdate);
+ repo_config_get_int(r, "rerere.locktimeout", &lock_timeout_ms);
if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
- rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE);
+ cache.autoupdate = !!(flags & RERERE_AUTOUPDATE);
if ((flags & RERERE_READONLY) && (flags & RERERE_NOWAIT))
BUG("RERERE_NOWAIT does not apply with RERERE_READONLY");
if (flags & RERERE_READONLY) {
fd = 0;
} else {
int lock_flags = LOCK_DIE_ON_ERROR;
- int timeout_ms = rerere_lock_timeout_ms;
if (flags & RERERE_NOWAIT) {
lock_flags = 0;
- timeout_ms = 0;
+ lock_timeout_ms = 0;
}
/*
@@ -930,9 +924,9 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
* "git rerere gc" while it prunes rr-cache, so by default
* wait for it instead of dying right away.
*/
- fd = repo_hold_lock_file_for_update_timeout(r, &write_lock,
+ fd = repo_hold_lock_file_for_update_timeout(r, &cache.write_lock,
git_path_merge_rr(r),
- lock_flags, timeout_ms);
+ lock_flags, lock_timeout_ms);
if (fd < 0)
return -1;
}
@@ -1369,7 +1363,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
"rr-cache/%s", to_remove.items[i].string));
string_list_clear(&to_remove, 0);
- rollback_lock_file(&write_lock);
+ rollback_lock_file(&cache.write_lock);
strbuf_release(&buf);
}
@@ -1399,5 +1393,5 @@ void rerere_clear(struct repository *r, struct string_list *merge_rr)
strbuf_release(&buf);
}
unlink_or_warn(git_path_merge_rr(r));
- rollback_lock_file(&write_lock);
+ rollback_lock_file(&cache.write_lock);
}
diff --git a/rerere.h b/rerere.h
index ee8d97917d..570667d866 100644
--- a/rerere.h
+++ b/rerere.h
@@ -2,7 +2,9 @@
#define RERERE_H
#include "gettext.h"
+#include "lockfile.h"
#include "string-list.h"
+#include "strmap.h"
struct pathspec;
struct repository;
@@ -20,6 +22,15 @@ struct repository;
*/
extern void *RERERE_RESOLVED;
+struct rerere_cache {
+ /* Automatically update cleanly resolved paths to the index. */
+ int autoupdate;
+
+ struct strmap dirs;
+
+ struct lock_file write_lock;
+};
+
struct rerere_dir;
struct rerere_id {
struct rerere_dir *collection;
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/7] rerere: get rid of global rerere cache state
2026-10-23 12:50 [PATCH 0/7] rerere: remove global state Patrick Steinhardt
` (5 preceding siblings ...)
2026-10-23 12:50 ` [PATCH 6/7] rerere: bundle global state into a struct Patrick Steinhardt
@ 2026-10-23 12:50 ` Patrick Steinhardt
2026-10-23 17:04 ` Junio C Hamano
6 siblings, 1 reply; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-23 12:50 UTC (permalink / raw)
To: git
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.
For now, all callers are updated to do so ad-hoc. This of course means
that state that we could carry over across multiple calls would have to
be set up multiple times. But that doesn't really matter much right now
anyway, as the number of times we call those functions is bounded.
Note that `rerere_gc_needed()` is the only caller where we explicitly
don't set up the rerere cache state. This is intentional and matches the
previous status quo, where we didn't honor "rerere.enabled", either. The
function only needs access to `struct rerere_cache::dirs` anyway though,
so it's fine to partially initialize it, only.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
builtin/rerere.c | 15 ++++--
rerere.c | 138 +++++++++++++++++++++++++++++++++----------------------
rerere.h | 9 +++-
3 files changed, 102 insertions(+), 60 deletions(-)
diff --git a/builtin/rerere.c b/builtin/rerere.c
index e900e302c3..2c17f02f56 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -106,11 +106,15 @@ int cmd_rerere(int argc,
rerere_gc(the_repository, &merge_rr,
skip_locked ? RERERE_NOWAIT : 0);
else if (!strcmp(argv[0], "status")) {
- if (setup_rerere(the_repository, &merge_rr,
- flags | RERERE_READONLY) < 0)
+ struct rerere_cache cache;
+
+ if (rerere_cache_setup(&cache, the_repository, &merge_rr,
+ flags | RERERE_READONLY) < 0)
return 0;
for (size_t i = 0; i < merge_rr.nr; i++)
printf("%s\n", merge_rr.items[i].string);
+
+ rerere_cache_release(&cache);
} else if (!strcmp(argv[0], "remaining")) {
rerere_remaining(the_repository, &merge_rr);
for (size_t i = 0; i < merge_rr.nr; i++) {
@@ -123,8 +127,10 @@ int cmd_rerere(int argc,
}
} else if (!strcmp(argv[0], "diff")) {
struct strbuf buf = STRBUF_INIT;
- if (setup_rerere(the_repository, &merge_rr,
- flags | RERERE_READONLY) < 0)
+ struct rerere_cache cache;
+
+ if (rerere_cache_setup(&cache, the_repository, &merge_rr,
+ flags | RERERE_READONLY) < 0)
return 0;
for (size_t i = 0; i < merge_rr.nr; i++) {
const char *path = merge_rr.items[i].string;
@@ -135,6 +141,7 @@ int cmd_rerere(int argc,
rerere_path(the_repository, &buf, id, NULL));
}
+ rerere_cache_release(&cache);
strbuf_release(&buf);
} else
usage_with_options(rerere_usage, options);
diff --git a/rerere.c b/rerere.c
index 23bcb8bc31..e089761208 100644
--- a/rerere.c
+++ b/rerere.c
@@ -21,10 +21,6 @@
#include "odb.h"
#include "strmap.h"
-static struct rerere_cache cache = {
- .dirs = STRMAP_INIT,
-};
-
#define RESOLVED 0
#define PUNTED 1
#define THREE_STAGED 2
@@ -38,17 +34,19 @@ struct rerere_dir {
char name[FLEX_ARRAY];
};
-static void free_rerere_dirs(void)
+void rerere_cache_release(struct rerere_cache *cache)
{
struct hashmap_iter iter;
struct strmap_entry *ent;
- strmap_for_each_entry(&cache.dirs, &iter, ent) {
+ strmap_for_each_entry(&cache->dirs, &iter, ent) {
struct rerere_dir *rr_dir = ent->value;
free(rr_dir->status);
free(rr_dir);
}
- strmap_clear(&cache.dirs, 0);
+ strmap_clear(&cache->dirs, 0);
+
+ rollback_lock_file(&cache->write_lock);
}
static void free_rerere_id(struct string_list_item *item)
@@ -121,13 +119,13 @@ static int is_rr_file(const char *name, const char *filename, int *variant)
return 1;
}
-static void scan_rerere_dir(struct repository *r, struct rerere_dir *rr_dir)
+static void scan_rerere_dir(struct rerere_cache *cache, struct rerere_dir *rr_dir)
{
struct dirent *de;
char *path;
DIR *dir;
- path = repo_git_path(r, "rr-cache/%s", rr_dir->name);
+ path = repo_git_path(cache->repo, "rr-cache/%s", rr_dir->name);
dir = opendir(path);
free(path);
if (!dir)
@@ -146,19 +144,20 @@ static void scan_rerere_dir(struct repository *r, struct rerere_dir *rr_dir)
closedir(dir);
}
-static struct rerere_dir *find_rerere_dir(struct repository *r, const char *hex)
+static struct rerere_dir *find_rerere_dir(struct rerere_cache *cache,
+ const char *hex)
{
struct rerere_dir *rr_dir;
- rr_dir = strmap_get(&cache.dirs, hex);
+ rr_dir = strmap_get(&cache->dirs, hex);
if (!rr_dir) {
FLEX_ALLOC_STR(rr_dir, name, hex);
rr_dir->status = NULL;
rr_dir->status_nr = 0;
rr_dir->status_alloc = 0;
- strmap_put(&cache.dirs, hex, rr_dir);
+ strmap_put(&cache->dirs, hex, rr_dir);
- scan_rerere_dir(r, rr_dir);
+ scan_rerere_dir(cache, rr_dir);
}
return rr_dir;
}
@@ -173,17 +172,18 @@ static int has_rerere_resolution(const struct rerere_id *id)
return ((id->collection->status[variant] & both) == both);
}
-static struct rerere_id *new_rerere_id_hex(struct repository *r, char *hex)
+static struct rerere_id *new_rerere_id_hex(struct rerere_cache *cache,
+ char *hex)
{
struct rerere_id *id = xmalloc(sizeof(*id));
- id->collection = find_rerere_dir(r, hex);
+ id->collection = find_rerere_dir(cache, hex);
id->variant = -1; /* not known yet */
return id;
}
-static struct rerere_id *new_rerere_id(struct repository *r, unsigned char *hash)
+static struct rerere_id *new_rerere_id(struct rerere_cache *cache, unsigned char *hash)
{
- return new_rerere_id_hex(r, hash_to_hex_algop(hash, r->hash_algo));
+ return new_rerere_id_hex(cache, hash_to_hex_algop(hash, cache->repo->hash_algo));
}
/*
@@ -193,10 +193,10 @@ static struct rerere_id *new_rerere_id(struct repository *r, unsigned char *hash
* work on (i.e. what is left by the previous invocation of "git
* rerere" during the current conflict resolution session).
*/
-static void read_rr(struct repository *r, struct string_list *rr)
+static void read_rr(struct rerere_cache *cache, struct string_list *rr)
{
struct strbuf buf = STRBUF_INIT;
- FILE *in = fopen_or_warn(git_path_merge_rr(r), "r");
+ FILE *in = fopen_or_warn(git_path_merge_rr(cache->repo), "r");
if (!in)
return;
@@ -205,11 +205,11 @@ static void read_rr(struct repository *r, struct string_list *rr)
unsigned char hash[GIT_MAX_RAWSZ];
struct rerere_id *id;
int variant;
- const unsigned hexsz = r->hash_algo->hexsz;
+ const unsigned hexsz = cache->repo->hash_algo->hexsz;
/* There has to be the hash, tab, path and then NUL */
if (buf.len < hexsz + 2 ||
- get_hash_hex_algop(buf.buf, hash, r->hash_algo))
+ get_hash_hex_algop(buf.buf, hash, cache->repo->hash_algo))
die(_("corrupt MERGE_RR"));
if (buf.buf[hexsz] != '.') {
@@ -224,7 +224,7 @@ static void read_rr(struct repository *r, struct string_list *rr)
if (*(path++) != '\t')
die(_("corrupt MERGE_RR"));
buf.buf[hexsz] = '\0';
- id = new_rerere_id_hex(r, buf.buf);
+ id = new_rerere_id_hex(cache, buf.buf);
id->variant = variant;
/*
* make sure id->collection->status has enough space
@@ -237,7 +237,8 @@ static void read_rr(struct repository *r, struct string_list *rr)
fclose(in);
}
-static int write_rr(struct string_list *rr, int out_fd)
+static int write_rr(struct rerere_cache *cache,
+ struct string_list *rr, int out_fd)
{
int i;
for (i = 0; i < rr->nr; i++) {
@@ -264,7 +265,7 @@ static int write_rr(struct string_list *rr, int out_fd)
strbuf_release(&buf);
}
- if (commit_lock_file(&cache.write_lock) != 0)
+ if (commit_lock_file(&cache->write_lock) != 0)
die(_("unable to write rerere record"));
return 0;
}
@@ -564,12 +565,15 @@ static int find_conflict(struct repository *r, struct string_list *conflict)
*/
int rerere_remaining(struct repository *r, struct string_list *merge_rr)
{
- int i;
+ struct rerere_cache cache;
+ int ret, i;
- if (setup_rerere(r, merge_rr, RERERE_READONLY))
+ if (rerere_cache_setup(&cache, r, merge_rr, RERERE_READONLY))
return 0;
- if (repo_read_index(r) < 0)
- return error(_("index file corrupt"));
+ if (repo_read_index(r) < 0) {
+ ret = error(_("index file corrupt"));
+ goto out;
+ }
for (i = 0; i < r->index->cache_nr;) {
int conflict_type;
@@ -586,7 +590,12 @@ int rerere_remaining(struct repository *r, struct string_list *merge_rr)
}
}
}
- return 0;
+
+ ret = 0;
+
+out:
+ rerere_cache_release(&cache);
+ return ret;
}
/*
@@ -714,10 +723,11 @@ static void remove_variant(struct repository *r, struct rerere_id *id)
* only have the preimage for that conflict, in which case the result
* needs to be recorded as a resolution in a postimage file.
*/
-static void do_rerere_one_path(struct index_state *istate,
+static void do_rerere_one_path(struct rerere_cache *cache,
struct string_list_item *rr_item,
struct string_list *update)
{
+ struct index_state *istate = cache->repo->index;
const char *path = rr_item->string;
struct rerere_id *id = rr_item->util;
struct rerere_dir *rr_dir = id->collection;
@@ -779,7 +789,7 @@ static void do_rerere_one_path(struct index_state *istate,
if (0 <= id->variant && id->variant != variant)
remove_variant(istate->repo, id);
- if (cache.autoupdate)
+ if (cache->autoupdate)
string_list_insert(update, path);
else
fprintf_ln(stderr,
@@ -810,12 +820,13 @@ static void do_rerere_one_path(struct index_state *istate,
strbuf_release(&buf);
}
-static int do_plain_rerere(struct repository *r,
+static int do_plain_rerere(struct rerere_cache *cache,
struct string_list *rr, int fd)
{
struct string_list conflict = STRING_LIST_INIT_DUP;
struct string_list update = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
+ struct repository *r = cache->repo;
int i;
find_conflict(r, &conflict);
@@ -845,7 +856,7 @@ static int do_plain_rerere(struct repository *r,
if (ret < 1)
continue;
- id = new_rerere_id(r, hash);
+ id = new_rerere_id(cache, hash);
string_list_insert(rr, path)->util = id;
/* Ensure that the directory exists. */
@@ -853,7 +864,7 @@ static int do_plain_rerere(struct repository *r,
}
for (i = 0; i < rr->nr; i++)
- do_rerere_one_path(r->index, &rr->items[i], &update);
+ do_rerere_one_path(cache, &rr->items[i], &update);
if (update.nr)
update_paths(r, &update);
@@ -861,7 +872,7 @@ static int do_plain_rerere(struct repository *r,
string_list_clear(&conflict, 0);
string_list_clear(&update, 0);
strbuf_release(&buf);
- return write_rr(rr, fd);
+ return write_rr(cache, rr, fd);
}
static int is_rerere_enabled(struct repository *r)
@@ -894,7 +905,10 @@ static int is_rerere_enabled(struct repository *r)
return r->rerere_cache_enabled;
}
-int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
+int rerere_cache_setup(struct rerere_cache *cache,
+ struct repository *r,
+ struct string_list *merge_rr,
+ int flags)
{
int lock_timeout_ms = 1000;
int fd;
@@ -902,11 +916,14 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
if (!is_rerere_enabled(r))
return -1;
- repo_config_get_bool(r, "rerere.autoupdate", &cache.autoupdate);
+ memset(cache, 0, sizeof(*cache));
+ cache->repo = r;
+ strmap_init(&cache->dirs);
+ repo_config_get_bool(r, "rerere.autoupdate", &cache->autoupdate);
repo_config_get_int(r, "rerere.locktimeout", &lock_timeout_ms);
if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE))
- cache.autoupdate = !!(flags & RERERE_AUTOUPDATE);
+ cache->autoupdate = !!(flags & RERERE_AUTOUPDATE);
if ((flags & RERERE_READONLY) && (flags & RERERE_NOWAIT))
BUG("RERERE_NOWAIT does not apply with RERERE_READONLY");
if (flags & RERERE_READONLY) {
@@ -924,13 +941,13 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
* "git rerere gc" while it prunes rr-cache, so by default
* wait for it instead of dying right away.
*/
- fd = repo_hold_lock_file_for_update_timeout(r, &cache.write_lock,
+ fd = repo_hold_lock_file_for_update_timeout(r, &cache->write_lock,
git_path_merge_rr(r),
lock_flags, lock_timeout_ms);
if (fd < 0)
return -1;
}
- read_rr(r, merge_rr);
+ read_rr(cache, merge_rr);
return fd;
}
@@ -942,13 +959,15 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
int repo_rerere(struct repository *r, int flags)
{
struct string_list merge_rr = STRING_LIST_INIT_DUP;
+ struct rerere_cache cache;
int fd, status;
- fd = setup_rerere(r, &merge_rr, flags);
+ fd = rerere_cache_setup(&cache, r, &merge_rr, flags);
if (fd < 0)
return 0;
- status = do_plain_rerere(r, &merge_rr, fd);
- free_rerere_dirs();
+ status = do_plain_rerere(&cache, &merge_rr, fd);
+
+ rerere_cache_release(&cache);
string_list_clear(&merge_rr, 1);
return status;
}
@@ -1057,10 +1076,11 @@ static int handle_cache(struct index_state *istate,
return has_conflicts;
}
-static int rerere_forget_one_path(struct index_state *istate,
+static int rerere_forget_one_path(struct rerere_cache *cache,
const char *path,
struct string_list *rr)
{
+ struct index_state *istate = cache->repo->index;
const char *filename;
struct rerere_id *id;
unsigned char hash[GIT_MAX_RAWSZ];
@@ -1077,7 +1097,7 @@ static int rerere_forget_one_path(struct index_state *istate,
return error(_("could not parse conflict hunks in '%s'"), path);
/* Nuke the recorded resolution for the conflict */
- id = new_rerere_id(istate->repo, hash);
+ id = new_rerere_id(cache, hash);
for (id->variant = 0;
id->variant < id->collection->status_nr;
@@ -1145,11 +1165,12 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)
int i, fd, ret;
struct string_list conflict = STRING_LIST_INIT_DUP;
struct string_list merge_rr = STRING_LIST_INIT_DUP;
+ struct rerere_cache cache;
if (repo_read_index(r) < 0)
return error(_("index file corrupt"));
- fd = setup_rerere(r, &merge_rr, RERERE_NOAUTOUPDATE);
+ fd = rerere_cache_setup(&cache, r, &merge_rr, RERERE_NOAUTOUPDATE);
if (fd < 0)
return 0;
@@ -1165,13 +1186,14 @@ int rerere_forget(struct repository *r, struct pathspec *pathspec)
if (!match_pathspec(r->index, pathspec, it->string,
strlen(it->string), 0, NULL, 0))
continue;
- rerere_forget_one_path(r->index, it->string, &merge_rr);
+ rerere_forget_one_path(&cache, it->string, &merge_rr);
}
- ret = write_rr(&merge_rr, fd);
+ ret = write_rr(&cache, &merge_rr, fd);
string_list_clear(&conflict, 0);
string_list_clear(&merge_rr, 1);
+ rerere_cache_release(&cache);
return ret;
}
@@ -1267,6 +1289,10 @@ static int is_rr_cache_dirname(struct repository *r, const char *path)
bool rerere_gc_needed(struct repository *r, size_t limit)
{
+ struct rerere_cache cache = {
+ .repo = r,
+ .dirs = STRMAP_INIT,
+ };
timestamp_t cutoff_resolve, cutoff_noresolve;
struct strbuf buf = STRBUF_INIT;
bool needed = false;
@@ -1292,7 +1318,7 @@ bool rerere_gc_needed(struct repository *r, size_t limit)
!is_rr_cache_dirname(r, e->d_name))
continue;
- id.collection = find_rerere_dir(r, e->d_name);
+ id.collection = find_rerere_dir(&cache, e->d_name);
for (id.variant = 0;
id.variant < id.collection->status_nr;
id.variant++) {
@@ -1310,13 +1336,14 @@ bool rerere_gc_needed(struct repository *r, size_t limit)
out:
if (dir)
closedir(dir);
- free_rerere_dirs();
+ rerere_cache_release(&cache);
strbuf_release(&buf);
return needed;
}
void rerere_gc(struct repository *r, struct string_list *rr, int flags)
{
+ struct rerere_cache cache;
struct string_list to_remove = STRING_LIST_INIT_DUP;
DIR *dir;
struct dirent *e;
@@ -1325,7 +1352,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
timestamp_t cutoff_resolve;
struct strbuf buf = STRBUF_INIT;
- if (setup_rerere(r, rr, flags) < 0)
+ if (rerere_cache_setup(&cache, r, rr, flags) < 0)
return;
rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
@@ -1342,7 +1369,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
if (!is_rr_cache_dirname(r, e->d_name))
continue; /* or should we remove e->d_name? */
- rr_dir = find_rerere_dir(r, e->d_name);
+ rr_dir = find_rerere_dir(&cache, e->d_name);
now_empty = 1;
for (id.variant = 0, id.collection = rr_dir;
@@ -1363,7 +1390,7 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
"rr-cache/%s", to_remove.items[i].string));
string_list_clear(&to_remove, 0);
- rollback_lock_file(&cache.write_lock);
+ rerere_cache_release(&cache);
strbuf_release(&buf);
}
@@ -1376,9 +1403,10 @@ void rerere_gc(struct repository *r, struct string_list *rr, int flags)
*/
void rerere_clear(struct repository *r, struct string_list *merge_rr)
{
+ struct rerere_cache cache;
int i;
- if (setup_rerere(r, merge_rr, 0) < 0)
+ if (rerere_cache_setup(&cache, r, merge_rr, 0) < 0)
return;
for (i = 0; i < merge_rr->nr; i++) {
@@ -1393,5 +1421,5 @@ void rerere_clear(struct repository *r, struct string_list *merge_rr)
strbuf_release(&buf);
}
unlink_or_warn(git_path_merge_rr(r));
- rollback_lock_file(&cache.write_lock);
+ rerere_cache_release(&cache);
}
diff --git a/rerere.h b/rerere.h
index 570667d866..d3cbd10e13 100644
--- a/rerere.h
+++ b/rerere.h
@@ -23,6 +23,8 @@ struct repository;
extern void *RERERE_RESOLVED;
struct rerere_cache {
+ struct repository *repo;
+
/* Automatically update cleanly resolved paths to the index. */
int autoupdate;
@@ -37,7 +39,12 @@ struct rerere_id {
int variant;
};
-int setup_rerere(struct repository *,struct string_list *, int);
+int rerere_cache_setup(struct rerere_cache *cache,
+ struct repository *r,
+ struct string_list *merge_rr,
+ int flags);
+void rerere_cache_release(struct rerere_cache *cache);
+
int repo_rerere(struct repository *, int);
/*
* Given the conflict ID and the name of a "file" used for replaying
--
2.56.0.307.g5fe7bd680c.dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 7/7] rerere: get rid of global rerere cache state
2026-10-23 12:50 ` [PATCH 7/7] rerere: get rid of global rerere cache state Patrick Steinhardt
@ 2026-10-23 17:04 ` Junio C Hamano
2026-10-26 7:28 ` Patrick Steinhardt
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2026-10-23 17:04 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
Patrick Steinhardt <ps@pks.im> 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.
Queued.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 7/7] rerere: get rid of global rerere cache state
2026-10-23 17:04 ` Junio C Hamano
@ 2026-10-26 7:28 ` Patrick Steinhardt
0 siblings, 0 replies; 10+ messages in thread
From: Patrick Steinhardt @ 2026-10-26 7:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Fri, Oct 23, 2026 at 10:04:57AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> 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,
^ permalink raw reply related [flat|nested] 10+ messages in thread