Re: [PATCH 09/21] maintenance: add commit-graph task

On 7/9/2020 6:52 PM, Jeff King wrote:

On Thu, Jul 09, 2020 at 07:14:41AM -0400, Derrick Stolee wrote:

quoted

On 7/8/2020 10:29 PM, Jonathan Tan wrote:

quoted

quoted

+static int run_write_commit_graph(struct repository *r)
+{
+	int result;
+	struct argv_array cmd = ARGV_ARRAY_INIT;
+
+	argv_array_pushl(&cmd, "-C", r->worktree,
+			 "commit-graph", "write",
+			 "--split", "--reachable",
+			 NULL);

As mentioned in my reply to an earlier patch (sent a few minutes ago),
this won't work if there are environment variables like GIT_DIR present.

Do we not pass GIT_DIR to the subcommand? Or does using "-C" override
the GIT_DIR?

We do pass GIT_DIR to the subcommand, and "-C" does not override it. I
think this code would work as long as "r" is the_repository, which it
would be in the current code. But then the "-C" would be doing nothing
useful (it might change to the top of the worktree if we weren't there
for some reason, but I don't think "commit-graph write" would care
either way).

But if "r" is some other repository, "commit-graph" would continue to
operate in the parent process repository because of the inherited
GIT_DIR. Using "--git-dir" would solve that, but as a general practice,
if you're spawning a sub-process that might be in another repository,
you should clear any repo-specific environment variables. The list is in
local_repo_env, which you can feed to the "env" or "env_array" parameter
of a child_process (see the use in connect.c for an example).

Even in the current scheme where "r" is always the_repository, I suspect
this might still be buggy. If we're in a bare repository, presumably
r->worktree would be NULL.

Ah. I'll investigate this more and work to create a way to
run a subcommand in a given repository. Your pointers will
help a lot.

Thanks,
-Stolee