"D. Ben Knoble" <ben.knoble+github@xxxxxxxxx> writes: > 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. I can understand why we want to do this for things like editors, but doing this in start_command() would affect running of things like hook scripts (which may in turn invoke "git" commands) and command running on the other side of the pipe that does a local transport (e.g., "git fetch --upload-pack cmd"), and other commands specified via the configuration (e.g., "trailer.<key>.cmd") that may in turn invoke "git" commands, wouldn't it? > @@ -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); Regardless of the answer to the above question, I wonder if the API change for this helper function should go the other direction, by passing the entire "child_process" structure to the function. That would give prep_childenv() more information to work with to decide when futzing with PATH is appropriate. Yes, I am assuming that the answer to the above question is "true, unconditionally futzing with PATH is a bad idea" and we need this fix to be more focused, e.g., limit to "launch_specified_editor()". If that is the case, we can - add a member "drop_git_exec_path" bit to "struct child_process"; - update selected callers, e.g., launch_specified_editor(), that prepare child_process to set that bit; - update prep_childenv() to accept the entire "struct child_process", instead of cmd->env.v, and base the decision in your logic to look at the cmd->drop_git_exec_path bit; perhaps. It would give us a more surgical and focused update, hopefully?
