* [PATCH 0/5] refs: reduce reliance on the_repository global state
@ 2026-03-25 16:44 Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
` (5 more replies)
0 siblings, 6 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
This series continues the effort to reduce reliance on the_repository
global state by making repository context explicit across the refs
subsystem. The patches focus on passing struct repository through various
ref helpers and backends, and replacing uses of global state such as
the_repository and the_hash_algo with the appropriate repository instance.
Patch 1/5: Making branch name helper functions (copy_branchname(),
check_branch_ref(), validate_branchname(), and validate_new_branchname())
repository-aware. (built on top of jw/object-name-bitset-to-enum)
Patch 2/5: Updating get_files_ref_lock_timeout_ms() to take a repository
and propagating it through files-backend, including callback paths.
Patch 3/5: Replacing uses of the_hash_algo in refs.c with the hash
algorithm from the appropriate repository.
Patch 4/5: Removing remaining uses of the_repository in reftable-backend.c
where a repository instance is already available.
Patch 5/5: Replacing the single instance of the_repository in
packed-backend.c, thus dropping the USE_THE_REPOSITORY_VARIABLE macro.
Shreyansh Paliwal (5):
refs: make branchname helpers repository aware
refs: make get_files_ref_lock_timeout_ms() repostory aware
refs: remove the_hash_algo global state
refs/reftable-backend: drop uses of the_repository
refs/packed-backend: use ref_store->repo instead of the_repository
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 28 +++++++++++++---------------
refs.h | 5 +++--
refs/files-backend.c | 19 +++++++++++++------
refs/packed-backend.c | 3 +--
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 6 +++---
13 files changed, 63 insertions(+), 55 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH 1/5] refs: make branchname helpers repository aware
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
@ 2026-03-25 16:44 ` Shreyansh Paliwal
2026-03-27 7:49 ` Patrick Steinhardt
2026-03-25 16:44 ` [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
` (4 subsequent siblings)
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
copy_branchname() in refs.c relies on the_repository when calling
repo_interpret_branch_name(), introducing an implicit dependency on global
state. Add a struct repository parameter and use it instead.
Update check_branch_ref() to take a repository parameter as well, since it
calls copy_branchname(). Propagate this change to higher-level helpers
validate_branchname() and validate_new_branchname(), which also lack access
to a repository instance. Most callers of these helpers reside in builtin
code and already operate on the_repository, so pass it explicitly at those
call sites (builtin/checkout and builtin/worktree) otherwise pass struct
repository where available.
This makes branch name handling explicitly repository-aware and aligns with
ongoing efforts to remove reliance on global state. This change builds on
top of jw/object-name-bitset-to-enum (2026-03-18), which introduced the
enum interpret_branch_kind parameter to copy_branchname().
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 9 +++++----
refs.h | 5 +++--
9 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..65189823c3 100644
--- a/branch.c
+++ b/branch.c
@@ -370,16 +370,16 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref)
+int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo)
{
- if (check_branch_ref(ref, name)) {
+ if (check_branch_ref(ref, name, repo)) {
int code = die_message(_("'%s' is not a valid branch name"), name);
advise_if_enabled(ADVICE_REF_SYNTAX,
_("See 'git help check-ref-format'"));
exit(code);
}
- return refs_ref_exists(get_main_ref_store(the_repository), ref->buf);
+ return refs_ref_exists(get_main_ref_store(repo), ref->buf);
}
static int initialized_checked_out_branches;
@@ -468,10 +468,11 @@ const char *branch_checked_out(const char *refname)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+int validate_new_branchname(const char *name, struct strbuf *ref, int force,
+ struct repository *repo)
{
const char *path;
- if (!validate_branchname(name, ref))
+ if (!validate_branchname(name, ref, repo))
return 0;
if (!force)
@@ -613,8 +614,8 @@ void create_branch(struct repository *r,
BUG("'clobber_head_ok' can only be used with 'force'");
if (clobber_head_ok ?
- validate_branchname(name, &ref) :
- validate_new_branchname(name, &ref, force)) {
+ validate_branchname(name, &ref, r) :
+ validate_new_branchname(name, &ref, force, r)) {
forcing = 1;
}
diff --git a/branch.h b/branch.h
index 3dc6e2a0ff..3aa53eb243 100644
--- a/branch.h
+++ b/branch.h
@@ -111,7 +111,7 @@ const char *branch_checked_out(const char *refname);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref);
+int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo);
/*
* Check if a branch 'name' can be created as a new branch; die otherwise.
@@ -119,7 +119,8 @@ int validate_branchname(const char *name, struct strbuf *ref);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+int validate_new_branchname(const char *name, struct strbuf *ref, int force,
+ struct repository *repo);
/*
* Remove information about the merge state on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..9c86b9f525 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -259,7 +259,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
char *target = NULL;
int flags = 0;
- copy_branchname(&bname, argv[i], allowed_interpret);
+ copy_branchname(&bname, argv[i], allowed_interpret, the_repository);
free(name);
name = mkpathdup(fmt, bname.buf);
@@ -581,7 +581,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
int recovery = 0, oldref_usage = 0;
struct worktree **worktrees = get_worktrees();
- if (check_branch_ref(&oldref, oldname)) {
+ if (check_branch_ref(&oldref, oldname, the_repository)) {
/*
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
@@ -619,9 +619,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
* cause the worktree to become inconsistent with HEAD, so allow it.
*/
if (!strcmp(oldname, newname))
- validate_branchname(newname, &newref);
+ validate_branchname(newname, &newref, the_repository);
else
- validate_new_branchname(newname, &newref, force);
+ validate_new_branchname(newname, &newref, force, the_repository);
reject_rebase_or_bisect_branch(worktrees, oldref.buf);
@@ -898,7 +898,7 @@ int cmd_branch(int argc,
die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
branch_name = buf.buf;
} else {
die(_("cannot edit description of more than one branch"));
@@ -941,7 +941,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to set new upstream"));
@@ -971,7 +971,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to unset upstream"));
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 5d80afeec0..8222a289c9 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -5,6 +5,7 @@
#include "refs.h"
#include "setup.h"
#include "strbuf.h"
+#include "repository.c"
static const char builtin_check_ref_format_usage[] =
"git check-ref-format [--normalize] [<options>] <refname>\n"
@@ -42,7 +43,7 @@ static int check_ref_format_branch(const char *arg)
int nongit;
setup_git_directory_gently(&nongit);
- if (check_branch_ref(&sb, arg) ||
+ if (check_branch_ref(&sb, arg, the_repository) ||
!skip_prefix(sb.buf, "refs/heads/", &name))
die("'%s' is not a valid branch name", arg);
printf("%s\n", name);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e031e61886..7570a5664f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -743,7 +743,7 @@ static void setup_branch_path(struct branch_info *branch)
&branch->oid, &branch->refname, 0))
repo_get_oid_committish(the_repository, branch->name, &branch->oid);
- copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL, the_repository);
if (strcmp(buf.buf, branch->name)) {
free(branch->name);
branch->name = xstrdup(buf.buf);
@@ -2014,10 +2014,10 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
struct strbuf buf = STRBUF_INIT;
if (opts->new_branch_force)
- opts->branch_exists = validate_branchname(opts->new_branch, &buf);
+ opts->branch_exists = validate_branchname(opts->new_branch, &buf, the_repository);
else
opts->branch_exists =
- validate_new_branchname(opts->new_branch, &buf, 0);
+ validate_new_branchname(opts->new_branch, &buf, 0, the_repository);
strbuf_release(&buf);
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 2cbce56f8d..854490afef 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -552,7 +552,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
char *found_ref = NULL;
int len, early;
- copy_branchname(&bname, remote, 0);
+ copy_branchname(&bname, remote, 0, the_repository);
remote = bname.buf;
oidclr(&branch_head, the_repository->hash_algo);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4035b1cb06..6ec2f02bf0 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -415,7 +415,7 @@ static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
struct strbuf symref = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
- validate_new_branchname(ref, &symref, 0);
+ validate_new_branchname(ref, &symref, 0, the_repository);
strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
if (opts->quiet)
strvec_push(&cp.args, "--quiet");
@@ -481,7 +481,7 @@ static int add_worktree(const char *path, const char *refname,
worktrees = NULL;
/* is 'refname' a branch or commit? */
- if (!opts->detach && !check_branch_ref(&symref, refname) &&
+ if (!opts->detach && !check_branch_ref(&symref, refname, the_repository) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
is_branch = 1;
if (!opts->force)
@@ -649,7 +649,7 @@ static void print_preparing_worktree_line(int detach,
fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
} else {
struct strbuf s = STRBUF_INIT;
- if (!detach && !check_branch_ref(&s, branch) &&
+ if (!detach && !check_branch_ref(&s, branch, the_repository) &&
refs_ref_exists(get_main_ref_store(the_repository), s.buf))
fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
branch);
@@ -788,7 +788,7 @@ static char *dwim_branch(const char *path, char **new_branch)
char *branchname = xstrndup(s, n);
struct strbuf ref = STRBUF_INIT;
- branch_exists = !check_branch_ref(&ref, branchname) &&
+ branch_exists = !check_branch_ref(&ref, branchname, the_repository) &&
refs_ref_exists(get_main_ref_store(the_repository),
ref.buf);
strbuf_release(&ref);
@@ -885,7 +885,7 @@ static int add(int ac, const char **av, const char *prefix,
new_branch = new_branch_force;
if (!opts.force &&
- !check_branch_ref(&symref, new_branch) &&
+ !check_branch_ref(&symref, new_branch, the_repository) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
die_if_checked_out(symref.buf, 0);
strbuf_release(&symref);
diff --git a/refs.c b/refs.c
index 685a0c247b..840965519e 100644
--- a/refs.c
+++ b/refs.c
@@ -744,13 +744,14 @@ static char *substitute_branch_name(struct repository *r,
}
void copy_branchname(struct strbuf *sb, const char *name,
- enum interpret_branch_kind allowed)
+ enum interpret_branch_kind allowed,
+ struct repository *repo)
{
int len = strlen(name);
struct interpret_branch_name_options options = {
.allowed = allowed
};
- int used = repo_interpret_branch_name(the_repository, name, len, sb,
+ int used = repo_interpret_branch_name(repo, name, len, sb,
&options);
if (used < 0)
@@ -758,10 +759,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
strbuf_add(sb, name + used, len - used);
}
-int check_branch_ref(struct strbuf *sb, const char *name)
+int check_branch_ref(struct strbuf *sb, const char *name, struct repository *repo)
{
if (startup_info->have_repository)
- copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL, repo);
else
strbuf_addstr(sb, name);
diff --git a/refs.h b/refs.h
index d65de6ab5f..72b8ea609a 100644
--- a/refs.h
+++ b/refs.h
@@ -226,7 +226,8 @@ char *repo_default_branch_name(struct repository *r, int quiet);
* repo_interpret_branch_name() for details.
*/
void copy_branchname(struct strbuf *sb, const char *name,
- enum interpret_branch_kind allowed);
+ enum interpret_branch_kind allowed,
+ struct repository *repo);
/*
* Like copy_branchname() above, but confirm that the result is
@@ -234,7 +235,7 @@ void copy_branchname(struct strbuf *sb, const char *name,
*
* The return value is "0" if the result is valid, and "-1" otherwise.
*/
-int check_branch_ref(struct strbuf *sb, const char *name);
+int check_branch_ref(struct strbuf *sb, const char *name, struct repository *repo);
/*
* Similar for a tag name in refs/tags/.
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
@ 2026-03-25 16:44 ` Shreyansh Paliwal
2026-03-27 7:50 ` Patrick Steinhardt
2026-03-27 9:23 ` Burak Kaan Karaçay
2026-03-25 16:44 ` [PATCH 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
` (3 subsequent siblings)
5 siblings, 2 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.
Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
This removes reliance on the_repository global and makes the timeout lookup
operate on the correct repository context.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 840965519e..e7256b3a84 100644
--- a/refs.c
+++ b/refs.c
@@ -990,7 +990,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,
return REF_WORKTREE_SHARED;
}
-long get_files_ref_lock_timeout_ms(void)
+long get_files_ref_lock_timeout_ms(struct repository *repo)
{
static int configured = 0;
@@ -998,7 +998,7 @@ long get_files_ref_lock_timeout_ms(void)
static int timeout_ms = 100;
if (!configured) {
- repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
+ repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
configured = 1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ce0d57478..ee8dd771a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
if (hold_lock_file_for_update_timeout(
&lock->lk, ref_file.buf, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0) {
+ get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) {
int myerr = errno;
errno = 0;
if (myerr == ENOENT && --attempts_remaining > 0) {
@@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
+struct create_reflock_cb {
+ struct lock_file *lk;
+ struct repository *repo;
+};
+
static int create_reflock(const char *path, void *cb)
{
- struct lock_file *lk = cb;
-
+ struct create_reflock_cb *data = cb;
return hold_lock_file_for_update_timeout(
- lk, path, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
+ data->lk, path, LOCK_NO_DEREF,
+ get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
}
/*
@@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ struct create_reflock_cb cb_data;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
lock->ref_name = xstrdup(refname);
lock->count = 1;
+ cb_data.lk = &lock->lk;
+ cb_data.repo = refs->base.repo;
- if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) {
+ if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) {
unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..e4cfd9e19e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,7 +43,7 @@ struct ref_transaction;
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
*/
-long get_files_ref_lock_timeout_ms(void);
+long get_files_ref_lock_timeout_ms(struct repository *repo);
/*
* Return true iff refname is minimally safe. "Safe" here means that
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 3/5] refs: remove the_hash_algo global state
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
@ 2026-03-25 16:44 ` Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
` (2 subsequent siblings)
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.
This removes implicit reliance on global state. With no remaining uses of
the_repository in this file, drop USE_THE_REPOSITORY_VARIABLE and the
dependency on environment.h.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index e7256b3a84..7762babf5f 100644
--- a/refs.c
+++ b/refs.c
@@ -2,13 +2,10 @@
* The backend-independent part of the reference module.
*/
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "abspath.h"
#include "advice.h"
#include "config.h"
-#include "environment.h"
#include "strmap.h"
#include "gettext.h"
#include "hex.h"
@@ -1473,7 +1470,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
return 1;
}
return ref_transaction_update(transaction, refname, new_oid,
- null_oid(the_hash_algo), new_target, NULL, flags,
+ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
msg, err);
}
@@ -1492,7 +1489,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (old_target && !(flags & REF_NO_DEREF))
BUG("delete cannot operate on symrefs with deref mode");
return ref_transaction_update(transaction, refname,
- null_oid(the_hash_algo), old_oid,
+ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
NULL, old_target, flags,
msg, err);
}
@@ -2380,7 +2377,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
subrepo = xmalloc(sizeof(*subrepo));
if (repo_submodule_init(subrepo, repo, submodule,
- null_oid(the_hash_algo))) {
+ null_oid(repo->hash_algo))) {
free(subrepo);
goto done;
}
@@ -2572,14 +2569,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
strbuf_reset(buf);
if (!(update->flags & REF_HAVE_OLD))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->old_target)
strbuf_addf(buf, "ref:%s ", update->old_target);
else
strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
if (!(update->flags & REF_HAVE_NEW))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
@@ -3154,7 +3151,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(data->transaction->ref_store->repo->hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 4/5] refs/reftable-backend: drop uses of the_repository
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (2 preceding siblings ...)
2026-03-25 16:44 ` [PATCH 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-03-25 16:44 ` Shreyansh Paliwal
2026-03-27 7:50 ` Patrick Steinhardt
2026-03-25 16:44 ` [PATCH 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.
Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.
Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663..7c8a992fcb 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo,
default:
BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
}
- refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask);
+ refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask);
refs->write_options.disable_auto_compact =
!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
refs->write_options.lock_timeout_ms = 100;
refs->write_options.fsync = reftable_be_fsync;
- repo_config(the_repository, reftable_be_config, &refs->write_options);
+ repo_config(repo, reftable_be_config, &refs->write_options);
/*
* It is somewhat unfortunate that we have to mirror the default block
@@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store,
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
- safe_create_dir(the_repository, sb.buf, 1);
+ safe_create_dir(ref_store->repo, sb.buf, 1);
strbuf_reset(&sb);
strbuf_release(&sb);
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH 5/5] refs/packed-backend: use ref_store->repo instead of the_repository
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (3 preceding siblings ...)
2026-03-25 16:44 ` [PATCH 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-03-25 16:44 ` Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-25 16:44 UTC (permalink / raw)
To: git; +Cc: Shreyansh Paliwal
In refs/packed-backend.c, repo_config_get_int() is called using the global
the_repository, even though a repository instance is available via struct
ref_store.
Replace the use of the_repository with ref_store->repo to make the code
explicitly repository-aware. With no remaining users of the_repository in
this file, drop the USE_THE_REPOSITORY_VARIABLE macro.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/packed-backend.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 23ed62984b..ebc10dab4d 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "../git-compat-util.h"
@@ -1223,7 +1222,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
static int timeout_value = 1000;
if (!timeout_configured) {
- repo_config_get_int(the_repository, "core.packedrefstimeout", &timeout_value);
+ repo_config_get_int(ref_store->repo, "core.packedrefstimeout", &timeout_value);
timeout_configured = 1;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH 1/5] refs: make branchname helpers repository aware
2026-03-25 16:44 ` [PATCH 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
@ 2026-03-27 7:49 ` Patrick Steinhardt
2026-03-28 12:45 ` Shreyansh Paliwal
0 siblings, 1 reply; 48+ messages in thread
From: Patrick Steinhardt @ 2026-03-27 7:49 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git
On Wed, Mar 25, 2026 at 10:14:18PM +0530, Shreyansh Paliwal wrote:
> diff --git a/branch.h b/branch.h
> index 3dc6e2a0ff..3aa53eb243 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -111,7 +111,7 @@ const char *branch_checked_out(const char *refname);
> * Return 1 if the named branch already exists; return 0 otherwise.
> * Fill ref with the full refname for the branch.
> */
> -int validate_branchname(const char *name, struct strbuf *ref);
> +int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo);
>
> /*
> * Check if a branch 'name' can be created as a new branch; die otherwise.
> @@ -119,7 +119,8 @@ int validate_branchname(const char *name, struct strbuf *ref);
> * Return 1 if the named branch already exists; return 0 otherwise.
> * Fill ref with the full refname for the branch.
> */
> -int validate_new_branchname(const char *name, struct strbuf *ref, int force);
> +int validate_new_branchname(const char *name, struct strbuf *ref, int force,
> + struct repository *repo);
>
> /*
> * Remove information about the merge state on the current
It's more customary in our code base to have the repository be the first
parameter. Other than that this patch looks good to me.
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware
2026-03-25 16:44 ` [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
@ 2026-03-27 7:50 ` Patrick Steinhardt
2026-03-27 9:23 ` Burak Kaan Karaçay
1 sibling, 0 replies; 48+ messages in thread
From: Patrick Steinhardt @ 2026-03-27 7:50 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git
On Wed, Mar 25, 2026 at 10:14:19PM +0530, Shreyansh Paliwal wrote:
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 7ce0d57478..ee8dd771a4 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
> return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
> }
>
> +struct create_reflock_cb {
> + struct lock_file *lk;
> + struct repository *repo;
> +};
> +
> static int create_reflock(const char *path, void *cb)
> {
> - struct lock_file *lk = cb;
> -
> + struct create_reflock_cb *data = cb;
> return hold_lock_file_for_update_timeout(
> - lk, path, LOCK_NO_DEREF,
> - get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
> + data->lk, path, LOCK_NO_DEREF,
> + get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
> }
>
> /*
Makes sense. This function is used as a callback to
`raceproof_create_file()`, and that callsite is adapted accordingly to
pass the new struct as payload.
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 4/5] refs/reftable-backend: drop uses of the_repository
2026-03-25 16:44 ` [PATCH 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-03-27 7:50 ` Patrick Steinhardt
0 siblings, 0 replies; 48+ messages in thread
From: Patrick Steinhardt @ 2026-03-27 7:50 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git
On Wed, Mar 25, 2026 at 10:14:21PM +0530, Shreyansh Paliwal wrote:
> reftable_be_init() and reftable_be_create_on_disk() use the_repository even
> though a repository instance is already available, either directly or via
> struct ref_store.
>
> Replace these uses with the appropriate local repository instance (repo or
> ref_store->repo) to avoid relying on global state.
>
> Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
> is_bare_repository() is still there in the file.
Yeah, `is_bare_repository()` is a blocker in many files :(
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware
2026-03-25 16:44 ` [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
2026-03-27 7:50 ` Patrick Steinhardt
@ 2026-03-27 9:23 ` Burak Kaan Karaçay
2026-03-28 12:51 ` Shreyansh Paliwal
1 sibling, 1 reply; 48+ messages in thread
From: Burak Kaan Karaçay @ 2026-03-27 9:23 UTC (permalink / raw)
To: Shreyansh Paliwal, git
On Wed Mar 25, 2026 at 7:44 PM +03, Shreyansh Paliwal wrote:
> -long get_files_ref_lock_timeout_ms(void)
> +long get_files_ref_lock_timeout_ms(struct repository *repo)
> {
> static int configured = 0;
>
> @@ -998,7 +998,7 @@ long get_files_ref_lock_timeout_ms(void)
> static int timeout_ms = 100;
>
> if (!configured) {
> - repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
> + repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
> configured = 1;
> }
>
Looks like the existing code uses static local variables for
performance. They can behave unexpectedly in multi-repo cases.
I think moving the config into 'repo-settings' should be considered. The
config is already lazy-parsed, migrating it shouldn't be a problem.
Best,
Burak Kaan Karaçay
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 1/5] refs: make branchname helpers repository aware
2026-03-27 7:49 ` Patrick Steinhardt
@ 2026-03-28 12:45 ` Shreyansh Paliwal
0 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 12:45 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On Fri, Mar 27, 2026 at 1:20 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Wed, Mar 25, 2026 at 10:14:18PM +0530, Shreyansh Paliwal wrote:
> > diff --git a/branch.h b/branch.h
> > index 3dc6e2a0ff..3aa53eb243 100644
> > --- a/branch.h
> > +++ b/branch.h
> > @@ -111,7 +111,7 @@ const char *branch_checked_out(const char *refname);
> > * Return 1 if the named branch already exists; return 0 otherwise.
> > * Fill ref with the full refname for the branch.
> > */
> > -int validate_branchname(const char *name, struct strbuf *ref);
> > +int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo);
> >
> > /*
> > * Check if a branch 'name' can be created as a new branch; die otherwise.
> > @@ -119,7 +119,8 @@ int validate_branchname(const char *name, struct strbuf *ref);
> > * Return 1 if the named branch already exists; return 0 otherwise.
> > * Fill ref with the full refname for the branch.
> > */
> > -int validate_new_branchname(const char *name, struct strbuf *ref, int force);
> > +int validate_new_branchname(const char *name, struct strbuf *ref, int force,
> > + struct repository *repo);
> >
> > /*
> > * Remove information about the merge state on the current
>
> It's more customary in our code base to have the repository be the first
> parameter. Other than that this patch looks good to me.
Got it. I will send a reroll for that.
Thanks for reviewing.
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware
2026-03-27 9:23 ` Burak Kaan Karaçay
@ 2026-03-28 12:51 ` Shreyansh Paliwal
0 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 12:51 UTC (permalink / raw)
To: Burak Kaan Karaçay; +Cc: git
On Fri, Mar 27, 2026 at 2:54 PM Burak Kaan Karaçay <bkkaracay@gmail.com> wrote:
>
> On Wed Mar 25, 2026 at 7:44 PM +03, Shreyansh Paliwal wrote:
> > -long get_files_ref_lock_timeout_ms(void)
> > +long get_files_ref_lock_timeout_ms(struct repository *repo)
> > {
> > static int configured = 0;
> >
> > @@ -998,7 +998,7 @@ long get_files_ref_lock_timeout_ms(void)
> > static int timeout_ms = 100;
> >
> > if (!configured) {
> > - repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
> > + repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
> > configured = 1;
> > }
> >
>
> Looks like the existing code uses static local variables for
> performance. They can behave unexpectedly in multi-repo cases.
>
> I think moving the config into 'repo-settings' should be considered. The
> config is already lazy-parsed, migrating it shouldn't be a problem.
Yup, I agree that would be a logical change.
We can take that up in a follow-up patch to this. Thanks.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 0/5] refs: reduce reliance on the_repository global state
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (4 preceding siblings ...)
2026-03-25 16:44 ` [PATCH 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
` (5 more replies)
5 siblings, 6 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
This series continues the effort to reduce reliance on the_repository
global state by making repository context explicit across the refs
subsystem. The patches focus on passing struct repository through various
ref helpers and backends, and replacing uses of global state such as
the_repository and the_hash_algo with the appropriate repository instance.
Patch 1/5: Making branch name helper functions (copy_branchname(),
check_branch_ref(), validate_branchname(), and validate_new_branchname())
repository-aware. (built on top of jw/object-name-bitset-to-enum)
Patch 2/5: Updating get_files_ref_lock_timeout_ms() to take a repository
and propagating it through files-backend, including callback paths.
Patch 3/5: Replacing uses of the_hash_algo in refs.c with the hash
algorithm from the appropriate repository.
Patch 4/5: Removing remaining uses of the_repository in reftable-backend.c
where a repository instance is already available.
Patch 5/5: Replacing the single instance of the_repository in
packed-backend.c, thus dropping the USE_THE_REPOSITORY_VARIABLE macro.
Shreyansh Paliwal (5):
refs: make branchname helpers repository aware
refs: make get_files_ref_lock_timeout_ms() repostory aware
refs: remove the_hash_algo global state
refs/reftable-backend: drop uses of the_repository
refs/packed-backend: use ref_store->repo instead of the_repository
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 27 ++++++++++++---------------
refs.h | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/packed-backend.c | 3 +--
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 6 +++---
13 files changed, 61 insertions(+), 55 deletions(-)
---
Changes in v2:
- Made struct repository the first argument in function parameters.
Range-diff against v1:
1: 68ca12412b ! 1: c0182252c4 refs: make branchname helpers repository aware
@@ branch.c: int read_branch_desc(struct strbuf *buf, const char *branch_name)
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref)
-+int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo)
++int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref)
{
- if (check_branch_ref(ref, name)) {
-+ if (check_branch_ref(ref, name, repo)) {
++ if (check_branch_ref(repo, ref, name)) {
int code = die_message(_("'%s' is not a valid branch name"), name);
advise_if_enabled(ADVICE_REF_SYNTAX,
_("See 'git help check-ref-format'"));
@@ branch.c: const char *branch_checked_out(const char *refname)
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
-+int validate_new_branchname(const char *name, struct strbuf *ref, int force,
-+ struct repository *repo)
++int validate_new_branchname(struct repository *repo, const char *name,
++ struct strbuf *ref, int force)
{
const char *path;
- if (!validate_branchname(name, ref))
-+ if (!validate_branchname(name, ref, repo))
++ if (!validate_branchname(repo, name, ref))
return 0;
if (!force)
@@ branch.c: void create_branch(struct repository *r,
if (clobber_head_ok ?
- validate_branchname(name, &ref) :
- validate_new_branchname(name, &ref, force)) {
-+ validate_branchname(name, &ref, r) :
-+ validate_new_branchname(name, &ref, force, r)) {
++ validate_branchname(r, name, &ref) :
++ validate_new_branchname(r, name, &ref, force)) {
forcing = 1;
}
@@ branch.h: const char *branch_checked_out(const char *refname);
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref);
-+int validate_branchname(const char *name, struct strbuf *ref, struct repository *repo);
++int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref);
/*
* Check if a branch 'name' can be created as a new branch; die otherwise.
@@ branch.h: int validate_branchname(const char *name, struct strbuf *ref);
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
-+int validate_new_branchname(const char *name, struct strbuf *ref, int force,
-+ struct repository *repo);
++int validate_new_branchname(struct repository *repo, const char *name,
++ struct strbuf *ref, int force);
/*
* Remove information about the merge state on the current
@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int fo
int flags = 0;
- copy_branchname(&bname, argv[i], allowed_interpret);
-+ copy_branchname(&bname, argv[i], allowed_interpret, the_repository);
++ copy_branchname(the_repository, &bname, argv[i], allowed_interpret);
free(name);
name = mkpathdup(fmt, bname.buf);
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
struct worktree **worktrees = get_worktrees();
- if (check_branch_ref(&oldref, oldname)) {
-+ if (check_branch_ref(&oldref, oldname, the_repository)) {
++ if (check_branch_ref(the_repository, &oldref, oldname)) {
/*
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const c
*/
if (!strcmp(oldname, newname))
- validate_branchname(newname, &newref);
-+ validate_branchname(newname, &newref, the_repository);
++ validate_branchname(the_repository, newname, &newref);
else
- validate_new_branchname(newname, &newref, force);
-+ validate_new_branchname(newname, &newref, force, the_repository);
++ validate_new_branchname(the_repository, newname, &newref, force);
reject_rebase_or_bisect_branch(worktrees, oldref.buf);
@@ builtin/branch.c: int cmd_branch(int argc,
branch_name = head;
} else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
++ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch_name = buf.buf;
} else {
die(_("cannot edit description of more than one branch"));
@@ builtin/branch.c: int cmd_branch(int argc,
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
++ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to set new upstream"));
@@ builtin/branch.c: int cmd_branch(int argc,
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL, the_repository);
++ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to unset upstream"));
@@ builtin/check-ref-format.c: static int check_ref_format_branch(const char *arg)
setup_git_directory_gently(&nongit);
- if (check_branch_ref(&sb, arg) ||
-+ if (check_branch_ref(&sb, arg, the_repository) ||
++ if (check_branch_ref(the_repository, &sb, arg) ||
!skip_prefix(sb.buf, "refs/heads/", &name))
die("'%s' is not a valid branch name", arg);
printf("%s\n", name);
@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
repo_get_oid_committish(the_repository, branch->name, &branch->oid);
- copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL, the_repository);
++ copy_branchname(the_repository, &buf, branch->name, INTERPRET_BRANCH_LOCAL);
if (strcmp(buf.buf, branch->name)) {
free(branch->name);
branch->name = xstrdup(buf.buf);
@@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const
if (opts->new_branch_force)
- opts->branch_exists = validate_branchname(opts->new_branch, &buf);
-+ opts->branch_exists = validate_branchname(opts->new_branch, &buf, the_repository);
++ opts->branch_exists = validate_branchname(the_repository, opts->new_branch, &buf);
else
opts->branch_exists =
- validate_new_branchname(opts->new_branch, &buf, 0);
-+ validate_new_branchname(opts->new_branch, &buf, 0, the_repository);
++ validate_new_branchname(the_repository, opts->new_branch, &buf, 0);
strbuf_release(&buf);
}
@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg)
int len, early;
- copy_branchname(&bname, remote, 0);
-+ copy_branchname(&bname, remote, 0, the_repository);
++ copy_branchname(the_repository, &bname, remote, 0);
remote = bname.buf;
oidclr(&branch_head, the_repository->hash_algo);
@@ builtin/worktree.c: static int make_worktree_orphan(const char * ref, const stru
struct child_process cp = CHILD_PROCESS_INIT;
- validate_new_branchname(ref, &symref, 0);
-+ validate_new_branchname(ref, &symref, 0, the_repository);
++ validate_new_branchname(the_repository, ref, &symref, 0);
strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
if (opts->quiet)
strvec_push(&cp.args, "--quiet");
@@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam
/* is 'refname' a branch or commit? */
- if (!opts->detach && !check_branch_ref(&symref, refname) &&
-+ if (!opts->detach && !check_branch_ref(&symref, refname, the_repository) &&
++ if (!opts->detach && !check_branch_ref(the_repository, &symref, refname) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
is_branch = 1;
if (!opts->force)
@@ builtin/worktree.c: static void print_preparing_worktree_line(int detach,
} else {
struct strbuf s = STRBUF_INIT;
- if (!detach && !check_branch_ref(&s, branch) &&
-+ if (!detach && !check_branch_ref(&s, branch, the_repository) &&
++ if (!detach && !check_branch_ref(the_repository, &s, branch) &&
refs_ref_exists(get_main_ref_store(the_repository), s.buf))
fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
branch);
@@ builtin/worktree.c: static char *dwim_branch(const char *path, char **new_branch
struct strbuf ref = STRBUF_INIT;
- branch_exists = !check_branch_ref(&ref, branchname) &&
-+ branch_exists = !check_branch_ref(&ref, branchname, the_repository) &&
++ branch_exists = !check_branch_ref(the_repository, &ref, branchname) &&
refs_ref_exists(get_main_ref_store(the_repository),
ref.buf);
strbuf_release(&ref);
@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
if (!opts.force &&
- !check_branch_ref(&symref, new_branch) &&
-+ !check_branch_ref(&symref, new_branch, the_repository) &&
++ !check_branch_ref(the_repository, &symref, new_branch) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
die_if_checked_out(symref.buf, 0);
strbuf_release(&symref);
## refs.c ##
@@ refs.c: static char *substitute_branch_name(struct repository *r,
+ return NULL;
}
- void copy_branchname(struct strbuf *sb, const char *name,
-- enum interpret_branch_kind allowed)
-+ enum interpret_branch_kind allowed,
-+ struct repository *repo)
+-void copy_branchname(struct strbuf *sb, const char *name,
++void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
+ enum interpret_branch_kind allowed)
{
int len = strlen(name);
struct interpret_branch_name_options options = {
@@ refs.c: void copy_branchname(struct strbuf *sb, const char *name,
}
-int check_branch_ref(struct strbuf *sb, const char *name)
-+int check_branch_ref(struct strbuf *sb, const char *name, struct repository *repo)
++int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
{
if (startup_info->have_repository)
- copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL, repo);
++ copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
else
strbuf_addstr(sb, name);
## refs.h ##
@@ refs.h: char *repo_default_branch_name(struct repository *r, int quiet);
+ * If "allowed" is non-zero, restrict the set of allowed expansions. See
* repo_interpret_branch_name() for details.
*/
- void copy_branchname(struct strbuf *sb, const char *name,
-- enum interpret_branch_kind allowed);
-+ enum interpret_branch_kind allowed,
-+ struct repository *repo);
+-void copy_branchname(struct strbuf *sb, const char *name,
++void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
+ enum interpret_branch_kind allowed);
/*
- * Like copy_branchname() above, but confirm that the result is
@@ refs.h: void copy_branchname(struct strbuf *sb, const char *name,
*
* The return value is "0" if the result is valid, and "-1" otherwise.
*/
-int check_branch_ref(struct strbuf *sb, const char *name);
-+int check_branch_ref(struct strbuf *sb, const char *name, struct repository *repo);
++int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name);
/*
* Similar for a tag name in refs/tags/.
2: 9bfbfc38b8 = 2: 857a8c40fe refs: make get_files_ref_lock_timeout_ms() repostory aware
3: 6f9854845e = 3: 46d5272aec refs: remove the_hash_algo global state
4: 1c02f780d1 = 4: 11aa886259 refs/reftable-backend: drop uses of the_repository
5: 33946503e1 = 5: c763732964 refs/packed-backend: use ref_store->repo instead of the_repository
--
2.53.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v2 1/5] refs: make branchname helpers repository aware
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 16:54 ` Tian Yuchen
2026-03-28 14:09 ` [PATCH v2 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
` (4 subsequent siblings)
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
copy_branchname() in refs.c relies on the_repository when calling
repo_interpret_branch_name(), introducing an implicit dependency on global
state. Add a struct repository parameter and use it instead.
Update check_branch_ref() to take a repository parameter as well, since it
calls copy_branchname(). Propagate this change to higher-level helpers
validate_branchname() and validate_new_branchname(), which also lack access
to a repository instance. Most callers of these helpers reside in builtin
code and already operate on the_repository, so pass it explicitly at those
call sites (builtin/checkout and builtin/worktree) otherwise pass struct
repository where available.
This makes branch name handling explicitly repository-aware and aligns with
ongoing efforts to remove reliance on global state. This change builds on
top of jw/object-name-bitset-to-enum (2026-03-18), which introduced the
enum interpret_branch_kind parameter to copy_branchname().
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 8 ++++----
refs.h | 4 ++--
9 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..63a9c0c238 100644
--- a/branch.c
+++ b/branch.c
@@ -370,16 +370,16 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref)
+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref)
{
- if (check_branch_ref(ref, name)) {
+ if (check_branch_ref(repo, ref, name)) {
int code = die_message(_("'%s' is not a valid branch name"), name);
advise_if_enabled(ADVICE_REF_SYNTAX,
_("See 'git help check-ref-format'"));
exit(code);
}
- return refs_ref_exists(get_main_ref_store(the_repository), ref->buf);
+ return refs_ref_exists(get_main_ref_store(repo), ref->buf);
}
static int initialized_checked_out_branches;
@@ -468,10 +468,11 @@ const char *branch_checked_out(const char *refname)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+int validate_new_branchname(struct repository *repo, const char *name,
+ struct strbuf *ref, int force)
{
const char *path;
- if (!validate_branchname(name, ref))
+ if (!validate_branchname(repo, name, ref))
return 0;
if (!force)
@@ -613,8 +614,8 @@ void create_branch(struct repository *r,
BUG("'clobber_head_ok' can only be used with 'force'");
if (clobber_head_ok ?
- validate_branchname(name, &ref) :
- validate_new_branchname(name, &ref, force)) {
+ validate_branchname(r, name, &ref) :
+ validate_new_branchname(r, name, &ref, force)) {
forcing = 1;
}
diff --git a/branch.h b/branch.h
index 3dc6e2a0ff..b41176ee7f 100644
--- a/branch.h
+++ b/branch.h
@@ -111,7 +111,7 @@ const char *branch_checked_out(const char *refname);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref);
+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref);
/*
* Check if a branch 'name' can be created as a new branch; die otherwise.
@@ -119,7 +119,8 @@ int validate_branchname(const char *name, struct strbuf *ref);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+int validate_new_branchname(struct repository *repo, const char *name,
+ struct strbuf *ref, int force);
/*
* Remove information about the merge state on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..ea4109f893 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -259,7 +259,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
char *target = NULL;
int flags = 0;
- copy_branchname(&bname, argv[i], allowed_interpret);
+ copy_branchname(the_repository, &bname, argv[i], allowed_interpret);
free(name);
name = mkpathdup(fmt, bname.buf);
@@ -581,7 +581,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
int recovery = 0, oldref_usage = 0;
struct worktree **worktrees = get_worktrees();
- if (check_branch_ref(&oldref, oldname)) {
+ if (check_branch_ref(the_repository, &oldref, oldname)) {
/*
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
@@ -619,9 +619,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
* cause the worktree to become inconsistent with HEAD, so allow it.
*/
if (!strcmp(oldname, newname))
- validate_branchname(newname, &newref);
+ validate_branchname(the_repository, newname, &newref);
else
- validate_new_branchname(newname, &newref, force);
+ validate_new_branchname(the_repository, newname, &newref, force);
reject_rebase_or_bisect_branch(worktrees, oldref.buf);
@@ -898,7 +898,7 @@ int cmd_branch(int argc,
die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch_name = buf.buf;
} else {
die(_("cannot edit description of more than one branch"));
@@ -941,7 +941,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to set new upstream"));
@@ -971,7 +971,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to unset upstream"));
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 5d80afeec0..28e77102d3 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -5,6 +5,7 @@
#include "refs.h"
#include "setup.h"
#include "strbuf.h"
+#include "repository.c"
static const char builtin_check_ref_format_usage[] =
"git check-ref-format [--normalize] [<options>] <refname>\n"
@@ -42,7 +43,7 @@ static int check_ref_format_branch(const char *arg)
int nongit;
setup_git_directory_gently(&nongit);
- if (check_branch_ref(&sb, arg) ||
+ if (check_branch_ref(the_repository, &sb, arg) ||
!skip_prefix(sb.buf, "refs/heads/", &name))
die("'%s' is not a valid branch name", arg);
printf("%s\n", name);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e031e61886..93ad894dc2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -743,7 +743,7 @@ static void setup_branch_path(struct branch_info *branch)
&branch->oid, &branch->refname, 0))
repo_get_oid_committish(the_repository, branch->name, &branch->oid);
- copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, branch->name, INTERPRET_BRANCH_LOCAL);
if (strcmp(buf.buf, branch->name)) {
free(branch->name);
branch->name = xstrdup(buf.buf);
@@ -2014,10 +2014,10 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
struct strbuf buf = STRBUF_INIT;
if (opts->new_branch_force)
- opts->branch_exists = validate_branchname(opts->new_branch, &buf);
+ opts->branch_exists = validate_branchname(the_repository, opts->new_branch, &buf);
else
opts->branch_exists =
- validate_new_branchname(opts->new_branch, &buf, 0);
+ validate_new_branchname(the_repository, opts->new_branch, &buf, 0);
strbuf_release(&buf);
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 2cbce56f8d..3f4b9dc47a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -552,7 +552,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
char *found_ref = NULL;
int len, early;
- copy_branchname(&bname, remote, 0);
+ copy_branchname(the_repository, &bname, remote, 0);
remote = bname.buf;
oidclr(&branch_head, the_repository->hash_algo);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4035b1cb06..a5e116d8f9 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -415,7 +415,7 @@ static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
struct strbuf symref = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
- validate_new_branchname(ref, &symref, 0);
+ validate_new_branchname(the_repository, ref, &symref, 0);
strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
if (opts->quiet)
strvec_push(&cp.args, "--quiet");
@@ -481,7 +481,7 @@ static int add_worktree(const char *path, const char *refname,
worktrees = NULL;
/* is 'refname' a branch or commit? */
- if (!opts->detach && !check_branch_ref(&symref, refname) &&
+ if (!opts->detach && !check_branch_ref(the_repository, &symref, refname) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
is_branch = 1;
if (!opts->force)
@@ -649,7 +649,7 @@ static void print_preparing_worktree_line(int detach,
fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
} else {
struct strbuf s = STRBUF_INIT;
- if (!detach && !check_branch_ref(&s, branch) &&
+ if (!detach && !check_branch_ref(the_repository, &s, branch) &&
refs_ref_exists(get_main_ref_store(the_repository), s.buf))
fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
branch);
@@ -788,7 +788,7 @@ static char *dwim_branch(const char *path, char **new_branch)
char *branchname = xstrndup(s, n);
struct strbuf ref = STRBUF_INIT;
- branch_exists = !check_branch_ref(&ref, branchname) &&
+ branch_exists = !check_branch_ref(the_repository, &ref, branchname) &&
refs_ref_exists(get_main_ref_store(the_repository),
ref.buf);
strbuf_release(&ref);
@@ -885,7 +885,7 @@ static int add(int ac, const char **av, const char *prefix,
new_branch = new_branch_force;
if (!opts.force &&
- !check_branch_ref(&symref, new_branch) &&
+ !check_branch_ref(the_repository, &symref, new_branch) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
die_if_checked_out(symref.buf, 0);
strbuf_release(&symref);
diff --git a/refs.c b/refs.c
index 685a0c247b..5cdc8858c5 100644
--- a/refs.c
+++ b/refs.c
@@ -743,14 +743,14 @@ static char *substitute_branch_name(struct repository *r,
return NULL;
}
-void copy_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
enum interpret_branch_kind allowed)
{
int len = strlen(name);
struct interpret_branch_name_options options = {
.allowed = allowed
};
- int used = repo_interpret_branch_name(the_repository, name, len, sb,
+ int used = repo_interpret_branch_name(repo, name, len, sb,
&options);
if (used < 0)
@@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
strbuf_add(sb, name + used, len - used);
}
-int check_branch_ref(struct strbuf *sb, const char *name)
+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
{
if (startup_info->have_repository)
- copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
else
strbuf_addstr(sb, name);
diff --git a/refs.h b/refs.h
index d65de6ab5f..5407a4c4d6 100644
--- a/refs.h
+++ b/refs.h
@@ -225,7 +225,7 @@ char *repo_default_branch_name(struct repository *r, int quiet);
* If "allowed" is non-zero, restrict the set of allowed expansions. See
* repo_interpret_branch_name() for details.
*/
-void copy_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
enum interpret_branch_kind allowed);
/*
@@ -234,7 +234,7 @@ void copy_branchname(struct strbuf *sb, const char *name,
*
* The return value is "0" if the result is valid, and "-1" otherwise.
*/
-int check_branch_ref(struct strbuf *sb, const char *name);
+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name);
/*
* Similar for a tag name in refs/tags/.
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
` (3 subsequent siblings)
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.
Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
This removes reliance on the_repository global and makes the timeout lookup
operate on the correct repository context.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5cdc8858c5..2f8c8427cd 100644
--- a/refs.c
+++ b/refs.c
@@ -989,7 +989,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,
return REF_WORKTREE_SHARED;
}
-long get_files_ref_lock_timeout_ms(void)
+long get_files_ref_lock_timeout_ms(struct repository *repo)
{
static int configured = 0;
@@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
static int timeout_ms = 100;
if (!configured) {
- repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
+ repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
configured = 1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ce0d57478..ee8dd771a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
if (hold_lock_file_for_update_timeout(
&lock->lk, ref_file.buf, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0) {
+ get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) {
int myerr = errno;
errno = 0;
if (myerr == ENOENT && --attempts_remaining > 0) {
@@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
+struct create_reflock_cb {
+ struct lock_file *lk;
+ struct repository *repo;
+};
+
static int create_reflock(const char *path, void *cb)
{
- struct lock_file *lk = cb;
-
+ struct create_reflock_cb *data = cb;
return hold_lock_file_for_update_timeout(
- lk, path, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
+ data->lk, path, LOCK_NO_DEREF,
+ get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
}
/*
@@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ struct create_reflock_cb cb_data;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
lock->ref_name = xstrdup(refname);
lock->count = 1;
+ cb_data.lk = &lock->lk;
+ cb_data.repo = refs->base.repo;
- if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) {
+ if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) {
unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..e4cfd9e19e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,7 +43,7 @@ struct ref_transaction;
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
*/
-long get_files_ref_lock_timeout_ms(void);
+long get_files_ref_lock_timeout_ms(struct repository *repo);
/*
* Return true iff refname is minimally safe. "Safe" here means that
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 3/5] refs: remove the_hash_algo global state
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 17:03 ` Tian Yuchen
2026-03-28 14:09 ` [PATCH v2 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
` (2 subsequent siblings)
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.
This removes implicit reliance on global state. With no remaining uses of
the_repository in this file, drop USE_THE_REPOSITORY_VARIABLE and the
dependency on environment.h.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 2f8c8427cd..54ca87eda9 100644
--- a/refs.c
+++ b/refs.c
@@ -2,13 +2,10 @@
* The backend-independent part of the reference module.
*/
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "abspath.h"
#include "advice.h"
#include "config.h"
-#include "environment.h"
#include "strmap.h"
#include "gettext.h"
#include "hex.h"
@@ -1472,7 +1469,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
return 1;
}
return ref_transaction_update(transaction, refname, new_oid,
- null_oid(the_hash_algo), new_target, NULL, flags,
+ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
msg, err);
}
@@ -1491,7 +1488,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (old_target && !(flags & REF_NO_DEREF))
BUG("delete cannot operate on symrefs with deref mode");
return ref_transaction_update(transaction, refname,
- null_oid(the_hash_algo), old_oid,
+ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
NULL, old_target, flags,
msg, err);
}
@@ -2379,7 +2376,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
subrepo = xmalloc(sizeof(*subrepo));
if (repo_submodule_init(subrepo, repo, submodule,
- null_oid(the_hash_algo))) {
+ null_oid(repo->hash_algo))) {
free(subrepo);
goto done;
}
@@ -2571,14 +2568,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
strbuf_reset(buf);
if (!(update->flags & REF_HAVE_OLD))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->old_target)
strbuf_addf(buf, "ref:%s ", update->old_target);
else
strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
if (!(update->flags & REF_HAVE_NEW))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
@@ -3153,7 +3150,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(data->transaction->ref_store->repo->hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 4/5] refs/reftable-backend: drop uses of the_repository
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (2 preceding siblings ...)
2026-03-28 14:09 ` [PATCH v2 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.
Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.
Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663..7c8a992fcb 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo,
default:
BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
}
- refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask);
+ refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask);
refs->write_options.disable_auto_compact =
!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
refs->write_options.lock_timeout_ms = 100;
refs->write_options.fsync = reftable_be_fsync;
- repo_config(the_repository, reftable_be_config, &refs->write_options);
+ repo_config(repo, reftable_be_config, &refs->write_options);
/*
* It is somewhat unfortunate that we have to mirror the default block
@@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store,
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
- safe_create_dir(the_repository, sb.buf, 1);
+ safe_create_dir(ref_store->repo, sb.buf, 1);
strbuf_reset(&sb);
strbuf_release(&sb);
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead of the_repository
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (3 preceding siblings ...)
2026-03-28 14:09 ` [PATCH v2 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-03-28 14:09 ` Shreyansh Paliwal
2026-03-28 17:08 ` Tian Yuchen
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-28 14:09 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
In refs/packed-backend.c, repo_config_get_int() is called using the global
the_repository, even though a repository instance is available via struct
ref_store.
Replace the use of the_repository with ref_store->repo to make the code
explicitly repository-aware. With no remaining users of the_repository in
this file, drop the USE_THE_REPOSITORY_VARIABLE macro.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/packed-backend.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 23ed62984b..ebc10dab4d 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "../git-compat-util.h"
@@ -1223,7 +1222,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
static int timeout_value = 1000;
if (!timeout_configured) {
- repo_config_get_int(the_repository, "core.packedrefstimeout", &timeout_value);
+ repo_config_get_int(ref_store->repo, "core.packedrefstimeout", &timeout_value);
timeout_configured = 1;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 1/5] refs: make branchname helpers repository aware
2026-03-28 14:09 ` [PATCH v2 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
@ 2026-03-28 16:54 ` Tian Yuchen
2026-03-29 9:55 ` Shreyansh Paliwal
0 siblings, 1 reply; 48+ messages in thread
From: Tian Yuchen @ 2026-03-28 16:54 UTC (permalink / raw)
To: Shreyansh Paliwal, git; +Cc: ps
Hi Shreyansh,
On 3/28/26 22:09, Shreyansh Paliwal wrote:
> @@ -5,6 +5,7 @@
> #include "refs.h"
> #include "setup.h"
> #include "strbuf.h"
> +#include "repository.c"
I'm surprised that it doesn't cause any errors. Or maybe you haven't
build it yet?
---
> -int check_branch_ref(struct strbuf *sb, const char *name)
> +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> {
> if (startup_info->have_repository)
> - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> else
> strbuf_addstr(sb, name);
>
startup_info itself is a global variable, isn't it?
I think a more appropriate approach is something like:
if (repo && repo->gitdir)
copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
I dunno, just a thought.
---
Regards, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 3/5] refs: remove the_hash_algo global state
2026-03-28 14:09 ` [PATCH v2 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-03-28 17:03 ` Tian Yuchen
0 siblings, 0 replies; 48+ messages in thread
From: Tian Yuchen @ 2026-03-28 17:03 UTC (permalink / raw)
To: Shreyansh Paliwal, git; +Cc: ps
On 3/28/26 22:09, Shreyansh Paliwal wrote:
> @@ -3153,7 +3150,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
> if (ret < 0)
> goto done;
>
> - ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
> + ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(data->transaction->ref_store->repo->hash_algo),
> symref_target.buf, NULL,
> REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
> if (ret < 0)
I have no objection to the logic itself at all, but this line of code
looks too long. ;)
If I were you, I would write:
const struct git_hash_algo *algo =
data->transaction->ref_store->repo->hash_algo;
then:
null_oid(algo)
Just wanted to remind you to pay attention to readability. There’s no
need to reply to this email.
Thanks, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead of the_repository
2026-03-28 14:09 ` [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
@ 2026-03-28 17:08 ` Tian Yuchen
2026-03-29 9:54 ` Shreyansh Paliwal
0 siblings, 1 reply; 48+ messages in thread
From: Tian Yuchen @ 2026-03-28 17:08 UTC (permalink / raw)
To: Shreyansh Paliwal, git; +Cc: ps
On 3/28/26 22:09, Shreyansh Paliwal wrote:
> In refs/packed-backend.c, repo_config_get_int() is called using the global
> the_repository, even though a repository instance is available via struct
> ref_store.
>
> Replace the use of the_repository with ref_store->repo to make the code
> explicitly repository-aware. With no remaining users of the_repository in
> this file, drop the USE_THE_REPOSITORY_VARIABLE macro.
>
> Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> ---
> refs/packed-backend.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> index 23ed62984b..ebc10dab4d 100644
> --- a/refs/packed-backend.c
> +++ b/refs/packed-backend.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> #define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "../git-compat-util.h"
> @@ -1223,7 +1222,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
> static int timeout_value = 1000;
Burak already pointed out the issue with the static keyword in patch 3
of v1. Notice that it's here as well.
>
> if (!timeout_configured) {
> - repo_config_get_int(the_repository, "core.packedrefstimeout", &timeout_value);
> + repo_config_get_int(ref_store->repo, "core.packedrefstimeout", &timeout_value);
> timeout_configured = 1;
> }
>
Regards, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead of the_repository
2026-03-28 17:08 ` Tian Yuchen
@ 2026-03-29 9:54 ` Shreyansh Paliwal
0 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 9:54 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, ps
On Sat, Mar 28, 2026 at 10:38 PM Tian Yuchen <a3205153416@gmail.com> wrote:
>
> On 3/28/26 22:09, Shreyansh Paliwal wrote:
> > In refs/packed-backend.c, repo_config_get_int() is called using the global
> > the_repository, even though a repository instance is available via struct
> > ref_store.
> >
> > Replace the use of the_repository with ref_store->repo to make the code
> > explicitly repository-aware. With no remaining users of the_repository in
> > this file, drop the USE_THE_REPOSITORY_VARIABLE macro.
> >
> > Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> > ---
> > refs/packed-backend.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/refs/packed-backend.c b/refs/packed-backend.c
> > index 23ed62984b..ebc10dab4d 100644
> > --- a/refs/packed-backend.c
> > +++ b/refs/packed-backend.c
> > @@ -1,4 +1,3 @@
> > -#define USE_THE_REPOSITORY_VARIABLE
> > #define DISABLE_SIGN_COMPARE_WARNINGS
> >
> > #include "../git-compat-util.h"
> > @@ -1223,7 +1222,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
> > static int timeout_value = 1000;
>
> Burak already pointed out the issue with the static keyword in patch 3
> of v1. Notice that it's here as well.
Hi Yuchen,
I have acknowledged this in a previous reply to Burak. As stated there,
this is a valid issue and would require moving the config into
repo-settings struct.
In this patch, I focused on removing the dependency on
'the_repository' while preserving existing behavior. Global state
removal and multi-repo correctness is an incremental process,
so I would prefer to handle this in a follow-up change.
I'll also update the patch title in the next version to better reflect
the scope of the change.
Btw, I forgot to cc the other recipients previously, so it might have ended
in your inbox twice :)
Thanks,
Shreyansh
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v2 1/5] refs: make branchname helpers repository aware
2026-03-28 16:54 ` Tian Yuchen
@ 2026-03-29 9:55 ` Shreyansh Paliwal
2026-03-29 15:37 ` Tian Yuchen
0 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 9:55 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, ps
On Sat, Mar 28, 2026 at 10:24 PM Tian Yuchen <a3205153416@gmail.com> wrote:
>
> Hi Shreyansh,
>
> On 3/28/26 22:09, Shreyansh Paliwal wrote:
>
>
> > @@ -5,6 +5,7 @@
> > #include "refs.h"
> > #include "setup.h"
> > #include "strbuf.h"
> > +#include "repository.c"
>
> I'm surprised that it doesn't cause any errors. Or maybe you haven't
> build it yet?
>
Thanks for pointing this out. Apparently it didn't raise any errors in the
build or test suite. Will send a reroll.
>
> ---
>
>
> > -int check_branch_ref(struct strbuf *sb, const char *name)
> > +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> > {
> > if (startup_info->have_repository)
> > - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> > + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> > else
> > strbuf_addstr(sb, name);
> >
>
> startup_info itself is a global variable, isn't it?
>
> I think a more appropriate approach is something like:
>
> if (repo && repo->gitdir)
> copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
>
> I dunno, just a thought.
I hadn’t considered this as it is outside my scope of this patch series.
While the change makes sense, I am not very sure whether it could cause
any behavioral change or not.
Thanks.
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 0/5] replace the_repository with local repository instances
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
` (4 preceding siblings ...)
2026-03-28 14:09 ` [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers Shreyansh Paliwal
` (5 more replies)
5 siblings, 6 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
This series continues the effort to reduce reliance on the_repository
global state by making repository context explicit across the refs
subsystem. The patches focus on passing struct repository through various
ref helpers and backends, and replacing uses of global state such as
the_repository and the_hash_algo with the appropriate repository instance.
Patch 1/5: Making branch name helper functions (copy_branchname(),
check_branch_ref(), validate_branchname(), and validate_new_branchname())
repository-aware. (built on top of jw/object-name-bitset-to-enum)
Patch 2/5: Updating get_files_ref_lock_timeout_ms() to take a repository
and propagating it through files-backend, including callback paths.
Patch 3/5: Replacing uses of the_hash_algo in refs.c with the hash
algorithm from the appropriate repository.
Patch 4/5: Removing remaining uses of the_repository in reftable-backend.c
where a repository instance is already available.
Patch 5/5: Replacing the single instance of the_repository in
packed-backend.c, thus dropping the USE_THE_REPOSITORY_VARIABLE macro.
Shreyansh Paliwal (5):
refs: add struct repository parameter to branchname helpers
refs: add struct repository parameter in
get_files_ref_lock_timeout_ms()
refs: remove the_hash_algo global state
refs/reftable-backend: drop uses of the_repository
refs/packed-backend: use ref_store->repo instead of the_repository
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 28 +++++++++++++---------------
refs.h | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/packed-backend.c | 3 +--
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 6 +++---
13 files changed, 62 insertions(+), 55 deletions(-)
---
Changes in v3:
- Fixed an import
- better readability in patch 3/5
Changes in v2:
- Made struct repository the first argument in function parameters.
Range-diff against v2:
1: c0182252c4 ! 1: 5844440b73 refs: make branchname helpers repository aware
@@ Metadata
Author: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
## Commit message ##
- refs: make branchname helpers repository aware
+ refs: add struct repository parameter to branchname helpers
copy_branchname() in refs.c relies on the_repository when calling
repo_interpret_branch_name(), introducing an implicit dependency on global
@@ builtin/branch.c: int cmd_branch(int argc,
## builtin/check-ref-format.c ##
@@
+ /*
+ * GIT - The information manager from hell
+ */
++#define USE_THE_REPOSITORY_VARIABLE
+ #include "builtin.h"
#include "refs.h"
#include "setup.h"
- #include "strbuf.h"
-+#include "repository.c"
-
- static const char builtin_check_ref_format_usage[] =
- "git check-ref-format [--normalize] [<options>] <refname>\n"
@@ builtin/check-ref-format.c: static int check_ref_format_branch(const char *arg)
int nongit;
2: 857a8c40fe ! 2: f7a9ea4204 refs: make get_files_ref_lock_timeout_ms() repostory aware
@@ Metadata
Author: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
## Commit message ##
- refs: make get_files_ref_lock_timeout_ms() repostory aware
+ refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
@@ Commit message
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
- This removes reliance on the_repository global and makes the timeout lookup
- operate on the correct repository context.
+ This reduces reliance on the_repository global.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
3: 46d5272aec ! 3: 00dba1a96a refs: remove the_hash_algo global state
@@ refs.c: static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, v
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
+@@ refs.c: struct migration_data {
+ static int migrate_one_ref(const struct reference *ref, void *cb_data)
+ {
+ struct migration_data *data = cb_data;
++ const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo;
+ struct strbuf symref_target = STRBUF_INIT;
+ int ret;
+
@@ refs.c: static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
-+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(data->transaction->ref_store->repo->hash_algo),
++ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
4: 11aa886259 = 4: 5653c418e1 refs/reftable-backend: drop uses of the_repository
5: c763732964 = 5: 18c1c67083 refs/packed-backend: use ref_store->repo instead of the_repository
--
2.53.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-04-02 7:27 ` Patrick Steinhardt
2026-03-29 10:16 ` [PATCH v3 2/5] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
` (4 subsequent siblings)
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
copy_branchname() in refs.c relies on the_repository when calling
repo_interpret_branch_name(), introducing an implicit dependency on global
state. Add a struct repository parameter and use it instead.
Update check_branch_ref() to take a repository parameter as well, since it
calls copy_branchname(). Propagate this change to higher-level helpers
validate_branchname() and validate_new_branchname(), which also lack access
to a repository instance. Most callers of these helpers reside in builtin
code and already operate on the_repository, so pass it explicitly at those
call sites (builtin/checkout and builtin/worktree) otherwise pass struct
repository where available.
This makes branch name handling explicitly repository-aware and aligns with
ongoing efforts to remove reliance on global state. This change builds on
top of jw/object-name-bitset-to-enum (2026-03-18), which introduced the
enum interpret_branch_kind parameter to copy_branchname().
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
branch.c | 15 ++++++++-------
branch.h | 5 +++--
builtin/branch.c | 14 +++++++-------
builtin/check-ref-format.c | 3 ++-
builtin/checkout.c | 6 +++---
builtin/merge.c | 2 +-
builtin/worktree.c | 10 +++++-----
refs.c | 8 ++++----
refs.h | 4 ++--
9 files changed, 35 insertions(+), 32 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..63a9c0c238 100644
--- a/branch.c
+++ b/branch.c
@@ -370,16 +370,16 @@ int read_branch_desc(struct strbuf *buf, const char *branch_name)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref)
+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref)
{
- if (check_branch_ref(ref, name)) {
+ if (check_branch_ref(repo, ref, name)) {
int code = die_message(_("'%s' is not a valid branch name"), name);
advise_if_enabled(ADVICE_REF_SYNTAX,
_("See 'git help check-ref-format'"));
exit(code);
}
- return refs_ref_exists(get_main_ref_store(the_repository), ref->buf);
+ return refs_ref_exists(get_main_ref_store(repo), ref->buf);
}
static int initialized_checked_out_branches;
@@ -468,10 +468,11 @@ const char *branch_checked_out(const char *refname)
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force)
+int validate_new_branchname(struct repository *repo, const char *name,
+ struct strbuf *ref, int force)
{
const char *path;
- if (!validate_branchname(name, ref))
+ if (!validate_branchname(repo, name, ref))
return 0;
if (!force)
@@ -613,8 +614,8 @@ void create_branch(struct repository *r,
BUG("'clobber_head_ok' can only be used with 'force'");
if (clobber_head_ok ?
- validate_branchname(name, &ref) :
- validate_new_branchname(name, &ref, force)) {
+ validate_branchname(r, name, &ref) :
+ validate_new_branchname(r, name, &ref, force)) {
forcing = 1;
}
diff --git a/branch.h b/branch.h
index 3dc6e2a0ff..b41176ee7f 100644
--- a/branch.h
+++ b/branch.h
@@ -111,7 +111,7 @@ const char *branch_checked_out(const char *refname);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_branchname(const char *name, struct strbuf *ref);
+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref);
/*
* Check if a branch 'name' can be created as a new branch; die otherwise.
@@ -119,7 +119,8 @@ int validate_branchname(const char *name, struct strbuf *ref);
* Return 1 if the named branch already exists; return 0 otherwise.
* Fill ref with the full refname for the branch.
*/
-int validate_new_branchname(const char *name, struct strbuf *ref, int force);
+int validate_new_branchname(struct repository *repo, const char *name,
+ struct strbuf *ref, int force);
/*
* Remove information about the merge state on the current
diff --git a/builtin/branch.c b/builtin/branch.c
index 1572a4f9ef..ea4109f893 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -259,7 +259,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
char *target = NULL;
int flags = 0;
- copy_branchname(&bname, argv[i], allowed_interpret);
+ copy_branchname(the_repository, &bname, argv[i], allowed_interpret);
free(name);
name = mkpathdup(fmt, bname.buf);
@@ -581,7 +581,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
int recovery = 0, oldref_usage = 0;
struct worktree **worktrees = get_worktrees();
- if (check_branch_ref(&oldref, oldname)) {
+ if (check_branch_ref(the_repository, &oldref, oldname)) {
/*
* Bad name --- this could be an attempt to rename a
* ref that we used to allow to be created by accident.
@@ -619,9 +619,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
* cause the worktree to become inconsistent with HEAD, so allow it.
*/
if (!strcmp(oldname, newname))
- validate_branchname(newname, &newref);
+ validate_branchname(the_repository, newname, &newref);
else
- validate_new_branchname(newname, &newref, force);
+ validate_new_branchname(the_repository, newname, &newref, force);
reject_rebase_or_bisect_branch(worktrees, oldref.buf);
@@ -898,7 +898,7 @@ int cmd_branch(int argc,
die(_("cannot give description to detached HEAD"));
branch_name = head;
} else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch_name = buf.buf;
} else {
die(_("cannot edit description of more than one branch"));
@@ -941,7 +941,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to set new upstream"));
@@ -971,7 +971,7 @@ int cmd_branch(int argc,
if (!argc)
branch = branch_get(NULL);
else if (argc == 1) {
- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
branch = branch_get(buf.buf);
} else
die(_("too many arguments to unset upstream"));
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 5d80afeec0..b7ba098363 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -1,6 +1,7 @@
/*
* GIT - The information manager from hell
*/
+#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "refs.h"
#include "setup.h"
@@ -42,7 +43,7 @@ static int check_ref_format_branch(const char *arg)
int nongit;
setup_git_directory_gently(&nongit);
- if (check_branch_ref(&sb, arg) ||
+ if (check_branch_ref(the_repository, &sb, arg) ||
!skip_prefix(sb.buf, "refs/heads/", &name))
die("'%s' is not a valid branch name", arg);
printf("%s\n", name);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e031e61886..93ad894dc2 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -743,7 +743,7 @@ static void setup_branch_path(struct branch_info *branch)
&branch->oid, &branch->refname, 0))
repo_get_oid_committish(the_repository, branch->name, &branch->oid);
- copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(the_repository, &buf, branch->name, INTERPRET_BRANCH_LOCAL);
if (strcmp(buf.buf, branch->name)) {
free(branch->name);
branch->name = xstrdup(buf.buf);
@@ -2014,10 +2014,10 @@ static int checkout_main(int argc, const char **argv, const char *prefix,
struct strbuf buf = STRBUF_INIT;
if (opts->new_branch_force)
- opts->branch_exists = validate_branchname(opts->new_branch, &buf);
+ opts->branch_exists = validate_branchname(the_repository, opts->new_branch, &buf);
else
opts->branch_exists =
- validate_new_branchname(opts->new_branch, &buf, 0);
+ validate_new_branchname(the_repository, opts->new_branch, &buf, 0);
strbuf_release(&buf);
}
diff --git a/builtin/merge.c b/builtin/merge.c
index 2cbce56f8d..3f4b9dc47a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -552,7 +552,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
char *found_ref = NULL;
int len, early;
- copy_branchname(&bname, remote, 0);
+ copy_branchname(the_repository, &bname, remote, 0);
remote = bname.buf;
oidclr(&branch_head, the_repository->hash_algo);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 4035b1cb06..a5e116d8f9 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -415,7 +415,7 @@ static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
struct strbuf symref = STRBUF_INIT;
struct child_process cp = CHILD_PROCESS_INIT;
- validate_new_branchname(ref, &symref, 0);
+ validate_new_branchname(the_repository, ref, &symref, 0);
strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
if (opts->quiet)
strvec_push(&cp.args, "--quiet");
@@ -481,7 +481,7 @@ static int add_worktree(const char *path, const char *refname,
worktrees = NULL;
/* is 'refname' a branch or commit? */
- if (!opts->detach && !check_branch_ref(&symref, refname) &&
+ if (!opts->detach && !check_branch_ref(the_repository, &symref, refname) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
is_branch = 1;
if (!opts->force)
@@ -649,7 +649,7 @@ static void print_preparing_worktree_line(int detach,
fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
} else {
struct strbuf s = STRBUF_INIT;
- if (!detach && !check_branch_ref(&s, branch) &&
+ if (!detach && !check_branch_ref(the_repository, &s, branch) &&
refs_ref_exists(get_main_ref_store(the_repository), s.buf))
fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
branch);
@@ -788,7 +788,7 @@ static char *dwim_branch(const char *path, char **new_branch)
char *branchname = xstrndup(s, n);
struct strbuf ref = STRBUF_INIT;
- branch_exists = !check_branch_ref(&ref, branchname) &&
+ branch_exists = !check_branch_ref(the_repository, &ref, branchname) &&
refs_ref_exists(get_main_ref_store(the_repository),
ref.buf);
strbuf_release(&ref);
@@ -885,7 +885,7 @@ static int add(int ac, const char **av, const char *prefix,
new_branch = new_branch_force;
if (!opts.force &&
- !check_branch_ref(&symref, new_branch) &&
+ !check_branch_ref(the_repository, &symref, new_branch) &&
refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
die_if_checked_out(symref.buf, 0);
strbuf_release(&symref);
diff --git a/refs.c b/refs.c
index 685a0c247b..5cdc8858c5 100644
--- a/refs.c
+++ b/refs.c
@@ -743,14 +743,14 @@ static char *substitute_branch_name(struct repository *r,
return NULL;
}
-void copy_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
enum interpret_branch_kind allowed)
{
int len = strlen(name);
struct interpret_branch_name_options options = {
.allowed = allowed
};
- int used = repo_interpret_branch_name(the_repository, name, len, sb,
+ int used = repo_interpret_branch_name(repo, name, len, sb,
&options);
if (used < 0)
@@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
strbuf_add(sb, name + used, len - used);
}
-int check_branch_ref(struct strbuf *sb, const char *name)
+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
{
if (startup_info->have_repository)
- copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
else
strbuf_addstr(sb, name);
diff --git a/refs.h b/refs.h
index d65de6ab5f..5407a4c4d6 100644
--- a/refs.h
+++ b/refs.h
@@ -225,7 +225,7 @@ char *repo_default_branch_name(struct repository *r, int quiet);
* If "allowed" is non-zero, restrict the set of allowed expansions. See
* repo_interpret_branch_name() for details.
*/
-void copy_branchname(struct strbuf *sb, const char *name,
+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
enum interpret_branch_kind allowed);
/*
@@ -234,7 +234,7 @@ void copy_branchname(struct strbuf *sb, const char *name,
*
* The return value is "0" if the result is valid, and "-1" otherwise.
*/
-int check_branch_ref(struct strbuf *sb, const char *name);
+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name);
/*
* Similar for a tag name in refs/tags/.
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 2/5] refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
` (3 subsequent siblings)
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.
Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
This reduces reliance on the_repository global.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5cdc8858c5..2f8c8427cd 100644
--- a/refs.c
+++ b/refs.c
@@ -989,7 +989,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,
return REF_WORKTREE_SHARED;
}
-long get_files_ref_lock_timeout_ms(void)
+long get_files_ref_lock_timeout_ms(struct repository *repo)
{
static int configured = 0;
@@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
static int timeout_ms = 100;
if (!configured) {
- repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
+ repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
configured = 1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ce0d57478..ee8dd771a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
if (hold_lock_file_for_update_timeout(
&lock->lk, ref_file.buf, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0) {
+ get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) {
int myerr = errno;
errno = 0;
if (myerr == ENOENT && --attempts_remaining > 0) {
@@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
+struct create_reflock_cb {
+ struct lock_file *lk;
+ struct repository *repo;
+};
+
static int create_reflock(const char *path, void *cb)
{
- struct lock_file *lk = cb;
-
+ struct create_reflock_cb *data = cb;
return hold_lock_file_for_update_timeout(
- lk, path, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
+ data->lk, path, LOCK_NO_DEREF,
+ get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
}
/*
@@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ struct create_reflock_cb cb_data;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
lock->ref_name = xstrdup(refname);
lock->count = 1;
+ cb_data.lk = &lock->lk;
+ cb_data.repo = refs->base.repo;
- if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) {
+ if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) {
unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..e4cfd9e19e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,7 +43,7 @@ struct ref_transaction;
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
*/
-long get_files_ref_lock_timeout_ms(void);
+long get_files_ref_lock_timeout_ms(struct repository *repo);
/*
* Return true iff refname is minimally safe. "Safe" here means that
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 3/5] refs: remove the_hash_algo global state
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 2/5] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
` (2 subsequent siblings)
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.
This removes implicit reliance on global state. With no remaining uses of
the_repository in this file, drop USE_THE_REPOSITORY_VARIABLE and the
dependency on environment.h.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 2f8c8427cd..d3abce0318 100644
--- a/refs.c
+++ b/refs.c
@@ -2,13 +2,10 @@
* The backend-independent part of the reference module.
*/
-#define USE_THE_REPOSITORY_VARIABLE
-
#include "git-compat-util.h"
#include "abspath.h"
#include "advice.h"
#include "config.h"
-#include "environment.h"
#include "strmap.h"
#include "gettext.h"
#include "hex.h"
@@ -1472,7 +1469,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
return 1;
}
return ref_transaction_update(transaction, refname, new_oid,
- null_oid(the_hash_algo), new_target, NULL, flags,
+ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
msg, err);
}
@@ -1491,7 +1488,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (old_target && !(flags & REF_NO_DEREF))
BUG("delete cannot operate on symrefs with deref mode");
return ref_transaction_update(transaction, refname,
- null_oid(the_hash_algo), old_oid,
+ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
NULL, old_target, flags,
msg, err);
}
@@ -2379,7 +2376,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
subrepo = xmalloc(sizeof(*subrepo));
if (repo_submodule_init(subrepo, repo, submodule,
- null_oid(the_hash_algo))) {
+ null_oid(repo->hash_algo))) {
free(subrepo);
goto done;
}
@@ -2571,14 +2568,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
strbuf_reset(buf);
if (!(update->flags & REF_HAVE_OLD))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->old_target)
strbuf_addf(buf, "ref:%s ", update->old_target);
else
strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
if (!(update->flags & REF_HAVE_NEW))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
@@ -3145,6 +3142,7 @@ struct migration_data {
static int migrate_one_ref(const struct reference *ref, void *cb_data)
{
struct migration_data *data = cb_data;
+ const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo;
struct strbuf symref_target = STRBUF_INIT;
int ret;
@@ -3153,7 +3151,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
` (2 preceding siblings ...)
2026-03-29 10:16 ` [PATCH v3 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-04-02 7:27 ` Patrick Steinhardt
2026-03-29 10:16 ` [PATCH v3 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
5 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.
Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.
Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663..7c8a992fcb 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo,
default:
BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
}
- refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask);
+ refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask);
refs->write_options.disable_auto_compact =
!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
refs->write_options.lock_timeout_ms = 100;
refs->write_options.fsync = reftable_be_fsync;
- repo_config(the_repository, reftable_be_config, &refs->write_options);
+ repo_config(repo, reftable_be_config, &refs->write_options);
/*
* It is somewhat unfortunate that we have to mirror the default block
@@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store,
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
- safe_create_dir(the_repository, sb.buf, 1);
+ safe_create_dir(ref_store->repo, sb.buf, 1);
strbuf_reset(&sb);
strbuf_release(&sb);
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v3 5/5] refs/packed-backend: use ref_store->repo instead of the_repository
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
` (3 preceding siblings ...)
2026-03-29 10:16 ` [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-03-29 10:16 ` Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
5 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-03-29 10:16 UTC (permalink / raw)
To: git; +Cc: ps, Shreyansh Paliwal
In refs/packed-backend.c, repo_config_get_int() is called using the global
the_repository, even though a repository instance is available via struct
ref_store.
Replace the use of the_repository with ref_store->repo to make the code
explicitly repository-aware. With no remaining users of the_repository in
this file, drop the USE_THE_REPOSITORY_VARIABLE macro.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/packed-backend.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 23ed62984b..ebc10dab4d 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
#define DISABLE_SIGN_COMPARE_WARNINGS
#include "../git-compat-util.h"
@@ -1223,7 +1222,7 @@ int packed_refs_lock(struct ref_store *ref_store, int flags, struct strbuf *err)
static int timeout_value = 1000;
if (!timeout_configured) {
- repo_config_get_int(the_repository, "core.packedrefstimeout", &timeout_value);
+ repo_config_get_int(ref_store->repo, "core.packedrefstimeout", &timeout_value);
timeout_configured = 1;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v2 1/5] refs: make branchname helpers repository aware
2026-03-29 9:55 ` Shreyansh Paliwal
@ 2026-03-29 15:37 ` Tian Yuchen
0 siblings, 0 replies; 48+ messages in thread
From: Tian Yuchen @ 2026-03-29 15:37 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git, ps
Hi,
On 3/29/26 17:55, Shreyansh Paliwal wrote:
>
> Thanks for pointing this out. Apparently it didn't raise any errors in the
> build or test suite. Will send a reroll.
Oh, then that’s likely because the static libraries have some kind of
on-demand
linking mechanism or something. Interesting.
> I hadn’t considered this as it is outside my scope of this patch series.
> While the change makes sense, I am not very sure whether it could cause
> any behavioral change or not.
> Thanks.
That's certainly true, I also think it’s better not to make drastic
changes. ;)
Regards, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-03-29 10:16 ` [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers Shreyansh Paliwal
@ 2026-04-02 7:27 ` Patrick Steinhardt
2026-04-02 17:03 ` Burak Kaan Karaçay
0 siblings, 1 reply; 48+ messages in thread
From: Patrick Steinhardt @ 2026-04-02 7:27 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git
On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
> diff --git a/refs.c b/refs.c
> index 685a0c247b..5cdc8858c5 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
> strbuf_add(sb, name + used, len - used);
> }
>
> -int check_branch_ref(struct strbuf *sb, const char *name)
> +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> {
> if (startup_info->have_repository)
> - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> else
> strbuf_addstr(sb, name);
>
I have to agree with Tian's comment on v2, this part here looks wrong. I
don't think we should depend on `startup_info` here, but we should
exclusively rely on whether or not the caller has passed in a
repository. And that will likely require a bit more scrutiny to figure
out whether there are any callers that shouldn't pass in a repository
because it's not initialized.
Alternatively, we could go with Tian's suggestion of checking for `repo
&& repo->gitdir`.
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository
2026-03-29 10:16 ` [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-04-02 7:27 ` Patrick Steinhardt
2026-04-03 10:43 ` Shreyansh Paliwal
0 siblings, 1 reply; 48+ messages in thread
From: Patrick Steinhardt @ 2026-04-02 7:27 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git
On Sun, Mar 29, 2026 at 03:46:42PM +0530, Shreyansh Paliwal wrote:
> reftable_be_init() and reftable_be_create_on_disk() use the_repository even
> though a repository instance is already available, either directly or via
> struct ref_store.
>
> Replace these uses with the appropriate local repository instance (repo or
> ref_store->repo) to avoid relying on global state.
>
> Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
> is_bare_repository() is still there in the file.
I've got a patch series cooking now that'll eventually de-globalize
`is_bare_repository()`. So there's light at the end of the tunnel here :)
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-04-02 7:27 ` Patrick Steinhardt
@ 2026-04-02 17:03 ` Burak Kaan Karaçay
2026-04-02 17:48 ` Tian Yuchen
2026-04-02 18:57 ` Patrick Steinhardt
0 siblings, 2 replies; 48+ messages in thread
From: Burak Kaan Karaçay @ 2026-04-02 17:03 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Shreyansh Paliwal, git, Jeff King, Tian Yuchen
Hi,
On Thu, Apr 02, 2026 at 09:27:33AM +0200, Patrick Steinhardt wrote:
>On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
>> diff --git a/refs.c b/refs.c
>> index 685a0c247b..5cdc8858c5 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
>> strbuf_add(sb, name + used, len - used);
>> }
>>
>> -int check_branch_ref(struct strbuf *sb, const char *name)
>> +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
>> {
>> if (startup_info->have_repository)
>> - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
>> + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
>> else
>> strbuf_addstr(sb, name);
>>
>
>I have to agree with Tian's comment on v2, this part here looks wrong. I
>don't think we should depend on `startup_info` here, but we should
>exclusively rely on whether or not the caller has passed in a
>repository. And that will likely require a bit more scrutiny to figure
>out whether there are any callers that shouldn't pass in a repository
>because it's not initialized.
>
>Alternatively, we could go with Tian's suggestion of checking for `repo
>&& repo->gitdir`.
>
>Patrick
This approach actually leads to a bug and segfault in a specific edge
case when running 'git check-ref-format'. The current tests don't cover
this scenario, but they can be extended to catch it.
If GIT_DIR is set to a non-existent path,
'startup_info->have_repository' becomes '0' but 'repo->gitdir' still
holds the invalid path. As a result, the code enters the first condition
and crashes. The case can be tested with this command:
$ git --git-dir='non-existing' check-ref-format --branch @{-1}
Modifying the behavior of 'repo->gitdir' might solve the issue, but I
belive that falls outside the scope of this patch. After a quick search,
I found a prophecy from Peff about the 'startup_info->have_repository':
[1] https://lore.kernel.org/git/20190806124954.GA13649@sigill.intra.peff.net/
Thanks,
Burak Kaan Karaçay
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-04-02 17:03 ` Burak Kaan Karaçay
@ 2026-04-02 17:48 ` Tian Yuchen
2026-04-02 18:57 ` Patrick Steinhardt
1 sibling, 0 replies; 48+ messages in thread
From: Tian Yuchen @ 2026-04-02 17:48 UTC (permalink / raw)
To: Burak Kaan Karaçay, Patrick Steinhardt
Cc: Shreyansh Paliwal, git, Jeff King
On 4/3/26 01:03, Burak Kaan Karaçay wrote:
> Hi,
>
> On Thu, Apr 02, 2026 at 09:27:33AM +0200, Patrick Steinhardt wrote:
>> On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
>>> diff --git a/refs.c b/refs.c
>>> index 685a0c247b..5cdc8858c5 100644
>>> --- a/refs.c
>>> +++ b/refs.c
>>> @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const
>>> char *name,
>>> strbuf_add(sb, name + used, len - used);
>>> }
>>>
>>> -int check_branch_ref(struct strbuf *sb, const char *name)
>>> +int check_branch_ref(struct repository *repo, struct strbuf *sb,
>>> const char *name)
>>> {
>>> if (startup_info->have_repository)
>>> - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
>>> + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
>>> else
>>> strbuf_addstr(sb, name);
>>>
>>
>> I have to agree with Tian's comment on v2, this part here looks wrong. I
>> don't think we should depend on `startup_info` here, but we should
>> exclusively rely on whether or not the caller has passed in a
>> repository. And that will likely require a bit more scrutiny to figure
>> out whether there are any callers that shouldn't pass in a repository
>> because it's not initialized.
>>
>> Alternatively, we could go with Tian's suggestion of checking for `repo
>> && repo->gitdir`.
>>
>> Patrick
>
> This approach actually leads to a bug and segfault in a specific edge
> case when running 'git check-ref-format'. The current tests don't cover
> this scenario, but they can be extended to catch it.
>
> If GIT_DIR is set to a non-existent path,
> 'startup_info->have_repository' becomes '0' but 'repo->gitdir' still
> holds the invalid path. As a result, the code enters the first condition
> and crashes. The case can be tested with this command:
>
> $ git --git-dir='non-existing' check-ref-format --branch @{-1}
>
> Modifying the behavior of 'repo->gitdir' might solve the issue, but I
> belive that falls outside the scope of this patch. After a quick search,
> I found a prophecy from Peff about the 'startup_info->have_repository':
>
> [1] https://lore.kernel.org/
> git/20190806124954.GA13649@sigill.intra.peff.net/
>
> Thanks,
> Burak Kaan Karaçay
You’re absolutely right.
Actually, I’d already spotted the error when I wrote that bit of code,
which is why I said 'I dunno' - simply to give an idea of what I was
trying to achieve. It looks like we’ll have to go with a much uglier
solution. ;)
One possible approach (albeit temporary and useless) is to wrap the code
in a macro or an inline function (in repository.h, I guess?):
> static inline int repo_has_repository(struct repository *repo)
> {
> /*
> * NEEDSWORK...
> */> if (repo == the_repository)
> return startup_info->have_repository;
>
> return repo && repo->gitdir;
> }
This will remove a large amount of startup_info->have_repository...But
that doesn’t solve anything, just as stuffing rubbish into a bin doesn’t
change the fact that it’s rubbish. The 'startup_info->have_repository'
still requires a significant amount of effort to refactor.
So still, I dunno.
Regards, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-04-02 17:03 ` Burak Kaan Karaçay
2026-04-02 17:48 ` Tian Yuchen
@ 2026-04-02 18:57 ` Patrick Steinhardt
2026-04-03 10:39 ` Shreyansh Paliwal
1 sibling, 1 reply; 48+ messages in thread
From: Patrick Steinhardt @ 2026-04-02 18:57 UTC (permalink / raw)
To: Burak Kaan Karaçay; +Cc: Shreyansh Paliwal, git, Jeff King, Tian Yuchen
On Thu, Apr 02, 2026 at 08:03:45PM +0300, Burak Kaan Karaçay wrote:
> Hi,
>
> On Thu, Apr 02, 2026 at 09:27:33AM +0200, Patrick Steinhardt wrote:
> > On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
> > > diff --git a/refs.c b/refs.c
> > > index 685a0c247b..5cdc8858c5 100644
> > > --- a/refs.c
> > > +++ b/refs.c
> > > @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
> > > strbuf_add(sb, name + used, len - used);
> > > }
> > >
> > > -int check_branch_ref(struct strbuf *sb, const char *name)
> > > +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> > > {
> > > if (startup_info->have_repository)
> > > - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> > > + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> > > else
> > > strbuf_addstr(sb, name);
> > >
> >
> > I have to agree with Tian's comment on v2, this part here looks wrong. I
> > don't think we should depend on `startup_info` here, but we should
> > exclusively rely on whether or not the caller has passed in a
> > repository. And that will likely require a bit more scrutiny to figure
> > out whether there are any callers that shouldn't pass in a repository
> > because it's not initialized.
> >
> > Alternatively, we could go with Tian's suggestion of checking for `repo
> > && repo->gitdir`.
> >
> > Patrick
>
> This approach actually leads to a bug and segfault in a specific edge
> case when running 'git check-ref-format'. The current tests don't cover
> this scenario, but they can be extended to catch it.
> If GIT_DIR is set to a non-existent path,
> 'startup_info->have_repository' becomes '0' but 'repo->gitdir' still
> holds the invalid path. As a result, the code enters the first condition
> and crashes. The case can be tested with this command:
>
> $ git --git-dir='non-existing' check-ref-format --branch @{-1}
>
> Modifying the behavior of 'repo->gitdir' might solve the issue, but I
> belive that falls outside the scope of this patch. After a quick search,
> I found a prophecy from Peff about the 'startup_info->have_repository':
>
> [1] https://lore.kernel.org/git/20190806124954.GA13649@sigill.intra.peff.net/
If we cannot make it work in this patch series, the next question is
whether we actually want to give the false sense of `check_branch_ref()`
being independent of global state, or whether we want to leave it as-is
for now and then do a follow-up patch series where we fix the issue and
adapt the interface.
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers
2026-04-02 18:57 ` Patrick Steinhardt
@ 2026-04-03 10:39 ` Shreyansh Paliwal
0 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 10:39 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Burak Kaan Karaçay, git, Jeff King, Tian Yuchen
On Fri, Apr 3, 2026 at 12:28 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Apr 02, 2026 at 08:03:45PM +0300, Burak Kaan Karaçay wrote:
> > Hi,
> >
> > On Thu, Apr 02, 2026 at 09:27:33AM +0200, Patrick Steinhardt wrote:
> > > On Sun, Mar 29, 2026 at 03:46:39PM +0530, Shreyansh Paliwal wrote:
> > > > diff --git a/refs.c b/refs.c
> > > > index 685a0c247b..5cdc8858c5 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -758,10 +758,10 @@ void copy_branchname(struct strbuf *sb, const char *name,
> > > > strbuf_add(sb, name + used, len - used);
> > > > }
> > > >
> > > > -int check_branch_ref(struct strbuf *sb, const char *name)
> > > > +int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
> > > > {
> > > > if (startup_info->have_repository)
> > > > - copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
> > > > + copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
> > > > else
> > > > strbuf_addstr(sb, name);
> > > >
> > >
> > > I have to agree with Tian's comment on v2, this part here looks wrong. I
> > > don't think we should depend on `startup_info` here, but we should
> > > exclusively rely on whether or not the caller has passed in a
> > > repository. And that will likely require a bit more scrutiny to figure
> > > out whether there are any callers that shouldn't pass in a repository
> > > because it's not initialized.
> > >
> > > Alternatively, we could go with Tian's suggestion of checking for `repo
> > > && repo->gitdir`.
> > >
> > > Patrick
> >
> > This approach actually leads to a bug and segfault in a specific edge
> > case when running 'git check-ref-format'. The current tests don't cover
> > this scenario, but they can be extended to catch it.
>
> > If GIT_DIR is set to a non-existent path,
> > 'startup_info->have_repository' becomes '0' but 'repo->gitdir' still
> > holds the invalid path. As a result, the code enters the first condition
> > and crashes. The case can be tested with this command:
> >
> > $ git --git-dir='non-existing' check-ref-format --branch @{-1}
> >
> > Modifying the behavior of 'repo->gitdir' might solve the issue, but I
> > belive that falls outside the scope of this patch. After a quick search,
> > I found a prophecy from Peff about the 'startup_info->have_repository':
> >
> > [1] https://lore.kernel.org/git/20190806124954.GA13649@sigill.intra.peff.net/
>
> If we cannot make it work in this patch series, the next question is
> whether we actually want to give the false sense of `check_branch_ref()`
> being independent of global state, or whether we want to leave it as-is
> for now and then do a follow-up patch series where we fix the issue and
> adapt the interface.
I think it makes sense to drop patch 1/5 from this series for now, which
introduces changes to the branch name helper functions.
It would be much better to address this separately after replacing
startup_info->have_repository.
For now, I'll reroll the series with the remaining patches and send this
part later as an rfc.
Thanks everyone,
Shreyansh
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository
2026-04-02 7:27 ` Patrick Steinhardt
@ 2026-04-03 10:43 ` Shreyansh Paliwal
0 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 10:43 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
On Thu, Apr 2, 2026 at 12:57 PM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Sun, Mar 29, 2026 at 03:46:42PM +0530, Shreyansh Paliwal wrote:
> > reftable_be_init() and reftable_be_create_on_disk() use the_repository even
> > though a repository instance is already available, either directly or via
> > struct ref_store.
> >
> > Replace these uses with the appropriate local repository instance (repo or
> > ref_store->repo) to avoid relying on global state.
> >
> > Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
> > is_bare_repository() is still there in the file.
>
> I've got a patch series cooking now that'll eventually de-globalize
> `is_bare_repository()`. So there's light at the end of the tunnel here :)
That's great to hear, thanks for the heads-up :)
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 0/3] refs: reduce reliance on global state
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
` (4 preceding siblings ...)
2026-03-29 10:16 ` [PATCH v3 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
@ 2026-04-03 12:08 ` Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
` (3 more replies)
5 siblings, 4 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 12:08 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
This series continues the effort to reduce reliance on the_repository
global state by making repository context explicit across the refs
subsystem. The patches focus on passing struct repository through various
ref helpers and backends, and replacing uses of global state such as
the_repository and the_hash_algo with the appropriate repository instance.
Patch 1/3: Updating get_files_ref_lock_timeout_ms() to take a repository
and propagating it through files-backend, including callback paths.
Patch 2/3:Replacing uses of the_hash_algo in refs.c with the hash
algorithm from the appropriate repository.
Patch 3/3:Removing remaining uses of the_repository in reftable-backend.c
where a repository instance is already available.
Shreyansh Paliwal (3):
refs: add struct repository parameter in
get_files_ref_lock_timeout_ms()
refs: remove the_hash_algo global state
refs/reftable-backend: drop uses of the_repository
refs.c | 17 +++++++++--------
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 6 +++---
4 files changed, 26 insertions(+), 18 deletions(-)
---
Changes in v4:
- Dropped patches 1/5 and 5/5, as they require further refactoring and
discussion. I will send them separately as a follow-up.
Changes in v3:
- Fixed an import
- better readability in patch 3/5
Changes in v2:
- Made struct repository the first argument in function parameters.
Range-diff against v3:
2: f7a9ea4204 = 1: 11c134b3f5 refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
1: 5844440b73 ! 2: d144e879ad refs: add struct repository parameter to branchname helpers
@@ Metadata
Author: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
## Commit message ##
- refs: add struct repository parameter to branchname helpers
+ refs: remove the_hash_algo global state
- copy_branchname() in refs.c relies on the_repository when calling
- repo_interpret_branch_name(), introducing an implicit dependency on global
- state. Add a struct repository parameter and use it instead.
-
- Update check_branch_ref() to take a repository parameter as well, since it
- calls copy_branchname(). Propagate this change to higher-level helpers
- validate_branchname() and validate_new_branchname(), which also lack access
- to a repository instance. Most callers of these helpers reside in builtin
- code and already operate on the_repository, so pass it explicitly at those
- call sites (builtin/checkout and builtin/worktree) otherwise pass struct
- repository where available.
-
- This makes branch name handling explicitly repository-aware and aligns with
- ongoing efforts to remove reliance on global state. This change builds on
- top of jw/object-name-bitset-to-enum (2026-03-18), which introduced the
- enum interpret_branch_kind parameter to copy_branchname().
+ refs.c uses the_hash_algo in multiple places, relying on global state for
+ the object hash algorithm. Replace these uses with the appropriate
+ repository-specific hash_algo. In transaction-related functions
+ (ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
+ transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
+ In other cases, such as repo_get_submodule_ref_store(), use
+ repo->hash_algo.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
- ## branch.c ##
-@@ branch.c: int read_branch_desc(struct strbuf *buf, const char *branch_name)
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
--int validate_branchname(const char *name, struct strbuf *ref)
-+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref)
- {
-- if (check_branch_ref(ref, name)) {
-+ if (check_branch_ref(repo, ref, name)) {
- int code = die_message(_("'%s' is not a valid branch name"), name);
- advise_if_enabled(ADVICE_REF_SYNTAX,
- _("See 'git help check-ref-format'"));
- exit(code);
+ ## refs.c ##
+@@ refs.c: int ref_transaction_create(struct ref_transaction *transaction,
+ return 1;
}
-
-- return refs_ref_exists(get_main_ref_store(the_repository), ref->buf);
-+ return refs_ref_exists(get_main_ref_store(repo), ref->buf);
+ return ref_transaction_update(transaction, refname, new_oid,
+- null_oid(the_hash_algo), new_target, NULL, flags,
++ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
+ msg, err);
}
- static int initialized_checked_out_branches;
-@@ branch.c: const char *branch_checked_out(const char *refname)
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
--int validate_new_branchname(const char *name, struct strbuf *ref, int force)
-+int validate_new_branchname(struct repository *repo, const char *name,
-+ struct strbuf *ref, int force)
- {
- const char *path;
-- if (!validate_branchname(name, ref))
-+ if (!validate_branchname(repo, name, ref))
- return 0;
-
- if (!force)
-@@ branch.c: void create_branch(struct repository *r,
- BUG("'clobber_head_ok' can only be used with 'force'");
-
- if (clobber_head_ok ?
-- validate_branchname(name, &ref) :
-- validate_new_branchname(name, &ref, force)) {
-+ validate_branchname(r, name, &ref) :
-+ validate_new_branchname(r, name, &ref, force)) {
- forcing = 1;
+@@ refs.c: int ref_transaction_delete(struct ref_transaction *transaction,
+ if (old_target && !(flags & REF_NO_DEREF))
+ BUG("delete cannot operate on symrefs with deref mode");
+ return ref_transaction_update(transaction, refname,
+- null_oid(the_hash_algo), old_oid,
++ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
+ NULL, old_target, flags,
+ msg, err);
+ }
+@@ refs.c: struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
+ subrepo = xmalloc(sizeof(*subrepo));
+
+ if (repo_submodule_init(subrepo, repo, submodule,
+- null_oid(the_hash_algo))) {
++ null_oid(repo->hash_algo))) {
+ free(subrepo);
+ goto done;
}
-
-
- ## branch.h ##
-@@ branch.h: const char *branch_checked_out(const char *refname);
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
--int validate_branchname(const char *name, struct strbuf *ref);
-+int validate_branchname(struct repository *repo, const char *name, struct strbuf *ref);
-
- /*
- * Check if a branch 'name' can be created as a new branch; die otherwise.
-@@ branch.h: int validate_branchname(const char *name, struct strbuf *ref);
- * Return 1 if the named branch already exists; return 0 otherwise.
- * Fill ref with the full refname for the branch.
- */
--int validate_new_branchname(const char *name, struct strbuf *ref, int force);
-+int validate_new_branchname(struct repository *repo, const char *name,
-+ struct strbuf *ref, int force);
-
- /*
- * Remove information about the merge state on the current
-
- ## builtin/branch.c ##
-@@ builtin/branch.c: static int delete_branches(int argc, const char **argv, int force, int kinds,
- char *target = NULL;
- int flags = 0;
-
-- copy_branchname(&bname, argv[i], allowed_interpret);
-+ copy_branchname(the_repository, &bname, argv[i], allowed_interpret);
- free(name);
- name = mkpathdup(fmt, bname.buf);
-
-@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const char *newname, int
- int recovery = 0, oldref_usage = 0;
- struct worktree **worktrees = get_worktrees();
-
-- if (check_branch_ref(&oldref, oldname)) {
-+ if (check_branch_ref(the_repository, &oldref, oldname)) {
- /*
- * Bad name --- this could be an attempt to rename a
- * ref that we used to allow to be created by accident.
-@@ builtin/branch.c: static void copy_or_rename_branch(const char *oldname, const char *newname, int
- * cause the worktree to become inconsistent with HEAD, so allow it.
- */
- if (!strcmp(oldname, newname))
-- validate_branchname(newname, &newref);
-+ validate_branchname(the_repository, newname, &newref);
+@@ refs.c: static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
+ strbuf_reset(buf);
+
+ if (!(update->flags & REF_HAVE_OLD))
+- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
++ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
+ else if (update->old_target)
+ strbuf_addf(buf, "ref:%s ", update->old_target);
else
-- validate_new_branchname(newname, &newref, force);
-+ validate_new_branchname(the_repository, newname, &newref, force);
-
- reject_rebase_or_bisect_branch(worktrees, oldref.buf);
+ strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
-@@ builtin/branch.c: int cmd_branch(int argc,
- die(_("cannot give description to detached HEAD"));
- branch_name = head;
- } else if (argc == 1) {
-- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
- branch_name = buf.buf;
- } else {
- die(_("cannot edit description of more than one branch"));
-@@ builtin/branch.c: int cmd_branch(int argc,
- if (!argc)
- branch = branch_get(NULL);
- else if (argc == 1) {
-- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
- branch = branch_get(buf.buf);
- } else
- die(_("too many arguments to set new upstream"));
-@@ builtin/branch.c: int cmd_branch(int argc,
- if (!argc)
- branch = branch_get(NULL);
- else if (argc == 1) {
-- copy_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(the_repository, &buf, argv[0], INTERPRET_BRANCH_LOCAL);
- branch = branch_get(buf.buf);
- } else
- die(_("too many arguments to unset upstream"));
-
- ## builtin/check-ref-format.c ##
-@@
- /*
- * GIT - The information manager from hell
- */
-+#define USE_THE_REPOSITORY_VARIABLE
- #include "builtin.h"
- #include "refs.h"
- #include "setup.h"
-@@ builtin/check-ref-format.c: static int check_ref_format_branch(const char *arg)
- int nongit;
-
- setup_git_directory_gently(&nongit);
-- if (check_branch_ref(&sb, arg) ||
-+ if (check_branch_ref(the_repository, &sb, arg) ||
- !skip_prefix(sb.buf, "refs/heads/", &name))
- die("'%s' is not a valid branch name", arg);
- printf("%s\n", name);
-
- ## builtin/checkout.c ##
-@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
- &branch->oid, &branch->refname, 0))
- repo_get_oid_committish(the_repository, branch->name, &branch->oid);
-
-- copy_branchname(&buf, branch->name, INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(the_repository, &buf, branch->name, INTERPRET_BRANCH_LOCAL);
- if (strcmp(buf.buf, branch->name)) {
- free(branch->name);
- branch->name = xstrdup(buf.buf);
-@@ builtin/checkout.c: static int checkout_main(int argc, const char **argv, const char *prefix,
- struct strbuf buf = STRBUF_INIT;
-
- if (opts->new_branch_force)
-- opts->branch_exists = validate_branchname(opts->new_branch, &buf);
-+ opts->branch_exists = validate_branchname(the_repository, opts->new_branch, &buf);
- else
- opts->branch_exists =
-- validate_new_branchname(opts->new_branch, &buf, 0);
-+ validate_new_branchname(the_repository, opts->new_branch, &buf, 0);
- strbuf_release(&buf);
- }
-
-
- ## builtin/merge.c ##
-@@ builtin/merge.c: static void merge_name(const char *remote, struct strbuf *msg)
- char *found_ref = NULL;
- int len, early;
-
-- copy_branchname(&bname, remote, 0);
-+ copy_branchname(the_repository, &bname, remote, 0);
- remote = bname.buf;
-
- oidclr(&branch_head, the_repository->hash_algo);
-
- ## builtin/worktree.c ##
-@@ builtin/worktree.c: static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
- struct strbuf symref = STRBUF_INIT;
- struct child_process cp = CHILD_PROCESS_INIT;
-
-- validate_new_branchname(ref, &symref, 0);
-+ validate_new_branchname(the_repository, ref, &symref, 0);
- strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
- if (opts->quiet)
- strvec_push(&cp.args, "--quiet");
-@@ builtin/worktree.c: static int add_worktree(const char *path, const char *refname,
- worktrees = NULL;
-
- /* is 'refname' a branch or commit? */
-- if (!opts->detach && !check_branch_ref(&symref, refname) &&
-+ if (!opts->detach && !check_branch_ref(the_repository, &symref, refname) &&
- refs_ref_exists(get_main_ref_store(the_repository), symref.buf)) {
- is_branch = 1;
- if (!opts->force)
-@@ builtin/worktree.c: static void print_preparing_worktree_line(int detach,
- fprintf_ln(stderr, _("Preparing worktree (new branch '%s')"), new_branch);
- } else {
- struct strbuf s = STRBUF_INIT;
-- if (!detach && !check_branch_ref(&s, branch) &&
-+ if (!detach && !check_branch_ref(the_repository, &s, branch) &&
- refs_ref_exists(get_main_ref_store(the_repository), s.buf))
- fprintf_ln(stderr, _("Preparing worktree (checking out '%s')"),
- branch);
-@@ builtin/worktree.c: static char *dwim_branch(const char *path, char **new_branch)
- char *branchname = xstrndup(s, n);
- struct strbuf ref = STRBUF_INIT;
-
-- branch_exists = !check_branch_ref(&ref, branchname) &&
-+ branch_exists = !check_branch_ref(the_repository, &ref, branchname) &&
- refs_ref_exists(get_main_ref_store(the_repository),
- ref.buf);
- strbuf_release(&ref);
-@@ builtin/worktree.c: static int add(int ac, const char **av, const char *prefix,
- new_branch = new_branch_force;
-
- if (!opts.force &&
-- !check_branch_ref(&symref, new_branch) &&
-+ !check_branch_ref(the_repository, &symref, new_branch) &&
- refs_ref_exists(get_main_ref_store(the_repository), symref.buf))
- die_if_checked_out(symref.buf, 0);
- strbuf_release(&symref);
-
- ## refs.c ##
-@@ refs.c: static char *substitute_branch_name(struct repository *r,
- return NULL;
- }
-
--void copy_branchname(struct strbuf *sb, const char *name,
-+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
- enum interpret_branch_kind allowed)
- {
- int len = strlen(name);
- struct interpret_branch_name_options options = {
- .allowed = allowed
- };
-- int used = repo_interpret_branch_name(the_repository, name, len, sb,
-+ int used = repo_interpret_branch_name(repo, name, len, sb,
- &options);
-
- if (used < 0)
-@@ refs.c: void copy_branchname(struct strbuf *sb, const char *name,
- strbuf_add(sb, name + used, len - used);
- }
-
--int check_branch_ref(struct strbuf *sb, const char *name)
-+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name)
- {
- if (startup_info->have_repository)
-- copy_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
-+ copy_branchname(repo, sb, name, INTERPRET_BRANCH_LOCAL);
+ if (!(update->flags & REF_HAVE_NEW))
+- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
++ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
+ else if (update->new_target)
+ strbuf_addf(buf, "ref:%s ", update->new_target);
else
- strbuf_addstr(sb, name);
-
-
- ## refs.h ##
-@@ refs.h: char *repo_default_branch_name(struct repository *r, int quiet);
- * If "allowed" is non-zero, restrict the set of allowed expansions. See
- * repo_interpret_branch_name() for details.
- */
--void copy_branchname(struct strbuf *sb, const char *name,
-+void copy_branchname(struct repository *repo, struct strbuf *sb, const char *name,
- enum interpret_branch_kind allowed);
-
- /*
-@@ refs.h: void copy_branchname(struct strbuf *sb, const char *name,
- *
- * The return value is "0" if the result is valid, and "-1" otherwise.
- */
--int check_branch_ref(struct strbuf *sb, const char *name);
-+int check_branch_ref(struct repository *repo, struct strbuf *sb, const char *name);
-
- /*
- * Similar for a tag name in refs/tags/.
+@@ refs.c: struct migration_data {
+ static int migrate_one_ref(const struct reference *ref, void *cb_data)
+ {
+ struct migration_data *data = cb_data;
++ const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo;
+ struct strbuf symref_target = STRBUF_INIT;
+ int ret;
+
+@@ refs.c: static int migrate_one_ref(const struct reference *ref, void *cb_data)
+ if (ret < 0)
+ goto done;
+
+- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
++ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo),
+ symref_target.buf, NULL,
+ REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
+ if (ret < 0)
3: 00dba1a96a < -: ---------- refs: remove the_hash_algo global state
4: 5653c418e1 = 3: 76c14eb320 refs/reftable-backend: drop uses of the_repository
5: 18c1c67083 < -: ---------- refs/packed-backend: use ref_store->repo instead of the_repository
--
2.53.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
@ 2026-04-03 12:08 ` Shreyansh Paliwal
2026-04-03 17:40 ` Tian Yuchen
2026-04-03 12:08 ` [PATCH v4 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
` (2 subsequent siblings)
3 siblings, 1 reply; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 12:08 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.
Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
This reduces reliance on the_repository global.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 685a0c247b..214ebfd5ce 100644
--- a/refs.c
+++ b/refs.c
@@ -989,7 +989,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,
return REF_WORKTREE_SHARED;
}
-long get_files_ref_lock_timeout_ms(void)
+long get_files_ref_lock_timeout_ms(struct repository *repo)
{
static int configured = 0;
@@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
static int timeout_ms = 100;
if (!configured) {
- repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
+ repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
configured = 1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ce0d57478..ee8dd771a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
if (hold_lock_file_for_update_timeout(
&lock->lk, ref_file.buf, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0) {
+ get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) {
int myerr = errno;
errno = 0;
if (myerr == ENOENT && --attempts_remaining > 0) {
@@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
+struct create_reflock_cb {
+ struct lock_file *lk;
+ struct repository *repo;
+};
+
static int create_reflock(const char *path, void *cb)
{
- struct lock_file *lk = cb;
-
+ struct create_reflock_cb *data = cb;
return hold_lock_file_for_update_timeout(
- lk, path, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
+ data->lk, path, LOCK_NO_DEREF,
+ get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
}
/*
@@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ struct create_reflock_cb cb_data;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
lock->ref_name = xstrdup(refname);
lock->count = 1;
+ cb_data.lk = &lock->lk;
+ cb_data.repo = refs->base.repo;
- if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) {
+ if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) {
unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..e4cfd9e19e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,7 +43,7 @@ struct ref_transaction;
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
*/
-long get_files_ref_lock_timeout_ms(void);
+long get_files_ref_lock_timeout_ms(struct repository *repo);
/*
* Return true iff refname is minimally safe. "Safe" here means that
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 2/3] refs: remove the_hash_algo global state
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
@ 2026-04-03 12:08 ` Shreyansh Paliwal
2026-04-03 12:09 ` [PATCH v4 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
3 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 12:08 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 214ebfd5ce..cb58e10dc6 100644
--- a/refs.c
+++ b/refs.c
@@ -1472,7 +1472,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
return 1;
}
return ref_transaction_update(transaction, refname, new_oid,
- null_oid(the_hash_algo), new_target, NULL, flags,
+ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
msg, err);
}
@@ -1491,7 +1491,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (old_target && !(flags & REF_NO_DEREF))
BUG("delete cannot operate on symrefs with deref mode");
return ref_transaction_update(transaction, refname,
- null_oid(the_hash_algo), old_oid,
+ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
NULL, old_target, flags,
msg, err);
}
@@ -2379,7 +2379,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
subrepo = xmalloc(sizeof(*subrepo));
if (repo_submodule_init(subrepo, repo, submodule,
- null_oid(the_hash_algo))) {
+ null_oid(repo->hash_algo))) {
free(subrepo);
goto done;
}
@@ -2571,14 +2571,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
strbuf_reset(buf);
if (!(update->flags & REF_HAVE_OLD))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->old_target)
strbuf_addf(buf, "ref:%s ", update->old_target);
else
strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
if (!(update->flags & REF_HAVE_NEW))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
@@ -3145,6 +3145,7 @@ struct migration_data {
static int migrate_one_ref(const struct reference *ref, void *cb_data)
{
struct migration_data *data = cb_data;
+ const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo;
struct strbuf symref_target = STRBUF_INIT;
int ret;
@@ -3153,7 +3154,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v4 3/3] refs/reftable-backend: drop uses of the_repository
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-04-03 12:09 ` Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
3 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-03 12:09 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.
Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.
Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663..7c8a992fcb 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo,
default:
BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
}
- refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask);
+ refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask);
refs->write_options.disable_auto_compact =
!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
refs->write_options.lock_timeout_ms = 100;
refs->write_options.fsync = reftable_be_fsync;
- repo_config(the_repository, reftable_be_config, &refs->write_options);
+ repo_config(repo, reftable_be_config, &refs->write_options);
/*
* It is somewhat unfortunate that we have to mirror the default block
@@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store,
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
- safe_create_dir(the_repository, sb.buf, 1);
+ safe_create_dir(ref_store->repo, sb.buf, 1);
strbuf_reset(&sb);
strbuf_release(&sb);
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
2026-04-03 12:08 ` [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
@ 2026-04-03 17:40 ` Tian Yuchen
0 siblings, 0 replies; 48+ messages in thread
From: Tian Yuchen @ 2026-04-03 17:40 UTC (permalink / raw)
To: Shreyansh Paliwal, git; +Cc: ps, gitster, bkkaracay
On 4/3/26 20:08, Shreyansh Paliwal wrote:
> -long get_files_ref_lock_timeout_ms(void)
> +long get_files_ref_lock_timeout_ms(struct repository *repo)
> {
> static int configured = 0;
>
> @@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
> static int timeout_ms = 100;
>
A very minor and trivial question: the 'static' keyword is still present
here. This is entirely understandable, given that you mentioned earlier
that...
> Hi Yuchen,
>
> I have acknowledged this in a previous reply to Burak. As stated there,
> this is a valid issue and would require moving the config into
> repo-settings struct.
> In this patch, I focused on removing the dependency on
> 'the_repository' while preserving existing behavior. Global state
> removal and multi-repo correctness is an incremental process,
> so I would prefer to handle this in a follow-up change.
> I'll also update the patch title in the next version to better reflect
> the scope of the change.
But if that is the case, the accuracy of this line in the commit message:
> This reduces reliance on the_repository global.
..is open to question. Or perhaps it would be worth mentioning:
"Note: This function still uses static variables, which means it does
not fully support in-process multi-repo usage yet. This will be
addressed in a follow-up by moving the configuration to the
'repo-settings' struct, but changing the signature is a necessary first
step..."
or something (shorter)?
To reiterate, I think this is a minor issue, so it would be better if
you decide for yourself. Other parts look good to me. ;)
Regards, Yuchen
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 0/3] refs: reduce reliance on global state
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
` (2 preceding siblings ...)
2026-04-03 12:09 ` [PATCH v4 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-04-04 13:58 ` Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
` (3 more replies)
3 siblings, 4 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-04 13:58 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
This series continues the effort to reduce reliance on the_repository
global state by making repository context explicit across the refs
subsystem. The patches focus on passing struct repository through various
ref helpers and backends, and replacing uses of global state such as
the_repository and the_hash_algo with the appropriate repository instance.
Patch 1/3: Updating get_files_ref_lock_timeout_ms() to take a repository
and propagating it through files-backend, including callback paths.
Patch 2/3:Replacing uses of the_hash_algo in refs.c with the hash
algorithm from the appropriate repository.
Patch 3/3:Removing remaining uses of the_repository in reftable-backend.c
where a repository instance is already available.
Shreyansh Paliwal (3):
refs: add struct repository parameter in
get_files_ref_lock_timeout_ms()
refs: remove the_hash_algo global state
refs/reftable-backend: drop uses of the_repository
refs.c | 17 +++++++++--------
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 6 +++---
4 files changed, 26 insertions(+), 18 deletions(-)
---
Changes in v5:
- made the commit message of patch 1/3 more explicit.
Changes in v4:
- Dropped patches 1/5 and 5/5, as they require further refactoring and
discussion. I will send them separately as a follow-up.
Changes in v3:
- Fixed an import
- better readability in patch 3/5
Changes in v2:
- Made struct repository the first argument in function parameters.
Range-diff against v4:
1: 11c134b3f5 ! 1: 59c4662031 refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
@@ Commit message
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
- This reduces reliance on the_repository global.
+ This reduces reliance on the_repository global, though the function
+ still uses static variables and is not yet fully repository-scoped.
+ This can be addressed in a follow-up change.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
2: d144e879ad = 2: 9dd20df759 refs: remove the_hash_algo global state
3: 76c14eb320 = 3: 04c88f7ed4 refs/reftable-backend: drop uses of the_repository
--
2.53.0
^ permalink raw reply [flat|nested] 48+ messages in thread
* [PATCH v5 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms()
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
@ 2026-04-04 13:58 ` Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
` (2 subsequent siblings)
3 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-04 13:58 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
get_files_ref_lock_timeout_ms() calls repo_config_get_int() using
the_repository, as no repository instance is available in its scope. Add a
struct repository parameter and use it instead of the_repository.
Update all callers accordingly. In files-backend.c, lock_raw_ref() can
obtain repository instance from the struct ref_transaction via
transaction->ref_store->repo and pass it down. For create_reflock(), which
is used as a callback, introduce a small wrapper struct to pass both struct
lock_file and struct repository through the callback data.
This reduces reliance on the_repository global, though the function
still uses static variables and is not yet fully repository-scoped.
This can be addressed in a follow-up change.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 4 ++--
refs/files-backend.c | 19 +++++++++++++------
refs/refs-internal.h | 2 +-
3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 685a0c247b..214ebfd5ce 100644
--- a/refs.c
+++ b/refs.c
@@ -989,7 +989,7 @@ enum ref_worktree_type parse_worktree_ref(const char *maybe_worktree_ref,
return REF_WORKTREE_SHARED;
}
-long get_files_ref_lock_timeout_ms(void)
+long get_files_ref_lock_timeout_ms(struct repository *repo)
{
static int configured = 0;
@@ -997,7 +997,7 @@ long get_files_ref_lock_timeout_ms(void)
static int timeout_ms = 100;
if (!configured) {
- repo_config_get_int(the_repository, "core.filesreflocktimeout", &timeout_ms);
+ repo_config_get_int(repo, "core.filesreflocktimeout", &timeout_ms);
configured = 1;
}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 7ce0d57478..ee8dd771a4 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -792,7 +792,7 @@ static enum ref_transaction_error lock_raw_ref(struct files_ref_store *refs,
if (hold_lock_file_for_update_timeout(
&lock->lk, ref_file.buf, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0) {
+ get_files_ref_lock_timeout_ms(transaction->ref_store->repo)) < 0) {
int myerr = errno;
errno = 0;
if (myerr == ENOENT && --attempts_remaining > 0) {
@@ -1190,13 +1190,17 @@ static int remove_empty_directories(struct strbuf *path)
return remove_dir_recursively(path, REMOVE_DIR_EMPTY_ONLY);
}
+struct create_reflock_cb {
+ struct lock_file *lk;
+ struct repository *repo;
+};
+
static int create_reflock(const char *path, void *cb)
{
- struct lock_file *lk = cb;
-
+ struct create_reflock_cb *data = cb;
return hold_lock_file_for_update_timeout(
- lk, path, LOCK_NO_DEREF,
- get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;
+ data->lk, path, LOCK_NO_DEREF,
+ get_files_ref_lock_timeout_ms(data->repo)) < 0 ? -1 : 0;
}
/*
@@ -1208,6 +1212,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
{
struct strbuf ref_file = STRBUF_INIT;
struct ref_lock *lock;
+ struct create_reflock_cb cb_data;
files_assert_main_repository(refs, "lock_ref_oid_basic");
assert(err);
@@ -1229,8 +1234,10 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs,
lock->ref_name = xstrdup(refname);
lock->count = 1;
+ cb_data.lk = &lock->lk;
+ cb_data.repo = refs->base.repo;
- if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) {
+ if (raceproof_create_file(ref_file.buf, create_reflock, &cb_data)) {
unable_to_lock_message(ref_file.buf, errno, err);
goto error_return;
}
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index d79e35fd26..e4cfd9e19e 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -43,7 +43,7 @@ struct ref_transaction;
* Return the length of time to retry acquiring a loose reference lock
* before giving up, in milliseconds:
*/
-long get_files_ref_lock_timeout_ms(void);
+long get_files_ref_lock_timeout_ms(struct repository *repo);
/*
* Return true iff refname is minimally safe. "Safe" here means that
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 2/3] refs: remove the_hash_algo global state
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
@ 2026-04-04 13:58 ` Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-04-08 8:46 ` [PATCH v5 0/3] refs: reduce reliance on global state Patrick Steinhardt
3 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-04 13:58 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
refs.c uses the_hash_algo in multiple places, relying on global state for
the object hash algorithm. Replace these uses with the appropriate
repository-specific hash_algo. In transaction-related functions
(ref_transaction_create, ref_transaction_delete, migrate_one_ref, and
transaction_hook_feed_stdin), use transaction->ref_store->repo->hash_algo.
In other cases, such as repo_get_submodule_ref_store(), use
repo->hash_algo.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 214ebfd5ce..cb58e10dc6 100644
--- a/refs.c
+++ b/refs.c
@@ -1472,7 +1472,7 @@ int ref_transaction_create(struct ref_transaction *transaction,
return 1;
}
return ref_transaction_update(transaction, refname, new_oid,
- null_oid(the_hash_algo), new_target, NULL, flags,
+ null_oid(transaction->ref_store->repo->hash_algo), new_target, NULL, flags,
msg, err);
}
@@ -1491,7 +1491,7 @@ int ref_transaction_delete(struct ref_transaction *transaction,
if (old_target && !(flags & REF_NO_DEREF))
BUG("delete cannot operate on symrefs with deref mode");
return ref_transaction_update(transaction, refname,
- null_oid(the_hash_algo), old_oid,
+ null_oid(transaction->ref_store->repo->hash_algo), old_oid,
NULL, old_target, flags,
msg, err);
}
@@ -2379,7 +2379,7 @@ struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
subrepo = xmalloc(sizeof(*subrepo));
if (repo_submodule_init(subrepo, repo, submodule,
- null_oid(the_hash_algo))) {
+ null_oid(repo->hash_algo))) {
free(subrepo);
goto done;
}
@@ -2571,14 +2571,14 @@ static int transaction_hook_feed_stdin(int hook_stdin_fd, void *pp_cb, void *pp_
strbuf_reset(buf);
if (!(update->flags & REF_HAVE_OLD))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->old_target)
strbuf_addf(buf, "ref:%s ", update->old_target);
else
strbuf_addf(buf, "%s ", oid_to_hex(&update->old_oid));
if (!(update->flags & REF_HAVE_NEW))
- strbuf_addf(buf, "%s ", oid_to_hex(null_oid(the_hash_algo)));
+ strbuf_addf(buf, "%s ", oid_to_hex(null_oid(transaction->ref_store->repo->hash_algo)));
else if (update->new_target)
strbuf_addf(buf, "ref:%s ", update->new_target);
else
@@ -3145,6 +3145,7 @@ struct migration_data {
static int migrate_one_ref(const struct reference *ref, void *cb_data)
{
struct migration_data *data = cb_data;
+ const struct git_hash_algo *hash_algo = data->transaction->ref_store->repo->hash_algo;
struct strbuf symref_target = STRBUF_INIT;
int ret;
@@ -3153,7 +3154,7 @@ static int migrate_one_ref(const struct reference *ref, void *cb_data)
if (ret < 0)
goto done;
- ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(the_hash_algo),
+ ret = ref_transaction_update(data->transaction, ref->name, NULL, null_oid(hash_algo),
symref_target.buf, NULL,
REF_SKIP_CREATE_REFLOG | REF_NO_DEREF, NULL, data->errbuf);
if (ret < 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* [PATCH v5 3/3] refs/reftable-backend: drop uses of the_repository
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
@ 2026-04-04 13:58 ` Shreyansh Paliwal
2026-04-08 8:46 ` [PATCH v5 0/3] refs: reduce reliance on global state Patrick Steinhardt
3 siblings, 0 replies; 48+ messages in thread
From: Shreyansh Paliwal @ 2026-04-04 13:58 UTC (permalink / raw)
To: git; +Cc: ps, gitster, a3205153416, bkkaracay, Shreyansh Paliwal
reftable_be_init() and reftable_be_create_on_disk() use the_repository even
though a repository instance is already available, either directly or via
struct ref_store.
Replace these uses with the appropriate local repository instance (repo or
ref_store->repo) to avoid relying on global state.
Note that USE_THE_REPOSITORY_VARIABLE cannot be removed yet, as
is_bare_repository() is still there in the file.
Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index b124404663..7c8a992fcb 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -404,13 +404,13 @@ static struct ref_store *reftable_be_init(struct repository *repo,
default:
BUG("unknown hash algorithm %d", repo->hash_algo->format_id);
}
- refs->write_options.default_permissions = calc_shared_perm(the_repository, 0666 & ~mask);
+ refs->write_options.default_permissions = calc_shared_perm(repo, 0666 & ~mask);
refs->write_options.disable_auto_compact =
!git_env_bool("GIT_TEST_REFTABLE_AUTOCOMPACTION", 1);
refs->write_options.lock_timeout_ms = 100;
refs->write_options.fsync = reftable_be_fsync;
- repo_config(the_repository, reftable_be_config, &refs->write_options);
+ repo_config(repo, reftable_be_config, &refs->write_options);
/*
* It is somewhat unfortunate that we have to mirror the default block
@@ -492,7 +492,7 @@ static int reftable_be_create_on_disk(struct ref_store *ref_store,
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/reftable", refs->base.gitdir);
- safe_create_dir(the_repository, sb.buf, 1);
+ safe_create_dir(ref_store->repo, sb.buf, 1);
strbuf_reset(&sb);
strbuf_release(&sb);
--
2.53.0
^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: [PATCH v5 0/3] refs: reduce reliance on global state
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
` (2 preceding siblings ...)
2026-04-04 13:58 ` [PATCH v5 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
@ 2026-04-08 8:46 ` Patrick Steinhardt
2026-04-08 17:09 ` Junio C Hamano
3 siblings, 1 reply; 48+ messages in thread
From: Patrick Steinhardt @ 2026-04-08 8:46 UTC (permalink / raw)
To: Shreyansh Paliwal; +Cc: git, gitster, a3205153416, bkkaracay
On Sat, Apr 04, 2026 at 07:28:37PM +0530, Shreyansh Paliwal wrote:
> Changes in v5:
> - made the commit message of patch 1/3 more explicit.
>
> Changes in v4:
> - Dropped patches 1/5 and 5/5, as they require further refactoring and
> discussion. I will send them separately as a follow-up.
>
> Changes in v3:
> - Fixed an import
> - better readability in patch 3/5
>
> Changes in v2:
> - Made struct repository the first argument in function parameters.
This version looks good to me. We bail on some of the more intricate
pieces, but that's totally fine as we can still fix these in a
subsequent patch series.
Thanks!
Patrick
^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: [PATCH v5 0/3] refs: reduce reliance on global state
2026-04-08 8:46 ` [PATCH v5 0/3] refs: reduce reliance on global state Patrick Steinhardt
@ 2026-04-08 17:09 ` Junio C Hamano
0 siblings, 0 replies; 48+ messages in thread
From: Junio C Hamano @ 2026-04-08 17:09 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Shreyansh Paliwal, git, a3205153416, bkkaracay
Patrick Steinhardt <ps@pks.im> writes:
> On Sat, Apr 04, 2026 at 07:28:37PM +0530, Shreyansh Paliwal wrote:
>> Changes in v5:
>> - made the commit message of patch 1/3 more explicit.
>>
>> Changes in v4:
>> - Dropped patches 1/5 and 5/5, as they require further refactoring and
>> discussion. I will send them separately as a follow-up.
>>
>> Changes in v3:
>> - Fixed an import
>> - better readability in patch 3/5
>>
>> Changes in v2:
>> - Made struct repository the first argument in function parameters.
>
> This version looks good to me. We bail on some of the more intricate
> pieces, but that's totally fine as we can still fix these in a
> subsequent patch series.
>
> Thanks!
>
> Patrick
Thanks for this ack---all of these iterations somehow escaped from
my radar. Will apply.
^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2026-04-08 17:10 UTC | newest]
Thread overview: 48+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-25 16:44 [PATCH 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
2026-03-27 7:49 ` Patrick Steinhardt
2026-03-28 12:45 ` Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
2026-03-27 7:50 ` Patrick Steinhardt
2026-03-27 9:23 ` Burak Kaan Karaçay
2026-03-28 12:51 ` Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
2026-03-25 16:44 ` [PATCH 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-03-27 7:50 ` Patrick Steinhardt
2026-03-25 16:44 ` [PATCH 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 0/5] refs: reduce reliance on the_repository global state Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 1/5] refs: make branchname helpers repository aware Shreyansh Paliwal
2026-03-28 16:54 ` Tian Yuchen
2026-03-29 9:55 ` Shreyansh Paliwal
2026-03-29 15:37 ` Tian Yuchen
2026-03-28 14:09 ` [PATCH v2 2/5] refs: make get_files_ref_lock_timeout_ms() repostory aware Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
2026-03-28 17:03 ` Tian Yuchen
2026-03-28 14:09 ` [PATCH v2 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-03-28 14:09 ` [PATCH v2 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-03-28 17:08 ` Tian Yuchen
2026-03-29 9:54 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 0/5] replace the_repository with local repository instances Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 1/5] refs: add struct repository parameter to branchname helpers Shreyansh Paliwal
2026-04-02 7:27 ` Patrick Steinhardt
2026-04-02 17:03 ` Burak Kaan Karaçay
2026-04-02 17:48 ` Tian Yuchen
2026-04-02 18:57 ` Patrick Steinhardt
2026-04-03 10:39 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 2/5] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 3/5] refs: remove the_hash_algo global state Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 4/5] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-04-02 7:27 ` Patrick Steinhardt
2026-04-03 10:43 ` Shreyansh Paliwal
2026-03-29 10:16 ` [PATCH v3 5/5] refs/packed-backend: use ref_store->repo instead " Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-03 12:08 ` [PATCH v4 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
2026-04-03 17:40 ` Tian Yuchen
2026-04-03 12:08 ` [PATCH v4 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
2026-04-03 12:09 ` [PATCH v4 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 0/3] refs: reduce reliance on global state Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 1/3] refs: add struct repository parameter in get_files_ref_lock_timeout_ms() Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 2/3] refs: remove the_hash_algo global state Shreyansh Paliwal
2026-04-04 13:58 ` [PATCH v5 3/3] refs/reftable-backend: drop uses of the_repository Shreyansh Paliwal
2026-04-08 8:46 ` [PATCH v5 0/3] refs: reduce reliance on global state Patrick Steinhardt
2026-04-08 17:09 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox