Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s

Michal Ostrowski <mostrows@watson.ibm.com> writes:

> Calls to git_setup_exec_path() are inserted on paths that will execute
> other git programs. git_setup_exec_path() will ensure that the git
> installation directories are in the path.

About fetch-clone.c (which is shared by fetch-pack and
clone-pack), it runs "git-index-pack" from finish_pack and
"git-unpack-objects" from unpack_pack, so spelling these exec
with execlp("git", "git", "index-pack", ...) might be cleaner,
since "git" is required to be in users' PATH even though git-*
may be moved out of the PATH in later versions of git.  I
dunno...

In send-pack.c, I wonder why you didn't do a setup_exec_path()
at the beginning of main() instead of having two calls close to
exec*() call site.

The same comment applies for run-command.c; you do it once for
each child, but calling it once at the beginning of receive-pack
would be good enough.  The same thing for daemon.c.

I suspect you are trying to limit the extent of damage, but I do
not think of a downside if we just call setup_exec_path() once
at the beginning of main().  $GIT_EXEC_PATH _could_ have a
private copy of broken "diff" to confuse diff-* family, but you
cannot say "git diff" in such a setup anyway because "git" does
the PATH prefixing already, so it would be a moot point.

Here is the list my "nm | grep ' exec[vlpe]*\($\|@@\)'" found
that use some variant of exec* family (except "git-diff-*"):

	clone-pack
        daemon
        fetch-pack
        merge-index
        peek-remote
        receive-pack
        send-pack
        shell
        ssh-fetch/ssh-pull
        ssh-upload/ssh-push
        upload-pack

I do not care too much about ssh-* commit walkers (users can say
e.g. GIT_SSH_PUSH themselves).

Anyway, thanks for starting this.  I need a bit more thought and
a bit of list discussion to convince myself this is a good
change.


-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html