Merge branch 'gb/maint-submodule-env'

* gb/maint-submodule-env:
  is_submodule_modified(): clear environment properly
  submodules: ensure clean environment when operating in a submodule
  shell setup: clear_local_git_env() function
  rev-parse: --local-env-vars option
  Refactor list of of repo-local env vars

8 files changed, 69 insertions, 33 deletions

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 1a613aa108..8db600f6ba 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -148,6 +148,12 @@ shown.  If the pattern does not contain a globbing character (`?`,
 --is-bare-repository::
 	When the repository is bare print "true", otherwise "false".
 
+--local-env-vars::
+	List the GIT_* environment variables that are local to the
+	repository (e.g. GIT_DIR or GIT_WORK_TREE, but not GIT_EDITOR).
+	Only the names of the variables are listed, not their value,
+	even if they are set.
+
 --short::
 --short=number::
 	Instead of outputting the full SHA1 values of object names try to
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 88bad9af3c..8fbf9d0db6 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -455,6 +455,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 	if (argc > 1 && !strcmp("--sq-quote", argv[1]))
 		return cmd_sq_quote(argc - 2, argv + 2);
 
+	if (argc == 2 && !strcmp("--local-env-vars", argv[1])) {
+		int i;
+		for (i = 0; local_repo_env[i]; i++)
+			printf("%s\n", local_repo_env[i]);
+		return 0;
+	}
+
 	if (argc > 1 && !strcmp("-h", argv[1]))
 		usage(builtin_rev_parse_usage);
 
diff --git a/cache.h b/cache.h
index c286310ded..89f6a40d1a 100644
--- a/cache.h
+++ b/cache.h
@@ -388,6 +388,15 @@ static inline enum object_type object_type(unsigned int mode)
 #define GIT_NOTES_REF_ENVIRONMENT "GIT_NOTES_REF"
 #define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
 
+/*
+ * Repository-local GIT_* environment variables
+ * The array is NULL-terminated to simplify its usage in contexts such
+ * environment creation or simple walk of the list.
+ * The number of non-NULL entries is available as a macro.
+ */
+#define LOCAL_REPO_ENV_SIZE 8
+extern const char *const local_repo_env[LOCAL_REPO_ENV_SIZE + 1];
+
 extern int is_bare_repository_cfg;
 extern int is_bare_repository(void);
 extern int is_inside_git_dir(void);
diff --git a/connect.c b/connect.c
index 9f39038f6f..323a771b69 100644
--- a/connect.c
+++ b/connect.c
@@ -582,18 +582,8 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		*arg++ = host;
 	}
 	else {
-		/* remove these from the environment */
-		const char *env[] = {
-			ALTERNATE_DB_ENVIRONMENT,
-			DB_ENVIRONMENT,
-			GIT_DIR_ENVIRONMENT,
-			GIT_WORK_TREE_ENVIRONMENT,
-			GRAFT_ENVIRONMENT,
-			INDEX_ENVIRONMENT,
-			NO_REPLACE_OBJECTS_ENVIRONMENT,
-			NULL
-		};
-		conn->env = env;
+		/* remove repo-local variables from the environment */
+		conn->env = local_repo_env;
 		conn->use_shell = 1;
 	}
 	*arg++ = cmd.buf;
diff --git a/environment.c b/environment.c
index 739ec27040..876c5e5341 100644
--- a/environment.c
+++ b/environment.c
@@ -63,6 +63,23 @@ static char *work_tree;
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
 
+/*
+ * Repository-local GIT_* environment variables
+ * Remember to update local_repo_env_size in cache.h when
+ * the size of the list changes
+ */
+const char * const local_repo_env[LOCAL_REPO_ENV_SIZE + 1] = {
+	ALTERNATE_DB_ENVIRONMENT,
+	CONFIG_ENVIRONMENT,
+	DB_ENVIRONMENT,
+	GIT_DIR_ENVIRONMENT,
+	GIT_WORK_TREE_ENVIRONMENT,
+	GRAFT_ENVIRONMENT,
+	INDEX_ENVIRONMENT,
+	NO_REPLACE_OBJECTS_ENVIRONMENT,
+	NULL
+};
+
 static void setup_git_env(void)
 {
 	git_dir = getenv(GIT_DIR_ENVIRONMENT);
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 7a095665d7..6131670860 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -172,6 +172,13 @@ get_author_ident_from_commit () {
 	LANG=C LC_ALL=C sed -ne "$pick_author_script"
 }
 
+# Clear repo-local GIT_* environment variables. Useful when switching to
+# another repository (e.g. when entering a submodule). See also the env
+# list in git_connect()
+clear_local_git_env() {
+	unset $(git rev-parse --local-env-vars)
+}
+
 # Make sure we are in a valid repository of a vintage we understand,
 # if we require to be in a git repository.
 if test -z "$NONGIT_OK"
diff --git a/git-submodule.sh b/git-submodule.sh
index 383dc45119..f21d0bfce7 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -222,7 +222,7 @@ cmd_add()
 
 		module_clone "$path" "$realrepo" "$reference" || exit
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path" &&
 			# ash fails to wordsplit ${branch:+-b "$branch"...}
 			case "$branch" in
@@ -278,7 +278,7 @@ cmd_foreach()
 			name=$(module_name "$path")
 			(
 				prefix="$prefix$path/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				eval "$@" &&
 				if test -n "$recursive"
@@ -434,7 +434,7 @@ cmd_update()
 			module_clone "$path" "$url" "$reference"|| exit
 			subsha1=
 		else
-			subsha1=$(unset GIT_DIR; cd "$path" &&
+			subsha1=$(clear_local_git_env; cd "$path" &&
 				git rev-parse --verify HEAD) ||
 			die "Unable to find current revision in submodule path '$path'"
 		fi
@@ -454,7 +454,7 @@ cmd_update()
 
 			if test -z "$nofetch"
 			then
-				(unset GIT_DIR; cd "$path" &&
+				(clear_local_git_env; cd "$path" &&
 					git-fetch) ||
 				die "Unable to fetch in submodule path '$path'"
 			fi
@@ -477,14 +477,14 @@ cmd_update()
 				;;
 			esac
 
-			(unset GIT_DIR; cd "$path" && $command "$sha1") ||
+			(clear_local_git_env; cd "$path" && $command "$sha1") ||
 			die "Unable to $action '$sha1' in submodule path '$path'"
 			say "Submodule path '$path': $msg '$sha1'"
 		fi
 
 		if test -n "$recursive"
 		then
-			(unset GIT_DIR; cd "$path" && cmd_update $orig_args) ||
+			(clear_local_git_env; cd "$path" && cmd_update $orig_args) ||
 			die "Failed to recurse into submodule path '$path'"
 		fi
 	done
@@ -492,7 +492,7 @@ cmd_update()
 
 set_name_rev () {
 	revname=$( (
-		unset GIT_DIR
+		clear_local_git_env
 		cd "$1" && {
 			git describe "$2" 2>/dev/null ||
 			git describe --tags "$2" 2>/dev/null ||
@@ -760,7 +760,7 @@ cmd_status()
 		else
 			if test -z "$cached"
 			then
-				sha1=$(unset GIT_DIR; cd "$path" && git rev-parse --verify HEAD)
+				sha1=$(clear_local_git_env; cd "$path" && git rev-parse --verify HEAD)
 				set_name_rev "$path" "$sha1"
 			fi
 			say "+$sha1 $displaypath$revname"
@@ -770,7 +770,7 @@ cmd_status()
 		then
 			(
 				prefix="$displaypath/"
-				unset GIT_DIR
+				clear_local_git_env
 				cd "$path" &&
 				cmd_status $orig_args
 			) ||
@@ -821,7 +821,7 @@ cmd_sync()
 		if test -e "$path"/.git
 		then
 		(
-			unset GIT_DIR
+			clear_local_git_env
 			cd "$path"
 			remote=$(get_default_remote)
 			say "Synchronizing submodule url for '$name'"
diff --git a/submodule.c b/submodule.c
index 7d70c4f7bf..5d286e409e 100644
--- a/submodule.c
+++ b/submodule.c
@@ -123,16 +123,19 @@ void show_submodule_summary(FILE *f, const char *path,
 
 int is_submodule_modified(const char *path)
 {
-	int len;
+	int len, i;
 	struct child_process cp;
 	const char *argv[] = {
 		"status",
 		"--porcelain",
 		NULL,
 	};
-	char *env[4];
+	const char *env[LOCAL_REPO_ENV_SIZE + 3];
 	struct strbuf buf = STRBUF_INIT;
 
+	for (i = 0; i < LOCAL_REPO_ENV_SIZE; i++)
+		env[i] = local_repo_env[i];
+
 	strbuf_addf(&buf, "%s/.git/", path);
 	if (!is_directory(buf.buf)) {
 		strbuf_release(&buf);
@@ -143,16 +146,14 @@ int is_submodule_modified(const char *path)
 	strbuf_reset(&buf);
 
 	strbuf_addf(&buf, "GIT_WORK_TREE=%s", path);
-	env[0] = strbuf_detach(&buf, NULL);
+	env[i++] = strbuf_detach(&buf, NULL);
 	strbuf_addf(&buf, "GIT_DIR=%s/.git", path);
-	env[1] = strbuf_detach(&buf, NULL);
-	strbuf_addf(&buf, "GIT_INDEX_FILE");
-	env[2] = strbuf_detach(&buf, NULL);
-	env[3] = NULL;
+	env[i++] = strbuf_detach(&buf, NULL);
+	env[i] = NULL;
 
 	memset(&cp, 0, sizeof(cp));
 	cp.argv = argv;
-	cp.env = (const char *const *)env;
+	cp.env = env;
 	cp.git_cmd = 1;
 	cp.no_stdin = 1;
 	cp.out = -1;
@@ -165,9 +166,8 @@ int is_submodule_modified(const char *path)
 	if (finish_command(&cp))
 		die("git status --porcelain failed");
 
-	free(env[0]);
-	free(env[1]);
-	free(env[2]);
+	for (i = LOCAL_REPO_ENV_SIZE; env[i]; i++)
+		free((char *)env[i]);
 	strbuf_release(&buf);
 	return len != 0;
 }