Re: PATH modifications for git-hook processes

On Wed, Apr 15, 2015 at 11:00:20AM -0400, Matthew Rothenberg wrote:

Yeah, this behavior leaves me somewhat confused. It is obviously dangerous (since we have no clue what else is in the directory along with git, as opposed to the exec-path, which contains only git programs), and that is biting you here. But is it actually doing any good?

Surely we don't need to find the "git" executable from that directory; it should already be in the exec-path itself. Ditto for any dashed commands (built-ins or external commands) that git ships.

The only case I could come up with (that works now, but would not work if we stopped prepending the argv0 directory to the PATH) is:

  mkdir foo
  cp $(which git) foo
  cat >foo/git-bar <<\EOF
  #!/bin/sh
  echo this external command is not in your PATH!
  EOF
  chmod +x foo/git-bar
  foo/git bar

but I am not sure that is altogether sane.

I tried to find some reasoning for this behavior in the history, and this is what I found.

We originally started treating git programs as magical in 77cb17e (Exec git programs without using PATH., 2006-01-10), but it specifically warns against this:

  Modifying PATH is not desirable as it result in behavior differing
  from the user's intentions, as we may end up prepending "/usr/bin" to
  PATH.

So it uses a special git-specific version of exec, and doesn't touch the PATH. Later, we did 231af83 (Teach the "git" command to handle some commands internally, 2006-02-26), which says at the end:

   There's one other change: the search order for external programs is
   modified slightly, so that the first entry remains GIT_EXEC_DIR, but
   the second entry is the same directory as the git wrapper itself was
   executed out of - if we can figure it out from argv[0], of course.

There was some question of whether this would be a problem[1], but we realized it is OK due to 77cb17e, mentioned above. The use case described by Linus is more or less the "not altogether sane" scenario I described above:

  IOW, you can do things like

        alias git=/opt/my-git/git

  and all the "git" commands will automatically work fine, even if you
  didn't know at compile time where you would install them, and you didn't
  set GIT_EXEC_DIR at run-time. It will still first look in GIT_EXEC_DIR,
  but if that fails, it will take the git commands from /opt/my-git/ instead
  of from /usr/bin or whatever.

I think this is pretty much a non-issue for stock git commands (we do not even put them into /opt/my-git these days, so it cannot be helping there). It's really about finding third-party commands you have added.

Later we did 511707d (use only the $PATH for exec'ing git commands, 2007-10-28) which went back to munging the PATH, and dropping the protection we relied on when doing 231af83 (and causing the breakage you're seeing today).

So I dunno. I find the current behavior quite questionable. I guess I can see it coming in handy, but as you note it can also cause problems. And "put the git-* programs somewhere in your PATH or git's exec-dir if you want to use them" does not seem like that bad a thing to require people to do.

If we can get away with just dropping this element from the PATH, I'd much rather do that than try to implement a complicated path-precedence scheme.

-Peff