[PATCH v2 0/2] for-each-repo: work correctly in a worktree

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

This was reported by Matthew [1] and is a quick fix.

[1]
https://lore.kernel.org/git/CABpCjbY=wpStuhxqRJ5TSNV3A-CmN-g-xZGJOQGSSv3GYhs2fQ@mail.gmail.com/

I also took the liberty of removing the_repository as I wanted to make sure
that wasn't involved here.

Thanks, -Stolee

Derrick Stolee (2):
  for-each-repo: test outside of repo context
  for-each-repo: work correctly in a worktree

 builtin/for-each-repo.c  | 33 ++++++++++++++++++++++++++++++---
 t/t0068-for-each-repo.sh | 33 ++++++++++++++++++++++++---------
 2 files changed, 54 insertions(+), 12 deletions(-)


base-commit: 67ad42147a7acc2af6074753ebd03d904476118f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2056%2Fderrickstolee%2Ffor-each-repo-in-gitdir-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2056/derrickstolee/for-each-repo-in-gitdir-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2056

Range-diff vs v1:

 1:  86cd83f65b < -:  ---------- for-each-repo: stop using the_repository
 -:  ---------- > 1:  6e9d4f3029 for-each-repo: test outside of repo context
 2:  a47f9e9386 ! 2:  4e3f4aa6cd for-each-repo: work correctly in a worktree
     @@ Commit message
          repository's local config (demonstrating that it worked with
          non-worktree Git repositories).
      
     -    The fix is simple: unset the environment variable before looping over
     -    the repos.
     +    We need to be careful to unset the local Git environment variables and
     +    let the child process rediscover them, while also reinstating those
     +    variables in the parent process afterwards. Update run_command_on_repo()
     +    to store, unset, then reset the non-NULL variables.
      
          Reported-by: Matthew Gabeler-Lee <fastcat@gmail.com>
          Signed-off-by: Derrick Stolee <stolee@gmail.com>
      
       ## builtin/for-each-repo.c ##
      @@
     + 
       #include "builtin.h"
       #include "config.h"
      +#include "environment.h"
       #include "gettext.h"
       #include "parse-options.h"
       #include "path.h"
     -@@ builtin/for-each-repo.c: int cmd_for_each_repo(int argc,
     - 	else if (err)
     - 		return 0;
     +@@ builtin/for-each-repo.c: static const char * const for_each_repo_usage[] = {
     + 
     + static int run_command_on_repo(const char *path, int argc, const char ** argv)
     + {
     +-	int i;
     ++	int res;
     + 	struct child_process child = CHILD_PROCESS_INIT;
     ++	char **envvars;
     ++	size_t envvar_nr = 0;
     + 	char *abspath = interpolate_path(path, 0);
     + 
     ++	while (local_repo_env[envvar_nr])
     ++		envvar_nr++;
     ++
     ++	CALLOC_ARRAY(envvars, envvar_nr);
     ++
     ++	for (size_t i = 0; i < envvar_nr; i++) {
     ++		envvars[i] = getenv(local_repo_env[i]);
     ++
     ++		if (envvars[i]) {
     ++			unsetenv(local_repo_env[i]);
     ++			envvars[i] = xstrdup(envvars[i]);
     ++		}
     ++	}
     ++
     + 	child.git_cmd = 1;
     + 	strvec_pushl(&child.args, "-C", abspath, NULL);
     + 
     +-	for (i = 0; i < argc; i++)
     ++	for (int i = 0; i < argc; i++)
     + 		strvec_push(&child.args, argv[i]);
       
     -+	/* Be sure to not pass GIT_DIR to children. */
     -+	unsetenv(GIT_DIR_ENVIRONMENT);
     + 	free(abspath);
     + 
     +-	return run_command(&child);
     ++	res = run_command(&child);
     ++
     ++	for (size_t i = 0; i < envvar_nr; i++) {
     ++		if (envvars[i]) {
     ++			setenv(local_repo_env[i], envvars[i], 1);
     ++			free(envvars[i]);
     ++		}
     ++	}
      +
     - 	for (size_t i = 0; i < values->nr; i++) {
     - 		int ret = run_command_on_repo(values->items[i].string, argc, argv);
     - 		if (ret) {
     ++	free(envvars);
     ++	return res;
     + }
     + 
     + int cmd_for_each_repo(int argc,
      
       ## t/t0068-for-each-repo.sh ##
     -@@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'
     +@@ t/t0068-for-each-repo.sh: TEST_NO_CREATE_REPO=1
       test_expect_success 'run based on configured value' '
       	git init one &&
       	git init two &&
     @@ t/t0068-for-each-repo.sh: test_description='git for-each-repo builtin'
      +	git -C two worktree add --orphan ../three &&
       	git init ~/four &&
       	git -C two commit --allow-empty -m "DID NOT RUN" &&
     --	git config run.key "$TRASH_DIRECTORY/one" &&
     --	git config --add run.key "$TRASH_DIRECTORY/three" &&
     --	git config --add run.key "~/four" &&
     -+	git config --global run.key "$TRASH_DIRECTORY/one" &&
     -+	git config --global --add run.key "$TRASH_DIRECTORY/three" &&
     -+	git config --global --add run.key "~/four" &&
     -+
     - 	git for-each-repo --config=run.key commit --allow-empty -m "ran" &&
     - 	git -C one log -1 --pretty=format:%s >message &&
     - 	grep ran message &&
     -@@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
     - 	grep ran message &&
     - 	git -C ~/four log -1 --pretty=format:%s >message &&
     - 	grep ran message &&
     -+
     - 	git for-each-repo --config=run.key -- commit --allow-empty -m "ran again" &&
     - 	git -C one log -1 --pretty=format:%s >message &&
     - 	grep again message &&
     + 	git config --global run.key "$TRASH_DIRECTORY/one" &&
      @@ t/t0068-for-each-repo.sh: test_expect_success 'run based on configured value' '
       	git -C three log -1 --pretty=format:%s >message &&
       	grep again message &&

-- 
gitgitgadget