Re: [PATCH 00/18] setup: drop uses of `the_repository`

[Patrick Steinhardt <ps@pks.im>]

On Mon, Apr 20, 2026 at 10:11:09AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Message-Id: <20260420-pks-setup-wo-the-repository-v1-0-f4a81c4988e8@pks.im>
> 
> There seem to be some tool errors that made this marked as v1 (hence
> no References/In-Reply-To).

Yeah, I noticed this, too. As mentioned in the mail to Elijah, I think
this happens when I'm overeager and already switch branches while b4 is
still busy sending out mails. In that case, it doesn't update the series
to vN+1 :/

> >   -  9/18: setup: stop using `the_repository` in `setup_work_tree()`
> 
> > diff --git a/setup.c b/setup.c
> > index dca32addae..80dd94b261 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -26,7 +26,6 @@
> >  #include "trace2.h"
> >  #include "worktree.h"
> >  
> > -static int work_tree_config_is_bogus;
> 
> Once we start "setting up" more than one instance of "struct
> repository", "the worktree configuration given to us is bogus" bit
> cannot be a singleton global that applies across repositories.
> 
> Hopefully nobody sets core.bare and core.worktree in configuration
> files other than the repository local one.  Otherwise a lot of
> interesting would break ;-)
> 
> I am not quite sure the reasoning behind removing "initialized"
> global without any replacement was properly explained, with "should
> ultimately be idempotent".  This can give us a huge performance
> regression, if the callers have been relying on setup_work_tree() to
> be _cheap_ when they call it "just in case" no other potential
> callsites have called it, even though all of them know that the
> current code works only with the_repository (in other words, they
> call because the switched into the_repository---they do not know for
> sure if other call sites have called it).
> 
> Or does the current code never call setup_work_tree() twice, because
> everybody knows that we never work with anything other than
> the_repository?

`setup_work_tree()` is typically executed via "git.c" in case
NEED_WORK_TREE is set. Some other top-level commands end up executing it
manually in case the bit is not set.

Other than that it's called in a couple more sites:

  - "builtin/check-attr.c", "builtin/clone.c", "builtin/describe.c",
    "builtin/diff-index.c", "builtin/diff.c", "builtin/difftool.c",
    "builtin/grep.c", "builtin/ls-files.c", "builtin/read-tree.c",
    "builtin/reset.c", "builtin/rm.c", "builtin/submodule--helper.c", :
    Not marked with NEED_WORK_TREE, executed exactly once.

  - "builtin/sparse-checkout.c": we have multiple callsites here for the
    subcommands, but also call it via `update_working_directory()`. We
    execute it a fixed number of times only though, but I doubt that the
    performance overhead of this matters in practice. That being said,
    all sites leading up to this have already called `setup_work_tree()`
    beforehand, so it would be trivial to fix.

  - "builtin/update-index.c": it may be executed multiple times via
    command line options, e.g. in case you execute `git update-index
    --refresh --refresh`. Again, I doubt this matters.

  - "blame.c": executed exactly once via `setup_scoreboard()`, which in
    turn is executed once only via "builtin/blame.c", which is not
    marked with NEED_WORK_TREE. So this is fine.

  - "wt-status.c": executed once when printing a verbose status. This
    will result in a second invocation for git-status(1) and
    git-commit(1), but again it's bounded.

  - "t/helper/test-subprocess.c": executed exactly once, but doesn't
    matter much anyway.

So overall I think it's fine as in most cases we really only execute it
once, and in the remaining cases we only execute it a limited amount of
times (twice or thrice). In addition to that, getenv(3p) and setenv(3p)
shouldn't be inherently expensive, and the chnotify callbacks we
register are all rather cheap, too.

We _could_ prepare git-sparse-checkout(1) and git-update-index(1) to
handle this, but I'm not sure this is really needed. All the other
callsites are fine, as far as I can see.

Thanks!

Patrick