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

[Junio C Hamano <gitster@pobox.com>]

"D. Ben Knoble" <ben.knoble+github@gmail.com> 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?