From 2292ce4785170d5502c4c9ea860bb73c6379f029 Mon Sep 17 00:00:00 2001 From: Stephan Beyer Date: Fri, 16 Jan 2009 20:09:58 +0100 Subject: [PATCH 1/5] checkout: don't crash on file checkout before running post-checkout hook In the case of git init echo exit >.git/hooks/post-checkout chmod +x .git/hooks/post-checkout touch foo git add foo rm foo git checkout -- foo git-checkout resulted in a Segmentation fault, because there is no new branch set for the post-checkout hook. This patch makes use of the null SHA as it is set for the old branch. While at it, I removed the xstrdup() around the sha1_to_hex(...) calls in builtin-checkout.c/post_checkout_hook() because sha1_to_hex() uses four buffers for the hex-dumped SHA and we only need two. (Duplicating one buffer is only needed if we need more than four.) Signed-off-by: Stephan Beyer Signed-off-by: Junio C Hamano --- builtin-checkout.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index b5dd9c07b4..149343e3a9 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -47,8 +47,10 @@ static int post_checkout_hook(struct commit *old, struct commit *new, memset(&proc, 0, sizeof(proc)); argv[0] = name; - argv[1] = xstrdup(sha1_to_hex(old ? old->object.sha1 : null_sha1)); - argv[2] = xstrdup(sha1_to_hex(new->object.sha1)); + argv[1] = sha1_to_hex(old ? old->object.sha1 : null_sha1); + argv[2] = sha1_to_hex(new ? new->object.sha1 : null_sha1); + /* "new" can be NULL when checking out from the index before + a commit exists. */ argv[3] = changed ? "1" : "0"; argv[4] = NULL; proc.argv = argv; From ae98a0089ff7f7641ed15ddd595797de56eb49f1 Mon Sep 17 00:00:00 2001 From: Stephan Beyer Date: Fri, 16 Jan 2009 20:09:59 +0100 Subject: [PATCH 2/5] Move run_hook() from builtin-commit.c into run-command.c (libgit) A function that runs a hook is used in several Git commands. builtin-commit.c has the one that is most general for cases without piping. The one in builtin-gc.c prints some useful warnings. This patch moves a merged version of these variants into libgit and lets the other builtins use this libified run_hook(). The run_hook() function used in receive-pack.c feeds the standard input of the pre-receive or post-receive hooks. This function is renamed to run_receive_hook() because the libified run_hook() cannot handle this. Mentored-by: Daniel Barkalow Mentored-by: Christian Couder Signed-off-by: Stephan Beyer Signed-off-by: Junio C Hamano --- builtin-checkout.c | 22 +++++---------------- builtin-commit.c | 34 ------------------------------- builtin-gc.c | 30 +--------------------------- builtin-merge.c | 31 +---------------------------- builtin-receive-pack.c | 6 +++--- run-command.c | 45 ++++++++++++++++++++++++++++++++++++++++++ run-command.h | 2 ++ 7 files changed, 57 insertions(+), 113 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index 149343e3a9..275176d15d 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -38,25 +38,13 @@ struct checkout_opts { static int post_checkout_hook(struct commit *old, struct commit *new, int changed) { - struct child_process proc; - const char *name = git_path("hooks/post-checkout"); - const char *argv[5]; - - if (access(name, X_OK) < 0) - return 0; - - memset(&proc, 0, sizeof(proc)); - argv[0] = name; - argv[1] = sha1_to_hex(old ? old->object.sha1 : null_sha1); - argv[2] = sha1_to_hex(new ? new->object.sha1 : null_sha1); + return run_hook(NULL, "post-checkout", + sha1_to_hex(old ? old->object.sha1 : null_sha1), + sha1_to_hex(new ? new->object.sha1 : null_sha1), + changed ? "1" : "0", NULL); /* "new" can be NULL when checking out from the index before a commit exists. */ - argv[3] = changed ? "1" : "0"; - argv[4] = NULL; - proc.argv = argv; - proc.no_stdin = 1; - proc.stdout_to_stderr = 1; - return run_command(&proc); + } static int update_some(const unsigned char *sha1, const char *base, int baselen, diff --git a/builtin-commit.c b/builtin-commit.c index e88b78f811..6f8d9fbfe6 100644 --- a/builtin-commit.c +++ b/builtin-commit.c @@ -361,40 +361,6 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int return s.commitable; } -static int run_hook(const char *index_file, const char *name, ...) -{ - struct child_process hook; - const char *argv[10], *env[2]; - char index[PATH_MAX]; - va_list args; - int i; - - va_start(args, name); - argv[0] = git_path("hooks/%s", name); - i = 0; - do { - if (++i >= ARRAY_SIZE(argv)) - die ("run_hook(): too many arguments"); - argv[i] = va_arg(args, const char *); - } while (argv[i]); - va_end(args); - - snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file); - env[0] = index; - env[1] = NULL; - - if (access(argv[0], X_OK) < 0) - return 0; - - memset(&hook, 0, sizeof(hook)); - hook.argv = argv; - hook.no_stdin = 1; - hook.stdout_to_stderr = 1; - hook.env = env; - - return run_command(&hook); -} - static int is_a_merge(const unsigned char *sha1) { struct commit *commit = lookup_commit(sha1); diff --git a/builtin-gc.c b/builtin-gc.c index f8eae4adb4..a2014388da 100644 --- a/builtin-gc.c +++ b/builtin-gc.c @@ -144,34 +144,6 @@ static int too_many_packs(void) return gc_auto_pack_limit <= cnt; } -static int run_hook(void) -{ - const char *argv[2]; - struct child_process hook; - int ret; - - argv[0] = git_path("hooks/pre-auto-gc"); - argv[1] = NULL; - - if (access(argv[0], X_OK) < 0) - return 0; - - memset(&hook, 0, sizeof(hook)); - hook.argv = argv; - hook.no_stdin = 1; - hook.stdout_to_stderr = 1; - - ret = start_command(&hook); - if (ret) { - warning("Could not spawn %s", argv[0]); - return ret; - } - ret = finish_command(&hook); - if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL) - warning("%s exited due to uncaught signal", argv[0]); - return ret; -} - static int need_to_gc(void) { /* @@ -194,7 +166,7 @@ static int need_to_gc(void) else if (!too_many_loose_objects()) return 0; - if (run_hook()) + if (run_hook(NULL, "pre-auto-gc", NULL)) return 0; return 1; } diff --git a/builtin-merge.c b/builtin-merge.c index cf869751b4..e4555b0199 100644 --- a/builtin-merge.c +++ b/builtin-merge.c @@ -300,35 +300,6 @@ static void squash_message(void) strbuf_release(&out); } -static int run_hook(const char *name) -{ - struct child_process hook; - const char *argv[3], *env[2]; - char index[PATH_MAX]; - - argv[0] = git_path("hooks/%s", name); - if (access(argv[0], X_OK) < 0) - return 0; - - snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", get_index_file()); - env[0] = index; - env[1] = NULL; - - if (squash) - argv[1] = "1"; - else - argv[1] = "0"; - argv[2] = NULL; - - memset(&hook, 0, sizeof(hook)); - hook.argv = argv; - hook.no_stdin = 1; - hook.stdout_to_stderr = 1; - hook.env = env; - - return run_command(&hook); -} - static void finish(const unsigned char *new_head, const char *msg) { struct strbuf reflog_message = STRBUF_INIT; @@ -374,7 +345,7 @@ static void finish(const unsigned char *new_head, const char *msg) } /* Run a post-merge hook */ - run_hook("post-merge"); + run_hook(NULL, "post-merge", squash ? "1" : "0", NULL); strbuf_release(&reflog_message); } diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c index db67c3162c..6564a97cef 100644 --- a/builtin-receive-pack.c +++ b/builtin-receive-pack.c @@ -136,7 +136,7 @@ static int hook_status(int code, const char *hook_name) } } -static int run_hook(const char *hook_name) +static int run_receive_hook(const char *hook_name) { static char buf[sizeof(commands->old_sha1) * 2 + PATH_MAX + 4]; struct command *cmd; @@ -358,7 +358,7 @@ static void execute_commands(const char *unpacker_error) return; } - if (run_hook(pre_receive_hook)) { + if (run_receive_hook(pre_receive_hook)) { while (cmd) { cmd->error_string = "pre-receive hook declined"; cmd = cmd->next; @@ -627,7 +627,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix) unlink(pack_lockfile); if (report_status) report(unpack_status); - run_hook(post_receive_hook); + run_receive_hook(post_receive_hook); run_update_post_hook(commands); } return 0; diff --git a/run-command.c b/run-command.c index c90cdc50e3..49810a835e 100644 --- a/run-command.c +++ b/run-command.c @@ -342,3 +342,48 @@ int finish_async(struct async *async) #endif return ret; } + +int run_hook(const char *index_file, const char *name, ...) +{ + struct child_process hook; + const char *argv[10], *env[2]; + char index[PATH_MAX]; + va_list args; + int ret; + int i; + + va_start(args, name); + argv[0] = git_path("hooks/%s", name); + i = 0; + do { + if (++i >= ARRAY_SIZE(argv)) + die("run_hook(): too many arguments"); + argv[i] = va_arg(args, const char *); + } while (argv[i]); + va_end(args); + + if (access(argv[0], X_OK) < 0) + return 0; + + memset(&hook, 0, sizeof(hook)); + hook.argv = argv; + hook.no_stdin = 1; + hook.stdout_to_stderr = 1; + if (index_file) { + snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file); + env[0] = index; + env[1] = NULL; + hook.env = env; + } + + ret = start_command(&hook); + if (ret) { + warning("Could not spawn %s", argv[0]); + return ret; + } + ret = finish_command(&hook); + if (ret == -ERR_RUN_COMMAND_WAITPID_SIGNAL) + warning("%s exited due to uncaught signal", argv[0]); + + return ret; +} diff --git a/run-command.h b/run-command.h index a8b0c209e9..0211e1d471 100644 --- a/run-command.h +++ b/run-command.h @@ -49,6 +49,8 @@ int start_command(struct child_process *); int finish_command(struct child_process *); int run_command(struct child_process *); +extern int run_hook(const char *index_file, const char *name, ...); + #define RUN_COMMAND_NO_STDIN 1 #define RUN_GIT_CMD 2 /*If this is to be git sub-command */ #define RUN_COMMAND_STDOUT_TO_STDERR 4 From 35d5ae679c2a6bc2a5f70f9209844ae8c14ae9e3 Mon Sep 17 00:00:00 2001 From: Stephan Beyer Date: Fri, 16 Jan 2009 20:10:00 +0100 Subject: [PATCH 3/5] api-run-command.txt: talk about run_hook() Signed-off-by: Stephan Beyer Signed-off-by: Junio C Hamano --- Documentation/technical/api-run-command.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt index 82e9e831b6..13e7b6361b 100644 --- a/Documentation/technical/api-run-command.txt +++ b/Documentation/technical/api-run-command.txt @@ -52,6 +52,21 @@ Functions Wait for the completion of an asynchronous function that was started with start_async(). +`run_hook`:: + + Run a hook. + The first argument is a pathname to an index file, or NULL + if the hook uses the default index file or no index is needed. + The second argument is the name of the hook. + The further arguments (up to 9) correspond to the hook arguments. + The last argument has to be NULL to terminate the arguments list. + If the hook does not exist or is not executable, the return + value will be zero. + If it is executable, the hook will be executed and the exit + status of the hook is returned. + On execution, .stdout_to_stderr and .no_stdin will be set. + (See below.) + Data structures --------------- From cf94ca8ea98373d6f5e9caaa764eca89b20bfb63 Mon Sep 17 00:00:00 2001 From: Stephan Beyer Date: Fri, 16 Jan 2009 20:10:01 +0100 Subject: [PATCH 4/5] run_hook(): check the executability of the hook before filling argv Signed-off-by: Stephan Beyer Signed-off-by: Junio C Hamano --- run-command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index 49810a835e..fc54c07a93 100644 --- a/run-command.c +++ b/run-command.c @@ -352,6 +352,9 @@ int run_hook(const char *index_file, const char *name, ...) int ret; int i; + if (access(git_path("hooks/%s", name), X_OK) < 0) + return 0; + va_start(args, name); argv[0] = git_path("hooks/%s", name); i = 0; @@ -362,9 +365,6 @@ int run_hook(const char *index_file, const char *name, ...) } while (argv[i]); va_end(args); - if (access(argv[0], X_OK) < 0) - return 0; - memset(&hook, 0, sizeof(hook)); hook.argv = argv; hook.no_stdin = 1; From 14e6298f1215da503f0f65b63e13d674dd781868 Mon Sep 17 00:00:00 2001 From: Stephan Beyer Date: Sat, 17 Jan 2009 04:02:55 +0100 Subject: [PATCH 5/5] run_hook(): allow more than 9 hook arguments This is done using the ALLOC_GROW macro. Signed-off-by: Stephan Beyer Signed-off-by: Junio C Hamano --- Documentation/technical/api-run-command.txt | 2 +- run-command.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/technical/api-run-command.txt b/Documentation/technical/api-run-command.txt index 13e7b6361b..2efe7a40be 100644 --- a/Documentation/technical/api-run-command.txt +++ b/Documentation/technical/api-run-command.txt @@ -58,7 +58,7 @@ Functions The first argument is a pathname to an index file, or NULL if the hook uses the default index file or no index is needed. The second argument is the name of the hook. - The further arguments (up to 9) correspond to the hook arguments. + The further arguments correspond to the hook arguments. The last argument has to be NULL to terminate the arguments list. If the hook does not exist or is not executable, the return value will be zero. diff --git a/run-command.c b/run-command.c index fc54c07a93..db9ce59204 100644 --- a/run-command.c +++ b/run-command.c @@ -346,23 +346,22 @@ int finish_async(struct async *async) int run_hook(const char *index_file, const char *name, ...) { struct child_process hook; - const char *argv[10], *env[2]; + const char **argv = NULL, *env[2]; char index[PATH_MAX]; va_list args; int ret; - int i; + size_t i = 0, alloc = 0; if (access(git_path("hooks/%s", name), X_OK) < 0) return 0; va_start(args, name); - argv[0] = git_path("hooks/%s", name); - i = 0; - do { - if (++i >= ARRAY_SIZE(argv)) - die("run_hook(): too many arguments"); - argv[i] = va_arg(args, const char *); - } while (argv[i]); + ALLOC_GROW(argv, i + 1, alloc); + argv[i++] = git_path("hooks/%s", name); + while (argv[i-1]) { + ALLOC_GROW(argv, i + 1, alloc); + argv[i++] = va_arg(args, const char *); + } va_end(args); memset(&hook, 0, sizeof(hook)); @@ -377,6 +376,7 @@ int run_hook(const char *index_file, const char *name, ...) } ret = start_command(&hook); + free(argv); if (ret) { warning("Could not spawn %s", argv[0]); return ret;