[PATCH 4/4] drop git_exec_path() from non-Git commands' PATH

["D. Ben Knoble" <ben.knoble+github@gmail.com>]

We setup_path() with git_exec_path() unconditionally (8e3462837b (Modify
setup_path() to only add git_exec_path() to PATH, 2009-01-18)) when Git
starts; as a result, all child processes see Git's exec-path when run,
including editors and other programs that don't need it [1]. This can
cause confusion for such programs or shells, especially when they rely
on finding "git" in PATH to locate other nearby directories, in a
similar vein as a0b4507ef7 (stop putting argv[0] dirname at front of
PATH, 2015-04-22) solved.

Since we only need this for finding git-* subprocesses, drop it from
child processes that aren't Git commands.

[1]: https://public-inbox.org/git/CALnO6CDtGRRav8zK2GKi1oHTZWrHFTxZNmnOWu64-ab+oY3_Lw@mail.gmail.com/

Signed-off-by: D. Ben Knoble <ben.knoble+github@gmail.com>
Suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

Notes:
    A few interesting points:
    
    - I'm not sure how best to deal with the memory leak here.
    - Dscho suggested the essence of the patch in
      https://github.com/git-for-windows/build-extra/pull/616#pullrequestreview-2839055049,
      for the curious. My only major tweaks were this diff to skip past
      "PATH=" when searching for the matching path (but still modify the
      original buffer; b always points into buf.buf, so the later operations
      with p and buf.buf are valid).
    
    --->8---
    diff --git i/run-command.c w/run-command.c
    index b567e4fdd5..8a8b5c8455 100644
    --- i/run-command.c
    +++ w/run-command.c
    @@ -452,17 +452,24 @@ static void remove_git_exec_path(struct string_list_item *path_item) {
    	struct strbuf buf = STRBUF_INIT;
    	const char *exec_path = git_exec_path();
    	size_t exec_len = strlen(exec_path);
    -	char *p;
    +	char *b, *p;
    
    	/* Value comes from environ; we should not modify it directly. But
    	 * strbuf copies data, so we now have our own playground. */
    	strbuf_addstr(&buf, (const char *)path_item->util);
    -	for (p = strstr(buf.buf, exec_path); p; p = strstr(p, exec_path)) {
    -		if ((p[exec_len] && p[exec_len] != PATH_SEP) || (p != buf.buf && p[-1] != PATH_SEP))
    +
    +	/* skip past "PATH=" to start search */
    +	p = strchr(buf.buf, '=');
    +	if (!p || !*(p + 1))
    +		return;
    +	b = p + 1;
    +
    +	for (p = strstr(b, exec_path); p; p = strstr(p, exec_path)) {
    +		if ((p[exec_len] && p[exec_len] != PATH_SEP) || (p != b && p[-1] != PATH_SEP))
    			p += exec_len; /* false positive, skip */
    		else {
    			size_t offset = p - buf.buf, delete_len = exec_len;
    -			if (p != buf.buf) {
    +			if (p != b) {
    				/* include the preceding path separator */
    				offset--;
    				delete_len++;
    --->8---
    
    - I /think/ this resolves the issues in my earlier mail beyond just Git
      builtins; for example, git-jump also doesn't get exec-path because
      it's not invoked with git_cmd set during execv_dashed_external.

 run-command.c     | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 t/t7005-editor.sh | 13 +++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/run-command.c b/run-command.c
index dee6ae3e62..8a8b5c8455 100644
--- a/run-command.c
+++ b/run-command.c
@@ -448,11 +448,51 @@ static int prepare_cmd(struct strvec *out, const struct child_process *cmd)
 	return 0;
 }
 
-static char **prep_childenv(const char *const *deltaenv)
+static void remove_git_exec_path(struct string_list_item *path_item) {
+	struct strbuf buf = STRBUF_INIT;
+	const char *exec_path = git_exec_path();
+	size_t exec_len = strlen(exec_path);
+	char *b, *p;
+
+	/* Value comes from environ; we should not modify it directly. But
+	 * strbuf copies data, so we now have our own playground. */
+	strbuf_addstr(&buf, (const char *)path_item->util);
+
+	/* skip past "PATH=" to start search */
+	p = strchr(buf.buf, '=');
+	if (!p || !*(p + 1))
+		return;
+	b = p + 1;
+
+	for (p = strstr(b, exec_path); p; p = strstr(p, exec_path)) {
+		if ((p[exec_len] && p[exec_len] != PATH_SEP) || (p != b && p[-1] != PATH_SEP))
+			p += exec_len; /* false positive, skip */
+		else {
+			size_t offset = p - buf.buf, delete_len = exec_len;
+			if (p != b) {
+				/* include the preceding path separator */
+				offset--;
+				delete_len++;
+			} else if (p[exec_len] == PATH_SEP) {
+				/* include the path separator following GIT_EXEC_PATH */
+				delete_len++;
+			}
+			strbuf_splice(&buf, offset, delete_len, "", 0);
+		}
+	}
+
+	/* Overwrite PATH value with new (owned) data. This leaks memory because
+	 * the only future owner is a char** childenv, which is freed, but whose
+	 * contents are not (because most of them come from environ). */
+	path_item->util = (void *)strbuf_detach(&buf, NULL);
+}
+
+static char **prep_childenv(const char *const *deltaenv, unsigned git_cmd)
 {
 	extern char **environ;
 	char **childenv;
 	struct string_list env = STRING_LIST_INIT_DUP;
+	struct string_list_item *path_item;
 	struct strbuf key = STRBUF_INIT;
 	const char *const *p;
 	int i;
@@ -486,6 +526,9 @@ static char **prep_childenv(const char *const *deltaenv)
 		}
 	}
 
+	if (!git_cmd && (path_item = string_list_lookup(&env, "PATH")))
+		remove_git_exec_path(path_item);
+
 	/* Create an array of 'char *' to be used as the childenv */
 	ALLOC_ARRAY(childenv, env.nr + 1);
 	for (i = 0; i < env.nr; i++)
@@ -746,7 +789,7 @@ int start_command(struct child_process *cmd)
 	if (cmd->close_object_store)
 		close_object_store(the_repository->objects);
 
-	childenv = prep_childenv(cmd->env.v);
+	childenv = prep_childenv(cmd->env.v, cmd->git_cmd);
 
 #ifndef GIT_WINDOWS_NATIVE
 {
diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index 06fa1ecd91..560e500b53 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -127,4 +127,17 @@
 	test space = "$(git show -s --pretty=format:%s)"
 '
 
+test_expect_success 'editor does not see GIT_EXEC_PATH on PATH' '
+	cat >e-path <<-EOF &&
+	#!$SHELL_PATH
+	echo "\$PATH" | tr : "\\n" >actual
+	EOF
+	chmod +x e-path &&
+	(
+		test_set_editor ./e-path &&
+		git commit --amend
+	) &&
+	test_grep ! ^"$(git --exec-path)"\$ actual
+'
+
 test_done
-- 
2.48.1