[PATCH 0/4] wean start_command() off the_repository

[PATCH 0/4] wean start_command() off the_repository

[Burak Kaan Karaçay]

Hi,

start_command() relies on the_repository due to the 'close_object_store'
flag in 'struct child_process'. Introduce repo_start_command() to allow
working with arbitrary repositories. Turn start_command() into a macro
that wraps repo_start_command() and migrate the existing callers with a
cocci script.

For callers that cannot access 'the_repository' due to the lack of
USE_THE_REPOSITORY_VARIABLE, define the macro. If the caller already has
a local repository context, pass it explicitly instead of defining the
macro.

Thanks,
Burak Kaan Karaçay

Burak Kaan Karaçay (4):
  run-command: add repo_start_command()
  run-command: use repo_start_command() in strict callers
  run-command: redefine start_command() as a wrapper macro
  cocci: convert start_command() to repo_start_command()

 archive-tar.c                           |  2 +-
 branch.c                                |  2 +-
 builtin/credential-cache.c              |  4 +++-
 builtin/difftool.c                      |  4 ++--
 builtin/gc.c                            | 16 ++++++++--------
 builtin/help.c                          |  2 +-
 builtin/index-pack.c                    |  2 +-
 builtin/merge.c                         |  2 +-
 builtin/notes.c                         |  2 +-
 builtin/receive-pack.c                  |  6 +++---
 builtin/remote-ext.c                    |  4 +++-
 builtin/repack.c                        |  2 +-
 builtin/replace.c                       |  2 +-
 builtin/upload-archive.c                |  2 +-
 builtin/worktree.c                      |  2 +-
 bundle-uri.c                            |  2 +-
 bundle.c                                |  2 +-
 column.c                                |  3 ++-
 compat/mingw.c                          |  2 +-
 connect.c                               |  4 ++--
 connected.c                             |  2 +-
 contrib/coccinelle/the_repository.cocci |  3 +++
 convert.c                               |  2 +-
 credential.c                            |  3 ++-
 daemon.c                                |  6 +++---
 diff.c                                  |  2 +-
 editor.c                                |  2 +-
 fetch-pack.c                            |  4 ++--
 http-backend.c                          |  2 +-
 imap-send.c                             |  2 +-
 midx-write.c                            |  4 +++-
 odb.c                                   |  2 +-
 pager.c                                 |  2 +-
 parallel-checkout.c                     |  2 +-
 promisor-remote.c                       |  2 +-
 prompt.c                                |  3 ++-
 range-diff.c                            |  2 +-
 reachable.c                             |  2 +-
 remote-curl.c                           |  2 +-
 repack-cruft.c                          |  4 +++-
 repack-filtered.c                       |  4 +++-
 repack-midx.c                           |  4 +++-
 repack-promisor.c                       |  6 ++++--
 run-command.c                           | 12 ++++++------
 run-command.h                           |  6 ++++--
 send-pack.c                             |  4 ++--
 sub-process.c                           |  5 ++++-
 submodule.c                             | 12 ++++++------
 t/helper/test-run-command.c             |  6 ++++--
 transport-helper.c                      |  6 +++---
 upload-pack.c                           |  4 ++--
 51 files changed, 108 insertions(+), 81 deletions(-)

-- 
2.53.0

[PATCH 1/4] run-command: add repo_start_command()

[Burak Kaan Karaçay]

Currently start_command() relies on implicitly the_repository to close
object store before the spawning a child process. This prevents callers
from safely starting commands in context of a different repository.

Introduce repo_start_command() which takes 'struct repository *' as
argument. To avoid breaking existing callers, redefine start_command()
as a wrapper to pass the_repository to repo_start_command().

Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 run-command.c | 7 ++++++-
 run-command.h | 4 +++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/run-command.c b/run-command.c
index b27064ef57..fadc3d5283 100644
--- a/run-command.c
+++ b/run-command.c
@@ -675,6 +675,11 @@ static void trace_run_command(const struct child_process *cp)
 }
 
 int start_command(struct child_process *cmd)
+{
+	return repo_start_command(the_repository, cmd);
+}
+
+int repo_start_command(struct repository *repo, struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
 	int fdin[2], fdout[2], fderr[2];
@@ -743,7 +748,7 @@ int start_command(struct child_process *cmd)
 	fflush(NULL);
 
 	if (cmd->close_object_store)
-		odb_close(the_repository->objects);
+		odb_close(repo->objects);
 
 #ifndef GIT_WINDOWS_NATIVE
 {
diff --git a/run-command.h b/run-command.h
index e1ca965b5b..654ca659b3 100644
--- a/run-command.h
+++ b/run-command.h
@@ -2,7 +2,6 @@
 #define RUN_COMMAND_H
 
 #include "thread-utils.h"
-
 #include "strvec.h"
 
 /**
@@ -15,6 +14,7 @@
  * produces in the caller in order to process it.
  */
 
+struct repository;
 
 /**
  * This describes the arguments, redirections, and environment of a
@@ -205,7 +205,9 @@ char *git_shell_path(void);
  * that specifies the details and returns pipe FDs (if requested).
  * See below for details.
  */
+
 int start_command(struct child_process *);
+int repo_start_command(struct repository *, struct child_process *);
 
 /**
  * Wait for the completion of a sub-process that was started with
-- 
2.53.0

[PATCH 2/4] run-command: use repo_start_command() in strict callers

[Burak Kaan Karaçay]

Some callers have been freed from global state and they do not define
the 'USE_THE_REPOSITORY_VARIABLE' macro.

To complete the mitigation of 'start_command()', update these callers to
use repo_start_command() and pass their local 'struct repository' as an
argument, completely eliminating their hidden reliance on the global
state.

Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 builtin/difftool.c | 4 ++--
 odb.c              | 2 +-
 pager.c            | 2 +-
 repack-promisor.c  | 2 +-
 send-pack.c        | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/difftool.c b/builtin/difftool.c
index e4bc1f8316..15ac552edf 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -257,7 +257,7 @@ static void changed_files(struct repository *repo,
 	diff_files.out = -1;
 	diff_files.dir = workdir;
 	strvec_pushf(&diff_files.env, "GIT_INDEX_FILE=%s", index_path);
-	if (start_command(&diff_files))
+	if (repo_start_command(repo, &diff_files))
 		die("could not obtain raw diff");
 	fp = xfdopen(diff_files.out, "r");
 	while (!strbuf_getline_nul(&buf, fp)) {
@@ -437,7 +437,7 @@ static int run_dir_diff(struct repository *repo,
 	child->clean_on_exit = 1;
 	child->dir = prefix;
 	child->out = -1;
-	if (start_command(child))
+	if (repo_start_command(repo, child))
 		die("could not obtain raw diff");
 	fp = xfdopen(child->out, "r");
 
diff --git a/odb.c b/odb.c
index 776de5356c..8ec279f84e 100644
--- a/odb.c
+++ b/odb.c
@@ -535,7 +535,7 @@ static void read_alternate_refs(struct repository *repo,
 
 	fill_alternate_refs_command(repo, &cmd, path);
 
-	if (start_command(&cmd))
+	if (repo_start_command(repo, &cmd))
 		return;
 
 	fh = xfdopen(cmd.out, "r");
diff --git a/pager.c b/pager.c
index 5531fff50e..9a23ed958d 100644
--- a/pager.c
+++ b/pager.c
@@ -169,7 +169,7 @@ void setup_pager(struct repository *r)
 	prepare_pager_args(&pager_process, pager);
 	pager_process.in = -1;
 	strvec_push(&pager_process.env, "GIT_PAGER_IN_USE");
-	if (start_command(&pager_process))
+	if (repo_start_command(r, &pager_process))
 		die("unable to execute pager '%s'", pager);
 
 	/* original process continues, but writes to the pipe */
diff --git a/repack-promisor.c b/repack-promisor.c
index 90318ce150..dba161a11a 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -125,7 +125,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
 	prepare_pack_objects(&cmd, args, packtmp);
 	strvec_push(&cmd.args, "--stdin-packs");
 	cmd.in = -1;
-	if (start_command(&cmd))
+	if (repo_start_command(repo, &cmd))
 		die(_("could not start pack-objects to repack promisor packs"));
 
 	in = xfdopen(cmd.in, "w");
diff --git a/send-pack.c b/send-pack.c
index 67d6987b1c..c339c3d1ca 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -92,7 +92,7 @@ static int pack_objects(struct repository *r,
 	po.out = args->stateless_rpc ? -1 : fd;
 	po.git_cmd = 1;
 	po.clean_on_exit = 1;
-	if (start_command(&po))
+	if (repo_start_command(r, &po))
 		die_errno("git pack-objects failed");
 
 	/*
@@ -459,7 +459,7 @@ static void get_commons_through_negotiation(struct repository *r,
 		return;
 	}
 
-	if (start_command(&child))
+	if (repo_start_command(r, &child))
 		die(_("send-pack: unable to fork off fetch subprocess"));
 
 	do {
-- 
2.53.0

Re: [PATCH 2/4] run-command: use repo_start_command() in strict callers

[Junio C Hamano]

Burak Kaan Karaçay <bkkaracay@gmail.com> writes:

> Some callers have been freed from global state and they do not define
> the 'USE_THE_REPOSITORY_VARIABLE' macro.
>
> To complete the mitigation of 'start_command()', update these callers to
> use repo_start_command() and pass their local 'struct repository' as an
> argument, completely eliminating their hidden reliance on the global
> state.
>
> Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
> ---
>  builtin/difftool.c | 4 ++--
>  odb.c              | 2 +-
>  pager.c            | 2 +-
>  repack-promisor.c  | 2 +-
>  send-pack.c        | 4 ++--
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/difftool.c b/builtin/difftool.c
> index e4bc1f8316..15ac552edf 100644
> --- a/builtin/difftool.c
> +++ b/builtin/difftool.c
> @@ -257,7 +257,7 @@ static void changed_files(struct repository *repo,
>  	diff_files.out = -1;
>  	diff_files.dir = workdir;
>  	strvec_pushf(&diff_files.env, "GIT_INDEX_FILE=%s", index_path);
> -	if (start_command(&diff_files))
> +	if (repo_start_command(repo, &diff_files))
>  		die("could not obtain raw diff");
>  	fp = xfdopen(diff_files.out, "r");
>  	while (!strbuf_getline_nul(&buf, fp)) {
> @@ -437,7 +437,7 @@ static int run_dir_diff(struct repository *repo,
>  	child->clean_on_exit = 1;
>  	child->dir = prefix;
>  	child->out = -1;
> -	if (start_command(child))
> +	if (repo_start_command(repo, child))
>  		die("could not obtain raw diff");
>  	fp = xfdopen(child->out, "r");

Up to [1/4], we called start_command(), running the new process in
the context of the_repository, and if these "repo" were referring to
a repository different from the_repository, we risk changing the
behaviour without meaning to do so.

The difftool command, however, would not be dealing with multiple
repositories so it is very likely that the "repo" passed to these
two functions are the_repository _anyway_, so these changes are
correct, safe, and a move in the right direction.

> diff --git a/odb.c b/odb.c
> index 776de5356c..8ec279f84e 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -535,7 +535,7 @@ static void read_alternate_refs(struct repository *repo,
>  
>  	fill_alternate_refs_command(repo, &cmd, path);
>  
> -	if (start_command(&cmd))
> +	if (repo_start_command(repo, &cmd))
>  		return;
>  
>  	fh = xfdopen(cmd.out, "r");

The only semantic change brought in by [1/4] to start_command() is
that the object store of the named repository is closed, instead of
the object store of the_repository.  The read_alternate_refs()
function calls fill_alternate_refs_command() to run for-each-ref in
the named "repo" to find out the refs _they_ have.

But then, do we really want to close the object database we have
been using in the context of that "repo", that is different from
"the_repository" we have been using so far?

Of course, there is no guarantee that the_repository is always the
"current" repository object we are using and trying to read the refs
from this alternate repository in the codebase in an imaginary
future where we can start from one repository, add refs from an
alternate repository, and while doing so, we may add refs from an
alternate of the alternate repository, so always starting from
the_repository may not be correct, either.  But even before going
there, closing object store of "repo" feels more wrong than from
"the_repository".  Perhaps repo_start_command() needs to be aware of
two repositories, one that we have been using and neeed to close the
object store if instructed, and the other one that we want to launch
the new process in its context?  Keeping "the current repository" as
a global variable will drag us back to the similar issues these
efforts to wean us from "the_repository" global are trying to
address, so if our new repo_start_command() does need to be aware of
the two repositories, perhaps it needs to take two repository
parameters (i.e., where we are coming from, and where we are going
to)?  I dunno.

> diff --git a/pager.c b/pager.c
> index 5531fff50e..9a23ed958d 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -169,7 +169,7 @@ void setup_pager(struct repository *r)
>  	prepare_pager_args(&pager_process, pager);
>  	pager_process.in = -1;
>  	strvec_push(&pager_process.env, "GIT_PAGER_IN_USE");
> -	if (start_command(&pager_process))
> +	if (repo_start_command(r, &pager_process))
>  		die("unable to execute pager '%s'", pager);
>  
>  	/* original process continues, but writes to the pipe */

OK.

> diff --git a/repack-promisor.c b/repack-promisor.c
> index 90318ce150..dba161a11a 100644
> --- a/repack-promisor.c
> +++ b/repack-promisor.c
> @@ -125,7 +125,7 @@ void pack_geometry_repack_promisors(struct repository *repo,
>  	prepare_pack_objects(&cmd, args, packtmp);
>  	strvec_push(&cmd.args, "--stdin-packs");
>  	cmd.in = -1;
> -	if (start_command(&cmd))
> +	if (repo_start_command(repo, &cmd))
>  		die(_("could not start pack-objects to repack promisor packs"));
>  
>  	in = xfdopen(cmd.in, "w");

OK.

> diff --git a/send-pack.c b/send-pack.c
> index 67d6987b1c..c339c3d1ca 100644
> --- a/send-pack.c
> +++ b/send-pack.c
> @@ -92,7 +92,7 @@ static int pack_objects(struct repository *r,
>  	po.out = args->stateless_rpc ? -1 : fd;
>  	po.git_cmd = 1;
>  	po.clean_on_exit = 1;
> -	if (start_command(&po))
> +	if (repo_start_command(r, &po))
>  		die_errno("git pack-objects failed");

OK.

> @@ -459,7 +459,7 @@ static void get_commons_through_negotiation(struct repository *r,
>  		return;
>  	}
>  
> -	if (start_command(&child))
> +	if (repo_start_command(r, &child))
>  		die(_("send-pack: unable to fork off fetch subprocess"));
>  
>  	do {

OK.

[PATCH 3/4] run-command: redefine start_command() as a wrapper macro

[Burak Kaan Karaçay]

In the commit introducing 'repo_start_command()', 'start_command()' was
redefined as a wrapper function.

Now, redefine 'start_command()' as a wrapper macro to make 'the_repository'
dependency explicit at the caller's site. To successfully build, expose
'the_repository' dependency at the call sites by defining
'USE_THE_REPOSITORY_VARIABLE' or including 'repository.h' where necessary.

Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 builtin/credential-cache.c  | 2 ++
 builtin/remote-ext.c        | 2 ++
 column.c                    | 1 +
 credential.c                | 1 +
 midx-write.c                | 2 ++
 prompt.c                    | 1 +
 repack-cruft.c              | 2 ++
 repack-filtered.c           | 2 ++
 repack-midx.c               | 2 ++
 repack-promisor.c           | 2 ++
 run-command.c               | 5 -----
 run-command.h               | 2 +-
 sub-process.c               | 3 +++
 t/helper/test-run-command.c | 2 ++
 14 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index 7f733cb756..fb17aa87ba 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "builtin.h"
 #include "credential.h"
 #include "gettext.h"
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index bd2037f27d..ed2d551753 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "builtin.h"
 #include "transport.h"
 #include "run-command.h"
diff --git a/column.c b/column.c
index 93fae316b4..998b2ab458 100644
--- a/column.c
+++ b/column.c
@@ -1,3 +1,4 @@
+#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
diff --git a/credential.c b/credential.c
index 2594c0c422..a513355105 100644
--- a/credential.c
+++ b/credential.c
@@ -1,3 +1,4 @@
+#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
diff --git a/midx-write.c b/midx-write.c
index 6485cb6706..a53f77f13a 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "abspath.h"
 #include "config.h"
diff --git a/prompt.c b/prompt.c
index 706fba2a50..20a8c34438 100644
--- a/prompt.c
+++ b/prompt.c
@@ -7,6 +7,7 @@
 #include "strbuf.h"
 #include "prompt.h"
 #include "compat/terminal.h"
+#include "repository.h"
 
 static char *do_askpass(const char *cmd, const char *prompt)
 {
diff --git a/repack-cruft.c b/repack-cruft.c
index 0653e88792..0bfc77792a 100644
--- a/repack-cruft.c
+++ b/repack-cruft.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "repack.h"
 #include "packfile.h"
diff --git a/repack-filtered.c b/repack-filtered.c
index edcf7667c5..2f5d1dd709 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "repack.h"
 #include "repository.h"
diff --git a/repack-midx.c b/repack-midx.c
index 0682b80c42..8b4c0d95e3 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "repack.h"
 #include "hash.h"
diff --git a/repack-promisor.c b/repack-promisor.c
index dba161a11a..70ef19d04f 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -1,3 +1,5 @@
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "repack.h"
 #include "hex.h"
diff --git a/run-command.c b/run-command.c
index fadc3d5283..af26c636a9 100644
--- a/run-command.c
+++ b/run-command.c
@@ -674,11 +674,6 @@ static void trace_run_command(const struct child_process *cp)
 	strbuf_release(&buf);
 }
 
-int start_command(struct child_process *cmd)
-{
-	return repo_start_command(the_repository, cmd);
-}
-
 int repo_start_command(struct repository *repo, struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
diff --git a/run-command.h b/run-command.h
index 654ca659b3..890d7c5d72 100644
--- a/run-command.h
+++ b/run-command.h
@@ -206,7 +206,7 @@ char *git_shell_path(void);
  * See below for details.
  */
 
-int start_command(struct child_process *);
+#define start_command(cmd) repo_start_command(the_repository, cmd)
 int repo_start_command(struct repository *, struct child_process *);
 
 /**
diff --git a/sub-process.c b/sub-process.c
index 83bf0a0e82..ae7493eb5c 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -1,10 +1,13 @@
 /*
  * Generic implementation of background process infrastructure.
  */
+#define USE_THE_REPOSITORY_VARIABLE
+
 #include "git-compat-util.h"
 #include "sub-process.h"
 #include "sigchain.h"
 #include "pkt-line.h"
+#include "repository.h"
 
 int cmd2process_cmp(const void *cmp_data UNUSED,
 		    const struct hashmap_entry *eptr,
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index 4a56456894..dcd58f228c 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -8,6 +8,7 @@
  * published by the Free Software Foundation.
  */
 
+#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "test-tool.h"
@@ -18,6 +19,7 @@
 #include "string-list.h"
 #include "thread-utils.h"
 #include "wildmatch.h"
+#include "repository.h"
 
 static int number_callbacks;
 static int parallel_next(struct child_process *cp,
-- 
2.53.0

[PATCH 4/4] cocci: convert start_command() to repo_start_command()

[Burak Kaan Karaçay]

Add and apply a semantic patch to migrate the remaining
'start_command()' calls to 'repo_start_command()' by passing
'the_repository' as the first parameter.

Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 archive-tar.c                           |  2 +-
 branch.c                                |  2 +-
 builtin/credential-cache.c              |  2 +-
 builtin/gc.c                            | 16 ++++++++--------
 builtin/help.c                          |  2 +-
 builtin/index-pack.c                    |  2 +-
 builtin/merge.c                         |  2 +-
 builtin/notes.c                         |  2 +-
 builtin/receive-pack.c                  |  6 +++---
 builtin/remote-ext.c                    |  2 +-
 builtin/repack.c                        |  2 +-
 builtin/replace.c                       |  2 +-
 builtin/upload-archive.c                |  2 +-
 builtin/worktree.c                      |  2 +-
 bundle-uri.c                            |  2 +-
 bundle.c                                |  2 +-
 column.c                                |  2 +-
 compat/mingw.c                          |  2 +-
 connect.c                               |  4 ++--
 connected.c                             |  2 +-
 contrib/coccinelle/the_repository.cocci |  3 +++
 convert.c                               |  2 +-
 credential.c                            |  2 +-
 daemon.c                                |  6 +++---
 diff.c                                  |  2 +-
 editor.c                                |  2 +-
 fetch-pack.c                            |  4 ++--
 http-backend.c                          |  2 +-
 imap-send.c                             |  2 +-
 midx-write.c                            |  2 +-
 parallel-checkout.c                     |  2 +-
 promisor-remote.c                       |  2 +-
 prompt.c                                |  2 +-
 range-diff.c                            |  2 +-
 reachable.c                             |  2 +-
 remote-curl.c                           |  2 +-
 repack-cruft.c                          |  2 +-
 repack-filtered.c                       |  2 +-
 repack-midx.c                           |  2 +-
 repack-promisor.c                       |  2 +-
 run-command.c                           |  8 ++++----
 sub-process.c                           |  2 +-
 submodule.c                             | 12 ++++++------
 t/helper/test-run-command.c             |  4 ++--
 transport-helper.c                      |  6 +++---
 upload-pack.c                           |  4 ++--
 46 files changed, 73 insertions(+), 70 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 0fc70d13a8..78124443f5 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -503,7 +503,7 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	filter.in = -1;
 	filter.silent_exec_failure = 1;
 
-	if (start_command(&filter) < 0)
+	if (repo_start_command(the_repository, &filter) < 0)
 		die_errno(_("unable to start '%s' filter"), cmd.buf);
 	close(1);
 	if (dup2(filter.in, 1) < 0)
diff --git a/branch.c b/branch.c
index 243db7d0fc..878abff79d 100644
--- a/branch.c
+++ b/branch.c
@@ -728,7 +728,7 @@ static int submodule_create_branch(struct repository *r,
 
 	strvec_pushl(&child.args, name, start_oid, tracking_name, NULL);
 
-	if ((ret = start_command(&child)))
+	if ((ret = repo_start_command(the_repository, &child)))
 		return ret;
 	ret = finish_command(&child);
 	strbuf_read(&child_err, child.err, 0);
diff --git a/builtin/credential-cache.c b/builtin/credential-cache.c
index fb17aa87ba..642d94fca0 100644
--- a/builtin/credential-cache.c
+++ b/builtin/credential-cache.c
@@ -83,7 +83,7 @@ static void spawn_daemon(const char *socket)
 	daemon.no_stdin = 1;
 	daemon.out = -1;
 
-	if (start_command(&daemon))
+	if (repo_start_command(the_repository, &daemon))
 		die_errno("unable to start cache daemon");
 	r = read_in_full(daemon.out, buf, sizeof(buf));
 	if (r < 0)
diff --git a/builtin/gc.c b/builtin/gc.c
index fb329c2cff..01da48cc32 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1396,7 +1396,7 @@ static int pack_loose(struct maintenance_run_opts *opts)
 	 */
 	pack_proc.out = -1;
 
-	if (start_command(&pack_proc)) {
+	if (repo_start_command(the_repository, &pack_proc)) {
 		error(_("failed to start 'git pack-objects' process"));
 		return 1;
 	}
@@ -2435,7 +2435,7 @@ static int launchctl_boot_plist(int enable, const char *filename)
 	child.no_stderr = 1;
 	child.no_stdout = 1;
 
-	if (start_command(&child))
+	if (repo_start_command(the_repository, &child))
 		die(_("failed to start launchctl"));
 
 	result = finish_command(&child);
@@ -2474,7 +2474,7 @@ static int launchctl_list_contains_plist(const char *name, const char *cmd)
 	child.no_stderr = 1;
 	child.no_stdout = 1;
 
-	if (start_command(&child))
+	if (repo_start_command(the_repository, &child))
 		die(_("failed to start launchctl"));
 
 	/* Returns failure if 'name' doesn't exist. */
@@ -2766,7 +2766,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
 	child.no_stdout = 1;
 	child.no_stderr = 1;
 
-	if (start_command(&child))
+	if (repo_start_command(the_repository, &child))
 		die(_("failed to start schtasks"));
 	result = finish_command(&child);
 
@@ -2860,7 +2860,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
 	crontab_list.out = dup(fd);
 	crontab_list.git_cmd = 0;
 
-	if (start_command(&crontab_list)) {
+	if (repo_start_command(the_repository, &crontab_list)) {
 		result = error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
 		goto out;
 	}
@@ -2924,7 +2924,7 @@ static int crontab_update_schedule(int run_maintenance, int fd)
 	strvec_push(&crontab_edit.args, get_tempfile_path(tmpedit));
 	crontab_edit.git_cmd = 0;
 
-	if (start_command(&crontab_edit)) {
+	if (repo_start_command(the_repository, &crontab_edit)) {
 		result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
 		goto out;
 	}
@@ -2950,7 +2950,7 @@ static int real_is_systemd_timer_available(void)
 	child.no_stderr = 1;
 	child.silent_exec_failure = 1;
 
-	if (start_command(&child))
+	if (repo_start_command(the_repository, &child))
 		return 0;
 	if (finish_command(&child))
 		return 0;
@@ -3169,7 +3169,7 @@ static int systemd_timer_enable_unit(int enable,
 		     "--now", NULL);
 	strvec_pushf(&child.args, SYSTEMD_UNIT_FORMAT, frequency, "timer");
 
-	if (start_command(&child)) {
+	if (repo_start_command(the_repository, &child)) {
 		ret = error(_("failed to start systemctl"));
 		goto out;
 	}
diff --git a/builtin/help.c b/builtin/help.c
index 86a3d03a9b..3473a0ae9f 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -242,7 +242,7 @@ static int check_emacsclient_version(void)
 	strvec_pushl(&ec_process.args, "emacsclient", "--version", NULL);
 	ec_process.err = -1;
 	ec_process.stdout_to_stderr = 1;
-	if (start_command(&ec_process))
+	if (repo_start_command(the_repository, &ec_process))
 		return error(_("Failed to start emacsclient."));
 
 	strbuf_read(&buffer, ec_process.err, 20);
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index b67fb0256c..5be75cb0eb 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1840,7 +1840,7 @@ static void repack_local_links(void)
 			cmd.git_cmd = 1;
 			cmd.in = -1;
 			cmd.out = -1;
-			if (start_command(&cmd))
+			if (repo_start_command(the_repository, &cmd))
 				die(_("could not start pack-objects to repack local links"));
 		}
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 4e456a381c..fb5875d1b2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -357,7 +357,7 @@ static int save_state(struct object_id *stash)
 	cp.out = -1;
 	cp.git_cmd = 1;
 
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		die(_("could not run stash."));
 	len = strbuf_read(&buffer, cp.out, 1024);
 	close(cp.out);
diff --git a/builtin/notes.c b/builtin/notes.c
index 9af602bdd7..bf9a46461d 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -173,7 +173,7 @@ static void write_commented_object(int fd, const struct object_id *object)
 	show.out = -1;
 	show.err = 0;
 	show.git_cmd = 1;
-	if (start_command(&show))
+	if (repo_start_command(the_repository, &show))
 		die(_("unable to start 'show' for object '%s'"),
 		    oid_to_hex(object));
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d6225df890..9533179bd6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1179,7 +1179,7 @@ static int run_proc_receive_hook(struct command *commands,
 		proc.err = 0;
 	}
 
-	code = start_command(&proc);
+	code = repo_start_command(the_repository, &proc);
 	if (code) {
 		if (use_sideband)
 			finish_async(&muxer);
@@ -2411,7 +2411,7 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 		child.out = -1;
 		child.err = err_fd;
 		child.git_cmd = 1;
-		status = start_command(&child);
+		status = repo_start_command(the_repository, &child);
 		if (status)
 			return "index-pack fork failed";
 
@@ -2732,7 +2732,7 @@ int cmd_receive_pack(int argc,
 				proc.stdout_to_stderr = 1;
 				proc.err = use_sideband ? -1 : 0;
 
-				if (!start_command(&proc)) {
+				if (!repo_start_command(the_repository, &proc)) {
 					if (use_sideband)
 						copy_to_sideband(proc.err, -1, NULL);
 					finish_command(&proc);
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index ed2d551753..f12eb0243b 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -149,7 +149,7 @@ static int run_child(const char *arg, const char *service)
 	child.err = 0;
 	parse_argv(&child.args, arg, service);
 
-	if (start_command(&child) < 0)
+	if (repo_start_command(the_repository, &child) < 0)
 		die("Can't run specified command");
 
 	if (git_req)
diff --git a/builtin/repack.c b/builtin/repack.c
index f6bb04bef7..3f7d83238c 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -356,7 +356,7 @@ int cmd_repack(int argc,
 	else
 		cmd.no_stdin = 1;
 
-	ret = start_command(&cmd);
+	ret = repo_start_command(the_repository, &cmd);
 	if (ret)
 		goto cleanup;
 
diff --git a/builtin/replace.c b/builtin/replace.c
index 4c62c5ab58..0abf5bc004 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -278,7 +278,7 @@ static int import_object(struct object_id *oid, enum object_type type,
 		cmd.in = fd;
 		cmd.out = -1;
 
-		if (start_command(&cmd)) {
+		if (repo_start_command(the_repository, &cmd)) {
 			close(fd);
 			return error(_("unable to spawn mktree"));
 		}
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 25312bb2a5..a840a78f6c 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -106,7 +106,7 @@ struct repository *repo UNUSED)
 	writer.git_cmd = 1;
 	strvec_push(&writer.args, "upload-archive--writer");
 	strvec_pushv(&writer.args, argv + 1);
-	if (start_command(&writer)) {
+	if (repo_start_command(the_repository, &writer)) {
 		int err = errno;
 		packet_write_fmt(1, "NACK unable to spawn subprocess\n");
 		die("upload-archive: %s", strerror(err));
diff --git a/builtin/worktree.c b/builtin/worktree.c
index bc2d0d645b..eb509470e2 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1363,7 +1363,7 @@ static void check_clean_worktree(struct worktree *wt,
 	cp.git_cmd = 1;
 	cp.dir = wt->path;
 	cp.out = -1;
-	ret = start_command(&cp);
+	ret = repo_start_command(the_repository, &cp);
 	if (ret)
 		die_errno(_("failed to run 'git status' on '%s'"),
 			  original_path);
diff --git a/bundle-uri.c b/bundle-uri.c
index 3b2e347288..b27b909785 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -341,7 +341,7 @@ static int download_https_uri_to_file(const char *file, const char *uri)
 	cp.in = -1;
 	cp.out = -1;
 
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		return 1;
 
 	child_in = fdopen(cp.in, "w");
diff --git a/bundle.c b/bundle.c
index 42327f9739..d2d385fef6 100644
--- a/bundle.c
+++ b/bundle.c
@@ -355,7 +355,7 @@ static int write_pack_data(int bundle_fd, struct rev_info *revs, struct strvec *
 		}
 	}
 
-	if (start_command(&pack_objects))
+	if (repo_start_command(the_repository, &pack_objects))
 		return error(_("Could not spawn pack-objects"));
 
 	for (i = 0; i < revs->pending.nr; i++) {
diff --git a/column.c b/column.c
index 998b2ab458..a2e26b993d 100644
--- a/column.c
+++ b/column.c
@@ -388,7 +388,7 @@ int run_column_filter(int colopts, const struct column_options *opts)
 	column_process.out = dup(1);
 	column_process.git_cmd = 1;
 
-	if (start_command(&column_process))
+	if (repo_start_command(the_repository, &column_process))
 		return -2;
 
 	fd_out = dup(1);
diff --git a/compat/mingw.c b/compat/mingw.c
index c667a2dcda..4825cf596a 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -35,7 +35,7 @@ void open_in_gdb(void)
 	strvec_pushl(&cp.args, "mintty", "gdb", NULL);
 	strvec_pushf(&cp.args, "--pid=%d", getpid());
 	cp.clean_on_exit = 1;
-	if (start_command(&cp) < 0)
+	if (repo_start_command(the_repository, &cp) < 0)
 		die_errno("Could not start gdb");
 	sleep(1);
 }
diff --git a/connect.c b/connect.c
index a02583a102..b223fe7644 100644
--- a/connect.c
+++ b/connect.c
@@ -1054,7 +1054,7 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)
 	strvec_push(&proxy->args, port);
 	proxy->in = -1;
 	proxy->out = -1;
-	if (start_command(proxy))
+	if (repo_start_command(the_repository, proxy))
 		die(_("cannot start proxy %s"), git_proxy_command);
 	fd[0] = proxy->out; /* read from proxy stdout */
 	fd[1] = proxy->in;  /* write to proxy stdin */
@@ -1515,7 +1515,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 		}
 		strvec_push(&conn->args, cmd.buf);
 
-		if (start_command(conn))
+		if (repo_start_command(the_repository, conn))
 			die(_("unable to fork"));
 
 		fd[0] = conn->out; /* read from child's stdout */
diff --git a/connected.c b/connected.c
index 79403108dd..278cc6a123 100644
--- a/connected.c
+++ b/connected.c
@@ -127,7 +127,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 	else
 		rev_list.no_stderr = opt->quiet;
 
-	if (start_command(&rev_list)) {
+	if (repo_start_command(the_repository, &rev_list)) {
 		free(new_pack);
 		return error(_("Could not run 'git rev-list'"));
 	}
diff --git a/contrib/coccinelle/the_repository.cocci b/contrib/coccinelle/the_repository.cocci
index f1129f7985..ed0389ea12 100644
--- a/contrib/coccinelle/the_repository.cocci
+++ b/contrib/coccinelle/the_repository.cocci
@@ -12,6 +12,9 @@
 |
 - parse_tree_indirect
 + repo_parse_tree_indirect
+|
+- start_command
++ repo_start_command
 )
   (
 + the_repository,
diff --git a/convert.c b/convert.c
index a34ec6ecdc..db3e1996ea 100644
--- a/convert.c
+++ b/convert.c
@@ -662,7 +662,7 @@ static int filter_buffer_or_fd(int in UNUSED, int out, void *data)
 	child_process.in = -1;
 	child_process.out = out;
 
-	if (start_command(&child_process)) {
+	if (repo_start_command(the_repository, &child_process)) {
 		strbuf_release(&cmd);
 		return error(_("cannot fork to run external filter '%s'"),
 			     params->cmd);
diff --git a/credential.c b/credential.c
index a513355105..5497051df1 100644
--- a/credential.c
+++ b/credential.c
@@ -457,7 +457,7 @@ static int run_credential_helper(struct credential *c,
 	else
 		helper.no_stdout = 1;
 
-	if (start_command(&helper) < 0)
+	if (repo_start_command(the_repository, &helper) < 0)
 		return -1;
 
 	fp = xfdopen(helper.in, "w");
diff --git a/daemon.c b/daemon.c
index 0a7b1aae44..8eb22851da 100644
--- a/daemon.c
+++ b/daemon.c
@@ -328,7 +328,7 @@ static int run_access_hook(struct daemon_service *service, const char *dir,
 	child.no_stdin = 1;
 	child.no_stderr = 1;
 	child.out = -1;
-	if (start_command(&child)) {
+	if (repo_start_command(the_repository, &child)) {
 		logerror("daemon access hook '%s' failed to start",
 			 access_hook);
 		goto error_return;
@@ -454,7 +454,7 @@ static int run_service_command(struct child_process *cld)
 	strvec_push(&cld->args, ".");
 	cld->git_cmd = 1;
 	cld->err = -1;
-	if (start_command(cld))
+	if (repo_start_command(the_repository, cld))
 		return -1;
 
 	close(0);
@@ -906,7 +906,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 	cld.in = incoming;
 	cld.out = dup(incoming);
 
-	if (start_command(&cld))
+	if (repo_start_command(the_repository, &cld))
 		logerror("unable to fork");
 	else
 		add_child(&cld, addr, addrlen);
diff --git a/diff.c b/diff.c
index e87847fa4b..fbda03ac18 100644
--- a/diff.c
+++ b/diff.c
@@ -7467,7 +7467,7 @@ static char *run_textconv(struct repository *r,
 
 	child.use_shell = 1;
 	child.out = -1;
-	if (start_command(&child)) {
+	if (repo_start_command(the_repository, &child)) {
 		remove_tempfile();
 		return NULL;
 	}
diff --git a/editor.c b/editor.c
index fd174e6a03..c96311bb3f 100644
--- a/editor.c
+++ b/editor.c
@@ -92,7 +92,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 			strvec_pushv(&p.env, (const char **)env);
 		p.use_shell = 1;
 		p.trace2_child_class = "editor";
-		if (start_command(&p) < 0) {
+		if (repo_start_command(the_repository, &p) < 0) {
 			strbuf_release(&realpath);
 			return error("unable to start editor '%s'", editor);
 		}
diff --git a/fetch-pack.c b/fetch-pack.c
index 6ecd468ef7..64476f852f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1035,7 +1035,7 @@ static int get_pack(struct fetch_pack_args *args,
 
 	cmd.in = demux.out;
 	cmd.git_cmd = 1;
-	if (start_command(&cmd))
+	if (repo_start_command(the_repository, &cmd))
 		die(_("fetch-pack: unable to fork off %s"), cmd_name);
 	if (do_keep && (pack_lockfiles || fsck_objects)) {
 		int is_well_formed;
@@ -1841,7 +1841,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 		cmd.git_cmd = 1;
 		cmd.no_stdin = 1;
 		cmd.out = -1;
-		if (start_command(&cmd))
+		if (repo_start_command(the_repository, &cmd))
 			die("fetch-pack: unable to spawn http-fetch");
 
 		if (read_in_full(cmd.out, packname, 5) < 0 ||
diff --git a/http-backend.c b/http-backend.c
index 1a171c5c5a..08ffb4a393 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -501,7 +501,7 @@ static void run_service(const char **argv, int buffer_input)
 	cld.git_cmd = 1;
 	cld.clean_on_exit = 1;
 	cld.wait_after_clean = 1;
-	if (start_command(&cld))
+	if (repo_start_command(the_repository, &cld))
 		exit(1);
 
 	close(1);
diff --git a/imap-send.c b/imap-send.c
index 26dda7f328..fefdb7a4ba 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1155,7 +1155,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 		tunnel.use_shell = 1;
 		tunnel.in = -1;
 		tunnel.out = -1;
-		if (start_command(&tunnel))
+		if (repo_start_command(the_repository, &tunnel))
 			die("cannot start proxy %s", srvc->tunnel);
 
 		imap->buf.sock.fd[0] = tunnel.out;
diff --git a/midx-write.c b/midx-write.c
index a53f77f13a..a1051e7807 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1828,7 +1828,7 @@ int midx_repack(struct odb_source *source, size_t batch_size, unsigned flags)
 	cmd.git_cmd = 1;
 	cmd.in = cmd.out = -1;
 
-	if (start_command(&cmd)) {
+	if (repo_start_command(the_repository, &cmd)) {
 		error(_("could not start pack-objects"));
 		result = 1;
 		goto cleanup;
diff --git a/parallel-checkout.c b/parallel-checkout.c
index 0bf4bd6d4a..bd4e8afcf8 100644
--- a/parallel-checkout.c
+++ b/parallel-checkout.c
@@ -476,7 +476,7 @@ static struct pc_worker *setup_workers(struct checkout *state, int num_workers)
 		strvec_push(&cp->args, "checkout--worker");
 		if (state->base_dir_len)
 			strvec_pushf(&cp->args, "--prefix=%s", state->base_dir);
-		if (start_command(cp))
+		if (repo_start_command(the_repository, cp))
 			die("failed to spawn checkout worker");
 	}
 
diff --git a/promisor-remote.c b/promisor-remote.c
index 96fa215b06..4e861260fe 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -48,7 +48,7 @@ static int fetch_objects(struct repository *repo,
 		     "--filter=blob:none", "--stdin", NULL);
 	if (!repo_config_get_bool(the_repository, "promisor.quiet", &quiet) && quiet)
 		strvec_push(&child.args, "--quiet");
-	if (start_command(&child))
+	if (repo_start_command(the_repository, &child))
 		die(_("promisor-remote: unable to fork off fetch subprocess"));
 	child_in = xfdopen(child.in, "w");
 
diff --git a/prompt.c b/prompt.c
index 20a8c34438..7ff5e8adf9 100644
--- a/prompt.c
+++ b/prompt.c
@@ -20,7 +20,7 @@ static char *do_askpass(const char *cmd, const char *prompt)
 
 	pass.out = -1;
 
-	if (start_command(&pass))
+	if (repo_start_command(the_repository, &pass))
 		return NULL;
 
 	strbuf_reset(&buffer);
diff --git a/range-diff.c b/range-diff.c
index 57edff40a8..a698d84ae1 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -75,7 +75,7 @@ static int read_patches(const char *range, struct string_list *list,
 	cp.no_stdin = 1;
 	cp.git_cmd = 1;
 
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		return error_errno(_("could not start `log`"));
 	if (strbuf_read(&contents, cp.out, 0) < 0) {
 		error_errno(_("could not read `log` output"));
diff --git a/reachable.c b/reachable.c
index 101cfc2727..64dcaa6b23 100644
--- a/reachable.c
+++ b/reachable.c
@@ -138,7 +138,7 @@ static int run_one_gc_recent_objects_hook(struct oidset *set,
 
 	strvec_push(&cmd.args, args);
 
-	if (start_command(&cmd))
+	if (repo_start_command(the_repository, &cmd))
 		return -1;
 
 	out = xfdopen(cmd.out, "r");
diff --git a/remote-curl.c b/remote-curl.c
index 92e40bb682..5db038302e 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1093,7 +1093,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads,
 	client.out = -1;
 	client.git_cmd = 1;
 	strvec_pushv(&client.args, client_argv);
-	if (start_command(&client))
+	if (repo_start_command(the_repository, &client))
 		exit(1);
 	write_or_die(client.in, preamble->buf, preamble->len);
 	if (heads)
diff --git a/repack-cruft.c b/repack-cruft.c
index 0bfc77792a..af4665d029 100644
--- a/repack-cruft.c
+++ b/repack-cruft.c
@@ -62,7 +62,7 @@ int write_cruft_pack(const struct write_pack_opts *opts,
 
 	cmd.in = -1;
 
-	ret = start_command(&cmd);
+	ret = repo_start_command(the_repository, &cmd);
 	if (ret)
 		return ret;
 
diff --git a/repack-filtered.c b/repack-filtered.c
index 2f5d1dd709..b21a7d92a4 100644
--- a/repack-filtered.c
+++ b/repack-filtered.c
@@ -26,7 +26,7 @@ int write_filtered_pack(const struct write_pack_opts *opts,
 
 	cmd.in = -1;
 
-	ret = start_command(&cmd);
+	ret = repo_start_command(the_repository, &cmd);
 	if (ret)
 		return ret;
 
diff --git a/repack-midx.c b/repack-midx.c
index 8b4c0d95e3..32286b5507 100644
--- a/repack-midx.c
+++ b/repack-midx.c
@@ -344,7 +344,7 @@ int write_midx_included_packs(struct repack_write_midx_opts *opts)
 		strvec_pushf(&cmd.args, "--refs-snapshot=%s",
 			     opts->refs_snapshot);
 
-	ret = start_command(&cmd);
+	ret = repo_start_command(the_repository, &cmd);
 	if (ret)
 		goto done;
 
diff --git a/repack-promisor.c b/repack-promisor.c
index 70ef19d04f..03b17884a3 100644
--- a/repack-promisor.c
+++ b/repack-promisor.c
@@ -26,7 +26,7 @@ static int write_oid(const struct object_id *oid,
 	struct child_process *cmd = ctx->cmd;
 
 	if (cmd->in == -1) {
-		if (start_command(cmd))
+		if (repo_start_command(the_repository, cmd))
 			die(_("could not start pack-objects to repack promisor objects"));
 	}
 
diff --git a/run-command.c b/run-command.c
index af26c636a9..b7e7ebdf7d 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1015,7 +1015,7 @@ int run_command(struct child_process *cmd)
 	if (cmd->out < 0 || cmd->err < 0)
 		BUG("run_command with a pipe can cause deadlock");
 
-	code = start_command(cmd);
+	code = repo_start_command(the_repository, cmd);
 	if (code)
 		return code;
 	return finish_command(cmd);
@@ -1430,7 +1430,7 @@ int pipe_command(struct child_process *cmd,
 	if (err)
 		cmd->err = -1;
 
-	if (start_command(cmd) < 0)
+	if (repo_start_command(the_repository, cmd) < 0)
 		return -1;
 
 	if (in) {
@@ -1652,7 +1652,7 @@ static int pp_start_one(struct parallel_processes *pp,
 		pp->children[i].process.stdout_to_stderr = 1;
 	}
 
-	if (start_command(&pp->children[i].process)) {
+	if (repo_start_command(the_repository, &pp->children[i].process)) {
 		if (opts->start_failure)
 			code = opts->start_failure(opts->ungroup ? NULL :
 						   &pp->children[i].err,
@@ -2006,7 +2006,7 @@ enum start_bg_result start_bg_command(struct child_process *cmd,
 	if (!cmd->trace2_child_class)
 		cmd->trace2_child_class = "background";
 
-	ret = start_command(cmd);
+	ret = repo_start_command(the_repository, cmd);
 	if (ret) {
 		/*
 		 * We assume that if `start_command()` fails, we
diff --git a/sub-process.c b/sub-process.c
index ae7493eb5c..abfa856860 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -93,7 +93,7 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
 	process->clean_on_exit_handler = subprocess_exit_handler;
 	process->trace2_child_class = "subprocess";
 
-	err = start_command(process);
+	err = repo_start_command(the_repository, process);
 	if (err) {
 		error("cannot fork to run subprocess '%s'", cmd);
 		return err;
diff --git a/submodule.c b/submodule.c
index 4f9aaa2c75..40c474f927 100644
--- a/submodule.c
+++ b/submodule.c
@@ -718,7 +718,7 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 		strvec_push(&cp.env, GIT_WORK_TREE_ENVIRONMENT "=.");
 	}
 
-	if (start_command(&cp)) {
+	if (repo_start_command(the_repository, &cp)) {
 		diff_emit_submodule_error(o, "(diff failed)\n");
 		goto done;
 	}
@@ -1064,7 +1064,7 @@ static int submodule_needs_pushing(struct repository *r,
 		cp.no_stdin = 1;
 		cp.out = -1;
 		cp.dir = path;
-		if (start_command(&cp))
+		if (repo_start_command(the_repository, &cp))
 			die(_("Could not run 'git rev-list <commits> --not --remotes -n 1' command in submodule %s"),
 					path);
 		if (strbuf_read(&buf, cp.out, the_hash_algo->hexsz + 1))
@@ -1899,7 +1899,7 @@ unsigned is_submodule_modified(const char *path, int ignore_untracked)
 	cp.no_stdin = 1;
 	cp.out = -1;
 	cp.dir = path;
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		die(_("Could not run 'git status --porcelain=2' in submodule %s"), path);
 
 	fp = xfdopen(cp.out, "r");
@@ -2020,7 +2020,7 @@ int bad_to_remove_submodule(const char *path, unsigned flags)
 	cp.no_stdin = 1;
 	cp.out = -1;
 	cp.dir = path;
-	if (start_command(&cp)) {
+	if (repo_start_command(the_repository, &cp)) {
 		if (flags & SUBMODULE_REMOVAL_DIE_ON_ERROR)
 			die(_("could not start 'git status' in submodule '%s'"),
 				path);
@@ -2076,7 +2076,7 @@ static int submodule_has_dirty_index(const struct submodule *sub)
 	cp.no_stdin = 1;
 	cp.no_stdout = 1;
 	cp.dir = sub->path;
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		die(_("could not recurse into submodule '%s'"), sub->path);
 
 	return finish_command(&cp);
@@ -2636,7 +2636,7 @@ int get_superproject_working_tree(struct strbuf *buf)
 	cp.out = -1;
 	cp.git_cmd = 1;
 
-	if (start_command(&cp))
+	if (repo_start_command(the_repository, &cp))
 		die(_("could not start ls-files in .."));
 
 	len = strbuf_read(&sb, cp.out, PATH_MAX);
diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c
index dcd58f228c..fb2ba4b7b6 100644
--- a/t/helper/test-run-command.c
+++ b/t/helper/test-run-command.c
@@ -415,7 +415,7 @@ static int inherit_handle(const char *argv0)
 		     "test-tool", argv0, "inherited-handle-child", NULL);
 	cp.in = -1;
 	cp.no_stdout = cp.no_stderr = 1;
-	if (start_command(&cp) < 0)
+	if (repo_start_command(the_repository, &cp) < 0)
 		die("Could not start child process");
 
 	/* Then close it, and try to delete it. */
@@ -479,7 +479,7 @@ int cmd__run_command(int argc, const char **argv)
 	strvec_pushv(&proc.args, (const char **)argv + 2);
 
 	if (!strcmp(argv[1], "start-command-ENOENT")) {
-		if (start_command(&proc) < 0 && errno == ENOENT) {
+		if (repo_start_command(the_repository, &proc) < 0 && errno == ENOENT) {
 			ret = 0;
 			goto cleanup;
 		}
diff --git a/transport-helper.c b/transport-helper.c
index 4d95d84f9e..c2bfab4c6d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -154,7 +154,7 @@ static struct child_process *get_helper(struct transport *transport)
 
 	helper->trace2_child_class = helper->args.v[0]; /* "remote-<name>" */
 
-	code = start_command(helper);
+	code = repo_start_command(the_repository, helper);
 	if (code < 0 && errno == ENOENT)
 		die(_("unable to find remote helper for '%s'"), data->name);
 	else if (code != 0)
@@ -471,7 +471,7 @@ static int get_importer(struct transport *transport, struct child_process *fasti
 	}
 	fastimport->git_cmd = 1;
 
-	code = start_command(fastimport);
+	code = repo_start_command(the_repository, fastimport);
 	return code;
 }
 
@@ -500,7 +500,7 @@ static int get_exporter(struct transport *transport,
 		strvec_push(&fastexport->args, revlist_args->items[i].string);
 
 	fastexport->git_cmd = 1;
-	return start_command(fastexport);
+	return repo_start_command(the_repository, fastexport);
 }
 
 static int fetch_with_import(struct transport *transport,
diff --git a/upload-pack.c b/upload-pack.c
index e8c5cce1c7..a8143b7b18 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -338,7 +338,7 @@ static void create_pack_file(struct upload_pack_data *pack_data,
 	pack_objects.err = -1;
 	pack_objects.clean_on_exit = 1;
 
-	if (start_command(&pack_objects))
+	if (repo_start_command(the_repository, &pack_objects))
 		die("git upload-pack: unable to fork git-pack-objects");
 
 	pipe_fd = xfdopen(pack_objects.in, "w");
@@ -661,7 +661,7 @@ static int do_reachable_revlist(struct child_process *cmd,
 	 */
 	sigchain_push(SIGPIPE, SIG_IGN);
 
-	if (start_command(cmd))
+	if (repo_start_command(the_repository, cmd))
 		goto error;
 
 	cmd_in = xfdopen(cmd->in, "w");
-- 
2.53.0

Re: [PATCH 0/4] wean start_command() off the_repository

[René Scharfe]

On 3/11/26 4:19 PM, Burak Kaan Karaçay wrote:
> 
> start_command() relies on the_repository due to the 'close_object_store'
> flag in 'struct child_process'. Introduce repo_start_command() to allow
> working with arbitrary repositories. Turn start_command() into a macro
> that wraps repo_start_command() and migrate the existing callers with a
> cocci script.

Good idea to expose this hidden dependency.  It's different from the
other repo_* functions, though, in that most callers can safely pass
NULL as repo because they don't set close_object_store.

Only gc, pull and auto-maintenance set close_object_store.  If we
changed them to set a pointer to the object store they want to have
closed instead of a binary flag then we could leave the other callers
unchanged.

René

Re: [PATCH 0/4] wean start_command() off the_repository

[Jeff King]

On Wed, Mar 11, 2026 at 07:18:10PM +0100, René Scharfe wrote:

> On 3/11/26 4:19 PM, Burak Kaan Karaçay wrote:
> > 
> > start_command() relies on the_repository due to the 'close_object_store'
> > flag in 'struct child_process'. Introduce repo_start_command() to allow
> > working with arbitrary repositories. Turn start_command() into a macro
> > that wraps repo_start_command() and migrate the existing callers with a
> > cocci script.
> 
> Good idea to expose this hidden dependency.  It's different from the
> other repo_* functions, though, in that most callers can safely pass
> NULL as repo because they don't set close_object_store.
> 
> Only gc, pull and auto-maintenance set close_object_store.  If we
> changed them to set a pointer to the object store they want to have
> closed instead of a binary flag then we could leave the other callers
> unchanged.

FWIW, I was about to write the exact same suggestion.

-Peff

Re: [PATCH 0/4] wean start_command() off the_repository

[Burak Kaan Karaçay]

On Wed, Mar 11, 2026 at 07:18:10PM +0100, René Scharfe wrote:
>Only gc, pull and auto-maintenance set close_object_store.  If we
>changed them to set a pointer to the object store they want to have
>closed instead of a binary flag then we could leave the other callers
>unchanged.

Wow. That's... elegant! I have never considered that. I will start
preparing v2.

Thanks,
Burak Kaan Karaçay

Re: [PATCH 0/4] wean start_command() off the_repository

[Junio C Hamano]

René Scharfe <l.s.r@web.de> writes:

> On 3/11/26 4:19 PM, Burak Kaan Karaçay wrote:
>> 
>> start_command() relies on the_repository due to the 'close_object_store'
>> flag in 'struct child_process'. Introduce repo_start_command() to allow
>> working with arbitrary repositories. Turn start_command() into a macro
>> that wraps repo_start_command() and migrate the existing callers with a
>> cocci script.
>
> Good idea to expose this hidden dependency.  It's different from the
> other repo_* functions, though, in that most callers can safely pass
> NULL as repo because they don't set close_object_store.
>
> Only gc, pull and auto-maintenance set close_object_store.  If we
> changed them to set a pointer to the object store they want to have
> closed instead of a binary flag then we could leave the other callers
> unchanged.

You solved my "don't we need to know where we are coming from, in
addition to where we are going?" question elegantly.

Thanks.

Re: [PATCH 0/4] wean start_command() off the_repository

[Junio C Hamano]

Burak Kaan Karaçay <bkkaracay@gmail.com> writes:

> start_command() relies on the_repository due to the 'close_object_store'
> flag in 'struct child_process'. Introduce repo_start_command() to allow
> working with arbitrary repositories. Turn start_command() into a macro
> that wraps repo_start_command() and migrate the existing callers with a
> cocci script.
>
> For callers that cannot access 'the_repository' due to the lack of
> USE_THE_REPOSITORY_VARIABLE, define the macro. If the caller already has
> a local repository context, pass it explicitly instead of defining the
> macro.
>
> Thanks,
> Burak Kaan Karaçay
>
> Burak Kaan Karaçay (4):
>   run-command: add repo_start_command()
>   run-command: use repo_start_command() in strict callers
>   run-command: redefine start_command() as a wrapper macro
>   cocci: convert start_command() to repo_start_command()

The organization to start with a wrapper, and then moving to a macro
that is protected behind USE_THE_REPOSITORY_VARIABLE, makes quite a
lot of sense.  I do not know the answer to the question I asked on
"don't we need to know from which repository we are closing the
object store while switching to this new repository?", and without
knowing the answer, we cannot quite decide what the function
signature of repo_start_command() should look like, so the last step
might be a bit premature.

Other than that, a well reasoned series.

Thanks.

[PATCH v2] run-command: wean start_command() off the_repository

[Burak Kaan Karaçay]

The start_command() relies on the_repository due to the
close_object_store flag in 'struct child_process'. When this flag is
set, start_command() closes the object store associated with
the_repository before spawning a child process.

To eliminate this dependency, replace the 'close_object_store' with the
new 'odb_to_close' field. This allows callers to specify the object
store that needs to be closed.

Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 builtin/gc.c   | 14 +++++++++-----
 builtin/pull.c |  2 +-
 run-command.c  |  6 +++---
 run-command.h  |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index fb329c2cff..5d8d358f7a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1030,7 +1030,7 @@ int cmd_gc(int argc,
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
 		repack_cmd.git_cmd = 1;
-		repack_cmd.close_object_store = 1;
+		repack_cmd.odb_to_close = the_repository->objects;
 		strvec_pushv(&repack_cmd.args, repack_args.v);
 		if (run_command(&repack_cmd))
 			die(FAILED_RUN, repack_args.v[0]);
@@ -1199,7 +1199,8 @@ static int run_write_commit_graph(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "commit-graph", "write",
 		     "--split", "--reachable", NULL);
 
@@ -1268,7 +1269,8 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_push(&child.args, "gc");
 
 	if (opts->auto_flag)
@@ -1484,7 +1486,8 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "multi-pack-index", "expire", NULL);
 
 	if (opts->quiet)
@@ -1542,7 +1545,8 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "multi-pack-index", "repack", NULL);
 
 	if (opts->quiet)
diff --git a/builtin/pull.c b/builtin/pull.c
index 6ad420ce6f..7e67fdce97 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -454,7 +454,7 @@ static int run_fetch(const char *repo, const char **refspecs)
 	} else if (*refspecs)
 		BUG("refspecs without repo?");
 	cmd.git_cmd = 1;
-	cmd.close_object_store = 1;
+	cmd.odb_to_close = the_repository->objects;
 	return run_command(&cmd);
 }
 
diff --git a/run-command.c b/run-command.c
index b27064ef57..ed5e8be976 100644
--- a/run-command.c
+++ b/run-command.c
@@ -742,8 +742,8 @@ int start_command(struct child_process *cmd)
 
 	fflush(NULL);
 
-	if (cmd->close_object_store)
-		odb_close(the_repository->objects);
+	if (cmd->odb_to_close)
+		odb_close(cmd->odb_to_close);
 
 #ifndef GIT_WINDOWS_NATIVE
 {
@@ -1955,7 +1955,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
 	maint->git_cmd = 1;
-	maint->close_object_store = 1;
+	maint->odb_to_close = the_repository->objects;
 	strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL);
 	strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet");
 	strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach");
diff --git a/run-command.h b/run-command.h
index e1ca965b5b..af4c9da279 100644
--- a/run-command.h
+++ b/run-command.h
@@ -136,7 +136,7 @@ struct child_process {
 	 * want to repack because that would delete `.pack` files (and on
 	 * Windows, you cannot delete files that are still in use).
 	 */
-	unsigned close_object_store:1;
+	struct object_database *odb_to_close;
 
 	unsigned stdout_to_stderr:1;
 	unsigned clean_on_exit:1;
-- 
2.53.0

Re: [PATCH v2] run-command: wean start_command() off the_repository

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 11:53:41AM +0300, Burak Kaan Karaçay wrote:
> The start_command() relies on the_repository due to the
> close_object_store flag in 'struct child_process'. When this flag is
> set, start_command() closes the object store associated with
> the_repository before spawning a child process.
> 
> To eliminate this dependency, replace the 'close_object_store' with the
> new 'odb_to_close' field. This allows callers to specify the object
> store that needs to be closed.

I really like this solution.

There's now only a single other function that still uses
`the_repository` in `prepare_auto_maintenance()`. Do we maybe want to
add a second commit that converts this function and its caller
`run_auto_maintenance()` to receive the repository as parameter so that
we can drop the `USE_THE_REPOSITORY_VARIABLE` declaration?

Thanks!

Patrick

[PATCH v3 0/2] run-command: stop using the_repository

[Burak Kaan Karaçay]

Hi,

This patch series aims to remove the_repository dependency in
'run-command.c'.

The first patch removes the dependency in start_command() by replacing
the boolean 'close_object_store' flag with a pointer to the target
object store.

The second patch handles the prepare_auto_maintenance() and
run_auto_maintenance() functions by passing a 'struct repository *'
parameter. With no global repository dependencies left, it drops the
USE_THE_REPOSITORY_VARIABLE macro from the file.

Changes in v3:
- Added the second patch at the suggestion of Patrick to fully clean up
  the file.

Changes in v2:
- Dropped the wrapper approach and rewrote the patch around the approach
  suggested by René.

Thanks for all guidance,
Burak Kaan Karaçay

Burak Kaan Karaçay (2):
  run-command: wean start_command() off the_repository
  run-command: wean auto_maintenance() functions off the_repository

 builtin/am.c           |  2 +-
 builtin/commit.c       |  2 +-
 builtin/fetch.c        |  2 +-
 builtin/gc.c           | 14 +++++++++-----
 builtin/merge.c        |  2 +-
 builtin/pull.c         |  2 +-
 builtin/rebase.c       |  4 +++-
 builtin/receive-pack.c |  2 +-
 run-command.c          | 20 ++++++++++----------
 run-command.h          |  9 ++++++---
 10 files changed, 34 insertions(+), 25 deletions(-)

-- 
2.53.0

[PATCH v3 1/2] run-command: wean start_command() off the_repository

[Burak Kaan Karaçay]

The start_command() relies on the_repository due to the
close_object_store flag in 'struct child_process'. When this flag is
set, start_command() closes the object store associated with
the_repository before spawning a child process.

To eliminate this dependency, replace the 'close_object_store' with the
new 'struct object_database *odb_to_close' field. This allows callers to
specify the object store that needs to be closed.

Suggested-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 builtin/gc.c   | 14 +++++++++-----
 builtin/pull.c |  2 +-
 run-command.c  |  6 +++---
 run-command.h  |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index fb329c2cff..5d8d358f7a 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1030,7 +1030,7 @@ int cmd_gc(int argc,
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
 		repack_cmd.git_cmd = 1;
-		repack_cmd.close_object_store = 1;
+		repack_cmd.odb_to_close = the_repository->objects;
 		strvec_pushv(&repack_cmd.args, repack_args.v);
 		if (run_command(&repack_cmd))
 			die(FAILED_RUN, repack_args.v[0]);
@@ -1199,7 +1199,8 @@ static int run_write_commit_graph(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "commit-graph", "write",
 		     "--split", "--reachable", NULL);
 
@@ -1268,7 +1269,8 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_push(&child.args, "gc");
 
 	if (opts->auto_flag)
@@ -1484,7 +1486,8 @@ static int multi_pack_index_expire(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "multi-pack-index", "expire", NULL);
 
 	if (opts->quiet)
@@ -1542,7 +1545,8 @@ static int multi_pack_index_repack(struct maintenance_run_opts *opts)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 
-	child.git_cmd = child.close_object_store = 1;
+	child.git_cmd = 1;
+	child.odb_to_close = the_repository->objects;
 	strvec_pushl(&child.args, "multi-pack-index", "repack", NULL);
 
 	if (opts->quiet)
diff --git a/builtin/pull.c b/builtin/pull.c
index 6ad420ce6f..7e67fdce97 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -454,7 +454,7 @@ static int run_fetch(const char *repo, const char **refspecs)
 	} else if (*refspecs)
 		BUG("refspecs without repo?");
 	cmd.git_cmd = 1;
-	cmd.close_object_store = 1;
+	cmd.odb_to_close = the_repository->objects;
 	return run_command(&cmd);
 }
 
diff --git a/run-command.c b/run-command.c
index b27064ef57..ed5e8be976 100644
--- a/run-command.c
+++ b/run-command.c
@@ -742,8 +742,8 @@ int start_command(struct child_process *cmd)
 
 	fflush(NULL);
 
-	if (cmd->close_object_store)
-		odb_close(the_repository->objects);
+	if (cmd->odb_to_close)
+		odb_close(cmd->odb_to_close);
 
 #ifndef GIT_WINDOWS_NATIVE
 {
@@ -1955,7 +1955,7 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
 	maint->git_cmd = 1;
-	maint->close_object_store = 1;
+	maint->odb_to_close = the_repository->objects;
 	strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL);
 	strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet");
 	strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach");
diff --git a/run-command.h b/run-command.h
index e1ca965b5b..af4c9da279 100644
--- a/run-command.h
+++ b/run-command.h
@@ -136,7 +136,7 @@ struct child_process {
 	 * want to repack because that would delete `.pack` files (and on
 	 * Windows, you cannot delete files that are still in use).
 	 */
-	unsigned close_object_store:1;
+	struct object_database *odb_to_close;
 
 	unsigned stdout_to_stderr:1;
 	unsigned clean_on_exit:1;
-- 
2.53.0

[PATCH v3 2/2] run-command: wean auto_maintenance() functions off the_repository

[Burak Kaan Karaçay]

The prepare_auto_maintenance() relies on the_repository to read
configurations. Since run_auto_maintenance() calls
prepare_auto_maintenance(), it also implicitly depends the_repository.

Add 'struct repository *' as a parameter to both functions and update
all callers to pass the_repository.

With no global repository dependencies left in this file, remove the
USE_THE_REPOSITORY_VARIABLE macro.

Suggested-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Burak Kaan Karaçay <bkkaracay@gmail.com>
---
 builtin/am.c           |  2 +-
 builtin/commit.c       |  2 +-
 builtin/fetch.c        |  2 +-
 builtin/merge.c        |  2 +-
 builtin/rebase.c       |  4 +++-
 builtin/receive-pack.c |  2 +-
 run-command.c          | 16 ++++++++--------
 run-command.h          |  7 +++++--
 8 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e0c767e223..9d0b51c651 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1937,7 +1937,7 @@ static void am_run(struct am_state *state, int resume)
 	 */
 	if (!state->rebasing) {
 		am_destroy(state);
-		run_auto_maintenance(state->quiet);
+		run_auto_maintenance(the_repository, state->quiet);
 	}
 }
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 844bdcc728..7b23c1f883 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1958,7 +1958,7 @@ int cmd_commit(int argc,
 	git_test_write_commit_graph_or_die(the_repository->objects->sources);
 
 	repo_rerere(the_repository, 0);
-	run_auto_maintenance(quiet);
+	run_auto_maintenance(the_repository, quiet);
 	run_commit_hook(use_editor, repo_get_index_file(the_repository),
 			NULL, "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8a36cf67b5..4795b2a13c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -2873,7 +2873,7 @@ int cmd_fetch(int argc,
 			if (opt_val != 0)
 				git_config_push_parameter("maintenance.incremental-repack.auto=-1");
 		}
-		run_auto_maintenance(verbosity < 0);
+		run_auto_maintenance(the_repository, verbosity < 0);
 	}
 
  cleanup:
diff --git a/builtin/merge.c b/builtin/merge.c
index 4e456a381c..2cbce56f8d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -506,7 +506,7 @@ static void finish(struct commit *head_commit,
 			 * We ignore errors in 'gc --auto', since the
 			 * user should see them.
 			 */
-			run_auto_maintenance(verbosity < 0);
+			run_auto_maintenance(the_repository, verbosity < 0);
 		}
 	}
 	if (new_head && show_diffstat) {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index c487e10907..8c1316db38 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -562,7 +562,9 @@ static int finish_rebase(struct rebase_options *opts)
 	 * We ignore errors in 'git maintenance run --auto', since the
 	 * user should see them.
 	 */
-	run_auto_maintenance(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
+	run_auto_maintenance(the_repository,
+			!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
+
 	if (opts->type == REBASE_MERGE) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d6225df890..e34edff406 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -2727,7 +2727,7 @@ int cmd_receive_pack(int argc,
 		if (auto_gc) {
 			struct child_process proc = CHILD_PROCESS_INIT;
 
-			if (prepare_auto_maintenance(1, &proc)) {
+			if (prepare_auto_maintenance(the_repository, 1, &proc)) {
 				proc.no_stdin = 1;
 				proc.stdout_to_stderr = 1;
 				proc.err = use_sideband ? -1 : 0;
diff --git a/run-command.c b/run-command.c
index ed5e8be976..38f4c699f8 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1,4 +1,3 @@
-#define USE_THE_REPOSITORY_VARIABLE
 #define DISABLE_SIGN_COMPARE_WARNINGS
 
 #include "git-compat-util.h"
@@ -1937,11 +1936,12 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 		trace2_region_leave(tr2_category, tr2_label, NULL);
 }
 
-int prepare_auto_maintenance(int quiet, struct child_process *maint)
+int prepare_auto_maintenance(struct repository *r, int quiet,
+			     struct child_process *maint)
 {
 	int enabled, auto_detach;
 
-	if (!repo_config_get_bool(the_repository, "maintenance.auto", &enabled) &&
+	if (!repo_config_get_bool(r, "maintenance.auto", &enabled) &&
 	    !enabled)
 		return 0;
 
@@ -1950,12 +1950,12 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 	 * honoring `gc.autoDetach`. This is somewhat weird, but required to
 	 * retain behaviour from when we used to run git-gc(1) here.
 	 */
-	if (repo_config_get_bool(the_repository, "maintenance.autodetach", &auto_detach) &&
-	    repo_config_get_bool(the_repository, "gc.autodetach", &auto_detach))
+	if (repo_config_get_bool(r, "maintenance.autodetach", &auto_detach) &&
+	    repo_config_get_bool(r, "gc.autodetach", &auto_detach))
 		auto_detach = git_env_bool("GIT_TEST_MAINT_AUTO_DETACH", true);
 
 	maint->git_cmd = 1;
-	maint->odb_to_close = the_repository->objects;
+	maint->odb_to_close = r->objects;
 	strvec_pushl(&maint->args, "maintenance", "run", "--auto", NULL);
 	strvec_push(&maint->args, quiet ? "--quiet" : "--no-quiet");
 	strvec_push(&maint->args, auto_detach ? "--detach" : "--no-detach");
@@ -1963,10 +1963,10 @@ int prepare_auto_maintenance(int quiet, struct child_process *maint)
 	return 1;
 }
 
-int run_auto_maintenance(int quiet)
+int run_auto_maintenance(struct repository *r, int quiet)
 {
 	struct child_process maint = CHILD_PROCESS_INIT;
-	if (!prepare_auto_maintenance(quiet, &maint))
+	if (!prepare_auto_maintenance(r, quiet, &maint))
 		return 0;
 	return run_command(&maint);
 }
diff --git a/run-command.h b/run-command.h
index af4c9da279..ad25740fe6 100644
--- a/run-command.h
+++ b/run-command.h
@@ -5,6 +5,8 @@
 
 #include "strvec.h"
 
+struct repository;
+
 /**
  * The run-command API offers a versatile tool to run sub-processes with
  * redirected input and output as well as with a modified environment
@@ -227,12 +229,13 @@ int run_command(struct child_process *);
  * process has been prepared and is ready to run, or 0 in case auto-maintenance
  * should be skipped.
  */
-int prepare_auto_maintenance(int quiet, struct child_process *maint);
+int prepare_auto_maintenance(struct repository *r, int quiet,
+			     struct child_process *maint);
 
 /*
  * Trigger an auto-gc
  */
-int run_auto_maintenance(int quiet);
+int run_auto_maintenance(struct repository *r, int quiet);
 
 /**
  * Execute the given command, sending "in" to its stdin, and capturing its
-- 
2.53.0

Re: [PATCH v3 0/2] run-command: stop using the_repository

[Junio C Hamano]

Burak Kaan Karaçay <bkkaracay@gmail.com> writes:

> This patch series aims to remove the_repository dependency in
> 'run-command.c'.
>
> The first patch removes the dependency in start_command() by replacing
> the boolean 'close_object_store' flag with a pointer to the target
> object store.
>
> The second patch handles the prepare_auto_maintenance() and
> run_auto_maintenance() functions by passing a 'struct repository *'
> parameter. With no global repository dependencies left, it drops the
> USE_THE_REPOSITORY_VARIABLE macro from the file.
>
> Changes in v3:
> - Added the second patch at the suggestion of Patrick to fully clean up
>   the file.

[1/2] is now exactly as expected from the previous discussion.  The
only miniscule thing I found in [2/2] was a new blank line
introduced here, which seemed unnecessary and not in line with the
existing style in that function.

Otherwise, looking very good.  Will queue.  Thanks.


diff --git a/builtin/rebase.c b/builtin/rebase.c
index c487e10907..8c1316db38 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -562,7 +562,9 @@ static int finish_rebase(struct rebase_options *opts)
 	 * We ignore errors in 'git maintenance run --auto', since the
 	 * user should see them.
 	 */
-	run_auto_maintenance(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
+	run_auto_maintenance(the_repository,
+			!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
+
 	if (opts->type == REBASE_MERGE) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 

Re: [PATCH v3 0/2] run-command: stop using the_repository

[Patrick Steinhardt]

On Thu, Mar 12, 2026 at 08:29:52AM -0700, Junio C Hamano wrote:
> Burak Kaan Karaçay <bkkaracay@gmail.com> writes:
> 
> > This patch series aims to remove the_repository dependency in
> > 'run-command.c'.
> >
> > The first patch removes the dependency in start_command() by replacing
> > the boolean 'close_object_store' flag with a pointer to the target
> > object store.
> >
> > The second patch handles the prepare_auto_maintenance() and
> > run_auto_maintenance() functions by passing a 'struct repository *'
> > parameter. With no global repository dependencies left, it drops the
> > USE_THE_REPOSITORY_VARIABLE macro from the file.
> >
> > Changes in v3:
> > - Added the second patch at the suggestion of Patrick to fully clean up
> >   the file.
> 
> [1/2] is now exactly as expected from the previous discussion.  The
> only miniscule thing I found in [2/2] was a new blank line
> introduced here, which seemed unnecessary and not in line with the
> existing style in that function.
> 
> Otherwise, looking very good.  Will queue.  Thanks.

Agreed, this looks good to me. Thanks!

Patrick