patch, 3 partswt-status: reduce reliance on global state

In wt-status.c code still relies on some global variables, including the_repository and the_hash_algo, even in cases where a repository instance is already available via struct wt_status or struct worktree.

In patch 1/3, replace direct uses of the_repository with repository instances already available in local structs.

In patch 2/3, update function parameters and calls to pass struct repository or struct wt_status explicitly where no local repository access was available.

In patch 3/3, replace remaining uses of the global the_hash_algo with the hash algorithm stored in the respective repository instance.

These changes remove all direct uses of the_repository and the_hash_algo from wt-status.c and reduce its dependence on global state.

Shreyansh Paliwal (3):
  wt-status: replace uses of the_repository with local repository
    instances
  wt-status: pass struct repository and wt_status through function
    parameters
  wt-status: use hash_algo from local repository instead of global
    the_hash_algo

 wt-status.c | 76 ++++++++++++++++++++++++++---------------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

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.

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 <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Some functions in wt-status.c relied on the_repository because no repository instance was available in their local scope.

Update these functions to accept struct repository or struct wt_status as parameters, and adjust callers accordingly.

Replace remaining uses of the_repository in these functions with the passed-in repository instance.

This completely removes the use of the_repository global variable in wt-status.c.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

wt-status.c uses the global the_hash_algo even though a repository instance is already available via struct repository *r.

Replace uses of the_hash_algo with the hash algorithm stored in the associated repository (r->hash_algo).

This removes another dependency on global state and keeps wt-status consistent with local repository usage.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

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?

Wouldn't it make more sense to use the function argument 'r' here?

In the same file we make a call 'wt_status_check_rebase(NULL, state)', so wouldn't this break?

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

One final question is, does this mean we can remove `USE_THE_REPOSITORY_VARIABLE` after these changes? If not, why?

Karthik Nayak <karthik.188@gmail.com> writes:

Good questions.  Thanks for a quick review.

Yes I should have included explanation as well. I have explained below, let me know if this thought process is valid or not.

The replacement of all the_repository with s->repo in this patch are mostly to cases where a repository instance is already available via struct wt_status.

In the current flow, all functions operating on struct wt_status *s are called via commit.c. There, status_init_config() calls wt_status_prepare(), which initializes the struct wt_status and assigns s->repo from the repository instance passed in by the caller. As a result, s->repo is guaranteed to be initialized whenever these functions are invoked.

And commit.c itself still relies on the_repository, within wt-status.c, the local repository pointer refers to the same underlying repository object that the_repository would have pointed to, indirectly until we make commit.c also free of the_repository.

In wt_status_prepare(), s->repo is initialized to r at the top of the function, so both refer to the same repository instance. However, using r directly is more explicit and avoids indirect use. will change this in V2.

Yes my bad, it would throw a segfault error. I think the best way to handle this is to explicitly check for the wt to be valid like this,

    if (wt==NULL)
        return 0;

Falling back to the_repository in this case, would probably defeat the purpose.

We cannot remove `USE_THE_REPOSITORY_VARIABLE` yet, because the changes done only remove the direct use of the_hash_algo and the_repository, but 'USE_THE_REPOSITORY_VARIABLE' is for all the global variables that are still in use.

In particular wt-status.c still relies on the following globals,

* core_apply_sparse_checkout, this is already being addressed in an
ongoing patch series [1], so I intentionally did not modify it.

* comment_line_str and DEFAULT_ABBREV, these both still are used in
wt-status.c but they dont have any local instance in wt-status.c,
or in any other form.

Removing these would require a wider refactoring (adding in struct wt_status, adding helper functions etc) and I believe is better handled as a separate patch series.

Though I require some guidance on the preferred approach for handling comment_line_str and DEFAULT_ABBREV going forward.

Thanks for reviewing.

Best, Shreyansh

[1]- https://lore.kernel.org/git/5e56e1cc4172cfff9e917a068184e102aa70bf1d.1769256839.git.belkid98@gmail.com/t/#u

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

Would we benefit from further clean-up, either before or after this change, to lose the "struct repository *r" parameter, if we know that we can depend on s->repo being the repository we are collecting the status information in the wt_status structure for?

Junio C Hamano <gitster@pobox.com> writes:

Clarification. This function is like an initializer for the struct wt_status instance at 's', so it has to take both "struct repository" parameter, but what I meant was other wt_status_foo() functions that take both r and s as parameters. Once s has been initialized and s->repo becomes valid, passing r as a separate parameter, as if you can feed a different instance of "struct repo", becomes confusing and a source of bugs.

Actually wt_status_prepare() is the only function which is taking both struct wt_status *s and struct repository *r, because it has to initialize 's' with the help of 'r'. But all the other wt_status_.. helper functions only take one of the following, i.e. either they take struct wt_status *s which is fine, or they take struct repository *r.

If we trace the callers of the functions with struct repository *r, they are either being called in wt_status.c in which s->repo is being passed as a parameter at the end so it shouldn't cause any issues, and for any other file callers, the_repository is being passed, so to remove struct repository *r, we would have to setup struct wt_status in those files as well.

Best, Shreyansh

On 02/02/2026 18:57, Shreyansh Paliwal wrote:

Good explanation, that would be a very useful addition to the commit message

Yes, a few more context lines makes it is clear that either is safe.

That would change the behavior of the function though as it will no longer check if the rebase directories exist. You should pass the repository down from the caller.

Thanks for working on this

Phillip

On 03/02/2026 09:53, Shreyansh Paliwal wrote:

You're correct, but I had the same reaction as Junio initially as I was confused by the functions that take a "struct repository" and "struct wt_status_state" which does not contain a repository, but at first glance looks a lot like "struct wt_status" which does.

Thanks

Phillip

Sure I will include this in v2.

Yes agreed. I will pass s->repo as struct repository *r in this function. Thanks.

Best, Shreyansh

Ah, got it. Even I got confused by this once :)

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

Understandable.

Yeah, my intent wasn't to have all the cleanup done in this series, but rather to ensure we have enough information presented. You've already explained the reason. It would be great if you could add this information to the cover.

Happy to help out. Generally using an RFC patch series generally has the best outcome, since it provides material to base discussions on.

Yes sure, I will add in v2.

Thanks, I will try to send an RFC patch series regarding this after the v2.

Best, Shreyansh

In wt-status.c code still relies on some global variables, including the_repository and the_hash_algo, even in cases where a repository instance is already available via struct wt_status or struct worktree.

In patch 1/3, replace direct uses of the_repository with repository instances already available in local structs.

In patch 2/3, update function parameters and calls to pass struct repository or struct wt_status explicitly where no local repository access was available.

In patch 3/3, replace remaining uses of the global the_hash_algo with the hash algorithm stored in the respective repository instance.

These changes remove all direct uses of the_repository and the_hash_algo from wt-status.c and reduce its dependence on global state.

The 'USE_THE_REPOSITORY_VARIABLE' macro cannot yet be removed, since these patches only eliminate direct uses of the_repository and the_hash_algo, while other global variables are still referenced.

In particular wt-status.c still relies on the following globals,

* core_apply_sparse_checkout, this is already being addressed in an
ongoing patch series [1].

* comment_line_str and DEFAULT_ABBREV, these both still are used in
wt-status.c but they dont have any equivalent local instances.

[1]- https://lore.kernel.org/git/5e56e1cc4172cfff9e917a068184e102aa70bf1d.1769256839.git.belkid98@gmail.com/t/#u

Shreyansh Paliwal (3):
  wt-status: replace uses of the_repository with local repository
    instances
  wt-status: pass struct repository and wt_status through function
    parameters
  wt-status: use hash_algo from local repository instead of global
    the_hash_algo

 branch.c    |  2 +-
 worktree.c  |  2 +-
 wt-status.c | 85 +++++++++++++++++++++++++++--------------------------
 wt-status.h |  5 ++--
 4 files changed, 48 insertions(+), 46 deletions(-)

wt-status.c uses the global the_repository in several places even when a repository instance is already available via struct wt_status or struct worktree.

Replace these direct uses of the_repository with the repository carried by the local structs (e.g. s->repo, wt->repo).

The replacements of all the_repository with s->repo are mostly to cases where a repository instance is already available via struct wt_status. All functions operating on struct wt_status *s are only used after s is initialized by wt_status_prepare(), which sets s->repo from the repository provided by the caller. As a result, s->repo is guaranteed to be available and consistent whenever these functions are invoked.

This reduces reliance on global state and keeps wt-status consistent, though many functions operating on struct wt_status *s are called via commit.c and it still relies on the_repository, but within wt-status.c the local repository pointer refers to the same underlying repository object.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Some functions in wt-status.c relied on the_repository because no repository instance was available in their local scope. There is also a specific case in wt_status_check_rebase() where the worktree can be NULL, so accessing wt->repo may lead to a segfault.

Update these functions to accept a struct repository or struct wt_status parameter, and adjust callers accordingly. Replace the remaining uses of the_repository in these functions with the passed-in repository instance.

This removes the use of the_repository global variable from wt-status.c completely.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 branch.c    |  2 +-
 worktree.c  |  2 +-
 wt-status.c | 51 ++++++++++++++++++++++++++-------------------------
 wt-status.h |  5 +++--
 4 files changed, 31 insertions(+), 29 deletions(-)

wt-status.c still uses the global the_hash_algo even though a repository instance is already available via struct wt_status.

Replace uses of the_hash_algo with the hash algorithm stored in the associated repository (s->repo->hash_algo or r->hash_algo).

This removes another dependency on global state and keeps wt-status consistent with local repository usage.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

I forgot to include the changes in the version.

Changes in V2,
- Explained the changes and the reason more elaborately in commit message.
- Passed struct repository instead of accessing struct worktree in
   wt_status_check_rebase() and shifted this change to patch 2/3 instead of 1/3.
- Added information about leftover globals in the cover.

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

So if you look into `worktree_git_path()`, it has a certain check

  if (wt && wt->repo != r)
		BUG("worktree not connected to expected repository");

But this is okay with your change, the only question is, do we know wt is always defined here? Unfortunately, wt can be NULL here, in the same file we have:

  wt_status_check_rebase(NULL, state);
  -> get_branch(NULL, ...)

Which would crash, no? This is applicable for other parts of the code too were we're now using wt->repo.

This is also what I was requesting in the previous round, about explaining why it is safe to make a particular change.

[snip]

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

Okay, but this doesn't fix the issue I stated in the previous commit. I do wonder if we can re-order the commits and pass the repo struct to functions like 'get_branch()'.

[snip]

Karthik Nayak <karthik.188@gmail.com> writes:

One question, did you run the entire test suite with these changes? I would hope that we have tests which would fail if my inference is correct. If not, there's a gap in our tests too.

- Karthik

Sorry, I didn’t fully think through what you were pointing out in the previous version. I initially assumed you were only referring to the specific call involving wt_status_check_rebase(NULL, state).

So struct worktree *wt is not guaranteed to be defined and can be NULL. Because of that, relying on wt->repo is unsafe. Instead, rather than depending on the worktree for repository access (wt->repo), we can pass struct repository explicitly through the relevant call stack, like in functions get_branch() and wt_status_check_bisect(). I hope I have understood it correctly.

Best, Shreyansh

[...]

You’re right, I hadn’t run the tests initially, as I assumed this was a refactor-only change.

After running the tests, I do see failures, which confirms the issue. I’ll make sure to always run the tests before sending a patch going forward.

Thanks for pointing this out :)

On 05/02/2026 10:27, Shreyansh Paliwal wrote:

As well as describing the changes it is very helpful to include a range-diff to show what's changed - see the --range-diff option to "git format-patch". I've pasted the range-diff between V1 and V2 below

Thanks

Phillip

1:  1bf5449c7a8 ! 1:  7285c64e672 wt-status: replace uses of the_repository with local repository instances
     @@ Metadata
       ## Commit message ##
          wt-status: replace uses of the_repository with local repository instances
      
     -    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.
     -
     -    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.
     +    wt-status.c uses the global the_repository in several places even when
     +    a repository instance is already available via struct wt_status or
     +    struct worktree.
     +
     +    Replace these direct uses of the_repository with the repository carried
     +    by the local structs (e.g. s->repo, wt->repo).
     +
     +    The replacements of all the_repository with s->repo are mostly
     +    to cases where a repository instance is already available via
     +    struct wt_status. All functions operating on struct wt_status *s
     +    are only used after s is initialized by wt_status_prepare(),
     +    which sets s->repo from the repository provided by the caller.
     +    As a result, s->repo is guaranteed to be available and consistent
     +    whenever these functions are invoked.
     +
     +    This reduces reliance on global state and keeps wt-status consistent,
     +    though many functions operating on struct wt_status *s
     +    are called via commit.c and it still relies on the_repository,
     +    but within wt-status.c the local repository pointer
     +    refers to the same underlying repository object.
      
          Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
      
     @@ wt-status.c: void wt_status_prepare(struct repository *r, struct wt_status *s)
       	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),
     ++	s->branch = refs_resolve_refdup(get_main_ref_store(r),
       					"HEAD", 0, NULL, NULL);
       	s->reference = "HEAD";
       	s->fp = stdout;
     @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
       		strbuf_release(&cb.buf);
       		return;
       	}
     -@@ wt-status.c: 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)) {
     - 			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;
      @@ wt-status.c: int wt_status_check_bisect(const struct worktree *wt,
       {
       	struct stat st;
2:  8a0b489c3be ! 2:  0a04c957f12 wt-status: pass struct repository and wt_status through function parameters
     @@ Commit message
      
          Some functions in wt-status.c relied on the_repository because no
          repository instance was available in their local scope.
     -
     -    Update these functions to accept struct repository or struct
     -    wt_status as parameters, and adjust callers accordingly.
     -
     -    Replace remaining uses of the_repository in these functions with the
     +    There is also a specific case in wt_status_check_rebase() where the
     +    worktree can be NULL, so accessing wt->repo may lead to a segfault.
     +
     +    Update these functions to accept a struct repository or struct
     +    wt_status parameter, and adjust callers accordingly. Replace the
     +    remaining uses of the_repository in these functions with the
          passed-in repository instance.
      
     -    This completely removes the use of the_repository global variable
     -    in wt-status.c.
     +    This removes the use of the_repository global variable from
     +    wt-status.c completely.
      
          Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
      
     + ## branch.c ##
     +@@ branch.c: static void prepare_checked_out_branches(void)
     + 			free(old);
     + 		}
     +
     +-		if (wt_status_check_rebase(wt, &state) &&
     ++		if (wt_status_check_rebase(wt->repo, wt, &state) &&
     + 		    (state.rebase_in_progress || state.rebase_interactive_in_progress) &&
     + 		    state.branch) {
     + 			struct strbuf ref = STRBUF_INIT;
     +
     + ## worktree.c ##
     +@@ worktree.c: int is_worktree_being_rebased(const struct worktree *wt,
     + 	int found_rebase;
     +
     + 	memset(&state, 0, sizeof(state));
     +-	found_rebase = wt_status_check_rebase(wt, &state) &&
     ++	found_rebase = wt_status_check_rebase(wt->repo, wt, &state) &&
     + 		       (state.rebase_in_progress ||
     + 			state.rebase_interactive_in_progress) &&
     + 		       state.branch &&
     +
       ## wt-status.c ##
      @@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
       	return 0;
     @@ wt-status.c: static void show_rebase_information(struct wt_status *s,
       					 &yet_to_do))
       			status_printf_ln(s, color,
       				_("git-rebase-todo is missing."));
     +@@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
     + 	strbuf_release(&cb.buf);
     + }
     +
     +-int wt_status_check_rebase(const struct worktree *wt,
     +-			   struct wt_status_state *state)
     ++int wt_status_check_rebase(struct repository *r,
     ++			 const struct worktree *wt,
     ++			 struct wt_status_state *state)
     + {
     + 	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(r, wt, "rebase-apply"), &st)) {
     ++		if (!stat(worktree_git_path(r, wt, "rebase-apply/applying"), &st)) {
     + 			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(r, 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(r, wt, "rebase-merge"), &st)) {
     ++		if (!stat(worktree_git_path(r, wt, "rebase-merge/interactive"), &st))
     + 			state->rebase_interactive_in_progress = 1;
     + 		else
     + 			state->rebase_in_progress = 1;
     +@@ wt-status.c: void wt_status_get_state(struct repository *r,
     + 	enum replay_action action;
     +
     + 	if (!stat(git_path_merge_head(r), &st)) {
     +-		wt_status_check_rebase(NULL, state);
     ++		wt_status_check_rebase(r, NULL, state);
     + 		state->merge_in_progress = 1;
     +-	} else if (wt_status_check_rebase(NULL, state)) {
     ++	} else if (wt_status_check_rebase(r, NULL, state)) {
     + 		;		/* all set */
     + 	} else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
     + 		   !repo_get_oid(r, "CHERRY_PICK_HEAD", &oid)) {
      @@ wt-status.c: static void wt_porcelain_v2_print_tracking(struct wt_status *s)
        */
       static void wt_porcelain_v2_print_stash(struct wt_status *s)
     @@ wt-status.c: static void wt_porcelain_v2_print_tracking(struct wt_status *s)
       	char eol = s->null_termination ? '\0' : '\n';
       
       	if (stash_count > 0)
     +
     + ## wt-status.h ##
     +@@ wt-status.h: void wt_status_state_free_buffers(struct wt_status_state *s);
     + void wt_status_get_state(struct repository *repo,
     + 			 struct wt_status_state *state,
     + 			 int get_detached_from);
     +-int wt_status_check_rebase(const struct worktree *wt,
     +-			   struct wt_status_state *state);
     ++int wt_status_check_rebase(struct repository *r,
     ++			 const struct worktree *wt,
     ++			 struct wt_status_state *state);
     + int wt_status_check_bisect(const struct worktree *wt,
     + 			   struct wt_status_state *state);
     +
3:  e2cf365bad5 ! 3:  74b28d95299 wt-status: use hash_algo from local repository instead of global the_hash_algo
     @@ Metadata
       ## Commit message ##
          wt-status: use hash_algo from local repository instead of global the_hash_algo
      
     -    wt-status.c uses the global the_hash_algo even though a repository
     -    instance is already available via struct repository *r.
     +    wt-status.c still uses the global the_hash_algo even though a repository
     +    instance is already available via struct wt_status.
      
          Replace uses of the_hash_algo with the hash algorithm stored in the
     -    associated repository (r->hash_algo).
     +    associated repository (s->repo->hash_algo or r->hash_algo).
      
          This removes another dependency on global state and keeps wt-status
          consistent with local repository usage.

On 05/02/2026 11:09, Karthik Nayak wrote:

These callers pass wt->repo so it's important to explain that the callers all have a non-NULL worktree instance so passing wt->repo to wt_status_check_rebase() is safe.

That sounds like a good suggestion

Thanks

Phillip

On 05/02/2026 12:18, Shreyansh Paliwal wrote:

The whole point of regression tests is to check that refactors do not change the behavior. If you have an account on github or gitlab there are some instructions for setting the up the CI to run when you push in Documentation/SumbittingPatches.

Thanks

Phillip

Thanks, I will keep this in mind.

Got it, I will look into it.

Best, Shreyansh

On Thu, Feb 5, 2026, at 18:39, Shreyansh Paliwal wrote:

I’m personally a happy user of `--interdiff` in addition. It’s a nice supplement. :)

https://lore.kernel.org/git/CAPig+cSErj4ZB9bHB8mZfzNkiaN_EpjT6b4b=cfsf_+KMqytiA@mail.gmail.com/

Sounds good, I will try out both in the v3.

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

I would say its not that black and white. There's a lot of context around it. There are places where `struct worktree *wt` should definitely be defined. So we have to take it on a case by case basis. Which is what makes these refactors a bit tricky.

Karthik

But lets say if we do pass struct repository explicitly wherever it is doubtful, rather than relying on worktree, does it have any downside ?

I tried this out below, and it showed no fails in tests. After this we can just directly replace all the_repository with 'r' or 's->repo' without the hassle of checking the worktree is defined or not. Let me know what you think. Thanks.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 branch.c    |  4 ++--
 worktree.c  |  4 ++--
 wt-status.c | 48 +++++++++++++++++++++++++-----------------------
 wt-status.h |  6 ++++--
 4 files changed, 33 insertions(+), 29 deletions(-)

On 06/02/2026 12:57, Shreyansh Paliwal wrote:

As we're trying to remove uses of "the_repository" I think you should use "wt->repo" where we know always "wt != NULL". There are not many callers of these functions so is easy to do necessary analysis (see below).

As I said yesterday we know "wt != NULL" here so it is fine to use "wt->repo" rather than introduce a new use of "the_repository", you just need to explain that in the commit message.

The same is true here.

This function is called from builtin/branch.c:reject_rebase_or_bisect_branch() with "wt != NULL". It is also called from worktree.c:is_shared_symref() which dereferences wt before calling this function so we can assume "wt != NULL" there as well. That means we can use "wt->repo" here.

The same analysis for is_worktree_being_rebased() applies here.

The changes to get_branch() below look sensible

Thanks

Phillip

Thank you for explaining this and for each case. I have understood where wt->repo can be used safely. Will make changes and send a v3. Hopefully that will be good to go :)

Best, Shreyansh

In wt-status.c code still relies on some global variables, including the_repository and the_hash_algo, even in cases where a repository instance is already available via struct wt_status or struct worktree.

In patch 1/3, update function parameters and callers to pass struct repository where no local repository access was available.

In patch 2/3, replace direct uses of the_repository with repository instances already available in local structs.

In patch 3/3, replace remaining uses of the global the_hash_algo with the hash algorithm stored in the respective repository instance.

These changes remove all direct uses of the_repository and the_hash_algo from wt-status.c and reduce its dependence on global state.

The 'USE_THE_REPOSITORY_VARIABLE' macro cannot yet be removed, since these patches only eliminate direct uses of the_repository and the_hash_algo, while other global variables are still referenced.

In particular wt-status.c still relies on the following globals,

* core_apply_sparse_checkout, this is already being addressed in an
ongoing patch series [1].

* comment_line_str and DEFAULT_ABBREV, these both still are used in
wt-status.c but they dont have any equivalent local instances.

[1]- https://lore.kernel.org/git/5e56e1cc4172cfff9e917a068184e102aa70bf1d.1769256839.git.belkid98@gmail.com/t/#u

Shreyansh Paliwal (3):
  wt-status: pass struct repository through function parameters
  wt-status: replace uses of the_repository with local repository instances
  wt-status: use hash_algo from local repository instead of global the_hash_algo

 branch.c    |   4 +--
 worktree.c  |   4 +--
 wt-status.c | 102 ++++++++++++++++++++++++++--------------------------
 wt-status.h |   6 ++--
 4 files changed, 60 insertions(+), 56 deletions(-)

---
Changes in v2:
 - Reordered the first 2 patches for better flow.
 - Instead of relying on wt->repo in wt_status_check_rebase() and
    wt_status_check_bisect() addded struct repository.
 - Added extra explainations in the commit message for the usage of wt
    and being it not NULL.

Range-diff against v2:
2:  2af0113f6f ! 1:  960216e45c wt-status: pass struct repository and wt_status through function parameters
    @@ Metadata
     Author: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
     
      ## Commit message ##
    -    wt-status: pass struct repository and wt_status through function parameters
    +    wt-status: pass struct repository through function parameters
     
    -    Some functions in wt-status.c relied on the_repository because no
    -    repository instance was available in their local scope.
    -    There is also a specific case in wt_status_check_rebase() where the
    -    worktree can be NULL, so accessing wt->repo may lead to a segfault.
    +    Some functions in wt-status.c (count_stash_entries(),
    +    read_line_from_git_path(), abbrev_oid_in_line(), read_rebase_todolist())
    +    do not have access to a local repository instance and rely on the_repository.
     
    -    Update these functions to accept a struct repository or struct
    -    wt_status parameter, and adjust callers accordingly. Replace the
    -    remaining uses of the_repository in these functions with the
    -    passed-in repository instance.
    +    Add a struct repository *r parameter to these functions, and pass the local
    +    repository through the callers.
     
    -    This removes the use of the_repository global variable from
    -    wt-status.c completely.
    +    get_branch(), wt_status_check_rebase() and wt_status_check_bisect() already
    +    receive a struct worktree *, which can provide access to the repository.
    +    However, some callers pass NULL as the worktree like in wt_status_get_state(),
    +    which would make using wt->repo unsafe and lead to segfault issues.
    +    Add an explicit struct repository * parameter to these functions as well,
    +    and pass the repository through the callers.
    +
    +    Both wt_status_check_rebase() and  wt_status_check_bisect() are called from
    +    branch.c and worktree.c,
    +
    +    * In branch.c, wt is always non-NULL as the functions are called within an
    +    interation over worktrees in prepare_checked_out_branches().
    +    * In worktree.c the functions are called from is_worktree_being_rebased() and
    +    is_worktree_being_bisected() respectively which are further called from
    +    builtin/branch.c in reject_rebase_or_bisect_branch() which has a non-NULL
    +    worktree as it is called inside an iteration over worktrees as well.
     
         Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
     
    @@ branch.c: static void prepare_checked_out_branches(void)
      		    (state.rebase_in_progress || state.rebase_interactive_in_progress) &&
      		    state.branch) {
      			struct strbuf ref = STRBUF_INIT;
    +@@ branch.c: static void prepare_checked_out_branches(void)
    + 		}
    + 		wt_status_state_free_buffers(&state);
    + 
    +-		if (wt_status_check_bisect(wt, &state) &&
    ++		if (wt_status_check_bisect(wt->repo, wt, &state) &&
    + 		    state.bisecting_from) {
    + 			struct strbuf ref = STRBUF_INIT;
    + 			strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
     
      ## worktree.c ##
     @@ worktree.c: int is_worktree_being_rebased(const struct worktree *wt,
    @@ worktree.c: int is_worktree_being_rebased(const struct worktree *wt,
      		       (state.rebase_in_progress ||
      			state.rebase_interactive_in_progress) &&
      		       state.branch &&
    +@@ worktree.c: int is_worktree_being_bisected(const struct worktree *wt,
    + 	int found_bisect;
    + 
    + 	memset(&state, 0, sizeof(state));
    +-	found_bisect = wt_status_check_bisect(wt, &state) &&
    ++	found_bisect = wt_status_check_bisect(wt->repo, wt, &state) &&
    + 		       state.bisecting_from &&
    + 		       skip_prefix(target, "refs/heads/", &target) &&
    + 		       !strcmp(state.bisecting_from, target);
     
      ## wt-status.c ##
     @@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
    @@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
     +static int count_stash_entries(struct repository *r)
      {
      	int n = 0;
    --	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    -+	refs_for_each_reflog_ent(get_main_ref_store(r),
    - 				 "refs/stash", stash_count_refs, &n);
    - 	return n;
    - }
    + 	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    +@@ wt-status.c: static int count_stash_entries(void)
      
      static void wt_longstatus_print_stash_summary(struct wt_status *s)
      {
    @@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
      }
      
     -static char *read_line_from_git_path(const char *filename)
    -+static char *read_line_from_git_path(struct repository *r, const char *filename)
    ++static char *read_line_from_git_path(struct repository *r, char *filename)
      {
      	struct strbuf buf = STRBUF_INIT;
    --	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    -+	FILE *fp = fopen_or_warn(repo_git_path_append(r, &buf,
    - 						      "%s", filename), "r");
    - 
    - 	if (!fp) {
    + 	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
     @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      	if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF)
      		return 0;
    @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      {
      	struct string_list split = STRING_LIST_INIT_DUP;
      	struct object_id oid;
    -@@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
    - 		return;
    - 
    - 	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
    --	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
    -+	    !repo_get_oid(r, split.items[1].string, &oid)) {
    - 		strbuf_reset(line);
    - 		strbuf_addf(line, "%s ", split.items[0].string);
    - 		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
     @@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
      	string_list_clear(&split, 0);
      }
      
     -static int read_rebase_todolist(const char *fname, struct string_list *lines)
    -+static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)
    ++static int read_rebase_todolist(struct repository *r, char *fname, struct string_list *lines)
      {
      	struct strbuf buf = STRBUF_INIT;
    --	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    -+	FILE *f = fopen(repo_git_path_append(r, &buf, "%s", fname), "r");
    - 	int ret;
    - 
    - 	if (!f) {
    -@@ wt-status.c: static int read_rebase_todolist(const char *fname, struct string_list *lines)
    - 			goto out;
    - 		}
    - 		die_errno("Could not open file %s for reading",
    --			  repo_git_path_replace(the_repository, &buf, "%s", fname));
    -+			  repo_git_path_replace(r, &buf, "%s", fname));
    - 	}
    - 	while (!strbuf_getline_lf(&buf, f)) {
    - 		if (starts_with(buf.buf, comment_line_str))
    + 	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
     @@ wt-status.c: static int read_rebase_todolist(const char *fname, struct string_list *lines)
      		strbuf_trim(&buf);
      		if (!buf.len)
    @@ wt-status.c: static void show_rebase_information(struct wt_status *s,
      					 &yet_to_do))
      			status_printf_ln(s, color,
      				_("git-rebase-todo is missing."));
    +@@ wt-status.c: static void show_sparse_checkout_in_use(struct wt_status *s,
    + /*
    +  * Extract branch information from rebase/bisect
    +  */
    +-static char *get_branch(const struct worktree *wt, const char *path)
    ++static char *get_branch(struct repository *r, struct worktree *wt, const char *path)
    + {
    + 	struct strbuf sb = STRBUF_INIT;
    + 	struct object_id oid;
     @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      	strbuf_release(&cb.buf);
      }
    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
     -int wt_status_check_rebase(const struct worktree *wt,
     -			   struct wt_status_state *state)
     +int wt_status_check_rebase(struct repository *r,
    -+			 const struct worktree *wt,
    -+			 struct wt_status_state *state)
    ++	 			const struct worktree *wt,
    ++			    struct wt_status_state *state)
      {
      	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(r, wt, "rebase-apply"), &st)) {
    -+		if (!stat(worktree_git_path(r, wt, "rebase-apply/applying"), &st)) {
    - 			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(r, wt, "rebase-apply/patch"), &st) && !st.st_size)
    +@@ wt-status.c: int wt_status_check_rebase(const struct worktree *wt,
      				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");
    +-			state->branch = get_branch(wt, "rebase-apply/head-name");
    +-			state->onto = get_branch(wt, "rebase-apply/onto");
    ++			state->branch = get_branch(r, wt, "rebase-apply/head-name");
    ++			state->onto = get_branch(r, 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(r, wt, "rebase-merge"), &st)) {
    -+		if (!stat(worktree_git_path(r, wt, "rebase-merge/interactive"), &st))
    + 	} else if (!stat(worktree_git_path(the_repository, wt, "rebase-merge"), &st)) {
    + 		if (!stat(worktree_git_path(the_repository, wt, "rebase-merge/interactive"), &st))
      			state->rebase_interactive_in_progress = 1;
      		else
      			state->rebase_in_progress = 1;
    +-		state->branch = get_branch(wt, "rebase-merge/head-name");
    +-		state->onto = get_branch(wt, "rebase-merge/onto");
    ++		state->branch = get_branch(r, wt, "rebase-merge/head-name");
    ++		state->onto = get_branch(r, wt, "rebase-merge/onto");
    + 	} else
    + 		return 0;
    + 	return 1;
    + }
    + 
    +-int wt_status_check_bisect(const struct worktree *wt,
    ++int wt_status_check_bisect(struct repository *r, 
    ++			   struct worktree *wt,
    + 			   struct wt_status_state *state)
    + {
    + 	struct stat st;
    + 
    + 	if (!stat(worktree_git_path(the_repository, wt, "BISECT_LOG"), &st)) {
    + 		state->bisect_in_progress = 1;
    +-		state->bisecting_from = get_branch(wt, "BISECT_START");
    ++		state->bisecting_from = get_branch(r, wt, "BISECT_START");
    + 		return 1;
    + 	}
    + 	return 0;
     @@ wt-status.c: void wt_status_get_state(struct repository *r,
      	enum replay_action action;
      
    @@ wt-status.c: void wt_status_get_state(struct repository *r,
      		;		/* all set */
      	} else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
      		   !repo_get_oid(r, "CHERRY_PICK_HEAD", &oid)) {
    + 		state->cherry_pick_in_progress = 1;
    + 		oidcpy(&state->cherry_pick_head_oid, &oid);
    + 	}
    +-	wt_status_check_bisect(NULL, state);
    ++	wt_status_check_bisect(r, NULL, state);
    + 	if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD") &&
    + 	    !repo_get_oid(r, "REVERT_HEAD", &oid)) {
    + 		state->revert_in_progress = 1;
     @@ wt-status.c: static void wt_porcelain_v2_print_tracking(struct wt_status *s)
       */
      static void wt_porcelain_v2_print_stash(struct wt_status *s)
    @@ wt-status.h: void wt_status_state_free_buffers(struct wt_status_state *s);
      			 struct wt_status_state *state,
      			 int get_detached_from);
     -int wt_status_check_rebase(const struct worktree *wt,
    --			   struct wt_status_state *state);
     +int wt_status_check_rebase(struct repository *r,
    -+			 const struct worktree *wt,
    -+			 struct wt_status_state *state);
    - int wt_status_check_bisect(const struct worktree *wt,
    ++			   struct worktree *wt,
    + 			   struct wt_status_state *state);
    +-int wt_status_check_bisect(const struct worktree *wt,
    ++int wt_status_check_bisect(struct repository *r,
    ++			   struct worktree *wt,
      			   struct wt_status_state *state);
      
    + __attribute__((format (printf, 3, 4)))
1:  556735dfd4 ! 2:  906e682cd7 wt-status: replace uses of the_repository with local repository instances
    @@ Commit message
         wt-status: replace uses of the_repository with local repository instances
     
         wt-status.c uses the global the_repository in several places even when
    -    a repository instance is already available via struct wt_status or
    -    struct worktree.
    +    a repository instance is already available via struct wt_status *s or
    +    struct repository *r.
     
    -    Replace these direct uses of the_repository with the repository carried
    -    by the local structs (e.g. s->repo, wt->repo).
    +    Replace these uses of the_repository with the repository available
    +    in the local context (eg. s->repo or r).
     
         The replacements of all the_repository with s->repo are mostly
         to cases where a repository instance is already available via
    -    struct wt_status. All functions operating on struct wt_status *s
    -    are only used after s is initialized by wt_status_prepare(),
    +    struct wt_status *s and struct repository *r, all functions operating on
    +    struct wt_status *s are only used after s is initialized by wt_status_prepare(),
         which sets s->repo from the repository provided by the caller.
    -    As a result, s->repo is guaranteed to be available and consistent
    -    whenever these functions are invoked.
    +    As a result, s->repo is guaranteed to be available and consistent whenever
    +    these functions are invoked.
     
         This reduces reliance on global state and keeps wt-status consistent,
         though many functions operating on struct wt_status *s
    @@ wt-status.c: void wt_status_prepare(struct repository *r, struct wt_status *s)
      	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->index_file = repo_get_index_file(r);
      	s->change.strdup_strings = 1;
      	s->untracked.strdup_strings = 1;
      	s->ignored.strdup_strings = 1;
    @@ wt-status.c: static void wt_status_collect_changes_index(struct wt_status *s)
      	setup_revisions(0, NULL, &rev, &opt);
      
      	rev.diffopt.flags.override_submodule_config = 1;
    +@@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
    + static int count_stash_entries(struct repository *r)
    + {
    + 	int n = 0;
    +-	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    ++	refs_for_each_reflog_ent(get_main_ref_store(r),
    + 				 "refs/stash", stash_count_refs, &n);
    + 	return n;
    + }
     @@ wt-status.c: static void wt_longstatus_print_verbose(struct wt_status *s)
      	rev.diffopt.ita_invisible_in_index = 1;
      
    @@ wt-status.c: static void wt_longstatus_print_verbose(struct wt_status *s)
      	setup_revisions(0, NULL, &rev, &opt);
      
      	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
    +@@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
    + static char *read_line_from_git_path(struct repository *r, char *filename)
    + {
    + 	struct strbuf buf = STRBUF_INIT;
    +-	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    ++	FILE *fp = fopen_or_warn(repo_git_path_append(r, &buf,
    + 						      "%s", filename), "r");
    + 
    + 	if (!fp) {
     @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      	    !s->branch || strcmp(s->branch, "HEAD"))
      		return 0;
    @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      			       &orig_head_oid, &orig_head_flags))
      		return 0;
      	if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF)
    +@@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)
    + 		return;
    + 
    + 	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
    +-	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
    ++	    !repo_get_oid(r, split.items[1].string, &oid)) {
    + 		strbuf_reset(line);
    + 		strbuf_addf(line, "%s ", split.items[0].string);
    + 		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
    +@@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)
    + static int read_rebase_todolist(struct repository *r, char *fname, struct string_list *lines)
    + {
    + 	struct strbuf buf = STRBUF_INIT;
    +-	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    ++	FILE *f = fopen(repo_git_path_append(r, &buf, "%s", fname), "r");
    + 	int ret;
    + 
    + 	if (!f) {
    +@@ wt-status.c: static int read_rebase_todolist(struct repository *r, char *fname, struct string
    + 			goto out;
    + 		}
    + 		die_errno("Could not open file %s for reading",
    +-			  repo_git_path_replace(the_repository, &buf, "%s", fname));
    ++			  repo_git_path_replace(r, &buf, "%s", fname));
    + 	}
    + 	while (!strbuf_getline_lf(&buf, f)) {
    + 		if (starts_with(buf.buf, comment_line_str))
     @@ wt-status.c: static void show_rebase_information(struct wt_status *s,
      				i++)
      				status_printf_ln(s, color, "   %s", have_done.items[i].string);
    @@ wt-status.c: static void show_revert_in_progress(struct wt_status *s,
      						DEFAULT_ABBREV));
      	if (s->hints) {
      		if (has_unmerged(s))
    -@@ wt-status.c: static char *get_branch(const struct worktree *wt, const char *path)
    +@@ wt-status.c: static char *get_branch(struct repository *r, struct worktree *wt, const char *p
      	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)
    ++	if (strbuf_read_file(&sb, worktree_git_path(r, wt, "%s", path), 0) <= 0)
      		goto got_nothing;
      
      	while (sb.len && sb.buf[sb.len - 1] == '\n')
    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      		strbuf_release(&cb.buf);
      		return;
      	}
    -@@ wt-status.c: int wt_status_check_bisect(const struct worktree *wt,
    +@@ wt-status.c: int wt_status_check_rebase(struct repository *r,
    + {
    + 	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(r, wt, "rebase-apply"), &st)) {
    ++		if (!stat(worktree_git_path(r, wt, "rebase-apply/applying"), &st)) {
    + 			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(r, wt, "rebase-apply/patch"), &st) && !st.st_size)
    + 				state->am_empty_patch = 1;
    + 		} else {
    + 			state->rebase_in_progress = 1;
    + 			state->branch = get_branch(r, wt, "rebase-apply/head-name");
    + 			state->onto = get_branch(r, 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(r, wt, "rebase-merge"), &st)) {
    ++		if (!stat(worktree_git_path(r, wt, "rebase-merge/interactive"), &st))
    + 			state->rebase_interactive_in_progress = 1;
    + 		else
    + 			state->rebase_in_progress = 1;
    +@@ wt-status.c: int wt_status_check_bisect(struct repository *r,
      {
      	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)) {
    ++	if (!stat(worktree_git_path(r, wt, "BISECT_LOG"), &st)) {
      		state->bisect_in_progress = 1;
    - 		state->bisecting_from = get_branch(wt, "BISECT_START");
    + 		state->bisecting_from = get_branch(r, wt, "BISECT_START");
      		return 1;
     @@ wt-status.c: static void wt_shortstatus_print_tracking(struct wt_status *s)
      		upstream_is_gone = 1;
3:  b745a08d96 = 3:  9c0a1d82ad wt-status: use hash_algo from local repository instead of global the_hash_algo

Some functions in wt-status.c (count_stash_entries(), read_line_from_git_path(), abbrev_oid_in_line(), read_rebase_todolist()) do not have access to a local repository instance and rely on the_repository.

Add a struct repository *r parameter to these functions, and pass the local repository through the callers.

get_branch(), wt_status_check_rebase() and wt_status_check_bisect() already receive a struct worktree *, which can provide access to the repository. However, some callers pass NULL as the worktree like in wt_status_get_state(), which would make using wt->repo unsafe and lead to segfault issues. Add an explicit struct repository * parameter to these functions as well, and pass the repository through the callers.

Both wt_status_check_rebase() and wt_status_check_bisect() are called from branch.c and worktree.c,

* In branch.c, wt is always non-NULL as the functions are called within an
interation over worktrees in prepare_checked_out_branches().

* In worktree.c the functions are called from is_worktree_being_rebased() and
is_worktree_being_bisected() respectively which are further called from
builtin/branch.c in reject_rebase_or_bisect_branch() which has a non-NULL
worktree as it is called inside an iteration over worktrees as well.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 branch.c    |  4 ++--
 worktree.c  |  4 ++--
 wt-status.c | 48 +++++++++++++++++++++++++-----------------------
 wt-status.h |  6 ++++--
 4 files changed, 33 insertions(+), 29 deletions(-)

wt-status.c uses the global the_repository in several places even when a repository instance is already available via struct wt_status *s or struct repository *r.

Replace these uses of the_repository with the repository available in the local context (eg. s->repo or r).

The replacements of all the_repository with s->repo are mostly to cases where a repository instance is already available via struct wt_status *s and struct repository *r, all functions operating on struct wt_status *s are only used after s is initialized by wt_status_prepare(), which sets s->repo from the repository provided by the caller. As a result, s->repo is guaranteed to be available and consistent whenever these functions are invoked.

This reduces reliance on global state and keeps wt-status consistent, though many functions operating on struct wt_status *s are called via commit.c and it still relies on the_repository, but within wt-status.c the local repository pointer refers to the same underlying repository object.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

wt-status.c still uses the global the_hash_algo even though a repository instance is already available via struct wt_status.

Replace uses of the_hash_algo with the hash algorithm stored in the associated repository (s->repo->hash_algo or r->hash_algo).

This removes another dependency on global state and keeps wt-status consistent with local repository usage.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

I am not sure if this is an improvement for callers of the API.

Isn't wt_anything() that takes a worktree "wt" supposed to work with the wt->repo repository? Or is the API designed to be used to take any repository object that is _different_ from wt->repo? I am assuming it is the former, and if so, the only effect of adding a repository parameter to a function that already takes struct worktree is to invite a programming error to pass a repository that the wt is not designed to work with, isn't it?

Ditto.

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

Funny indentation.

Besides, should we adding a yet another repository parameter to the function? The worktree wt knows what repository it belongs to.

Same comment about "r" vs "wt->repo" applies here.

my bad, will fix it.

Actually adding another repository parameter to both of these functions is needed because of the calls like wt_status_check_rebase(NULL, state) and wt_status_check_bisect(NULL, state) from wt_status_get_state(). In the case where wt is NULL, accessing wt->repo can lead to issues.

You are right that when a worktree is present, the repository associated with should be wt->repo, and callers should not be expected to provide something different.

The reason I introduced an explicit struct repository *r parameter I have included in the previous reply.

So the intention is not to allow a repository different from wt->repo, but to make it possible to operate in the absence of a worktree. When wt is non-NULL, the expectation remains that callers pass wt->repo, and in the case in which passed repo and wt->repo are different worktree_git_path() would handle this case in a BUG().

Best, Shreyansh

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

But stopping thought at that point is not a reasonable thing to do, immediately after you notice that wt is sometimes NULL. It merely means that unconditionally dereferencing wt->repo without thinking is not good enough, doesn't it?

And what is the case where worktree is NULL? What are we doing with worktree set to NULL? Is it when secondary worktrees do not come into the picture at all and you can safely use the_repository?

    ... goes and looks ...

Ahh, I think the real culprit that needs cleaning up is the worktree API, where they pass NULL to mean "the primary worktree that has its .git/ directory at its natural place". So it may not necessarily be the_repository we are dealing with. There is *no* such client code right now, but we could imagine that a program that starts in a repository visits the primary worktree of another repository and asks the worktree status there, and once such a client code appears, we need to be able to say "we are dealing with the primary worktree for this repository".

In the longer run, I think we should fix the worktree API so that even for the primary worktree we will always have a non-NULL "struct worktree" object, perhaps with its .id member set to NULL to signal that it is the primary worktree, so that we do not have to have this strange "we must pass repository redundantly even though we are passing worktree" API elsewhere. Not just this code you are making worse, path.c:worktree_git_path() already is a victim of this misdesign of the worktree API. It has "if wt is given, then the r parameter should be the same as wt->repo" nonsense, which we wouldn't have had to have if we had a worktree object even for the primary worktree, Look at how ugly that code is, and weep X-<.

And the same misdesign of the worktree API has caused your [1/3] to pass 'r' but yet still depend on the_repository, which you had to fix in [2/3], in this function.

So, I dunno. If you are ambitious, you may want to clean up the worktree API before this series. Alternatively you may be able to punt on the parts of the wt-status that interact with worktree API, and move the rest of wt-status less dependent on the_repository, but I am not sure.

Thanks.

Thank you very much for the detailed explanation and for pointing towards the bigger picture.

From what I have understood, the worktree being NULL refers to the primary worktree (as it does not indicate which repository so it means in respect to the_repository). So if we want to access the primary worktree of a specific repository or even the local repository, NULL does not carry enough information. And obviously, using NULL as primary worktree introduces extra checks and measures as we saw in the previous discussion.

I would be very interested (and the more logical step) to fixing worktree api first, and then revisiting the wt-status series on top of that, once the API makes it possible to rely on wt->repo without the NULL risks.

So a possible in the worktree api cleanup approach could be,

* Make primary worktree as an instance of struct worktree but seperate
it by having a marker like id = NULL.

* Add this primary worktree in the struct repository (e.g. repo->primary_wt).

* Update/add functions, then find places that currently pass NULL
and convert them to use primary worktree object instead.

Let me know if I have the right understanding with this, and also would love to hear more guidance on the direction with this worktree api cleanup. Thanks.

Best, Shreyansh

Junio C Hamano <gitster@pobox.com> writes:

Absolutely, In a previous version, the patched directly used `wt->repo`. I pointed out that there are instances where `wt` could be NULL as per the existing flow. So this change is in accordance with that. Overall I think the fix should ideally be around how we work with worktrees.

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

[snip]

I would like to point out that we already have a function which provides a main worktree, see both `get_main_worktree()` & `is_main_worktree()`. In short, a worktree with id = NULL seems to be treated as the main worktree.

The harder part would be correcting all code where `struct worktree *` is passed and has special meaning for NULL vs non-NULL. See `strbuf_worktree_gitdir()` which also distinguishes between `wt == NULL`, `wt->id == NULL` and `wt->id != NULL`.

So cleanup would require identifying all such spots and fixing them too.

This also is tricky. We currently already store all worktrees in the repository in `struct strmap worktree_ref_stores`. Here, for the main worktree we use '\' (see `get_worktree_ref_store()`). So perhaps we should formalize using `\` for the main worktree everywhere.

Karthik

[...]

Thanks for these points, I definitely need a better understanding of the whole worktree api usage and flow before anything further. So I am going to spend some time on it. Once I have a clearer picture, I will send a seperate rfc attempt on this cleanup and we can discuss it further there.

Best, Shreyansh

Karthik Nayak <karthik.188@gmail.com> writes:

Yup.  That is why I upfront said "if you are ambitious" ;-)

Is this a joke, is my terminal broken, or is my MUA hallucinating? I see a couple of backslashes in the above, and in the code I have a forward slash instead.

But you are right, ref-store-map does use a slash to indicate the primary one, while worktree itself uses a NULL, which is somewhat understandable (NULL would not be a convenient hashmap key). And I do not think I see any downsides (other than "This used to take NULL as the sign of primari-ness but now we need to use a '/' instead" fixes we need everywhere) to use "/" on the wt->id side offhand.

Thanks.

Junio C Hamano <gitster@pobox.com> writes:

Seems like my fingers didn't type what my mind thought of.

Yeah I share the same sentiment.

In wt-status.c code still relies on some global variables, including the_repository and the_hash_algo, even in cases where a repository instance is already available via struct wt_status or struct repository.

In patch 1/3, update function parameters and callers to pass struct repository where no local repository access was available.

In patch 2/3, replace direct uses of the_repository with repository instances already available in local structs.

In patch 3/3, replace remaining uses of the global the_hash_algo with the hash algorithm stored in the respective repository instance.

These changes remove uses of the_repository and the_hash_algo from wt-status.c and reduce its dependence on global state.

The 'USE_THE_REPOSITORY_VARIABLE' macro cannot yet be removed, since these patches only eliminate some uses of the_repository and the_hash_algo, while some global variables are still referenced.

In particular wt-status.c still relies on the following globals,

* the_repository, this is still used in functions like worktree_git_path() and
wt_status_check_bisect/rebase() which are dependant on the worktree API and they
are being handled in a seperate patch series[1].

* core_apply_sparse_checkout, this is already being addressed in an
ongoing patch series [2].

* comment_line_str and DEFAULT_ABBREV, these both still are used in
wt-status.c but they dont have any equivalent local instances.

[1]- https://lore.kernel.org/git/20260213120529.15475-1-shreyanshpaliwalcmsmn@gmail.com/T/#mf664ad751faaf2eaca138302b1cc9d3856c9fec3 [2]- https://lore.kernel.org/git/5e56e1cc4172cfff9e917a068184e102aa70bf1d.1769256839.git.belkid98@gmail.com/t/#u

Shreyansh Paliwal (3):
  wt-status: pass struct repository through function parameters
  wt-status: replace uses of the_repository with local repository
    instances
  wt-status: use hash_algo from local repository instead of global
    the_hash_algo

 wt-status.c | 62 ++++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

---
Changes in v4:
 - removed the changes regarding worktree_git_path() and
    wt_status_check_bisect/rebase() as they are better handled seperately.

Range-diff against v3:
1:  960216e45c ! 1:  a3683a5e17 wt-status: pass struct repository through function parameters
    @@ Commit message
         do not have access to a local repository instance and rely on the_repository.

         Add a struct repository *r parameter to these functions, and pass the local
    -    repository through the callers.
    -
    -    get_branch(), wt_status_check_rebase() and wt_status_check_bisect() already
    -    receive a struct worktree *, which can provide access to the repository.
    -    However, some callers pass NULL as the worktree like in wt_status_get_state(),
    -    which would make using wt->repo unsafe and lead to segfault issues.
    -    Add an explicit struct repository * parameter to these functions as well,
    -    and pass the repository through the callers.
    -
    -    Both wt_status_check_rebase() and  wt_status_check_bisect() are called from
    -    branch.c and worktree.c,
    -
    -    * In branch.c, wt is always non-NULL as the functions are called within an
    -    interation over worktrees in prepare_checked_out_branches().
    -    * In worktree.c the functions are called from is_worktree_being_rebased() and
    -    is_worktree_being_bisected() respectively which are further called from
    -    builtin/branch.c in reject_rebase_or_bisect_branch() which has a non-NULL
    -    worktree as it is called inside an iteration over worktrees as well.
    +    repository through the callers where already they can access a local repository
    +    instance either directly by struct repository *r or
    +    by struct wt_state *s (s->repo).

         Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>

    - ## branch.c ##
    -@@ branch.c: static void prepare_checked_out_branches(void)
    - 			free(old);
    - 		}
    -
    --		if (wt_status_check_rebase(wt, &state) &&
    -+		if (wt_status_check_rebase(wt->repo, wt, &state) &&
    - 		    (state.rebase_in_progress || state.rebase_interactive_in_progress) &&
    - 		    state.branch) {
    - 			struct strbuf ref = STRBUF_INIT;
    -@@ branch.c: static void prepare_checked_out_branches(void)
    - 		}
    - 		wt_status_state_free_buffers(&state);
    -
    --		if (wt_status_check_bisect(wt, &state) &&
    -+		if (wt_status_check_bisect(wt->repo, wt, &state) &&
    - 		    state.bisecting_from) {
    - 			struct strbuf ref = STRBUF_INIT;
    - 			strbuf_addf(&ref, "refs/heads/%s", state.bisecting_from);
    -
    - ## worktree.c ##
    -@@ worktree.c: int is_worktree_being_rebased(const struct worktree *wt,
    - 	int found_rebase;
    -
    - 	memset(&state, 0, sizeof(state));
    --	found_rebase = wt_status_check_rebase(wt, &state) &&
    -+	found_rebase = wt_status_check_rebase(wt->repo, wt, &state) &&
    - 		       (state.rebase_in_progress ||
    - 			state.rebase_interactive_in_progress) &&
    - 		       state.branch &&
    -@@ worktree.c: int is_worktree_being_bisected(const struct worktree *wt,
    - 	int found_bisect;
    -
    - 	memset(&state, 0, sizeof(state));
    --	found_bisect = wt_status_check_bisect(wt, &state) &&
    -+	found_bisect = wt_status_check_bisect(wt->repo, wt, &state) &&
    - 		       state.bisecting_from &&
    - 		       skip_prefix(target, "refs/heads/", &target) &&
    - 		       !strcmp(state.bisecting_from, target);
    -
      ## wt-status.c ##
     @@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
      	return 0;
    @@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
      }

     -static char *read_line_from_git_path(const char *filename)
    -+static char *read_line_from_git_path(struct repository *r, char *filename)
    ++static char *read_line_from_git_path(struct repository *r, const char *filename)
      {
      	struct strbuf buf = STRBUF_INIT;
      	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    @@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
      }

     -static int read_rebase_todolist(const char *fname, struct string_list *lines)
    -+static int read_rebase_todolist(struct repository *r, char *fname, struct string_list *lines)
    ++static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)
      {
      	struct strbuf buf = STRBUF_INIT;
      	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    @@ wt-status.c: static void show_rebase_information(struct wt_status *s,
      					 &yet_to_do))
      			status_printf_ln(s, color,
      				_("git-rebase-todo is missing."));
    -@@ wt-status.c: static void show_sparse_checkout_in_use(struct wt_status *s,
    - /*
    -  * Extract branch information from rebase/bisect
    -  */
    --static char *get_branch(const struct worktree *wt, const char *path)
    -+static char *get_branch(struct repository *r, struct worktree *wt, const char *path)
    - {
    - 	struct strbuf sb = STRBUF_INIT;
    - 	struct object_id oid;
    -@@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
    - 	strbuf_release(&cb.buf);
    - }
    -
    --int wt_status_check_rebase(const struct worktree *wt,
    --			   struct wt_status_state *state)
    -+int wt_status_check_rebase(struct repository *r,
    -+	 			const struct worktree *wt,
    -+			    struct wt_status_state *state)
    - {
    - 	struct stat st;
    -
    -@@ wt-status.c: int wt_status_check_rebase(const struct worktree *wt,
    - 				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");
    -+			state->branch = get_branch(r, wt, "rebase-apply/head-name");
    -+			state->onto = get_branch(r, 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))
    - 			state->rebase_interactive_in_progress = 1;
    - 		else
    - 			state->rebase_in_progress = 1;
    --		state->branch = get_branch(wt, "rebase-merge/head-name");
    --		state->onto = get_branch(wt, "rebase-merge/onto");
    -+		state->branch = get_branch(r, wt, "rebase-merge/head-name");
    -+		state->onto = get_branch(r, wt, "rebase-merge/onto");
    - 	} else
    - 		return 0;
    - 	return 1;
    - }
    -
    --int wt_status_check_bisect(const struct worktree *wt,
    -+int wt_status_check_bisect(struct repository *r,
    -+			   struct worktree *wt,
    - 			   struct wt_status_state *state)
    - {
    - 	struct stat st;
    -
    - 	if (!stat(worktree_git_path(the_repository, wt, "BISECT_LOG"), &st)) {
    - 		state->bisect_in_progress = 1;
    --		state->bisecting_from = get_branch(wt, "BISECT_START");
    -+		state->bisecting_from = get_branch(r, wt, "BISECT_START");
    - 		return 1;
    - 	}
    - 	return 0;
    -@@ wt-status.c: void wt_status_get_state(struct repository *r,
    - 	enum replay_action action;
    -
    - 	if (!stat(git_path_merge_head(r), &st)) {
    --		wt_status_check_rebase(NULL, state);
    -+		wt_status_check_rebase(r, NULL, state);
    - 		state->merge_in_progress = 1;
    --	} else if (wt_status_check_rebase(NULL, state)) {
    -+	} else if (wt_status_check_rebase(r, NULL, state)) {
    - 		;		/* all set */
    - 	} else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
    - 		   !repo_get_oid(r, "CHERRY_PICK_HEAD", &oid)) {
    - 		state->cherry_pick_in_progress = 1;
    - 		oidcpy(&state->cherry_pick_head_oid, &oid);
    - 	}
    --	wt_status_check_bisect(NULL, state);
    -+	wt_status_check_bisect(r, NULL, state);
    - 	if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD") &&
    - 	    !repo_get_oid(r, "REVERT_HEAD", &oid)) {
    - 		state->revert_in_progress = 1;
     @@ wt-status.c: static void wt_porcelain_v2_print_tracking(struct wt_status *s)
       */
      static void wt_porcelain_v2_print_stash(struct wt_status *s)
    @@ wt-status.c: static void wt_porcelain_v2_print_tracking(struct wt_status *s)
      	char eol = s->null_termination ? '\0' : '\n';

      	if (stash_count > 0)
    -
    - ## wt-status.h ##
    -@@ wt-status.h: void wt_status_state_free_buffers(struct wt_status_state *s);
    - void wt_status_get_state(struct repository *repo,
    - 			 struct wt_status_state *state,
    - 			 int get_detached_from);
    --int wt_status_check_rebase(const struct worktree *wt,
    -+int wt_status_check_rebase(struct repository *r,
    -+			   struct worktree *wt,
    - 			   struct wt_status_state *state);
    --int wt_status_check_bisect(const struct worktree *wt,
    -+int wt_status_check_bisect(struct repository *r,
    -+			   struct worktree *wt,
    - 			   struct wt_status_state *state);
    -
    - __attribute__((format (printf, 3, 4)))
2:  906e682cd7 ! 2:  f3b4c3e972 wt-status: replace uses of the_repository with local repository instances
    @@ Commit message
         struct repository *r.

         Replace these uses of the_repository with the repository available
    -    in the local context (eg. s->repo or r).
    +    in the local context (i.e. s->repo or r).

         The replacements of all the_repository with s->repo are mostly
         to cases where a repository instance is already available via
    @@ wt-status.c: static void wt_longstatus_print_verbose(struct wt_status *s)

      	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
     @@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
    - static char *read_line_from_git_path(struct repository *r, char *filename)
    + static char *read_line_from_git_path(struct repository *r, const char *filename)
      {
      	struct strbuf buf = STRBUF_INIT;
     -	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    @@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf
      		strbuf_addf(line, "%s ", split.items[0].string);
      		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
     @@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)
    - static int read_rebase_todolist(struct repository *r, char *fname, struct string_list *lines)
    + static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)
      {
      	struct strbuf buf = STRBUF_INIT;
     -	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    @@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf
      	int ret;

      	if (!f) {
    -@@ wt-status.c: static int read_rebase_todolist(struct repository *r, char *fname, struct string
    +@@ wt-status.c: static int read_rebase_todolist(struct repository *r, const char *fname, struct
      			goto out;
      		}
      		die_errno("Could not open file %s for reading",
    @@ wt-status.c: static void show_revert_in_progress(struct wt_status *s,
      						DEFAULT_ABBREV));
      	if (s->hints) {
      		if (has_unmerged(s))
    -@@ wt-status.c: static char *get_branch(struct repository *r, struct worktree *wt, const char *p
    - 	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(r, wt, "%s", path), 0) <= 0)
    - 		goto got_nothing;
    -
    - 	while (sb.len && sb.buf[sb.len - 1] == '\n')
     @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      	char *ref = NULL;

    @@ wt-status.c: static void wt_status_get_detached_from(struct repository *r,
      		strbuf_release(&cb.buf);
      		return;
      	}
    -@@ wt-status.c: int wt_status_check_rebase(struct repository *r,
    - {
    - 	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(r, wt, "rebase-apply"), &st)) {
    -+		if (!stat(worktree_git_path(r, wt, "rebase-apply/applying"), &st)) {
    - 			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(r, wt, "rebase-apply/patch"), &st) && !st.st_size)
    - 				state->am_empty_patch = 1;
    - 		} else {
    - 			state->rebase_in_progress = 1;
    - 			state->branch = get_branch(r, wt, "rebase-apply/head-name");
    - 			state->onto = get_branch(r, 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(r, wt, "rebase-merge"), &st)) {
    -+		if (!stat(worktree_git_path(r, wt, "rebase-merge/interactive"), &st))
    - 			state->rebase_interactive_in_progress = 1;
    - 		else
    - 			state->rebase_in_progress = 1;
    -@@ wt-status.c: int wt_status_check_bisect(struct repository *r,
    - {
    - 	struct stat st;
    -
    --	if (!stat(worktree_git_path(the_repository, wt, "BISECT_LOG"), &st)) {
    -+	if (!stat(worktree_git_path(r, wt, "BISECT_LOG"), &st)) {
    - 		state->bisect_in_progress = 1;
    - 		state->bisecting_from = get_branch(r, wt, "BISECT_START");
    - 		return 1;
     @@ wt-status.c: static void wt_shortstatus_print_tracking(struct wt_status *s)
      		upstream_is_gone = 1;
      	}
3:  9c0a1d82ad = 3:  7efaf6b3fb wt-status: use hash_algo from local repository instead of global the_hash_algo
--
2.53.0

Some functions in wt-status.c (count_stash_entries(), read_line_from_git_path(), abbrev_oid_in_line(), read_rebase_todolist()) do not have access to a local repository instance and rely on the_repository.

Add a struct repository *r parameter to these functions, and pass the local repository through the callers where already they can access a local repository instance either directly by struct repository *r or by struct wt_state *s (s->repo).

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

wt-status.c uses the global the_repository in several places even when a repository instance is already available via struct wt_status *s or struct repository *r.

Replace these uses of the_repository with the repository available in the local context (i.e. s->repo or r).

The replacements of all the_repository with s->repo are mostly to cases where a repository instance is already available via struct wt_status *s and struct repository *r, all functions operating on struct wt_status *s are only used after s is initialized by wt_status_prepare(), which sets s->repo from the repository provided by the caller. As a result, s->repo is guaranteed to be available and consistent whenever these functions are invoked.

This reduces reliance on global state and keeps wt-status consistent, though many functions operating on struct wt_status *s are called via commit.c and it still relies on the_repository, but within wt-status.c the local repository pointer refers to the same underlying repository object.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

wt-status.c still uses the global the_hash_algo even though a repository instance is already available via struct wt_status.

Replace uses of the_hash_algo with the hash algorithm stored in the associated repository (s->repo->hash_algo or r->hash_algo).

This removes another dependency on global state and keeps wt-status consistent with local repository usage.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

On 17/02/2026 17:29, Shreyansh Paliwal wrote:

This breaks the build when running "make DEVELOPER=1"

wt-status.c: In function ‘count_stash_entries’:
wt-status.c:1011:51: error: unused parameter ‘r’ [-Werror=unused-parameter]
  1011 | static int count_stash_entries(struct repository *r)
       |                                ~~~~~~~~~~~~~~~~~~~^
wt-status.c: In function ‘read_line_from_git_path’:
wt-status.c:1314:57: error: unused parameter ‘r’ [-Werror=unused-parameter]
  1314 | static char *read_line_from_git_path(struct repository *r, 
const char *filename)
       |                                      ~~~~~~~~~~~~~~~~~~~^
wt-status.c: In function ‘abbrev_oid_in_line’:
wt-status.c:1377:51: error: unused parameter ‘r’ [-Werror=unused-parameter]
  1377 | static void abbrev_oid_in_line(struct repository *r, struct 
strbuf *line)
       |                                ~~~~~~~~~~~~~~~~~~~^

It would be better to use the new argument to replace "the_repository" in this patch. There aren't that many so the patch is still a manageable size.

These both look good, though I think the commit message for the second patch could be reflowed to give a more consistent line length.

Thanks

Phillip

Thanks for catching that. I ran the build and test suite after all the commits at once so it got overlooked.

I will send a revised version. Thanks.

Best, Shreyansh

In wt-status.c code still relies on some global variables, including the_repository and the_hash_algo, even in cases where a repository instance is already available via struct wt_status or struct repository.

In patch 1/3, update function parameters and callers to pass and use struct repository instead of the_repository where no local repository access was available.

In patch 2/3, replace direct uses of the_repository with repository instances already available in local structs.

In patch 3/3, replace remaining uses of the global the_hash_algo with the hash algorithm stored in the respective repository instance.

These changes remove uses of the_repository and the_hash_algo from wt-status.c and reduce its dependence on global state.

The 'USE_THE_REPOSITORY_VARIABLE' macro cannot yet be removed, since these patches only eliminate some uses of the_repository and the_hash_algo, while some global variables are still referenced.

In particular wt-status.c still relies on the following globals,

* the_repository, this is still used in functions like worktree_git_path()
  and wt_status_check_bisect/rebase() which are dependant on the worktree
  API and they are being handled in a seperate patch series[1].

* core_apply_sparse_checkout, this is already being addressed in an ongoing
  patch series [2].

* comment_line_str and DEFAULT_ABBREV, these both still are used in
  wt-status.c but they dont have any equivalent local instances.

[1]- https://lore.kernel.org/git/20260213120529.15475-1-shreyanshpaliwalcmsmn@gmail.com/T/#mf664ad751faaf2eaca138302b1cc9d3856c9fec3 [2]- https://lore.kernel.org/git/5e56e1cc4172cfff9e917a068184e102aa70bf1d.1769256839.git.belkid98@gmail.com/t/#u

Shreyansh Paliwal (3):
  wt-status: pass struct repository through function parameters
  wt-status: replace uses of the_repository with local repository
    instances
  wt-status: use hash_algo from local repository instead of global
    the_hash_algo

 wt-status.c | 62 ++++++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

---
Changes in v5:
 - Added the usage of struct repository *r in addition to adding it as a parameter to the functions,
    in patch 1/3 instead of doing that in 2/3.

Range-diff against v4:
1:  a3683a5e17 ! 1:  620cf8832b wt-status: pass struct repository through function parameters
    @@ Commit message
         wt-status: pass struct repository through function parameters

         Some functions in wt-status.c (count_stash_entries(),
    -    read_line_from_git_path(), abbrev_oid_in_line(), read_rebase_todolist())
    -    do not have access to a local repository instance and rely on the_repository.
    +    read_line_from_git_path(), abbrev_oid_in_line(), and
    +    read_rebase_todolist()) rely on the_repository as they do not have access
    +    to a local repository instance.

    -    Add a struct repository *r parameter to these functions, and pass the local
    -    repository through the callers where already they can access a local repository
    -    instance either directly by struct repository *r or
    -    by struct wt_state *s (s->repo).
    +    Add a struct repository *r parameter to these functions and pass the local
    +    repository instance through the callers, which already have access to it
    +    either directly by struct repository *r or indirectly by struct wt_state
    +    *s (s->repo).
    +
    +    Replace uses of the_repository in these functions with the passed parameter.

         Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>

    @@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
     +static int count_stash_entries(struct repository *r)
      {
      	int n = 0;
    - 	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    -@@ wt-status.c: static int count_stash_entries(void)
    +-	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    ++	refs_for_each_reflog_ent(get_main_ref_store(r),
    + 				 "refs/stash", stash_count_refs, &n);
    + 	return n;
    + }

      static void wt_longstatus_print_stash_summary(struct wt_status *s)
      {
    @@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
     +static char *read_line_from_git_path(struct repository *r, const char *filename)
      {
      	struct strbuf buf = STRBUF_INIT;
    - 	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    +-	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    ++	FILE *fp = fopen_or_warn(repo_git_path_append(r, &buf,
    + 						      "%s", filename), "r");
    +
    + 	if (!fp) {
     @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      	if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF)
      		return 0;
    @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      {
      	struct string_list split = STRING_LIST_INIT_DUP;
      	struct object_id oid;
    +@@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
    + 		return;
    +
    + 	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
    +-	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
    ++	    !repo_get_oid(r, split.items[1].string, &oid)) {
    + 		strbuf_reset(line);
    + 		strbuf_addf(line, "%s ", split.items[0].string);
    + 		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
     @@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
      	string_list_clear(&split, 0);
      }
    @@ wt-status.c: static void abbrev_oid_in_line(struct strbuf *line)
     +static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)
      {
      	struct strbuf buf = STRBUF_INIT;
    - 	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    +-	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    ++	FILE *f = fopen(repo_git_path_append(r, &buf, "%s", fname), "r");
    + 	int ret;
    +
    + 	if (!f) {
    +@@ wt-status.c: static int read_rebase_todolist(const char *fname, struct string_list *lines)
    + 			goto out;
    + 		}
    + 		die_errno("Could not open file %s for reading",
    +-			  repo_git_path_replace(the_repository, &buf, "%s", fname));
    ++			  repo_git_path_replace(r, &buf, "%s", fname));
    + 	}
    + 	while (!strbuf_getline_lf(&buf, f)) {
    + 		if (starts_with(buf.buf, comment_line_str))
     @@ wt-status.c: static int read_rebase_todolist(const char *fname, struct string_list *lines)
      		strbuf_trim(&buf);
      		if (!buf.len)
2:  f3b4c3e972 ! 2:  13e0c5bcd7 wt-status: replace uses of the_repository with local repository instances
    @@ Metadata
      ## Commit message ##
         wt-status: replace uses of the_repository with local repository instances

    -    wt-status.c uses the global the_repository in several places even when
    -    a repository instance is already available via struct wt_status *s or
    -    struct repository *r.
    +    wt-status.c uses the global the_repository in several places even when a
    +    repository instance is already available via struct wt_status *s or struct
    +    repository *r.

    -    Replace these uses of the_repository with the repository available
    -    in the local context (i.e. s->repo or r).
    +    Replace these uses of the_repository with the repository available in the
    +    local context (i.e. s->repo or r).

    -    The replacements of all the_repository with s->repo are mostly
    -    to cases where a repository instance is already available via
    -    struct wt_status *s and struct repository *r, all functions operating on
    -    struct wt_status *s are only used after s is initialized by wt_status_prepare(),
    -    which sets s->repo from the repository provided by the caller.
    -    As a result, s->repo is guaranteed to be available and consistent whenever
    -    these functions are invoked.
    +    The replacements of all the_repository with s->repo are mostly to cases
    +    where a repository instance is already available via struct wt_status *s
    +    and struct repository *r, all functions operating on struct wt_status *s
    +    are only used after s is initialized by wt_status_prepare(), which sets
    +    s->repo from the repository provided by the caller. As a result, s->repo is
    +    guaranteed to be available and consistent whenever these functions are
    +    invoked.

         This reduces reliance on global state and keeps wt-status consistent,
    -    though many functions operating on struct wt_status *s
    -    are called via commit.c and it still relies on the_repository,
    -    but within wt-status.c the local repository pointer
    -    refers to the same underlying repository object.
    +    though many functions operating on struct wt_status *s are called via
    +    commit.c and it still relies on the_repository, but within wt-status.c the
    +    local repository pointer refers to the same underlying repository object.

         Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>

    @@ wt-status.c: static void wt_status_collect_changes_index(struct wt_status *s)
      	setup_revisions(0, NULL, &rev, &opt);

      	rev.diffopt.flags.override_submodule_config = 1;
    -@@ wt-status.c: static int stash_count_refs(const char *refname UNUSED,
    - static int count_stash_entries(struct repository *r)
    - {
    - 	int n = 0;
    --	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
    -+	refs_for_each_reflog_ent(get_main_ref_store(r),
    - 				 "refs/stash", stash_count_refs, &n);
    - 	return n;
    - }
     @@ wt-status.c: static void wt_longstatus_print_verbose(struct wt_status *s)
      	rev.diffopt.ita_invisible_in_index = 1;

    @@ wt-status.c: static void wt_longstatus_print_verbose(struct wt_status *s)
      	setup_revisions(0, NULL, &rev, &opt);

      	rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
    -@@ wt-status.c: static void show_am_in_progress(struct wt_status *s,
    - static char *read_line_from_git_path(struct repository *r, const char *filename)
    - {
    - 	struct strbuf buf = STRBUF_INIT;
    --	FILE *fp = fopen_or_warn(repo_git_path_append(the_repository, &buf,
    -+	FILE *fp = fopen_or_warn(repo_git_path_append(r, &buf,
    - 						      "%s", filename), "r");
    -
    - 	if (!fp) {
     @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      	    !s->branch || strcmp(s->branch, "HEAD"))
      		return 0;
    @@ wt-status.c: static int split_commit_in_progress(struct wt_status *s)
      			       &orig_head_oid, &orig_head_flags))
      		return 0;
      	if (head_flags & REF_ISSYMREF || orig_head_flags & REF_ISSYMREF)
    -@@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)
    - 		return;
    -
    - 	if ((2 <= string_list_split(&split, line->buf, " ", 2)) &&
    --	    !repo_get_oid(the_repository, split.items[1].string, &oid)) {
    -+	    !repo_get_oid(r, split.items[1].string, &oid)) {
    - 		strbuf_reset(line);
    - 		strbuf_addf(line, "%s ", split.items[0].string);
    - 		strbuf_add_unique_abbrev(line, &oid, DEFAULT_ABBREV);
    -@@ wt-status.c: static void abbrev_oid_in_line(struct repository *r, struct strbuf *line)
    - static int read_rebase_todolist(struct repository *r, const char *fname, struct string_list *lines)
    - {
    - 	struct strbuf buf = STRBUF_INIT;
    --	FILE *f = fopen(repo_git_path_append(the_repository, &buf, "%s", fname), "r");
    -+	FILE *f = fopen(repo_git_path_append(r, &buf, "%s", fname), "r");
    - 	int ret;
    -
    - 	if (!f) {
    -@@ wt-status.c: static int read_rebase_todolist(struct repository *r, const char *fname, struct
    - 			goto out;
    - 		}
    - 		die_errno("Could not open file %s for reading",
    --			  repo_git_path_replace(the_repository, &buf, "%s", fname));
    -+			  repo_git_path_replace(r, &buf, "%s", fname));
    - 	}
    - 	while (!strbuf_getline_lf(&buf, f)) {
    - 		if (starts_with(buf.buf, comment_line_str))
     @@ wt-status.c: static void show_rebase_information(struct wt_status *s,
      				i++)
      				status_printf_ln(s, color, "   %s", have_done.items[i].string);
3:  7efaf6b3fb = 3:  2bf22dd925 wt-status: use hash_algo from local repository instead of global the_hash_algo
--
2.53.0

Some functions in wt-status.c (count_stash_entries(), read_line_from_git_path(), abbrev_oid_in_line(), and read_rebase_todolist()) rely on the_repository as they do not have access to a local repository instance.

Add a struct repository *r parameter to these functions and pass the local repository instance through the callers, which already have access to it either directly by struct repository *r or indirectly by struct wt_state *s (s->repo).

Replace uses of the_repository in these functions with the passed parameter.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

wt-status.c uses the global the_repository in several places even when a repository instance is already available via struct wt_status *s or struct repository *r.

Replace these uses of the_repository with the repository available in the local context (i.e. s->repo or r).

The replacements of all the_repository with s->repo are mostly to cases where a repository instance is already available via struct wt_status *s and struct repository *r, all functions operating on struct wt_status *s are only used after s is initialized by wt_status_prepare(), which sets s->repo from the repository provided by the caller. As a result, s->repo is guaranteed to be available and consistent whenever these functions are invoked.

This reduces reliance on global state and keeps wt-status consistent, though many functions operating on struct wt_status *s are called via commit.c and it still relies on the_repository, but within wt-status.c the local repository pointer refers to the same underlying repository object.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

wt-status.c still uses the global the_hash_algo even though a repository instance is already available via struct wt_status.

Replace uses of the_hash_algo with the hash algorithm stored in the associated repository (s->repo->hash_algo or r->hash_algo).

This removes another dependency on global state and keeps wt-status consistent with local repository usage.

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 wt-status.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com> writes:

We haven't seen any reviews on the latest round, but the previous iterations all had good discussions. Is everybody find this round satisfactory? If so, let me mark the topic for 'next'.

Thanks.

Junio C Hamano <gitster@pobox.com> writes:

I'm happy with the current version. Makes sense to move it to 'next'.

Thanks

On 06/03/2026 22:31, Junio C Hamano wrote:

Yes this looks good, lets merge it (sorry I thought I'd reviewed this round when it was posted but clearly didn't actually get round to it)

Thanks

Phillip

Karthik Nayak <karthik.188@gmail.com> writes:

Thanks for reviewing and helping the series to become in good shape.