From: Karthik Nayak Date: Mon, 02 Feb 2026 10:02:08 GMT Subject: Re: [PATCH 1/3] wt-status: replace uses of the_repository with local repository instances Message-ID: In-Reply-To: <20260131190106.389289-2-shreyanshpaliwalcmsmn@gmail.com> Shreyansh Paliwal writes: > Many instances of the_repository are used in wt-status.c even when a > local repository is already available via struct wt_status or struct > worktree. > One missing information is why is it safe to make this change? If is a repository field, is it holding the same information, is it always defined? > Replace direct uses of the global the_repository with the repository > instance carried by the local structs (e.g. s->repo, wt->repo). > > This helps reduce reliance on global repository state. > > Signed-off-by: Shreyansh Paliwal > --- > wt-status.c | 38 +++++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 19 deletions(-) > > diff --git a/wt-status.c b/wt-status.c > index e12adb26b9..9f4d8fda7f 100644 > --- a/wt-status.c > +++ b/wt-status.c > @@ -150,11 +150,11 @@ void wt_status_prepare(struct repository *r, struct wt_status *s) > s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES; > s->use_color = GIT_COLOR_UNKNOWN; > s->relative_paths = 1; > - s->branch = refs_resolve_refdup(get_main_ref_store(the_repository), > + s->branch = refs_resolve_refdup(get_main_ref_store(s->repo), > "HEAD", 0, NULL, NULL); Wouldn't it make more sense to use the function argument 'r' here? > s->reference = "HEAD"; > s->fp = stdout; > - s->index_file = repo_get_index_file(the_repository); > + s->index_file = repo_get_index_file(s->repo); > s->change.strdup_strings = 1; > s->untracked.strdup_strings = 1; > s->ignored.strdup_strings = 1; > @@ -646,7 +646,7 @@ static void wt_status_collect_changes_index(struct wt_status *s) > > repo_init_revisions(s->repo, &rev, NULL); > memset(&opt, 0, sizeof(opt)); > - opt.def = s->is_initial ? empty_tree_oid_hex(the_repository->hash_algo) : s->reference; > + opt.def = s->is_initial ? empty_tree_oid_hex(s->repo->hash_algo) : s->reference; > setup_revisions(0, NULL, &rev, &opt); > > rev.diffopt.flags.override_submodule_config = 1; > @@ -1146,7 +1146,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s) > rev.diffopt.ita_invisible_in_index = 1; > > memset(&opt, 0, sizeof(opt)); > - opt.def = s->is_initial ? empty_tree_oid_hex(the_repository->hash_algo) : s->reference; > + opt.def = s->is_initial ? empty_tree_oid_hex(s->repo->hash_algo) : s->reference; > setup_revisions(0, NULL, &rev, &opt); > > rev.diffopt.output_format |= DIFF_FORMAT_PATCH; > @@ -1317,9 +1317,9 @@ static int split_commit_in_progress(struct wt_status *s) > !s->branch || strcmp(s->branch, "HEAD")) > return 0; > > - if (refs_read_ref_full(get_main_ref_store(the_repository), "HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + if (refs_read_ref_full(get_main_ref_store(s->repo), "HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > &head_oid, &head_flags) || > - refs_read_ref_full(get_main_ref_store(the_repository), "ORIG_HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > + refs_read_ref_full(get_main_ref_store(s->repo), "ORIG_HEAD", RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, > &orig_head_oid, &orig_head_flags)) > return 0; > if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF) > @@ -1432,7 +1432,7 @@ static void show_rebase_information(struct wt_status *s, > i++) > status_printf_ln(s, color, " %s", have_done.items[i].string); > if (have_done.nr > nr_lines_to_show && s->hints) { > - char *path = repo_git_path(the_repository, "rebase-merge/done"); > + char *path = repo_git_path(s->repo, "rebase-merge/done"); > status_printf_ln(s, color, > _(" (see more in file %s)"), path); > free(path); > @@ -1534,7 +1534,7 @@ static void show_cherry_pick_in_progress(struct wt_status *s, > else > status_printf_ln(s, color, > _("You are currently cherry-picking commit %s."), > - repo_find_unique_abbrev(the_repository, &s->state.cherry_pick_head_oid, > + repo_find_unique_abbrev(s->repo, &s->state.cherry_pick_head_oid, > DEFAULT_ABBREV)); > > if (s->hints) { > @@ -1564,7 +1564,7 @@ static void show_revert_in_progress(struct wt_status *s, > else > status_printf_ln(s, color, > _("You are currently reverting commit %s."), > - repo_find_unique_abbrev(the_repository, &s->state.revert_head_oid, > + repo_find_unique_abbrev(s->repo, &s->state.revert_head_oid, > DEFAULT_ABBREV)); > if (s->hints) { > if (has_unmerged(s)) > @@ -1624,7 +1624,7 @@ static char *get_branch(const struct worktree *wt, const char *path) > struct object_id oid; > const char *branch_name; > > - if (strbuf_read_file(&sb, worktree_git_path(the_repository, wt, "%s", path), 0) <= 0) > + if (strbuf_read_file(&sb, worktree_git_path(wt->repo, wt, "%s", path), 0) <= 0) > goto got_nothing; > > while (sb.len && sb.buf[sb.len - 1] == '\n') > @@ -1691,7 +1691,7 @@ static void wt_status_get_detached_from(struct repository *r, > char *ref = NULL; > > strbuf_init(&cb.buf, 0); > - if (refs_for_each_reflog_ent_reverse(get_main_ref_store(the_repository), "HEAD", grab_1st_switch, &cb) <= 0) { > + if (refs_for_each_reflog_ent_reverse(get_main_ref_store(r), "HEAD", grab_1st_switch, &cb) <= 0) { > strbuf_release(&cb.buf); > return; > } > @@ -1723,18 +1723,18 @@ int wt_status_check_rebase(const struct worktree *wt, > { > struct stat st; > > - if (!stat(worktree_git_path(the_repository, wt, "rebase-apply"), &st)) { > - if (!stat(worktree_git_path(the_repository, wt, "rebase-apply/applying"), &st)) { > + if (!stat(worktree_git_path(wt->repo, wt, "rebase-apply"), &st)) { > + if (!stat(worktree_git_path(wt->repo, wt, "rebase-apply/applying"), &st)) { In the same file we make a call 'wt_status_check_rebase(NULL, state)', so wouldn't this break? > state->am_in_progress = 1; > - if (!stat(worktree_git_path(the_repository, wt, "rebase-apply/patch"), &st) && !st.st_size) > + if (!stat(worktree_git_path(wt->repo, wt, "rebase-apply/patch"), &st) && !st.st_size) > state->am_empty_patch = 1; > } else { > state->rebase_in_progress = 1; > state->branch = get_branch(wt, "rebase-apply/head-name"); > state->onto = get_branch(wt, "rebase-apply/onto"); > } > - } else if (!stat(worktree_git_path(the_repository, wt, "rebase-merge"), &st)) { > - if (!stat(worktree_git_path(the_repository, wt, "rebase-merge/interactive"), &st)) > + } else if (!stat(worktree_git_path(wt->repo, wt, "rebase-merge"), &st)) { > + if (!stat(worktree_git_path(wt->repo, wt, "rebase-merge/interactive"), &st)) > state->rebase_interactive_in_progress = 1; > else > state->rebase_in_progress = 1; > @@ -1750,7 +1750,7 @@ int wt_status_check_bisect(const struct worktree *wt, > { > struct stat st; > > - if (!stat(worktree_git_path(the_repository, wt, "BISECT_LOG"), &st)) { > + if (!stat(worktree_git_path(wt->repo, wt, "BISECT_LOG"), &st)) { > state->bisect_in_progress = 1; > state->bisecting_from = get_branch(wt, "BISECT_START"); > return 1; > @@ -2099,7 +2099,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) > upstream_is_gone = 1; > } > > - short_base = refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), > + short_base = refs_shorten_unambiguous_ref(get_main_ref_store(s->repo), > base, 0); > color_fprintf(s->fp, header_color, "..."); > color_fprintf(s->fp, branch_color_remote, "%s", short_base); > @@ -2233,7 +2233,7 @@ static void wt_porcelain_v2_print_tracking(struct wt_status *s) > ab_info = stat_tracking_info(branch, &nr_ahead, &nr_behind, > &base, 0, s->ahead_behind_flags); > if (base) { > - base = refs_shorten_unambiguous_ref(get_main_ref_store(the_repository), > + base = refs_shorten_unambiguous_ref(get_main_ref_store(s->repo), > base, 0); > fprintf(s->fp, "# branch.upstream %s%c", base, eol); > free((char *)base); > -- > 2.52.0