* [PATCH 0/2] Remember and use GIT_EXEC_PATH on exec()'s
@ 2006-01-09 23:34 Michal Ostrowski
2006-01-09 23:35 ` [PATCH 1/2] " Michal Ostrowski
2006-01-09 23:36 ` [PATCH 2/2] " Michal Ostrowski
0 siblings, 2 replies; 33+ messages in thread
From: Michal Ostrowski @ 2006-01-09 23:34 UTC (permalink / raw)
To: git; +Cc: Michal Ostrowski
I've been trying to setup a git repository for access via ssh, on a
system where git is to be installed under ~user/bin, and ~user/bin is
not included in the PATH that sshd provides.
Consequently I need to execute something like:
git-clone -u /home/user/bin/git-upload-pack \
ssh://user@system/home/user/repo.git repo
When git-upload-pack executes on the remote system, it tries to execute
git-rev-list and fails, since /home/user/bin is not in the path.
The following patches handle this issue by appending GIT_EXEC_PATH to
PATH prior to exec calls (via git_setup_exec_path()). Also, the value
of ${bindir} at build time is encoded and used as a default value for
"GIT_EXEC_PATH", if the latter is not present.
--
Michal Ostrowski <mostrows@watson.ibm.com>
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH 1/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-09 23:34 [PATCH 0/2] Remember and use GIT_EXEC_PATH on exec()'s Michal Ostrowski @ 2006-01-09 23:35 ` Michal Ostrowski 2006-01-10 2:53 ` Junio C Hamano 2006-01-09 23:36 ` [PATCH 2/2] " Michal Ostrowski 1 sibling, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-09 23:35 UTC (permalink / raw) To: git If git-upload-pack is invoked by ssh, it may have been invoked because ssh was explicitly told which program to execute on the remote end (i.e. --exec had been used with git-clone-pack). In this case, the git suite may not be in the PATH, and so subsequent exec's by git-upload-pack (i.e. git-rev-list, git-pack-objects) will fail. These changes provide for ${bindir} to be stored at compile time in environment.c. git_setup_exec_path() is implemented; this function will append GIT_EXEC_PATH or the saved ${bindir} to PATH. Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com> --- Makefile | 7 +++++++ cache.h | 1 + environment.c | 29 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 0 deletions(-) b24fde016c2d7382016b80bc0b9a011db3413bb3 diff --git a/Makefile b/Makefile index c9c15b5..ffd2a68 100644 --- a/Makefile +++ b/Makefile @@ -437,6 +437,13 @@ init-db.o: init-db.c $(CC) -c $(ALL_CFLAGS) \ -DDEFAULT_GIT_TEMPLATE_DIR=$(call shellquote,"$(template_dir)") $*.c +# Recompile environment.o if GIT_EXEC_PATH changes +.environment.GIT_EXEC_PATH: + @(test -e $@ && grep -h -e '^$(bindir)$$' $@) || echo $(bindir) > $@ +environment.o: .environment.GIT_EXEC_PATH +environment.o: CFLAGS+= -DGIT_EXEC_PATH=\"$(bindir)\" + + $(LIB_OBJS): $(LIB_H) $(patsubst git-%$X,%.o,$(PROGRAMS)): $(LIB_H) $(DIFF_OBJS): diffcore.h diff --git a/cache.h b/cache.h index 29c9e81..d73071e 100644 --- a/cache.h +++ b/cache.h @@ -244,6 +244,7 @@ unsigned long approxidate(const char *); extern int setup_ident(void); extern const char *git_author_info(void); extern const char *git_committer_info(void); +extern void git_setup_exec_path(void); struct checkout { const char *base_dir; diff --git a/environment.c b/environment.c index 0596fc6..4dc0249 100644 --- a/environment.c +++ b/environment.c @@ -9,6 +9,10 @@ */ #include "cache.h" +#ifndef GIT_EXEC_PATH +#define GIT_EXEC_PATH NULL +#endif + char git_default_email[MAX_GITNAME]; char git_default_name[MAX_GITNAME]; int trust_executable_bit = 1; @@ -16,6 +20,7 @@ int only_use_symrefs = 0; int repository_format_version = 0; char git_commit_encoding[MAX_ENCODING_LENGTH] = "utf-8"; int shared_repository = 0; +char *git_exec_path = GIT_EXEC_PATH; static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file; @@ -76,4 +81,28 @@ char *get_graft_file(void) return git_graft_file; } +void git_setup_exec_path(void) +{ + char *path, *old_path = getenv("PATH"); + int path_len, len, old_len; + char *exec_path = getenv("GIT_EXEC_PATH"); + + if (!exec_path) + exec_path = git_exec_path; + + len = strlen(exec_path); + + if (!old_path) + old_path = "/usr/local/bin:/usr/bin:/bin"; + old_len = strlen(old_path); + path_len = len + old_len + 1; + + path = malloc(path_len + 1); + + memcpy(path, old_path, old_len); + path[old_len] = ':'; + memcpy(path + old_len + 1, exec_path, len); + + setenv("PATH", path, 1); +} -- 0.99.9m-g02ad ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 1/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-09 23:35 ` [PATCH 1/2] " Michal Ostrowski @ 2006-01-10 2:53 ` Junio C Hamano 0 siblings, 0 replies; 33+ messages in thread From: Junio C Hamano @ 2006-01-10 2:53 UTC (permalink / raw) To: Michal Ostrowski; +Cc: git Michal Ostrowski <mostrows@watson.ibm.com> writes: > If git-upload-pack is invoked by ssh, it may have been invoked because > ssh was explicitly told which program to execute on the remote end > (i.e. --exec had been used with git-clone-pack). In this case, the > git suite may not be in the PATH, and so subsequent exec's by > git-upload-pack (i.e. git-rev-list, git-pack-objects) will fail. True. > +.environment.GIT_EXEC_PATH: > + @(test -e $@ && grep -h -e '^$(bindir)$$' $@) || echo $(bindir) > $@ Hmph. * I did not know "test -e" was portable (it is in POSIX.1), but since you are creating the file yourself anyway, wouldn't "test -f" look more familiar? * Perhaps grep -F (--fixed-strings), not as regexp? * I do not get the point of using "grep -h" here (it's not in POSIX.1). Perhaps just >/dev/null? But I like the timestamp trick here that uses ||. Maybe I should borrow it for GIT-VERSION-GEN. Maybe not. > --- a/environment.c > +++ b/environment.c > @@ -9,6 +9,10 @@ > */ > #include "cache.h" > > +#ifndef GIT_EXEC_PATH > +#define GIT_EXEC_PATH NULL > +#endif I wonder if not having GIT_EXEC_PATH defined should be an error here. > +void git_setup_exec_path(void) > +{ >... > +} Maybe move git.c::prepend_to_path() to a single library file and use it here? ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-09 23:34 [PATCH 0/2] Remember and use GIT_EXEC_PATH on exec()'s Michal Ostrowski 2006-01-09 23:35 ` [PATCH 1/2] " Michal Ostrowski @ 2006-01-09 23:36 ` Michal Ostrowski 2006-01-10 2:52 ` Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-09 23:36 UTC (permalink / raw) To: git 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. Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com> --- daemon.c | 2 ++ fetch-clone.c | 2 ++ run-command.c | 1 + send-pack.c | 2 ++ upload-pack.c | 4 +++- 5 files changed, 10 insertions(+), 1 deletions(-) bb14b4b61f53f755486695e5bdc45b5623a6f8c5 diff --git a/daemon.c b/daemon.c index 3bd1426..d653f33 100644 --- a/daemon.c +++ b/daemon.c @@ -226,6 +226,8 @@ static int upload(char *dir) snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); + git_setup_exec_path(); + /* git-upload-pack only ever reads stuff, so this is safe */ execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); return -1; diff --git a/fetch-clone.c b/fetch-clone.c index f46fe6e..afbbb79 100644 --- a/fetch-clone.c +++ b/fetch-clone.c @@ -98,6 +98,8 @@ int receive_unpack_pack(int fd[2], const int status; pid_t pid; + git_setup_exec_path(); + pid = fork(); if (pid < 0) die("%s: unable to fork off git-unpack-objects", me); diff --git a/run-command.c b/run-command.c index 8bf5922..993a3f9 100644 --- a/run-command.c +++ b/run-command.c @@ -9,6 +9,7 @@ int run_command_v_opt(int argc, char **a if (pid < 0) return -ERR_RUN_COMMAND_FORK; if (!pid) { + git_setup_exec_path(); if (flags & RUN_COMMAND_NO_STDIO) { int fd = open("/dev/null", O_RDWR); dup2(fd, 0); diff --git a/send-pack.c b/send-pack.c index cd36193..a241f00 100644 --- a/send-pack.c +++ b/send-pack.c @@ -30,6 +30,7 @@ static void exec_pack_objects(void) "--stdout", NULL }; + git_setup_exec_path(); execvp("git-pack-objects", args); die("git-pack-objects exec failed (%s)", strerror(errno)); } @@ -58,6 +59,7 @@ static void exec_rev_list(struct ref *re refs = refs->next; } args[i] = NULL; + git_setup_exec_path(); execvp("git-rev-list", args); die("git-rev-list exec failed (%s)", strerror(errno)); } diff --git a/upload-pack.c b/upload-pack.c index 1834b6b..f8d4fbe 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -270,7 +270,9 @@ int main(int argc, char **argv) break; } } - + + git_setup_exec_path(); + if (i != argc-1) usage(upload_pack_usage); dir = argv[i]; -- 0.99.9m-g02ad ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-09 23:36 ` [PATCH 2/2] " Michal Ostrowski @ 2006-01-10 2:52 ` Junio C Hamano 2006-01-10 13:36 ` Michal Ostrowski 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-10 2:52 UTC (permalink / raw) To: Michal Ostrowski; +Cc: git 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. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 2:52 ` Junio C Hamano @ 2006-01-10 13:36 ` Michal Ostrowski 2006-01-10 15:01 ` Andreas Ericsson 0 siblings, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-10 13:36 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Mon, 2006-01-09 at 18:52 -0800, Junio C Hamano wrote: > 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. > I'm not actually happy with the idea of mucking around with PATH, even within git.c. Hence I tried to only change PATH if the code had already committed to an exec. An approach that I think is better is to require all exec's of git programs from within git programs to use a specific git interface, rather than letting each one set up it's own exec parameters. Once you have that implemented, we can have a separate discussion of how the executable is to be found; - should we use PATH? - should we change PATH? - should we always exec using an absolute file name? (my preference) If a user invokes /home/user/bin/git-foo, and git-foo wants to call git-bar, is it legitimate for git-foo to call /usr/local/bin/git-bar, or should it require /home/user/bin/git-bar? Should the same rules be applied to the shell scripts? (In which case we'd want to do something like s:git-:$(GIT_EXEC_PATH)/git-:g.) Anyways, it may just be easier to show this in C (patch below). -- Michal Ostrowski <mostrows@watson.ibm.com> diff --git a/Makefile b/Makefile index c9c15b5..db57858 100644 --- a/Makefile +++ b/Makefile @@ -173,7 +173,7 @@ DIFF_OBJS = \ LIB_OBJS = \ blob.o commit.o connect.o count-delta.o csum-file.o \ - date.o diff-delta.o entry.o ident.o index.o \ + date.o diff-delta.o entry.o exec_cmd.o ident.o index.o \ object.o pack-check.o patch-delta.o path.o pkt-line.o \ quote.o read-cache.o refs.o run-command.o \ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \ @@ -184,6 +184,16 @@ LIB_OBJS = \ LIBS = $(LIB_FILE) LIBS += -lz + +# .exec_cmd.bindir stores $(bindir) used to compile exec_cmd.o +# If it has changed, store the new value and force exec_cmd.o to be rebuilt +ifneq ($(shell cat .exec_cmd.bindir 2>/dev/null),$(bindir)) +.PHONY: exec_cmd.c +$(shell echo $(bindir) > .exec_cmd.bindir) +endif + +exec_cmd.o: CFLAGS+=-DGIT_EXEC_PATH=\"$(bindir)\" + # Shell quote; # Result of this needs to be placed inside '' shq = $(subst ','\'',$(1)) diff --git a/daemon.c b/daemon.c index 3bd1426..ab793bd 100644 --- a/daemon.c +++ b/daemon.c @@ -9,6 +9,7 @@ #include <syslog.h> #include "pkt-line.h" #include "cache.h" +#include "exec_cmd.h" static int log_syslog; static int verbose; @@ -227,7 +228,7 @@ static int upload(char *dir) snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); /* git-upload-pack only ever reads stuff, so this is safe */ - execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); + exec_git_cmd("upload-pack", "--strict", timeout_buf, "."); return -1; } diff --git a/fetch-clone.c b/fetch-clone.c index f46fe6e..5ae9bda 100644 --- a/fetch-clone.c +++ b/fetch-clone.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "exec_cmd.h" #include <sys/wait.h> static int finish_pack(const char *pack_tmp_name, const char *me) @@ -27,8 +28,7 @@ static int finish_pack(const char *pack_ dup2(pipe_fd[1], 1); close(pipe_fd[0]); close(pipe_fd[1]); - execlp("git-index-pack","git-index-pack", - "-o", idx, pack_tmp_name, NULL); + exec_git_cmd("index-pack", "-o", idx, pack_tmp_name); error("cannot exec git-index-pack <%s> <%s>", idx, pack_tmp_name); exit(1); @@ -105,8 +105,7 @@ int receive_unpack_pack(int fd[2], const dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-unpack-objects", "git-unpack-objects", - quiet ? "-q" : NULL, NULL); + exec_git_cmd("unpack-objects", quiet ? "-q" : NULL); die("git-unpack-objects exec failed"); } close(fd[0]); diff --git a/git.c b/git.c index 5e7da74..a45dc54 100644 --- a/git.c +++ b/git.c @@ -10,6 +10,7 @@ #include <stdarg.h> #include <sys/ioctl.h> #include "git-compat-util.h" +#include "exec_cmd.h" #ifndef PATH_MAX # define PATH_MAX 4096 @@ -192,25 +193,6 @@ static void cmd_usage(const char *exec_p exit(1); } -static void prepend_to_path(const char *dir, int len) -{ - char *path, *old_path = getenv("PATH"); - int path_len = len; - - if (!old_path) - old_path = "/usr/local/bin:/usr/bin:/bin"; - - path_len = len + strlen(old_path) + 1; - - path = malloc(path_len + 1); - - memcpy(path, dir, len); - path[len] = ':'; - memcpy(path + len + 1, old_path, path_len - len); - - setenv("PATH", path, 1); -} - static void show_man_page(char *git_cmd) { char *page; @@ -233,14 +215,10 @@ int main(int argc, char **argv, char **e { char git_command[PATH_MAX + 1]; char wd[PATH_MAX + 1]; - int i, len, show_help = 0; - char *exec_path = getenv("GIT_EXEC_PATH"); + int i, show_help = 0; getcwd(wd, PATH_MAX); - if (!exec_path) - exec_path = GIT_EXEC_PATH; - for (i = 1; i < argc; i++) { char *arg = argv[i]; @@ -257,9 +235,9 @@ int main(int argc, char **argv, char **e if (!strncmp(arg, "exec-path", 9)) { arg += 9; if (*arg == '=') - exec_path = arg + 1; + git_set_exec_path(arg + 1); else { - puts(exec_path); + puts(git_exec_path()); exit(0); } } @@ -275,48 +253,18 @@ int main(int argc, char **argv, char **e if (i >= argc || show_help) { if (i >= argc) - cmd_usage(exec_path, NULL); + cmd_usage(git_exec_path(), NULL); show_man_page(argv[i]); } - if (*exec_path != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, - "git: cannot determine current directory\n"); - exit(1); - } - len = strlen(git_command); - - /* Trivial cleanup */ - while (!strncmp(exec_path, "./", 2)) { - exec_path += 2; - while (*exec_path == '/') - exec_path++; - } - snprintf(git_command + len, sizeof(git_command) - len, - "/%s", exec_path); - } - else - strcpy(git_command, exec_path); - len = strlen(git_command); - prepend_to_path(git_command, len); - - len += snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[i]); - if (sizeof(git_command) <= len) { - fprintf(stderr, "git: command name given is too long.\n"); - exit(1); - } - - /* execve() can only ever return if it fails */ - execve(git_command, &argv[i], envp); + execv_git_cmd(argv + i); if (errno == ENOENT) - cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); + cmd_usage(git_exec_path(), "'%s' is not a git-command", + argv[i]); fprintf(stderr, "Failed to run command '%s': %s\n", git_command, strerror(errno)); - return 1; } diff --git a/receive-pack.c b/receive-pack.c index f847ec2..8e78e32 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -257,7 +257,7 @@ static void read_head_info(void) static const char *unpack(int *error_code) { - int code = run_command(unpacker, NULL); + int code = run_command_v_opt(1, &unpacker, RUN_GIT_CMD); *error_code = 0; switch (code) { diff --git a/run-command.c b/run-command.c index 8bf5922..b3d287e 100644 --- a/run-command.c +++ b/run-command.c @@ -1,6 +1,7 @@ #include "cache.h" #include "run-command.h" #include <sys/wait.h> +#include "exec_cmd.h" int run_command_v_opt(int argc, char **argv, int flags) { @@ -13,9 +14,13 @@ int run_command_v_opt(int argc, char **a int fd = open("/dev/null", O_RDWR); dup2(fd, 0); dup2(fd, 1); - close(fd); + close(fd); + } + if (flags & RUN_GIT_CMD) { + execv_git_cmd(argv); + } else { + execvp(argv[0], (char *const*) argv); } - execvp(argv[0], (char *const*) argv); die("exec %s failed.", argv[0]); } for (;;) { diff --git a/run-command.h b/run-command.h index 2469eea..ef3ee05 100644 --- a/run-command.h +++ b/run-command.h @@ -12,7 +12,7 @@ enum { }; #define RUN_COMMAND_NO_STDIO 1 - +#define RUN_GIT_CMD 2 /*If this is to be git sub-command */ int run_command_v_opt(int argc, char **argv, int opt); int run_command_v(int argc, char **argv); int run_command(const char *cmd, ...); diff --git a/shell.c b/shell.c index cd31618..0d4891f 100644 --- a/shell.c +++ b/shell.c @@ -12,7 +12,7 @@ static int do_generic_cmd(const char *me my_argv[1] = arg; my_argv[2] = NULL; - return execvp(me, (char**) my_argv); + return execv_git_cmd((char**) my_argv); } static struct commands { diff --git a/upload-pack.c b/upload-pack.c index 1834b6b..6602d68 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -4,6 +4,7 @@ #include "tag.h" #include "object.h" #include "commit.h" +#include "exec_cmd.h" static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>"; @@ -60,7 +61,7 @@ static void create_pack_file(void) close(0); close(fd[0]); close(fd[1]); - *p++ = "git-rev-list"; + *p++ = "rev-list"; *p++ = "--objects"; if (create_full_pack || MAX_NEEDS <= nr_needs) *p++ = "--all"; @@ -79,13 +80,13 @@ static void create_pack_file(void) buf += 41; } *p++ = NULL; - execvp("git-rev-list", argv); + execv_git_cmd(argv); die("git-upload-pack: unable to exec git-rev-list"); } dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-pack-objects", "git-pack-objects", "--stdout", NULL); + exec_git_cmd("pack-objects", "--stdout"); die("git-upload-pack: unable to exec git-pack-objects"); } ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 13:36 ` Michal Ostrowski @ 2006-01-10 15:01 ` Andreas Ericsson 2006-01-10 16:26 ` Michal Ostrowski 0 siblings, 1 reply; 33+ messages in thread From: Andreas Ericsson @ 2006-01-10 15:01 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Junio C Hamano, git Michal Ostrowski wrote: > On Mon, 2006-01-09 at 18:52 -0800, Junio C Hamano wrote: > >>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. >> > > > I'm not actually happy with the idea of mucking around with PATH, even > within git.c. Hence I tried to only change PATH if the code had already > committed to an exec. > This is the case in the git potty already. git.c must prepend --exec-path to $PATH, or the whole idea of being able to move scripts out of the $PATH fails (at least it fails without changing quite a few of the scripts). Since it's already in place in the potty and that's required to be in the $PATH, I think Junio's suggestion of running execlp("git", "git", ...) is a good one. It will add one extra fork() and execve() for each clone/pull/push, but that isn't much of an issue, really. > An approach that I think is better is to require all exec's of git > programs from within git programs to use a specific git interface, > rather than letting each one set up it's own exec parameters. > A better idea would be to teach {send,upload}-pack about $GIT_EXEX_PATH and export it from your shells rc-file. > Once you have that implemented, we can have a separate discussion of how > the executable is to be found; > - should we use PATH? > - should we change PATH? > - should we always exec using an absolute file name? (my preference) > > If a user invokes /home/user/bin/git-foo, and git-foo wants to call > git-bar, is it legitimate for git-foo to call /usr/local/bin/git-bar, or > should it require /home/user/bin/git-bar? > If a user invokes "/home/user/bin/git-foo" rather than "/home/user/bin/git foo" he/she will have to have the rest of the git-suite in the $PATH. Prepending whatever directory any git-* program happens to reside in to $PATH is not a good idea. Trying to execute programs residing in the same directory is an even worse. > Should the same rules be applied to the shell scripts? (In which case > we'd want to do something like s:git-:$(GIT_EXEC_PATH)/git-:g.) > All shell-scripts (that I'm aware of) are porcelainish. They should be run through the git potty and thus should always run the git-programs from the same release as they themselves were built from regardless of whether they call them through the potty or directly. This is both sane and simple. It was also one of the reasons that the 'git' program was implemented in C to begin with. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 15:01 ` Andreas Ericsson @ 2006-01-10 16:26 ` Michal Ostrowski 2006-01-10 19:13 ` Andreas Ericsson 2006-01-10 19:47 ` [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s Junio C Hamano 0 siblings, 2 replies; 33+ messages in thread From: Michal Ostrowski @ 2006-01-10 16:26 UTC (permalink / raw) To: Andreas Ericsson; +Cc: Junio C Hamano, git On Tue, 2006-01-10 at 16:01 +0100, Andreas Ericsson wrote: > > This is the case in the git potty already. git.c must prepend > --exec-path to $PATH, or the whole idea of being able to move scripts > out of the $PATH fails (at least it fails without changing quite a few > of the scripts). One could make all the scripts depend on GIT_EXEC_PATH instead of PATH. At build time one could generate wrapper functions in git-sh-setup: function git-foo () { $(GIT_EXEC_PATH)/git-foo $*; } Presuming that all scripts include git-sh-setup, no other shell script changes would be needed. > > Since it's already in place in the potty and that's required to be in > the $PATH, I think Junio's suggestion of running execlp("git", "git", > ...) is a good one. It will add one extra fork() and execve() for each > clone/pull/push, but that isn't much of an issue, really. > The patch I posted most recently does something comparable; all exec's by C git programs go through exec_git_cmd, which actually implements the "git potty" logic (and git.c itself uses exec_git_cmd). If there is to be a consistent rule for how to exec a git program from a git C program, I think that it's reasonable that there be an API to enforce it. Note that the creation and use of such a function simply means that we hide the logic that handles PATH/GIT_EXEC_PATH; how git_exec_cmd() actually calls execve() and how PATH and GIT_EXEC_PATH are used is a separate issue. When it comes to the former, I think it is best to have all exec's of git programs go through an interface that imposes the same PATH/GIT_EXEC_PATH logics. As to the latter, my only concern is that we should never do 'setenv("PATH",....)'. > > > An approach that I think is better is to require all exec's of git > > programs from within git programs to use a specific git interface, > > rather than letting each one set up it's own exec parameters. > > > > A better idea would be to teach {send,upload}-pack about $GIT_EXEX_PATH > and export it from your shells rc-file. > My shell's rc-file doesn't get invoked when using ssh as a transport; that's part of the problem. > > > Once you have that implemented, we can have a separate discussion of how > > the executable is to be found; > > - should we use PATH? > > - should we change PATH? > > - should we always exec using an absolute file name? (my preference) > > > > If a user invokes /home/user/bin/git-foo, and git-foo wants to call > > git-bar, is it legitimate for git-foo to call /usr/local/bin/git-bar, or > > should it require /home/user/bin/git-bar? > > > > If a user invokes "/home/user/bin/git-foo" rather than > "/home/user/bin/git foo" he/she will have to have the rest of the > git-suite in the $PATH. Prepending whatever directory any git-* program > happens to reside in to $PATH is not a good idea. Isn't this exactly what git.c is doing currently via prepend_to_path()? git programs exec other git programs, but they also exec non-git programs. I think it is not appropriate to change PATH (via prepend_to_path) because this may result in unexpected behavior when exec'ing non-git programs: Suppose git is installed in /usr/bin, where a "diff" resided. I've got my own version of "diff" in /home/user/bin. PATH=/home/user/bin:/usr/bin. If git now tries to execute "diff", after having run prepend_to_path(), /usr/bin/diff gets executed, not /home/user/bin/diff. The user has set up PATH to ensure that /home/user/bin/diff is the diff, but by mucking with PATH we subvert their intentions. This is why in my original patch I tried to put the manipulations to PATH only in points where I knew that it would only affect the exec'ing of a git program. > > > > Should the same rules be applied to the shell scripts? (In which case > > we'd want to do something like s:git-:$(GIT_EXEC_PATH)/git-:g.) > > > > All shell-scripts (that I'm aware of) are porcelainish. They should be > run through the git potty and thus should always run the git-programs > from the same release as they themselves were built from regardless of > whether they call them through the potty or directly. This is both sane > and simple. It was also one of the reasons that the 'git' program was > implemented in C to begin with. > As described above, we can have shells scripts "always run the git-programs from the same release as they themselves were built from" without ever changing PATH. -- Michal Ostrowski <mostrows@watson.ibm.com> ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 16:26 ` Michal Ostrowski @ 2006-01-10 19:13 ` Andreas Ericsson 2006-01-10 20:15 ` Alex Riesen [not found] ` <7vu0cb6f1n.fsf@assigned-by-dhcp.cox.net> 2006-01-10 19:47 ` [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s Junio C Hamano 1 sibling, 2 replies; 33+ messages in thread From: Andreas Ericsson @ 2006-01-10 19:13 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Junio C Hamano, git Michal Ostrowski wrote: > On Tue, 2006-01-10 at 16:01 +0100, Andreas Ericsson wrote: > > >>This is the case in the git potty already. git.c must prepend >>--exec-path to $PATH, or the whole idea of being able to move scripts >>out of the $PATH fails (at least it fails without changing quite a few >>of the scripts). > > > One could make all the scripts depend on GIT_EXEC_PATH instead of PATH. > At build time one could generate wrapper functions in git-sh-setup: > > function git-foo () { > $(GIT_EXEC_PATH)/git-foo $*; > } > > Presuming that all scripts include git-sh-setup, no other shell script > changes would be needed. > Yuck, for two reasons. * Not all scripts include git-sh-setup, and for good reasons. If this is what you intend please make sure you don't break anything in the process. * This will spawn a sub-shell for each git-foo process called. Shells are way more expensive than the git potty, so the performance hit in iterations might be considerable. Think StGit and Cogito as well. On a side-note, $* will break quoting (you should use "$@" instead, with double-quotes attached), and $(GIT_EXEC_PATH)/git-foo $* will try to execute GIT_EXEC_PATH and prepend its output to the rest of the command, which is quite obviously wrong. > >>Since it's already in place in the potty and that's required to be in >>the $PATH, I think Junio's suggestion of running execlp("git", "git", >>...) is a good one. It will add one extra fork() and execve() for each >>clone/pull/push, but that isn't much of an issue, really. >> > > > The patch I posted most recently does something comparable; all exec's > by C git programs go through exec_git_cmd, which actually implements the > "git potty" logic (and git.c itself uses exec_git_cmd). If there is to > be a consistent rule for how to exec a git program from a git C program, > I think that it's reasonable that there be an API to enforce it. > True. Perhaps I misread your patch or your reasoning. > Note that the creation and use of such a function simply means that we > hide the logic that handles PATH/GIT_EXEC_PATH; how git_exec_cmd() > actually calls execve() and how PATH and GIT_EXEC_PATH are used is a > separate issue. When it comes to the former, I think it is best to have > all exec's of git programs go through an interface that imposes the same > PATH/GIT_EXEC_PATH logics. As to the latter, my only concern is that we > should never do 'setenv("PATH",....)'. > setenv("PATH", ..) is way preferrable over the git-setup.sh hackery suggested above, so long as it's only ever done in the git potty. That's what the potty is there for, after all. > >>>An approach that I think is better is to require all exec's of git >>>programs from within git programs to use a specific git interface, >>>rather than letting each one set up it's own exec parameters. >>> >> >>A better idea would be to teach {send,upload}-pack about $GIT_EXEX_PATH >>and export it from your shells rc-file. >> > > > My shell's rc-file doesn't get invoked when using ssh as a transport; > that's part of the problem. > It does for me and everybody else. $HOME/.bashrc is read even for non-interactive shells. $HOME/.bash_profile isn't. If you're using git-shell you're in to a whole different situation, but you didn't say so so I don't think you are. >> >>If a user invokes "/home/user/bin/git-foo" rather than >>"/home/user/bin/git foo" he/she will have to have the rest of the >>git-suite in the $PATH. Prepending whatever directory any git-* program >>happens to reside in to $PATH is not a good idea. > > > Isn't this exactly what git.c is doing currently via prepend_to_path()? > No. git prepends whatever was passed in --exec-path=/some/path, what's found in $GIT_EXEC_PATH or the GIT_EXEC_PATH pre-processor macro set at compile-time, in that order of preference. There's a very big difference. > git programs exec other git programs, but they also exec non-git > programs. I think it is not appropriate to change PATH (via > prepend_to_path) because this may result in unexpected behavior when > exec'ing non-git programs: > > Suppose git is installed in /usr/bin, where a "diff" resided. > I've got my own version of "diff" in /home/user/bin. > PATH=/home/user/bin:/usr/bin. > > If git now tries to execute "diff", after having run > prepend_to_path(), /usr/bin/diff gets executed, not /home/user/bin/diff. > The user has set up PATH to ensure that /home/user/bin/diff is the diff, > but by mucking with PATH we subvert their intentions. > Good point. Perhaps we should only prepend to path when the directory isn't already in $PATH, or append rather than prepend. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 19:13 ` Andreas Ericsson @ 2006-01-10 20:15 ` Alex Riesen 2006-01-10 20:32 ` Michal Ostrowski [not found] ` <7vu0cb6f1n.fsf@assigned-by-dhcp.cox.net> 1 sibling, 1 reply; 33+ messages in thread From: Alex Riesen @ 2006-01-10 20:15 UTC (permalink / raw) To: Andreas Ericsson; +Cc: Michal Ostrowski, Junio C Hamano, git Andreas Ericsson, Tue, Jan 10, 2006 20:13:34 +0100: > >My shell's rc-file doesn't get invoked when using ssh as a transport; > >that's part of the problem. > > It does for me and everybody else. $HOME/.bashrc is read even for > non-interactive shells. ... Not really: $ man bash When bash is started non-interactively, to run a shell script, for example, it looks for the variable BASH_ENV in the environment, expands its value if it appears there, and uses the expanded value as the name of a file to read and execute. Bash behaves as if the following com- mand were executed: if [ -n "$BASH_ENV" ]; then . "$BASH_ENV"; fi but the value of the PATH variable is not used to search for the file name. $ ssh host2 strace -e open bash -c : open("/etc/ld.so.cache", O_RDONLY) = 3 open("/lib/libncurses.so.5", O_RDONLY) = 3 open("/lib/tls/libdl.so.2", O_RDONLY) = 3 open("/lib/tls/libc.so.6", O_RDONLY) = 3 open("/dev/tty", O_RDWR|O_NONBLOCK|O_LARGEFILE) = -1 ENXIO (No such device or address) open("/etc/mtab", O_RDONLY) = 3 open("/proc/meminfo", O_RDONLY) = 3 open("/proc/sys/kernel/ngroups_max", O_RDONLY) = 3 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 20:15 ` Alex Riesen @ 2006-01-10 20:32 ` Michal Ostrowski 0 siblings, 0 replies; 33+ messages in thread From: Michal Ostrowski @ 2006-01-10 20:32 UTC (permalink / raw) To: Alex Riesen; +Cc: Andreas Ericsson, Junio C Hamano, git On Tue, 2006-01-10 at 21:15 +0100, Alex Riesen wrote: > Andreas Ericsson, Tue, Jan 10, 2006 20:13:34 +0100: > > >My shell's rc-file doesn't get invoked when using ssh as a transport; > > >that's part of the problem. > > > > It does for me and everybody else. $HOME/.bashrc is read even for > > non-interactive shells. ... On the system I'm dealing with, ssh does not invoke bash if a command is specified. If I do ssh user@system /home/user/bin/foo, ssh performs an exec of /home/user/bin/foo; my shell is never invoked. This isn't up to me; I don't have root on this system. -- Michal Ostrowski <mostrows@watson.ibm.com> ^ permalink raw reply [flat|nested] 33+ messages in thread
[parent not found: <7vu0cb6f1n.fsf@assigned-by-dhcp.cox.net>]
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s [not found] ` <7vu0cb6f1n.fsf@assigned-by-dhcp.cox.net> @ 2006-01-10 20:29 ` Michal Ostrowski 2006-01-11 0:06 ` Andreas Ericsson 2006-01-11 0:42 ` Junio C Hamano 0 siblings, 2 replies; 33+ messages in thread From: Michal Ostrowski @ 2006-01-10 20:29 UTC (permalink / raw) To: Junio C Hamano; +Cc: Andreas Ericsson, git On Tue, 2006-01-10 at 11:47 -0800, Junio C Hamano wrote: > > No. git prepends whatever was passed in --exec-path=/some/path, what's > > found in $GIT_EXEC_PATH or the GIT_EXEC_PATH pre-processor macro set > > at compile-time, in that order of preference. There's a very big > > difference. > > True. > > > Good point. Perhaps we should only prepend to path when the directory > > isn't already in $PATH, or append rather than prepend. > > I think appending not prepending would stop letting me say > > $ GIT_EXEC_PATH=/usr/libexec/git-core/0.99.9k git foo > > to try out older version, if I have more recent git in my PATH. > But I agree with Michal it is not nice to affect invocations of > "diff" (and things spawned from hooks, which would inherit PATH > from receive-pack). > > How about searching for executables in the following places, and in this order: 1. --exec-path setting, if any 2. GIT_EXEC_PATH env var, if set 3. PATH (never modified) 4. Value of ${bindir} at build time To accomplish this I'd suggest reworking the logic the git c programs use to exec another git program. The patch I sent out this morning attempts to do this. (I'll append again to avoid confusion with previous patches.) But note that the patch below doesn't implement this policy above; the point is that it provides for a single location where it has to be implemented (in exec_cmd.c). I'd be happy to clean it up and submit it formally as a clean-up patch (one that doesn't change the PATH/GIT_EXEC_PATH search behavior currently implemented). Secondly, the shell scripts as is cannot utilize this search order as long as they don't religiously use the git potty internally. If we were to "sed -e 's/git-/git /g' -i git*.sh" (grotesquely simplified of course) then they would. -- Michal Ostrowski <mostrows@watson.ibm.com> diff --git a/Makefile b/Makefile index c9c15b5..db57858 100644 --- a/Makefile +++ b/Makefile @@ -173,7 +173,7 @@ DIFF_OBJS = \ LIB_OBJS = \ blob.o commit.o connect.o count-delta.o csum-file.o \ - date.o diff-delta.o entry.o ident.o index.o \ + date.o diff-delta.o entry.o exec_cmd.o ident.o index.o \ object.o pack-check.o patch-delta.o path.o pkt-line.o \ quote.o read-cache.o refs.o run-command.o \ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \ @@ -184,6 +184,16 @@ LIB_OBJS = \ LIBS = $(LIB_FILE) LIBS += -lz + +# .exec_cmd.bindir stores $(bindir) used to compile exec_cmd.o +# If it has changed, store the new value and force exec_cmd.o to be rebuilt +ifneq ($(shell cat .exec_cmd.bindir 2>/dev/null),$(bindir)) +.PHONY: exec_cmd.c +$(shell echo $(bindir) > .exec_cmd.bindir) +endif + +exec_cmd.o: CFLAGS+=-DGIT_EXEC_PATH=\"$(bindir)\" + # Shell quote; # Result of this needs to be placed inside '' shq = $(subst ','\'',$(1)) diff --git a/daemon.c b/daemon.c index 3bd1426..ab793bd 100644 --- a/daemon.c +++ b/daemon.c @@ -9,6 +9,7 @@ #include <syslog.h> #include "pkt-line.h" #include "cache.h" +#include "exec_cmd.h" static int log_syslog; static int verbose; @@ -227,7 +228,7 @@ static int upload(char *dir) snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); /* git-upload-pack only ever reads stuff, so this is safe */ - execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); + exec_git_cmd("upload-pack", "--strict", timeout_buf, "."); return -1; } diff --git a/fetch-clone.c b/fetch-clone.c index f46fe6e..5ae9bda 100644 --- a/fetch-clone.c +++ b/fetch-clone.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "exec_cmd.h" #include <sys/wait.h> static int finish_pack(const char *pack_tmp_name, const char *me) @@ -27,8 +28,7 @@ static int finish_pack(const char *pack_ dup2(pipe_fd[1], 1); close(pipe_fd[0]); close(pipe_fd[1]); - execlp("git-index-pack","git-index-pack", - "-o", idx, pack_tmp_name, NULL); + exec_git_cmd("index-pack", "-o", idx, pack_tmp_name); error("cannot exec git-index-pack <%s> <%s>", idx, pack_tmp_name); exit(1); @@ -105,8 +105,7 @@ int receive_unpack_pack(int fd[2], const dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-unpack-objects", "git-unpack-objects", - quiet ? "-q" : NULL, NULL); + exec_git_cmd("unpack-objects", quiet ? "-q" : NULL); die("git-unpack-objects exec failed"); } close(fd[0]); diff --git a/git.c b/git.c index 5e7da74..a45dc54 100644 --- a/git.c +++ b/git.c @@ -10,6 +10,7 @@ #include <stdarg.h> #include <sys/ioctl.h> #include "git-compat-util.h" +#include "exec_cmd.h" #ifndef PATH_MAX # define PATH_MAX 4096 @@ -192,25 +193,6 @@ static void cmd_usage(const char *exec_p exit(1); } -static void prepend_to_path(const char *dir, int len) -{ - char *path, *old_path = getenv("PATH"); - int path_len = len; - - if (!old_path) - old_path = "/usr/local/bin:/usr/bin:/bin"; - - path_len = len + strlen(old_path) + 1; - - path = malloc(path_len + 1); - - memcpy(path, dir, len); - path[len] = ':'; - memcpy(path + len + 1, old_path, path_len - len); - - setenv("PATH", path, 1); -} - static void show_man_page(char *git_cmd) { char *page; @@ -233,14 +215,10 @@ int main(int argc, char **argv, char **e { char git_command[PATH_MAX + 1]; char wd[PATH_MAX + 1]; - int i, len, show_help = 0; - char *exec_path = getenv("GIT_EXEC_PATH"); + int i, show_help = 0; getcwd(wd, PATH_MAX); - if (!exec_path) - exec_path = GIT_EXEC_PATH; - for (i = 1; i < argc; i++) { char *arg = argv[i]; @@ -257,9 +235,9 @@ int main(int argc, char **argv, char **e if (!strncmp(arg, "exec-path", 9)) { arg += 9; if (*arg == '=') - exec_path = arg + 1; + git_set_exec_path(arg + 1); else { - puts(exec_path); + puts(git_exec_path()); exit(0); } } @@ -275,48 +253,18 @@ int main(int argc, char **argv, char **e if (i >= argc || show_help) { if (i >= argc) - cmd_usage(exec_path, NULL); + cmd_usage(git_exec_path(), NULL); show_man_page(argv[i]); } - if (*exec_path != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, - "git: cannot determine current directory\n"); - exit(1); - } - len = strlen(git_command); - - /* Trivial cleanup */ - while (!strncmp(exec_path, "./", 2)) { - exec_path += 2; - while (*exec_path == '/') - exec_path++; - } - snprintf(git_command + len, sizeof(git_command) - len, - "/%s", exec_path); - } - else - strcpy(git_command, exec_path); - len = strlen(git_command); - prepend_to_path(git_command, len); - - len += snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[i]); - if (sizeof(git_command) <= len) { - fprintf(stderr, "git: command name given is too long.\n"); - exit(1); - } - - /* execve() can only ever return if it fails */ - execve(git_command, &argv[i], envp); + execv_git_cmd(argv + i); if (errno == ENOENT) - cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); + cmd_usage(git_exec_path(), "'%s' is not a git-command", + argv[i]); fprintf(stderr, "Failed to run command '%s': %s\n", git_command, strerror(errno)); - return 1; } diff --git a/receive-pack.c b/receive-pack.c index f847ec2..8e78e32 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -257,7 +257,7 @@ static void read_head_info(void) static const char *unpack(int *error_code) { - int code = run_command(unpacker, NULL); + int code = run_command_v_opt(1, &unpacker, RUN_GIT_CMD); *error_code = 0; switch (code) { diff --git a/run-command.c b/run-command.c index 8bf5922..b3d287e 100644 --- a/run-command.c +++ b/run-command.c @@ -1,6 +1,7 @@ #include "cache.h" #include "run-command.h" #include <sys/wait.h> +#include "exec_cmd.h" int run_command_v_opt(int argc, char **argv, int flags) { @@ -13,9 +14,13 @@ int run_command_v_opt(int argc, char **a int fd = open("/dev/null", O_RDWR); dup2(fd, 0); dup2(fd, 1); - close(fd); + close(fd); + } + if (flags & RUN_GIT_CMD) { + execv_git_cmd(argv); + } else { + execvp(argv[0], (char *const*) argv); } - execvp(argv[0], (char *const*) argv); die("exec %s failed.", argv[0]); } for (;;) { diff --git a/run-command.h b/run-command.h index 2469eea..ef3ee05 100644 --- a/run-command.h +++ b/run-command.h @@ -12,7 +12,7 @@ enum { }; #define RUN_COMMAND_NO_STDIO 1 - +#define RUN_GIT_CMD 2 /*If this is to be git sub-command */ int run_command_v_opt(int argc, char **argv, int opt); int run_command_v(int argc, char **argv); int run_command(const char *cmd, ...); diff --git a/shell.c b/shell.c index cd31618..0d4891f 100644 --- a/shell.c +++ b/shell.c @@ -12,7 +12,7 @@ static int do_generic_cmd(const char *me my_argv[1] = arg; my_argv[2] = NULL; - return execvp(me, (char**) my_argv); + return execv_git_cmd((char**) my_argv); } static struct commands { diff --git a/upload-pack.c b/upload-pack.c index 1834b6b..6602d68 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -4,6 +4,7 @@ #include "tag.h" #include "object.h" #include "commit.h" +#include "exec_cmd.h" static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>"; @@ -60,7 +61,7 @@ static void create_pack_file(void) close(0); close(fd[0]); close(fd[1]); - *p++ = "git-rev-list"; + *p++ = "rev-list"; *p++ = "--objects"; if (create_full_pack || MAX_NEEDS <= nr_needs) *p++ = "--all"; @@ -79,13 +80,13 @@ static void create_pack_file(void) buf += 41; } *p++ = NULL; - execvp("git-rev-list", argv); + execv_git_cmd(argv); die("git-upload-pack: unable to exec git-rev-list"); } dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-pack-objects", "git-pack-objects", "--stdout", NULL); + exec_git_cmd("pack-objects", "--stdout"); die("git-upload-pack: unable to exec git-pack-objects"); } ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 20:29 ` Michal Ostrowski @ 2006-01-11 0:06 ` Andreas Ericsson 2006-01-11 0:42 ` Junio C Hamano 1 sibling, 0 replies; 33+ messages in thread From: Andreas Ericsson @ 2006-01-11 0:06 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Junio C Hamano, git Michal Ostrowski wrote: > On Tue, 2006-01-10 at 11:47 -0800, Junio C Hamano wrote: > >> >>>Good point. Perhaps we should only prepend to path when the directory >>>isn't already in $PATH, or append rather than prepend. >> >>I think appending not prepending would stop letting me say >> >> $ GIT_EXEC_PATH=/usr/libexec/git-core/0.99.9k git foo >> >>to try out older version, if I have more recent git in my PATH. >>But I agree with Michal it is not nice to affect invocations of >>"diff" (and things spawned from hooks, which would inherit PATH >>from receive-pack). >> So how about prepending only when the directory isn't already in the PATH? That can be done with a two-line patch to git.c only. It will break the "diff in another dir" scenario in the highly unlikely event that the other diff is located in the same dir as the git suite, isn't supposed to be used, and the directory in question isn't in $PATH already. People who have such a setup will be too ashamed to admit it, so we're not likely to be blamed for it either. ;) >> > > > How about searching for executables in the following places, and in this > order: > > 1. --exec-path setting, if any > 2. GIT_EXEC_PATH env var, if set > 3. PATH (never modified) > 4. Value of ${bindir} at build time > This is more or less what's done today, with the exception that $PATH isn't searched and it throws an error immediately no matter where exec_path (the git.c variable) came from. Adding $PATH to the search-pattern would be a simple matter of falling back to execvp() if execve(), but then we could end up with running programs from a different release while the user thinks he/she's specifically running 1.0.3... Tricky problem, really. > > Secondly, the shell scripts as is cannot utilize this search order as > long as they don't religiously use the git potty internally. If we were > to "sed -e 's/git-/git /g' -i git*.sh" (grotesquely simplified of > course) then they would. > I think this has been done, but as it happened to be convenient. I'd prefer if the git potty could keep prepending the GIT_EXEC_PATH to the path, really. We're bound to run into setup-related problems otherwise, such as; Alice writes a script that works fine for her and her friends, so she shares it freely with Bob and whoever else might be listening. Bob's git-tools aren't in the $PATH but he keeps the potty handy at all times. He can't always run it through the potty because in some code-paths Alice's script uses git-tools without going through the potty. Bob thinks Alice puts entropy in her programs on purpose, so Alice flees, sobbing and shaking in anger and betrayed trust. The two never speak again. Luckily, Bruce Schneier shows up and saves the day. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 20:29 ` Michal Ostrowski 2006-01-11 0:06 ` Andreas Ericsson @ 2006-01-11 0:42 ` Junio C Hamano 2006-01-11 2:09 ` Michal Ostrowski 2006-01-11 2:12 ` [PATCH] Exec git programs without using PATH Michal Ostrowski 1 sibling, 2 replies; 33+ messages in thread From: Junio C Hamano @ 2006-01-11 0:42 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Andreas Ericsson, git Michal Ostrowski <mostrows@watson.ibm.com> writes: > How about searching for executables in the following places, and in this > order: > > 1. --exec-path setting, if any > 2. GIT_EXEC_PATH env var, if set > 3. PATH (never modified) > 4. Value of ${bindir} at build time My preference is to first do this to the Makefile: -- >8 -- diff --git a/Makefile b/Makefile index 5817e86..b1e3055 100644 --- a/Makefile +++ b/Makefile @@ -71,6 +71,7 @@ ALL_LDFLAGS = $(LDFLAGS) prefix = $(HOME) bindir = $(prefix)/bin +gitexecdir = $(prefix)/bin template_dir = $(prefix)/share/git-core/templates/ GIT_PYTHON_DIR = $(prefix)/share/git-core/python # DESTDIR= @@ -144,7 +145,7 @@ PROGRAMS = \ git-describe$X # what 'all' will build and 'install' will install. -ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) git$X +ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) # Backward compatibility -- to be removed after 1.0 PROGRAMS += git-ssh-pull$X git-ssh-push$X @@ -368,13 +369,13 @@ LIB_OBJS += $(COMPAT_OBJS) export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir ### Build rules -all: $(ALL_PROGRAMS) +all: $(ALL_PROGRAMS) git$X all: $(MAKE) -C templates git$X: git.c $(LIB_FILE) - $(CC) -DGIT_EXEC_PATH='"$(bindir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \ + $(CC) -DGIT_EXEC_PATH='"$(gitexecdir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \ $(CFLAGS) $(COMPAT_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh @@ -470,7 +471,9 @@ check: install: all $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir)) - $(INSTALL) $(ALL_PROGRAMS) $(call shellquote,$(DESTDIR)$(bindir)) + $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(gitexecdir)) + $(INSTALL) $(ALL_PROGRAMS) $(call shellquote,$(DESTDIR)$(gitexecdir)) + $(INSTALL) git$X $(call shellquote,$(DESTDIR)$(bindir)) $(MAKE) -C templates install $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) $(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) -- 8< -- and then make the rule for git things: 1. --exec-path 2. GIT_EXEC_PATH environment 3. $(gitexecdir) in this order. Non git things should just use $PATH without looking at anything else --- as long as a hook script calls a git wrapper (i.e. "git foo" not "git-foo") I think things should work fine. > ... The patch I sent out this morning > attempts to do this. (I'll append again to avoid confusion with > previous patches)... > + if (flags & RUN_GIT_CMD) { > + execv_git_cmd(argv); > + } else { > + execvp(argv[0], (char *const*) argv); > } This bit sounds good, but if you were to go this route I'd suggest to rename your exec_git_cmd() to execl_git_cmd() (and terminate the vararg list with NULL) for naming consistency. execv_git_cmd() is good---we do not *want* execvp_git_cmd(), because we do not run git subcommand using PATH. ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-11 0:42 ` Junio C Hamano @ 2006-01-11 2:09 ` Michal Ostrowski 2006-01-11 2:12 ` [PATCH] Exec git programs without using PATH Michal Ostrowski 1 sibling, 0 replies; 33+ messages in thread From: Michal Ostrowski @ 2006-01-11 2:09 UTC (permalink / raw) To: Junio C Hamano; +Cc: Andreas Ericsson, git On Tue, 2006-01-10 at 16:42 -0800, Junio C Hamano wrote: > Michal Ostrowski <mostrows@watson.ibm.com> writes: > > > How about searching for executables in the following places, and in this > > order: > > > > 1. --exec-path setting, if any > > 2. GIT_EXEC_PATH env var, if set > > 3. PATH (never modified) > > 4. Value of ${bindir} at build time > > and then make the rule for git things: > > 1. --exec-path > 2. GIT_EXEC_PATH environment > 3. $(gitexecdir) > > in this order. Non git things should just use $PATH without > looking at anything else --- as long as a hook script calls a git > wrapper (i.e. "git foo" not "git-foo") I think things should > work fine. > The patch that follows implements your suggested search order and includes suggested Makefile changes. -- Michal Ostrowski <mostrows@watson.ibm.com> ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] Exec git programs without using PATH. 2006-01-11 0:42 ` Junio C Hamano 2006-01-11 2:09 ` Michal Ostrowski @ 2006-01-11 2:12 ` Michal Ostrowski 2006-01-11 6:13 ` Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-11 2:12 UTC (permalink / raw) To: Junio C Hamano; +Cc: Andreas Ericsson, git The git suite may not be in PATH (and thus programs such as git-send-pack could not exec git-rev-list). Thus there is a need for logic that will locate these programs. 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. - git C programs will use exec*_git_cmd() APIs to exec sub-commands. - exec*_git_cmd() will execute a git program by searching for it in the following directories: 1. --exec-path (as used by "git") 2. The GIT_EXEC_PATH environment variable. 3. $(gitexecdir) as set in Makefile (default value $(bindir)). - git potty will modify PATH as before to enable shell scripts to invoke "git-foo" commands. Ideally, shell scripts should use the git potty to become independent of PATH, and then modifying PATH will not be necessary. Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com> --- Makefile | 21 ++++++++-- daemon.c | 3 + exec_cmd.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ exec_cmd.h | 10 +++++ fetch-clone.c | 7 +-- git.c | 54 ++++++-------------------- receive-pack.c | 2 - run-command.c | 9 +++- run-command.h | 2 - send-pack.c | 8 ++-- shell.c | 2 - upload-pack.c | 7 ++- 12 files changed, 181 insertions(+), 62 deletions(-) create mode 100644 exec_cmd.c create mode 100644 exec_cmd.h d6dbbb5b9b47c37f44c3494962b9fa534677729f diff --git a/Makefile b/Makefile index c9c15b5..912c223 100644 --- a/Makefile +++ b/Makefile @@ -68,6 +68,7 @@ ALL_LDFLAGS = $(LDFLAGS) prefix = $(HOME) bindir = $(prefix)/bin +gitexecdir = $(prefix)/bin template_dir = $(prefix)/share/git-core/templates/ GIT_PYTHON_DIR = $(prefix)/share/git-core/python # DESTDIR= @@ -141,7 +142,7 @@ PROGRAMS = \ git-describe$X # what 'all' will build and 'install' will install. -ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) git$X +ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) # Backward compatibility -- to be removed after 1.0 PROGRAMS += git-ssh-pull$X git-ssh-push$X @@ -173,7 +174,7 @@ DIFF_OBJS = \ LIB_OBJS = \ blob.o commit.o connect.o count-delta.o csum-file.o \ - date.o diff-delta.o entry.o ident.o index.o \ + date.o diff-delta.o entry.o exec_cmd.o ident.o index.o \ object.o pack-check.o patch-delta.o path.o pkt-line.o \ quote.o read-cache.o refs.o run-command.o \ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \ @@ -184,6 +185,16 @@ LIB_OBJS = \ LIBS = $(LIB_FILE) LIBS += -lz + +# .exec_cmd.gitexecdir stores $(gitexecir) used to compile exec_cmd.o +# If it has changed, store the new value and force exec_cmd.o to be rebuilt +ifneq ($(shell cat .exec_cmd.gitexecdir 2>/dev/null),$(gitexecdir)) +.PHONY: exec_cmd.c +$(shell echo $(gitexecdir) > .exec_cmd.gitexecdir) +endif + +exec_cmd.o: CFLAGS+=-DGIT_EXEC_PATH=\"$(gitexecdir)\" + # Shell quote; # Result of this needs to be placed inside '' shq = $(subst ','\'',$(1)) @@ -366,13 +377,13 @@ LIB_OBJS += $(COMPAT_OBJS) export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir ### Build rules -all: $(ALL_PROGRAMS) +all: $(ALL_PROGRAMS) git$X all: $(MAKE) -C templates git$X: git.c $(LIB_FILE) - $(CC) -DGIT_EXEC_PATH='"$(bindir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \ + $(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \ $(CFLAGS) $(COMPAT_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh @@ -468,7 +479,9 @@ check: install: all $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir)) + $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(gitexecdir)) $(INSTALL) $(ALL_PROGRAMS) $(call shellquote,$(DESTDIR)$(bindir)) + $(INSTALL) git$X $(call shellquote,$(DESTDIR)$(gitexecdir)) $(MAKE) -C templates install $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) $(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) diff --git a/daemon.c b/daemon.c index 3bd1426..bb014fa 100644 --- a/daemon.c +++ b/daemon.c @@ -9,6 +9,7 @@ #include <syslog.h> #include "pkt-line.h" #include "cache.h" +#include "exec_cmd.h" static int log_syslog; static int verbose; @@ -227,7 +228,7 @@ static int upload(char *dir) snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); /* git-upload-pack only ever reads stuff, so this is safe */ - execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); + execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL); return -1; } diff --git a/exec_cmd.c b/exec_cmd.c new file mode 100644 index 0000000..a3bd40a --- /dev/null +++ b/exec_cmd.c @@ -0,0 +1,118 @@ +#include "cache.h" +#include "exec_cmd.h" +#define MAX_ARGS 32 + +extern char **environ; +static const char *builtin_exec_path = GIT_EXEC_PATH; +static const char *current_exec_path = NULL; + +void git_set_exec_path(const char *exec_path) +{ + current_exec_path = exec_path; +} + + +/* Returns the highest-priority, location to look for git programs. */ +const char *git_exec_path() +{ + const char *env; + + if (current_exec_path) + return current_exec_path; + + env = getenv("GIT_EXEC_PATH"); + if (env) { + return env; + } + + return builtin_exec_path; +} + + +int execv_git_cmd(char **argv) +{ + char git_command[PATH_MAX + 1]; + char *tmp; + int len, err, i; + const char *paths[] = { current_exec_path, + getenv("GIT_EXEC_PATH"), + builtin_exec_path, + NULL }; + + for (i = 0; i < 4; ++i) { + const char *exec_dir = paths[i]; + if (!exec_dir) continue; + + if (*exec_dir != '/') { + if (!getcwd(git_command, sizeof(git_command))) { + fprintf(stderr, "git: cannot determine " + "current directory\n"); + exit(1); + } + len = strlen(git_command); + + /* Trivial cleanup */ + while (!strncmp(exec_dir, "./", 2)) { + exec_dir += 2; + while (*exec_dir == '/') + exec_dir++; + } + snprintf(git_command + len, sizeof(git_command) - len, + "/%s", exec_dir); + } else { + strcpy(git_command, exec_dir); + } + + len = strlen(git_command); + len += snprintf(git_command + len, sizeof(git_command) - len, + "/git-%s", argv[0]); + + if (sizeof(git_command) <= len) { + fprintf(stderr, + "git: command name given is too long.\n"); + break; + } + + /* argv[0] must be the git command, but the argv array + * belongs to the caller, and my be reused in + * subsequent loop iterations. Save argv[0] and + * restore it on error. + */ + + tmp = argv[0]; + argv[0] = git_command; + + /* execve() can only ever return if it fails */ + execve(git_command, argv, environ); + + err = errno; + + argv[0] = tmp; + } + return -1; + +} + + +int execl_git_cmd(char *cmd,...) +{ + int argc; + char *argv[MAX_ARGS + 1]; + char *arg; + va_list param; + + va_start(param, cmd); + argv[0] = cmd; + argc = 1; + while (argc < MAX_ARGS) { + arg = argv[argc++] = va_arg(param, char *); + if (!arg) + break; + } + va_end(param); + if (MAX_ARGS <= argc) + return error("too many args to run %s", cmd); + + argv[argc] = NULL; + return execv_git_cmd(argv); +} diff --git a/exec_cmd.h b/exec_cmd.h new file mode 100644 index 0000000..06d5ec3 --- /dev/null +++ b/exec_cmd.h @@ -0,0 +1,10 @@ +#ifndef __GIT_EXEC_CMD_H_ +#define __GIT_EXEC_CMD_H_ + +extern void git_set_exec_path(const char *exec_path); +extern const char* git_exec_path(); +extern int execv_git_cmd(char **argv); /* NULL terminated */ +extern int execl_git_cmd(char *cmd, ...); + + +#endif /* __GIT_EXEC_CMD_H_ */ diff --git a/fetch-clone.c b/fetch-clone.c index f46fe6e..859f400 100644 --- a/fetch-clone.c +++ b/fetch-clone.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "exec_cmd.h" #include <sys/wait.h> static int finish_pack(const char *pack_tmp_name, const char *me) @@ -27,8 +28,7 @@ static int finish_pack(const char *pack_ dup2(pipe_fd[1], 1); close(pipe_fd[0]); close(pipe_fd[1]); - execlp("git-index-pack","git-index-pack", - "-o", idx, pack_tmp_name, NULL); + execl_git_cmd("index-pack", "-o", idx, pack_tmp_name, NULL); error("cannot exec git-index-pack <%s> <%s>", idx, pack_tmp_name); exit(1); @@ -105,8 +105,7 @@ int receive_unpack_pack(int fd[2], const dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-unpack-objects", "git-unpack-objects", - quiet ? "-q" : NULL, NULL); + execl_git_cmd("unpack-objects", quiet ? "-q" : NULL, NULL); die("git-unpack-objects exec failed"); } close(fd[0]); diff --git a/git.c b/git.c index 5e7da74..fdd02ed 100644 --- a/git.c +++ b/git.c @@ -10,6 +10,7 @@ #include <stdarg.h> #include <sys/ioctl.h> #include "git-compat-util.h" +#include "exec_cmd.h" #ifndef PATH_MAX # define PATH_MAX 4096 @@ -233,14 +234,11 @@ int main(int argc, char **argv, char **e { char git_command[PATH_MAX + 1]; char wd[PATH_MAX + 1]; - int i, len, show_help = 0; - char *exec_path = getenv("GIT_EXEC_PATH"); + int i, show_help = 0; + char *exec_path; getcwd(wd, PATH_MAX); - if (!exec_path) - exec_path = GIT_EXEC_PATH; - for (i = 1; i < argc; i++) { char *arg = argv[i]; @@ -256,10 +254,11 @@ int main(int argc, char **argv, char **e if (!strncmp(arg, "exec-path", 9)) { arg += 9; - if (*arg == '=') + if (*arg == '=') { exec_path = arg + 1; - else { - puts(exec_path); + git_set_exec_path(exec_path); + } else { + puts(git_exec_path()); exit(0); } } @@ -275,48 +274,21 @@ int main(int argc, char **argv, char **e if (i >= argc || show_help) { if (i >= argc) - cmd_usage(exec_path, NULL); + cmd_usage(git_exec_path(), NULL); show_man_page(argv[i]); } - if (*exec_path != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, - "git: cannot determine current directory\n"); - exit(1); - } - len = strlen(git_command); + exec_path = git_exec_path(); + prepend_to_path(exec_path, strlen(exec_path)); - /* Trivial cleanup */ - while (!strncmp(exec_path, "./", 2)) { - exec_path += 2; - while (*exec_path == '/') - exec_path++; - } - snprintf(git_command + len, sizeof(git_command) - len, - "/%s", exec_path); - } - else - strcpy(git_command, exec_path); - len = strlen(git_command); - prepend_to_path(git_command, len); - - len += snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[i]); - if (sizeof(git_command) <= len) { - fprintf(stderr, "git: command name given is too long.\n"); - exit(1); - } - - /* execve() can only ever return if it fails */ - execve(git_command, &argv[i], envp); + execv_git_cmd(argv + i); if (errno == ENOENT) - cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); + cmd_usage(git_exec_path(), "'%s' is not a git-command", + argv[i]); fprintf(stderr, "Failed to run command '%s': %s\n", git_command, strerror(errno)); - return 1; } diff --git a/receive-pack.c b/receive-pack.c index f847ec2..8e78e32 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -257,7 +257,7 @@ static void read_head_info(void) static const char *unpack(int *error_code) { - int code = run_command(unpacker, NULL); + int code = run_command_v_opt(1, &unpacker, RUN_GIT_CMD); *error_code = 0; switch (code) { diff --git a/run-command.c b/run-command.c index 8bf5922..b3d287e 100644 --- a/run-command.c +++ b/run-command.c @@ -1,6 +1,7 @@ #include "cache.h" #include "run-command.h" #include <sys/wait.h> +#include "exec_cmd.h" int run_command_v_opt(int argc, char **argv, int flags) { @@ -13,9 +14,13 @@ int run_command_v_opt(int argc, char **a int fd = open("/dev/null", O_RDWR); dup2(fd, 0); dup2(fd, 1); - close(fd); + close(fd); + } + if (flags & RUN_GIT_CMD) { + execv_git_cmd(argv); + } else { + execvp(argv[0], (char *const*) argv); } - execvp(argv[0], (char *const*) argv); die("exec %s failed.", argv[0]); } for (;;) { diff --git a/run-command.h b/run-command.h index 2469eea..ef3ee05 100644 --- a/run-command.h +++ b/run-command.h @@ -12,7 +12,7 @@ enum { }; #define RUN_COMMAND_NO_STDIO 1 - +#define RUN_GIT_CMD 2 /*If this is to be git sub-command */ int run_command_v_opt(int argc, char **argv, int opt); int run_command_v(int argc, char **argv); int run_command(const char *cmd, ...); diff --git a/send-pack.c b/send-pack.c index cd36193..4a420a6 100644 --- a/send-pack.c +++ b/send-pack.c @@ -26,11 +26,11 @@ static int is_zero_sha1(const unsigned c static void exec_pack_objects(void) { static char *args[] = { - "git-pack-objects", + "pack-objects", "--stdout", NULL }; - execvp("git-pack-objects", args); + execv_git_cmd(args); die("git-pack-objects exec failed (%s)", strerror(errno)); } @@ -39,7 +39,7 @@ static void exec_rev_list(struct ref *re static char *args[1000]; int i = 0; - args[i++] = "git-rev-list"; /* 0 */ + args[i++] = "rev-list"; /* 0 */ args[i++] = "--objects"; /* 1 */ while (refs) { char *buf = malloc(100); @@ -58,7 +58,7 @@ static void exec_rev_list(struct ref *re refs = refs->next; } args[i] = NULL; - execvp("git-rev-list", args); + execv_git_cmd(args); die("git-rev-list exec failed (%s)", strerror(errno)); } diff --git a/shell.c b/shell.c index cd31618..0d4891f 100644 --- a/shell.c +++ b/shell.c @@ -12,7 +12,7 @@ static int do_generic_cmd(const char *me my_argv[1] = arg; my_argv[2] = NULL; - return execvp(me, (char**) my_argv); + return execv_git_cmd((char**) my_argv); } static struct commands { diff --git a/upload-pack.c b/upload-pack.c index 1834b6b..d198055 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -4,6 +4,7 @@ #include "tag.h" #include "object.h" #include "commit.h" +#include "exec_cmd.h" static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>"; @@ -60,7 +61,7 @@ static void create_pack_file(void) close(0); close(fd[0]); close(fd[1]); - *p++ = "git-rev-list"; + *p++ = "rev-list"; *p++ = "--objects"; if (create_full_pack || MAX_NEEDS <= nr_needs) *p++ = "--all"; @@ -79,13 +80,13 @@ static void create_pack_file(void) buf += 41; } *p++ = NULL; - execvp("git-rev-list", argv); + execv_git_cmd(argv); die("git-upload-pack: unable to exec git-rev-list"); } dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-pack-objects", "git-pack-objects", "--stdout", NULL); + execl_git_cmd("pack-objects", "--stdout", NULL); die("git-upload-pack: unable to exec git-pack-objects"); } -- 0.99.9m-g5a22 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] Exec git programs without using PATH. 2006-01-11 2:12 ` [PATCH] Exec git programs without using PATH Michal Ostrowski @ 2006-01-11 6:13 ` Junio C Hamano 2006-01-11 17:05 ` [PATCH] (Updated) " Michal Ostrowski 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-11 6:13 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Andreas Ericsson, git Michal Ostrowski <mostrows@watson.ibm.com> writes: > exec_cmd.c | 118 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ This is a giveaway sign that your MUA is wrapping lines, so please double-check before sending things out next time around. It seems yours is not as bad as others I've seen; I could manage to trivially hand-edit to make it appliable (although I am inclined to ask you to polish it a bit further). > +ifneq ($(shell cat .exec_cmd.gitexecdir 2>/dev/null),$(gitexecdir)) > +.PHONY: exec_cmd.c > +$(shell echo $(gitexecdir) > .exec_cmd.gitexecdir) > +endif Although this check is much better than the last round, I changed my mind about this part. We do not do the equivalent checks and rebuild when somebody once builds with NO_OPENSSL and then tries to rebuild without, or for any other compile time configuration (template_dir, for example). Ideally we _could_ keep track of all of them but I suspect that would be an overkill, so I am inclined to drop this part. We need to make sure that we clean .exec_cmd.gitexecdir on the clean: target in the Makefile, if we _were_ to keep this, OTOH. > +exec_cmd.o: CFLAGS+=-DGIT_EXEC_PATH=\"$(gitexecdir)\" Please do not override CFLAGS like this; it breaks this common usage: $ make CFLAGS='-g -O1' exec_cmd.o > diff --git a/exec_cmd.c b/exec_cmd.c > +void git_set_exec_path(const char *exec_path) > +{ > + current_exec_path = exec_path; > +} I've always wondered what we should do when --exec-path is given more than once, but you decided it for me. I agree that silently overwriting is just fine. > + const char *paths[] = { current_exec_path, > + getenv("GIT_EXEC_PATH"), > + builtin_exec_path, > + NULL }; I do not think you need the last NULL element in paths[] here, given what the loop that uses this array does. > + for (i = 0; i < 4; ++i) { If your paths[] is fixed size, please do this instead: for (i = 0; sizeof(paths)/sizeof(paths[0]); i++) Then I do not have to remember that I need to update "4" when I update paths[]. > diff --git a/exec_cmd.h b/exec_cmd.h > +extern const char* git_exec_path(); ANSI C please: extern const char* git_exec_path(void); > diff --git a/git.c b/git.c > if (errno == ENOENT) > - cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); > + cmd_usage(git_exec_path(), "'%s' is not a git-command", > + argv[i]); You have assigned to exec_path before the failed execv_git_cmd() already; I think change in this line is unneeded. > diff --git a/receive-pack.c b/receive-pack.c > index f847ec2..8e78e32 100644 > --- a/receive-pack.c > +++ b/receive-pack.c > @@ -257,7 +257,7 @@ static void read_head_info(void) > > static const char *unpack(int *error_code) > { > - int code = run_command(unpacker, NULL); > + int code = run_command_v_opt(1, &unpacker, RUN_GIT_CMD); This one is questionable; run_command_v_opt() calls execv_git_cmd() which expects the second argument of run_command_v_opt() to be NULL terminated, doesn't it? > diff --git a/shell.c b/shell.c > index cd31618..0d4891f 100644 > --- a/shell.c > +++ b/shell.c > @@ -12,7 +12,7 @@ static int do_generic_cmd(const char *me > my_argv[1] = arg; > my_argv[2] = NULL; > > - return execvp(me, (char**) my_argv); > + return execv_git_cmd((char**) my_argv); > } Here my_argv comes from cmd_list[] which has "git-" prefix to their command name, but I thought your exec[vl]_git_cmd() took command names without. Here is my suggestion on top of your patch, summarizing all of the above comments. -- >8 -- diff --git a/Makefile b/Makefile index e7c7763..6694627 100644 --- a/Makefile +++ b/Makefile @@ -185,16 +185,6 @@ LIB_OBJS = \ LIBS = $(LIB_FILE) LIBS += -lz - -# .exec_cmd.gitexecdir stores $(gitexecir) used to compile exec_cmd.o -# If it has changed, store the new value and force exec_cmd.o to be rebuilt -ifneq ($(shell cat .exec_cmd.gitexecdir 2>/dev/null),$(gitexecdir)) -.PHONY: exec_cmd.c -$(shell echo $(gitexecdir) > .exec_cmd.gitexecdir) -endif - -exec_cmd.o: CFLAGS+=-DGIT_EXEC_PATH=\"$(gitexecdir)\" - # Shell quote; # Result of this needs to be placed inside '' shq = $(subst ','\'',$(1)) @@ -423,6 +413,8 @@ git$X git.spec \ %.o: %.S $(CC) -o $*.o -c $(ALL_CFLAGS) $< +exec_cmd.o: ALL_CFLAGS += -DGIT_EXEC_PATH=\"$(gitexecdir)\" + git-%$X: %.o $(LIB_FILE) $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) diff --git a/exec_cmd.c b/exec_cmd.c index a3bd40a..55af33b 100644 --- a/exec_cmd.c +++ b/exec_cmd.c @@ -36,10 +36,9 @@ int execv_git_cmd(char **argv) int len, err, i; const char *paths[] = { current_exec_path, getenv("GIT_EXEC_PATH"), - builtin_exec_path, - NULL }; + builtin_exec_path }; - for (i = 0; i < 4; ++i) { + for (i = 0; i < sizeof(paths)/sizeof(paths[0]); ++i) { const char *exec_dir = paths[i]; if (!exec_dir) continue; diff --git a/exec_cmd.h b/exec_cmd.h index 06d5ec3..5150ee2 100644 --- a/exec_cmd.h +++ b/exec_cmd.h @@ -2,7 +2,7 @@ #define __GIT_EXEC_CMD_H_ extern void git_set_exec_path(const char *exec_path); -extern const char* git_exec_path(); +extern const char* git_exec_path(void); extern int execv_git_cmd(char **argv); /* NULL terminated */ extern int execl_git_cmd(char *cmd, ...); diff --git a/git.c b/git.c index fdd02ed..e3a0b59 100644 --- a/git.c +++ b/git.c @@ -285,10 +285,10 @@ int main(int argc, char **argv, char **e execv_git_cmd(argv + i); if (errno == ENOENT) - cmd_usage(git_exec_path(), "'%s' is not a git-command", - argv[i]); + cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); fprintf(stderr, "Failed to run command '%s': %s\n", git_command, strerror(errno)); + return 1; } diff --git a/receive-pack.c b/receive-pack.c index 8e78e32..6120dbe 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -6,7 +6,7 @@ static const char receive_pack_usage[] = "git-receive-pack <git-dir>"; -static const char unpacker[] = "git-unpack-objects"; +static char *unpacker[] = { "git-unpack-objects", NULL }; static int report_status = 0; @@ -257,7 +257,7 @@ static void read_head_info(void) static const char *unpack(int *error_code) { - int code = run_command_v_opt(1, &unpacker, RUN_GIT_CMD); + int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD); *error_code = 0; switch (code) { diff --git a/shell.c b/shell.c index 0d4891f..d40dfe4 100644 --- a/shell.c +++ b/shell.c @@ -7,8 +7,10 @@ static int do_generic_cmd(const char *me if (!arg || !(arg = sq_dequote(arg))) die("bad argument"); + if (strncmp(me, "git-", 4)) + die("bad command"); - my_argv[0] = me; + my_argv[0] = me + 4; my_argv[1] = arg; my_argv[2] = NULL; ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 6:13 ` Junio C Hamano @ 2006-01-11 17:05 ` Michal Ostrowski 2006-01-11 20:33 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-11 17:05 UTC (permalink / raw) To: Junio C Hamano; +Cc: Andreas Ericsson, git The git suite may not be in PATH (and thus programs such as git-send-pack could not exec git-rev-list). Thus there is a need for logic that will locate these programs. 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. - git C programs will use exec*_git_cmd() APIs to exec sub-commands. - exec*_git_cmd() will execute a git program by searching for it in the following directories: 1. --exec-path (as used by "git") 2. The GIT_EXEC_PATH environment variable. 3. $(gitexecdir) as set in Makefile (default value $(bindir)). - all exec's of git programs will use exec*_git_cmd() interfaces, - C programs will not modify PATH If the git suite is not in PATH, C programs will function, but there is no guarantee that shell scripts will, as shell scripts may invoke "git-foo" rather than "git foo". Shell scripts that are modified to use the git potty should become independent of PATH. Includes modifications by Junio C Hamano <junkio@cox.net>. Signed-off-by: Michal Ostrowski <mostrows@watson.ibm.com> --- Makefile | 13 ++++-- daemon.c | 3 + exec_cmd.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ exec_cmd.h | 10 +++++ fetch-clone.c | 7 +-- git.c | 50 +++++------------------- receive-pack.c | 4 +- run-command.c | 9 +++- run-command.h | 2 - send-pack.c | 8 ++-- shell.c | 6 ++- upload-pack.c | 7 ++- 12 files changed, 174 insertions(+), 62 deletions(-) create mode 100644 exec_cmd.c create mode 100644 exec_cmd.h 565e290d6e7f24bd4e72ca7494297230799d118a diff --git a/Makefile b/Makefile index c9c15b5..abcb771 100644 --- a/Makefile +++ b/Makefile @@ -68,6 +68,7 @@ ALL_LDFLAGS = $(LDFLAGS) prefix = $(HOME) bindir = $(prefix)/bin +gitexecdir = $(prefix)/bin template_dir = $(prefix)/share/git-core/templates/ GIT_PYTHON_DIR = $(prefix)/share/git-core/python # DESTDIR= @@ -141,7 +142,7 @@ PROGRAMS = \ git-describe$X # what 'all' will build and 'install' will install. -ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) git$X +ALL_PROGRAMS = $(PROGRAMS) $(SIMPLE_PROGRAMS) $(SCRIPTS) # Backward compatibility -- to be removed after 1.0 PROGRAMS += git-ssh-pull$X git-ssh-push$X @@ -173,7 +174,7 @@ DIFF_OBJS = \ LIB_OBJS = \ blob.o commit.o connect.o count-delta.o csum-file.o \ - date.o diff-delta.o entry.o ident.o index.o \ + date.o diff-delta.o entry.o exec_cmd.o ident.o index.o \ object.o pack-check.o patch-delta.o path.o pkt-line.o \ quote.o read-cache.o refs.o run-command.o \ server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \ @@ -366,13 +367,13 @@ LIB_OBJS += $(COMPAT_OBJS) export prefix TAR INSTALL DESTDIR SHELL_PATH template_dir ### Build rules -all: $(ALL_PROGRAMS) +all: $(ALL_PROGRAMS) git$X all: $(MAKE) -C templates git$X: git.c $(LIB_FILE) - $(CC) -DGIT_EXEC_PATH='"$(bindir)"' -DGIT_VERSION='"$(GIT_VERSION)"' \ + $(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \ $(CFLAGS) $(COMPAT_CFLAGS) -o $@ $(filter %.c,$^) $(LIB_FILE) $(patsubst %.sh,%,$(SCRIPT_SH)) : % : %.sh @@ -412,6 +413,8 @@ git$X git.spec \ %.o: %.S $(CC) -o $*.o -c $(ALL_CFLAGS) $< +exec_cmd.o: ALL_CFLAGS += -DGIT_EXEC_PATH=\"$(gitexecdir)\" + git-%$X: %.o $(LIB_FILE) $(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS) @@ -468,7 +471,9 @@ check: install: all $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(bindir)) + $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(gitexecdir)) $(INSTALL) $(ALL_PROGRAMS) $(call shellquote,$(DESTDIR)$(bindir)) + $(INSTALL) git$X $(call shellquote,$(DESTDIR)$(gitexecdir)) $(MAKE) -C templates install $(INSTALL) -d -m755 $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) $(INSTALL) $(PYMODULES) $(call shellquote,$(DESTDIR)$(GIT_PYTHON_DIR)) diff --git a/daemon.c b/daemon.c index 3bd1426..bb014fa 100644 --- a/daemon.c +++ b/daemon.c @@ -9,6 +9,7 @@ #include <syslog.h> #include "pkt-line.h" #include "cache.h" +#include "exec_cmd.h" static int log_syslog; static int verbose; @@ -227,7 +228,7 @@ static int upload(char *dir) snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout); /* git-upload-pack only ever reads stuff, so this is safe */ - execlp("git-upload-pack", "git-upload-pack", "--strict", timeout_buf, ".", NULL); + execl_git_cmd("upload-pack", "--strict", timeout_buf, ".", NULL); return -1; } diff --git a/exec_cmd.c b/exec_cmd.c new file mode 100644 index 0000000..55af33b --- /dev/null +++ b/exec_cmd.c @@ -0,0 +1,117 @@ +#include "cache.h" +#include "exec_cmd.h" +#define MAX_ARGS 32 + +extern char **environ; +static const char *builtin_exec_path = GIT_EXEC_PATH; +static const char *current_exec_path = NULL; + +void git_set_exec_path(const char *exec_path) +{ + current_exec_path = exec_path; +} + + +/* Returns the highest-priority, location to look for git programs. */ +const char *git_exec_path() +{ + const char *env; + + if (current_exec_path) + return current_exec_path; + + env = getenv("GIT_EXEC_PATH"); + if (env) { + return env; + } + + return builtin_exec_path; +} + + +int execv_git_cmd(char **argv) +{ + char git_command[PATH_MAX + 1]; + char *tmp; + int len, err, i; + const char *paths[] = { current_exec_path, + getenv("GIT_EXEC_PATH"), + builtin_exec_path }; + + for (i = 0; i < sizeof(paths)/sizeof(paths[0]); ++i) { + const char *exec_dir = paths[i]; + if (!exec_dir) continue; + + if (*exec_dir != '/') { + if (!getcwd(git_command, sizeof(git_command))) { + fprintf(stderr, "git: cannot determine " + "current directory\n"); + exit(1); + } + len = strlen(git_command); + + /* Trivial cleanup */ + while (!strncmp(exec_dir, "./", 2)) { + exec_dir += 2; + while (*exec_dir == '/') + exec_dir++; + } + snprintf(git_command + len, sizeof(git_command) - len, + "/%s", exec_dir); + } else { + strcpy(git_command, exec_dir); + } + + len = strlen(git_command); + len += snprintf(git_command + len, sizeof(git_command) - len, + "/git-%s", argv[0]); + + if (sizeof(git_command) <= len) { + fprintf(stderr, + "git: command name given is too long.\n"); + break; + } + + /* argv[0] must be the git command, but the argv array + * belongs to the caller, and my be reused in + * subsequent loop iterations. Save argv[0] and + * restore it on error. + */ + + tmp = argv[0]; + argv[0] = git_command; + + /* execve() can only ever return if it fails */ + execve(git_command, argv, environ); + + err = errno; + + argv[0] = tmp; + } + return -1; + +} + + +int execl_git_cmd(char *cmd,...) +{ + int argc; + char *argv[MAX_ARGS + 1]; + char *arg; + va_list param; + + va_start(param, cmd); + argv[0] = cmd; + argc = 1; + while (argc < MAX_ARGS) { + arg = argv[argc++] = va_arg(param, char *); + if (!arg) + break; + } + va_end(param); + if (MAX_ARGS <= argc) + return error("too many args to run %s", cmd); + + argv[argc] = NULL; + return execv_git_cmd(argv); +} diff --git a/exec_cmd.h b/exec_cmd.h new file mode 100644 index 0000000..5150ee2 --- /dev/null +++ b/exec_cmd.h @@ -0,0 +1,10 @@ +#ifndef __GIT_EXEC_CMD_H_ +#define __GIT_EXEC_CMD_H_ + +extern void git_set_exec_path(const char *exec_path); +extern const char* git_exec_path(void); +extern int execv_git_cmd(char **argv); /* NULL terminated */ +extern int execl_git_cmd(char *cmd, ...); + + +#endif /* __GIT_EXEC_CMD_H_ */ diff --git a/fetch-clone.c b/fetch-clone.c index f46fe6e..859f400 100644 --- a/fetch-clone.c +++ b/fetch-clone.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "exec_cmd.h" #include <sys/wait.h> static int finish_pack(const char *pack_tmp_name, const char *me) @@ -27,8 +28,7 @@ static int finish_pack(const char *pack_ dup2(pipe_fd[1], 1); close(pipe_fd[0]); close(pipe_fd[1]); - execlp("git-index-pack","git-index-pack", - "-o", idx, pack_tmp_name, NULL); + execl_git_cmd("index-pack", "-o", idx, pack_tmp_name, NULL); error("cannot exec git-index-pack <%s> <%s>", idx, pack_tmp_name); exit(1); @@ -105,8 +105,7 @@ int receive_unpack_pack(int fd[2], const dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-unpack-objects", "git-unpack-objects", - quiet ? "-q" : NULL, NULL); + execl_git_cmd("unpack-objects", quiet ? "-q" : NULL, NULL); die("git-unpack-objects exec failed"); } close(fd[0]); diff --git a/git.c b/git.c index 5e7da74..4616df6 100644 --- a/git.c +++ b/git.c @@ -10,6 +10,7 @@ #include <stdarg.h> #include <sys/ioctl.h> #include "git-compat-util.h" +#include "exec_cmd.h" #ifndef PATH_MAX # define PATH_MAX 4096 @@ -233,14 +234,11 @@ int main(int argc, char **argv, char **e { char git_command[PATH_MAX + 1]; char wd[PATH_MAX + 1]; - int i, len, show_help = 0; - char *exec_path = getenv("GIT_EXEC_PATH"); + int i, show_help = 0; + const char *exec_path; getcwd(wd, PATH_MAX); - if (!exec_path) - exec_path = GIT_EXEC_PATH; - for (i = 1; i < argc; i++) { char *arg = argv[i]; @@ -256,10 +254,11 @@ int main(int argc, char **argv, char **e if (!strncmp(arg, "exec-path", 9)) { arg += 9; - if (*arg == '=') + if (*arg == '=') { exec_path = arg + 1; - else { - puts(exec_path); + git_set_exec_path(exec_path); + } else { + puts(git_exec_path()); exit(0); } } @@ -275,42 +274,15 @@ int main(int argc, char **argv, char **e if (i >= argc || show_help) { if (i >= argc) - cmd_usage(exec_path, NULL); + cmd_usage(git_exec_path(), NULL); show_man_page(argv[i]); } - if (*exec_path != '/') { - if (!getcwd(git_command, sizeof(git_command))) { - fprintf(stderr, - "git: cannot determine current directory\n"); - exit(1); - } - len = strlen(git_command); - - /* Trivial cleanup */ - while (!strncmp(exec_path, "./", 2)) { - exec_path += 2; - while (*exec_path == '/') - exec_path++; - } - snprintf(git_command + len, sizeof(git_command) - len, - "/%s", exec_path); - } - else - strcpy(git_command, exec_path); - len = strlen(git_command); - prepend_to_path(git_command, len); - - len += snprintf(git_command + len, sizeof(git_command) - len, - "/git-%s", argv[i]); - if (sizeof(git_command) <= len) { - fprintf(stderr, "git: command name given is too long.\n"); - exit(1); - } + exec_path = git_exec_path(); + prepend_to_path(exec_path, strlen(exec_path)); - /* execve() can only ever return if it fails */ - execve(git_command, &argv[i], envp); + execv_git_cmd(argv + i); if (errno == ENOENT) cmd_usage(exec_path, "'%s' is not a git-command", argv[i]); diff --git a/receive-pack.c b/receive-pack.c index f847ec2..6120dbe 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -6,7 +6,7 @@ static const char receive_pack_usage[] = "git-receive-pack <git-dir>"; -static const char unpacker[] = "git-unpack-objects"; +static char *unpacker[] = { "git-unpack-objects", NULL }; static int report_status = 0; @@ -257,7 +257,7 @@ static void read_head_info(void) static const char *unpack(int *error_code) { - int code = run_command(unpacker, NULL); + int code = run_command_v_opt(1, unpacker, RUN_GIT_CMD); *error_code = 0; switch (code) { diff --git a/run-command.c b/run-command.c index 8bf5922..b3d287e 100644 --- a/run-command.c +++ b/run-command.c @@ -1,6 +1,7 @@ #include "cache.h" #include "run-command.h" #include <sys/wait.h> +#include "exec_cmd.h" int run_command_v_opt(int argc, char **argv, int flags) { @@ -13,9 +14,13 @@ int run_command_v_opt(int argc, char **a int fd = open("/dev/null", O_RDWR); dup2(fd, 0); dup2(fd, 1); - close(fd); + close(fd); + } + if (flags & RUN_GIT_CMD) { + execv_git_cmd(argv); + } else { + execvp(argv[0], (char *const*) argv); } - execvp(argv[0], (char *const*) argv); die("exec %s failed.", argv[0]); } for (;;) { diff --git a/run-command.h b/run-command.h index 2469eea..ef3ee05 100644 --- a/run-command.h +++ b/run-command.h @@ -12,7 +12,7 @@ enum { }; #define RUN_COMMAND_NO_STDIO 1 - +#define RUN_GIT_CMD 2 /*If this is to be git sub-command */ int run_command_v_opt(int argc, char **argv, int opt); int run_command_v(int argc, char **argv); int run_command(const char *cmd, ...); diff --git a/send-pack.c b/send-pack.c index cd36193..4a420a6 100644 --- a/send-pack.c +++ b/send-pack.c @@ -26,11 +26,11 @@ static int is_zero_sha1(const unsigned c static void exec_pack_objects(void) { static char *args[] = { - "git-pack-objects", + "pack-objects", "--stdout", NULL }; - execvp("git-pack-objects", args); + execv_git_cmd(args); die("git-pack-objects exec failed (%s)", strerror(errno)); } @@ -39,7 +39,7 @@ static void exec_rev_list(struct ref *re static char *args[1000]; int i = 0; - args[i++] = "git-rev-list"; /* 0 */ + args[i++] = "rev-list"; /* 0 */ args[i++] = "--objects"; /* 1 */ while (refs) { char *buf = malloc(100); @@ -58,7 +58,7 @@ static void exec_rev_list(struct ref *re refs = refs->next; } args[i] = NULL; - execvp("git-rev-list", args); + execv_git_cmd(args); die("git-rev-list exec failed (%s)", strerror(errno)); } diff --git a/shell.c b/shell.c index cd31618..d40dfe4 100644 --- a/shell.c +++ b/shell.c @@ -7,12 +7,14 @@ static int do_generic_cmd(const char *me if (!arg || !(arg = sq_dequote(arg))) die("bad argument"); + if (strncmp(me, "git-", 4)) + die("bad command"); - my_argv[0] = me; + my_argv[0] = me + 4; my_argv[1] = arg; my_argv[2] = NULL; - return execvp(me, (char**) my_argv); + return execv_git_cmd((char**) my_argv); } static struct commands { diff --git a/upload-pack.c b/upload-pack.c index 1834b6b..d198055 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -4,6 +4,7 @@ #include "tag.h" #include "object.h" #include "commit.h" +#include "exec_cmd.h" static const char upload_pack_usage[] = "git-upload-pack [--strict] [--timeout=nn] <dir>"; @@ -60,7 +61,7 @@ static void create_pack_file(void) close(0); close(fd[0]); close(fd[1]); - *p++ = "git-rev-list"; + *p++ = "rev-list"; *p++ = "--objects"; if (create_full_pack || MAX_NEEDS <= nr_needs) *p++ = "--all"; @@ -79,13 +80,13 @@ static void create_pack_file(void) buf += 41; } *p++ = NULL; - execvp("git-rev-list", argv); + execv_git_cmd(argv); die("git-upload-pack: unable to exec git-rev-list"); } dup2(fd[0], 0); close(fd[0]); close(fd[1]); - execlp("git-pack-objects", "git-pack-objects", "--stdout", NULL); + execl_git_cmd("pack-objects", "--stdout", NULL); die("git-upload-pack: unable to exec git-pack-objects"); } -- 0.99.9m-g5a22 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 17:05 ` [PATCH] (Updated) " Michal Ostrowski @ 2006-01-11 20:33 ` Junio C Hamano 2006-01-11 20:42 ` Linus Torvalds 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-11 20:33 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Andreas Ericsson, git Michal Ostrowski <mostrows@watson.ibm.com> writes: > The git suite may not be in PATH (and thus programs such as >... > Includes modifications by Junio C Hamano <junkio@cox.net>. Thanks for the update (you did it even without line wrapping this time). I made further fixes last night and it is in the proposed updates ("pu") branch. This branch is used to hold and showcase what I plan to eventually merge into the master branch, and consists of the topic branches merged on top of what is in the master branch. To disect to see what shape I munged your patch into, please look at it with gitk or gitweb and find "Merge branch mo/exec" commit (my topic branches are named after patch author and topic keyword). The second parent of that commit is the tip of the topic branch mo/exec when the merge was made. A word of caution to people on the list (not limited to Michal) is needed again, because it's been a while since I talked about the proposed updates branch last time (which was pre 1.0.0 release IIRC). The "pu" branch is regularly rebuilt by merging private topic branches on top of the then-current master branch, by doing this: $ git checkout pu $ git reset --hard master ;# build from scratch! $ git pull . topic1 $ git pull . topic2 $ ... $ git pull . topicN What this means is that you (not just Michal---"people") should not base your development on top of "pu" branch proper, since you will have a hard time updating and merging. Disecting my "pu" branch to find the topic branch that corresponds to your topic, and doing your development and patch preparation based on that branch tip (and tell me that the patch is against your topic branch, or "pu") is possible, but even that is not always an easy option, since I may occasionally have to rebase my topic branches as well. You can always send in an updated patch based on my master, and I'll manage with three-way merge to pick out the real changes since your last iteration. For your (primarily Michal, but other interested parties as well) reference, here is the diff between your patch in the message I am replying to and what I placed in the "pu" branch last night. -- >8 -- Updates on top of comments last night. - git-unpack-objects are run via execv_git_cmd, so no "git-" prefix should be given. - two more files need #include "exec_cmd.h". --- diff --git a/receive-pack.c b/receive-pack.c index 6120dbe..eae31e3 100644 --- a/receive-pack.c +++ b/receive-pack.c @@ -6,7 +6,7 @@ static const char receive_pack_usage[] = "git-receive-pack <git-dir>"; -static char *unpacker[] = { "git-unpack-objects", NULL }; +static char *unpacker[] = { "unpack-objects", NULL }; static int report_status = 0; diff --git a/send-pack.c b/send-pack.c index 4a420a6..990be3f 100644 --- a/send-pack.c +++ b/send-pack.c @@ -3,6 +3,7 @@ #include "tag.h" #include "refs.h" #include "pkt-line.h" +#include "exec_cmd.h" static const char send_pack_usage[] = "git-send-pack [--all] [--exec=git-receive-pack] <remote> [<head>...]\n" diff --git a/shell.c b/shell.c index d40dfe4..fc0c73c 100644 --- a/shell.c +++ b/shell.c @@ -1,5 +1,6 @@ #include "cache.h" #include "quote.h" +#include "exec_cmd.h" static int do_generic_cmd(const char *me, char *arg) { ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 20:33 ` Junio C Hamano @ 2006-01-11 20:42 ` Linus Torvalds 2006-01-11 21:26 ` Michal Ostrowski 0 siblings, 1 reply; 33+ messages in thread From: Linus Torvalds @ 2006-01-11 20:42 UTC (permalink / raw) To: Junio C Hamano; +Cc: Michal Ostrowski, Andreas Ericsson, git On Wed, 11 Jan 2006, Junio C Hamano wrote: > > For your (primarily Michal, but other interested parties as > well) reference, here is the diff between your patch in the > message I am replying to and what I placed in the "pu" branch > last night. Tangentially related note: maybe we should try to move to a "spawn()" like interface so that it could port better to native Windows interfaces? Even under Linux, using vfork()+exec() is actually faster than a regular fork/exec, so there are potential advantages. The real advantage of fork+exec is how you can do arbitrary setup between the two, without needing insanely complex rules for file descriptors etc. But maybe we don't have tons of those issues? Linus ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 20:42 ` Linus Torvalds @ 2006-01-11 21:26 ` Michal Ostrowski 2006-01-11 21:32 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-11 21:26 UTC (permalink / raw) To: Linus Torvalds; +Cc: Junio C Hamano, Andreas Ericsson, git On Wed, 2006-01-11 at 12:42 -0800, Linus Torvalds wrote: > > On Wed, 11 Jan 2006, Junio C Hamano wrote: > > > > For your (primarily Michal, but other interested parties as > > well) reference, here is the diff between your patch in the > > message I am replying to and what I placed in the "pu" branch > > last night. > > Tangentially related note: maybe we should try to move to a "spawn()" like > interface so that it could port better to native Windows interfaces? > > Even under Linux, using vfork()+exec() is actually faster than a regular > fork/exec, so there are potential advantages. > > The real advantage of fork+exec is how you can do arbitrary setup between > the two, without needing insanely complex rules for file descriptors etc. > But maybe we don't have tons of those issues? > I briefly tried to consider if I could hide the various fork()+exec() sequences behind something like the run_command*() interfaces (which would move us down the direction of something "spawn()"-like). I found that there's a lot of variation between the various paths in terms of what happens between fork() and exec() on the various paths that does not lend itself to such consolidation. I'd love to be convinced otherwise. -- Michal Ostrowski <mostrows@watson.ibm.com> ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 21:26 ` Michal Ostrowski @ 2006-01-11 21:32 ` Junio C Hamano 2006-01-12 0:11 ` Andreas Ericsson 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-11 21:32 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Linus Torvalds, Andreas Ericsson, git Michal Ostrowski <mostrows@watson.ibm.com> writes: > I briefly tried to consider if I could hide the various fork()+exec() > sequences behind something like the run_command*() interfaces (which > would move us down the direction of something "spawn()"-like). I found > that there's a lot of variation between the various paths in terms of > what happens between fork() and exec() on the various paths that does > not lend itself to such consolidation. > > I'd love to be convinced otherwise. Unfortunately I am with Michal on this one (both "eh, I do not think that is feasible" and "I'd love to be..."). The run_command*() interfaces and its users were the best I could come up with as far as such consolidations could go when I did it. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-11 21:32 ` Junio C Hamano @ 2006-01-12 0:11 ` Andreas Ericsson 2006-01-12 5:38 ` H. Peter Anvin 0 siblings, 1 reply; 33+ messages in thread From: Andreas Ericsson @ 2006-01-12 0:11 UTC (permalink / raw) To: git Junio C Hamano wrote: > Michal Ostrowski <mostrows@watson.ibm.com> writes: > > >>I briefly tried to consider if I could hide the various fork()+exec() >>sequences behind something like the run_command*() interfaces (which >>would move us down the direction of something "spawn()"-like). I found >>that there's a lot of variation between the various paths in terms of >>what happens between fork() and exec() on the various paths that does >>not lend itself to such consolidation. >> >>I'd love to be convinced otherwise. > > > Unfortunately I am with Michal on this one (both "eh, I do not > think that is feasible" and "I'd love to be..."). > > The run_command*() interfaces and its users were the best I > could come up with as far as such consolidations could go when I > did it. > Not being entirely knowledgeable on what spawn() actually does and how its semantics differ from fork() and exec*() style API's (Google was depressingly unhelpful and wikipedia dredged up froglings...), I've got a decent "clone-lots-of-processes-and-multiplex-between-them" kind of library lying around. Would it be of any use? From the prototypes I've seen on spawn it doesn't seem to be much more than a fork() + execve(), either closing or dup2'ing all the file-descriptors, so I don't understand why that couldn't be implemented for git. Some pointers, anyone? -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] (Updated) Exec git programs without using PATH. 2006-01-12 0:11 ` Andreas Ericsson @ 2006-01-12 5:38 ` H. Peter Anvin 0 siblings, 0 replies; 33+ messages in thread From: H. Peter Anvin @ 2006-01-12 5:38 UTC (permalink / raw) To: Andreas Ericsson; +Cc: git Andreas Ericsson wrote: > > Not being entirely knowledgeable on what spawn() actually does and how > its semantics differ from fork() and exec*() style API's (Google was > depressingly unhelpful and wikipedia dredged up froglings...), I've got > a decent "clone-lots-of-processes-and-multiplex-between-them" kind of > library lying around. Would it be of any use? > > From the prototypes I've seen on spawn it doesn't seem to be much more > than a fork() + execve(), either closing or dup2'ing all the > file-descriptors, so I don't understand why that couldn't be implemented > for git. Some pointers, anyone? > RTFM(posix_spawn)... -hpa ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 16:26 ` Michal Ostrowski 2006-01-10 19:13 ` Andreas Ericsson @ 2006-01-10 19:47 ` Junio C Hamano 2006-01-10 19:55 ` Johannes Schindelin 1 sibling, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-10 19:47 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Andreas Ericsson, git Michal Ostrowski <mostrows@watson.ibm.com> writes: > One could make all the scripts depend on GIT_EXEC_PATH instead of PATH. > At build time one could generate wrapper functions in git-sh-setup: > > function git-foo () { > $(GIT_EXEC_PATH)/git-foo $*; > } > > Presuming that all scripts include git-sh-setup, no other shell script > changes would be needed. Is "git-foo" a valid name to define shell function as? > My shell's rc-file doesn't get invoked when using ssh as a transport; > that's part of the problem. Not any rc, or are you bitten by bash/ssh misfeature that noninteractive sessions do not start with .bash_profile? >> > Once you have that implemented, we can have a separate discussion of how >> > the executable is to be found; >> > - should we use PATH? >> > - should we change PATH? >> > - should we always exec using an absolute file name? (my preference) The goal here is to make sure we exec the program from the same release (unless user overrides it with GIT_EXEC_PATH to say "I want to try 0.99.9k, not the latest one"), but how? The last one feels the most correct way if done right. > git programs exec other git programs, but they also exec non-git > programs. I think it is not appropriate to change PATH (via > prepend_to_path) because this may result in unexpected behavior when > exec'ing non-git programs: This is a valid concern. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 19:47 ` [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s Junio C Hamano @ 2006-01-10 19:55 ` Johannes Schindelin 2006-01-10 20:31 ` Michal Ostrowski 2006-01-10 21:09 ` Junio C Hamano 0 siblings, 2 replies; 33+ messages in thread From: Johannes Schindelin @ 2006-01-10 19:55 UTC (permalink / raw) To: Junio C Hamano; +Cc: Michal Ostrowski, Andreas Ericsson, git Hi, On Tue, 10 Jan 2006, Junio C Hamano wrote: > Michal Ostrowski <mostrows@watson.ibm.com> writes: > > > git programs exec other git programs, but they also exec non-git > > programs. I think it is not appropriate to change PATH (via > > prepend_to_path) because this may result in unexpected behavior when > > exec'ing non-git programs: > > This is a valid concern. Why? If what is prepended to PATH only contains git programs? Ciao, Dscho ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 19:55 ` Johannes Schindelin @ 2006-01-10 20:31 ` Michal Ostrowski 2006-01-10 21:03 ` Johannes Schindelin 2006-01-10 21:09 ` Junio C Hamano 1 sibling, 1 reply; 33+ messages in thread From: Michal Ostrowski @ 2006-01-10 20:31 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Junio C Hamano, Andreas Ericsson, git On Tue, 2006-01-10 at 20:55 +0100, Johannes Schindelin wrote: > Hi, > > On Tue, 10 Jan 2006, Junio C Hamano wrote: > > > Michal Ostrowski <mostrows@watson.ibm.com> writes: > > > > > git programs exec other git programs, but they also exec non-git > > > programs. I think it is not appropriate to change PATH (via > > > prepend_to_path) because this may result in unexpected behavior when > > > exec'ing non-git programs: > > > > This is a valid concern. > > Why? If what is prepended to PATH only contains git programs? > If git is installed with prefix=/usr, then that won't be the case. -- Michal Ostrowski <mostrows@watson.ibm.com> ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 20:31 ` Michal Ostrowski @ 2006-01-10 21:03 ` Johannes Schindelin 2006-01-11 0:10 ` Andreas Ericsson 0 siblings, 1 reply; 33+ messages in thread From: Johannes Schindelin @ 2006-01-10 21:03 UTC (permalink / raw) To: Michal Ostrowski; +Cc: Junio C Hamano, Andreas Ericsson, git Hi, On Tue, 10 Jan 2006, Michal Ostrowski wrote: > On Tue, 2006-01-10 at 20:55 +0100, Johannes Schindelin wrote: > > Hi, > > > > On Tue, 10 Jan 2006, Junio C Hamano wrote: > > > > > Michal Ostrowski <mostrows@watson.ibm.com> writes: > > > > > > > git programs exec other git programs, but they also exec non-git > > > > programs. I think it is not appropriate to change PATH (via > > > > prepend_to_path) because this may result in unexpected behavior when > > > > exec'ing non-git programs: > > > > > > This is a valid concern. > > > > Why? If what is prepended to PATH only contains git programs? > > > > > If git is installed with prefix=/usr, then that won't be the case. Okay, so here we have the problem: Two completely different setups. One into a standard location on the PATH (which used to be the default), the other with a libexec/ directory (which some want in the future). And a git wrapper which makes no difference between both. Wouldn't it make much more sense to have a switch in the Makefile, which says *if* we have a libexec/ directory? This switch would decide if we ever prepend the path with the libexec/ directory or not. Ciao, Dscho ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 21:03 ` Johannes Schindelin @ 2006-01-11 0:10 ` Andreas Ericsson 2006-01-11 0:57 ` Junio C Hamano 0 siblings, 1 reply; 33+ messages in thread From: Andreas Ericsson @ 2006-01-11 0:10 UTC (permalink / raw) To: git Johannes Schindelin wrote: >>>>>git programs exec other git programs, but they also exec non-git >>>>>programs. I think it is not appropriate to change PATH (via >>>>>prepend_to_path) because this may result in unexpected behavior when >>>>>exec'ing non-git programs: >>>> >>>>This is a valid concern. >>> >>>Why? If what is prepended to PATH only contains git programs? >>> >> >> >>If git is installed with prefix=/usr, then that won't be the case. > > > Okay, so here we have the problem: Two completely different setups. One > into a standard location on the PATH (which used to be the default), the > other with a libexec/ directory (which some want in the future). And a git > wrapper which makes no difference between both. > > Wouldn't it make much more sense to have a switch in the Makefile, which > says *if* we have a libexec/ directory? No, it wouldn't, because then we can't use a different release of the git-tools without re-compiling the potty. void prepend_to_path(old_path, to_prep) { if (strstr(old_path, to_prep)) return; really_prepend_to_path(old_path, to_prep); } would work just fine though. -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-11 0:10 ` Andreas Ericsson @ 2006-01-11 0:57 ` Junio C Hamano 2006-01-11 11:57 ` Andreas Ericsson 0 siblings, 1 reply; 33+ messages in thread From: Junio C Hamano @ 2006-01-11 0:57 UTC (permalink / raw) To: Andreas Ericsson; +Cc: git Andreas Ericsson <ae@op5.se> writes: > Johannes Schindelin wrote: >> Wouldn't it make much more sense to have a switch in the Makefile, >> which says *if* we have a libexec/ directory? > > No, it wouldn't, because then we can't use a different release of the > git-tools without re-compiling the potty. True, but *please* stop calling "git wrapper" a potty. It gives me an impression that it is not connected to the plumbing. I do not do Porcelain, but I do not do plastics nor glass either ;-). Thanks. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-11 0:57 ` Junio C Hamano @ 2006-01-11 11:57 ` Andreas Ericsson 2006-01-11 17:11 ` Jon Loeliger 0 siblings, 1 reply; 33+ messages in thread From: Andreas Ericsson @ 2006-01-11 11:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: git Junio C Hamano wrote: > > True, but *please* stop calling "git wrapper" a potty. It gives > me an impression that it is not connected to the plumbing. > Aww... And here I was being quite pleased with this wording in the man-page: "The program git is just a wrapper to reach the core git programs (or a potty if you like, as it's not exactly porcelain but still brings your stuff to the plumbing)." -- Andreas Ericsson andreas.ericsson@op5.se OP5 AB www.op5.se Tel: +46 8-230225 Fax: +46 8-230231 ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-11 11:57 ` Andreas Ericsson @ 2006-01-11 17:11 ` Jon Loeliger 0 siblings, 0 replies; 33+ messages in thread From: Jon Loeliger @ 2006-01-11 17:11 UTC (permalink / raw) To: Andreas Ericsson; +Cc: Junio C Hamano, Git List On Wed, 2006-01-11 at 05:57, Andreas Ericsson wrote: > Junio C Hamano wrote: > > > > True, but *please* stop calling "git wrapper" a potty. It gives > > me an impression that it is not connected to the plumbing. > > > > Aww... And here I was being quite pleased with this wording in the man-page: > > "The program git is just a wrapper to reach the core git programs (or a > potty if you like, as it's not exactly porcelain but still brings your > stuff to the plumbing)." Oh man. So, do we have to go with "kitchen sink" instead? ;-) jdl ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s 2006-01-10 19:55 ` Johannes Schindelin 2006-01-10 20:31 ` Michal Ostrowski @ 2006-01-10 21:09 ` Junio C Hamano 1 sibling, 0 replies; 33+ messages in thread From: Junio C Hamano @ 2006-01-10 21:09 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Michal Ostrowski, Andreas Ericsson, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > Why? If what is prepended to PATH only contains git programs? Recall the "diff" example Michal gave us. ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2006-01-12 5:38 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-01-09 23:34 [PATCH 0/2] Remember and use GIT_EXEC_PATH on exec()'s Michal Ostrowski
2006-01-09 23:35 ` [PATCH 1/2] " Michal Ostrowski
2006-01-10 2:53 ` Junio C Hamano
2006-01-09 23:36 ` [PATCH 2/2] " Michal Ostrowski
2006-01-10 2:52 ` Junio C Hamano
2006-01-10 13:36 ` Michal Ostrowski
2006-01-10 15:01 ` Andreas Ericsson
2006-01-10 16:26 ` Michal Ostrowski
2006-01-10 19:13 ` Andreas Ericsson
2006-01-10 20:15 ` Alex Riesen
2006-01-10 20:32 ` Michal Ostrowski
[not found] ` <7vu0cb6f1n.fsf@assigned-by-dhcp.cox.net>
2006-01-10 20:29 ` Michal Ostrowski
2006-01-11 0:06 ` Andreas Ericsson
2006-01-11 0:42 ` Junio C Hamano
2006-01-11 2:09 ` Michal Ostrowski
2006-01-11 2:12 ` [PATCH] Exec git programs without using PATH Michal Ostrowski
2006-01-11 6:13 ` Junio C Hamano
2006-01-11 17:05 ` [PATCH] (Updated) " Michal Ostrowski
2006-01-11 20:33 ` Junio C Hamano
2006-01-11 20:42 ` Linus Torvalds
2006-01-11 21:26 ` Michal Ostrowski
2006-01-11 21:32 ` Junio C Hamano
2006-01-12 0:11 ` Andreas Ericsson
2006-01-12 5:38 ` H. Peter Anvin
2006-01-10 19:47 ` [PATCH 2/2] Remember and use GIT_EXEC_PATH on exec()'s Junio C Hamano
2006-01-10 19:55 ` Johannes Schindelin
2006-01-10 20:31 ` Michal Ostrowski
2006-01-10 21:03 ` Johannes Schindelin
2006-01-11 0:10 ` Andreas Ericsson
2006-01-11 0:57 ` Junio C Hamano
2006-01-11 11:57 ` Andreas Ericsson
2006-01-11 17:11 ` Jon Loeliger
2006-01-10 21:09 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).