From patchwork Thu Aug 19 03:34:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446087 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 42EA1C4320A for ; Thu, 19 Aug 2021 03:35:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1CA8C610CE for ; Thu, 19 Aug 2021 03:35:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236207AbhHSDfe (ORCPT ); Wed, 18 Aug 2021 23:35:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33584 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235743AbhHSDfc (ORCPT ); Wed, 18 Aug 2021 23:35:32 -0400 Received: from mail-qt1-x849.google.com (mail-qt1-x849.google.com [IPv6:2607:f8b0:4864:20::849]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 56AE5C061764 for ; Wed, 18 Aug 2021 20:34:57 -0700 (PDT) Received: by mail-qt1-x849.google.com with SMTP id w19-20020ac87e930000b029025a2609eb04so2014141qtj.17 for ; Wed, 18 Aug 2021 20:34:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=8BbJ6Qb2PqmNaSkvqwYErTCq3vQRssY7Pja9+YSzrXQ=; b=RhgLck4VFDWTPXo5CPe6xo/+mFG6J0h6oQ0Y0tf68OONhU8+scxnsFnh1qxmqmfJIl 63I6CxbFIsNk3BLgChA4eSKCMEe65B1+ceTHJnh+wMVCssFFDLozdO5ek5tah5ZMGM40 NJr8v+G7z384hTp5OwHH7kNGIOBOyTa8+TH/5zKXUhR9NB711OMUM1hP+cqnmF6WTk+0 jjOaxEKvRx7klZrUlIO8xGBWfiIcdD+vNDQDjpvXsehNHtQ1+Whz3xEzf0FDF75Jr/ZT oACgZQV2Iirq/ZDwGggIZv6NnBKdeXT4Mda1GaVl+MK8FDeaiMWM6MsTxtg8Wu4mnTbR Zeig== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=8BbJ6Qb2PqmNaSkvqwYErTCq3vQRssY7Pja9+YSzrXQ=; b=p/HDcetSO3Mkg/xPQd7l195v/JNrxmp+ZcTE+P9fCvP/qms6GZWMQmx7fVITf42LE7 /6Og22InAOJOB0j3khuN79+xl6tepv1mC1TspZlyEKN3J+oqesD+TbJP031hDd0mV3el EpzucnvCXr84g46/WRKLSHugx3pOBLasn7mXqIDjuEfF2sVebR8iTbl1Ywh6JQqMQvVN d82agE4kPBy56lDcWpBgCaz9i2gGNhedkq3Y27HdIjjlWxXAcnDhVv9mkmd6IOcOvskU jF69HsFMlpmQ5b7Ll1kVxPceMfCmotHgxJFw1als8R9mDeGP7IoKxr1H4/w3RCl+5k0f 6fUQ== X-Gm-Message-State: AOAM531b0rWmhlInrU8fO9A81U7612+tHk45icIX9U5jaMyfdP4bmKG2 MxAaSQWi6dzE4Gjp2UhP8/tgdic5YxdN0G3WW+PQ0JCWj2iqL/5hqkBgrJHIBCPT2NiOoOLdt3x q0po2nGpK9SBXDg4XEQfC2OXx9Sw6c6VZjBdfa18otTVpZa1ODcdcNDP1dv1HfwWpB/fdgKguvg == X-Google-Smtp-Source: ABdhPJz9FUSFAnGh5vZM2SgD0+G2hxdkYbL7sfIOLmX2ui0M2RMAcOsu0G4bqJ8/gD6p1EXDZxcQ5ooT+nzh5UE/3EI= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a05:6214:d04:: with SMTP id 4mr5108826qvh.36.1629344096527; Wed, 18 Aug 2021 20:34:56 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:45 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-2-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 1/6] hook: run a list of hooks instead From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org To prepare for multihook support, teach hook.[hc] to take a list of hooks at run_hooks and run_found_hooks. Right now the list is always one entry, but in the future we will allow users to supply more than one executable for a single hook event. Signed-off-by: Emily Shaffer --- builtin/hook.c | 14 ++++--- hook.c | 104 +++++++++++++++++++++++++++++++++++-------------- hook.h | 16 +++++++- 3 files changed, 96 insertions(+), 38 deletions(-) diff --git a/builtin/hook.c b/builtin/hook.c index 27dce6a2f0..e18357ba1f 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -25,7 +25,8 @@ static int run(int argc, const char **argv, const char *prefix) struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; int ignore_missing = 0; const char *hook_name; - const char *hook_path; + struct list_head *hooks; + struct option run_options[] = { OPT_BOOL(0, "ignore-missing", &ignore_missing, N_("exit quietly with a zero exit code if the requested hook cannot be found")), @@ -58,15 +59,16 @@ static int run(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); hook_name = argv[0]; - if (ignore_missing) - return run_hooks_oneshot(hook_name, &opt); - hook_path = find_hook(hook_name); - if (!hook_path) { + hooks = list_hooks(hook_name); + if (list_empty(hooks)) { + /* ... act like run_hooks_oneshot() under --ignore-missing */ + if (ignore_missing) + return 0; error("cannot find a hook named %s", hook_name); return 1; } - ret = run_hooks(hook_name, hook_path, &opt); + ret = run_hooks(hook_name, hooks, &opt); run_hooks_opt_clear(&opt); return ret; usage: diff --git a/hook.c b/hook.c index ee20b2e365..333cfd633a 100644 --- a/hook.c +++ b/hook.c @@ -4,6 +4,27 @@ #include "hook-list.h" #include "config.h" +static void free_hook(struct hook *ptr) +{ + if (ptr) + free(ptr->feed_pipe_cb_data); + free(ptr); +} + +static void remove_hook(struct list_head *to_remove) +{ + struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); + list_del(to_remove); + free_hook(hook_to_remove); +} + +void clear_hook_list(struct list_head *head) +{ + struct list_head *pos, *tmp; + list_for_each_safe(pos, tmp, head) + remove_hook(pos); +} + static int known_hook(const char *name) { const char **p; @@ -68,7 +89,31 @@ const char *find_hook(const char *name) int hook_exists(const char *name) { - return !!find_hook(name); + return !list_empty(list_hooks(name)); +} + +struct list_head *list_hooks(const char *hookname) +{ + struct list_head *hook_head = xmalloc(sizeof(struct list_head)); + + INIT_LIST_HEAD(hook_head); + + if (!hookname) + BUG("null hookname was provided to hook_list()!"); + + if (have_git_dir()) { + const char *hook_path = find_hook(hookname); + + /* Add the hook from the hookdir */ + if (hook_path) { + struct hook *to_add = xmalloc(sizeof(*to_add)); + to_add->hook_path = hook_path; + to_add->feed_pipe_cb_data = NULL; + list_add_tail(&to_add->list, hook_head); + } + } + + return hook_head; } void run_hooks_opt_clear(struct run_hooks_opt *o) @@ -128,7 +173,10 @@ static int pick_next_hook(struct child_process *cp, cp->dir = hook_cb->options->dir; /* add command */ - strvec_push(&cp->args, run_me->hook_path); + if (hook_cb->options->absolute_path) + strvec_push(&cp->args, absolute_path(run_me->hook_path)); + else + strvec_push(&cp->args, run_me->hook_path); /* * add passed-in argv, without expanding - let the user get back @@ -139,12 +187,12 @@ static int pick_next_hook(struct child_process *cp, /* Provide context for errors if necessary */ *pp_task_cb = run_me; - /* - * This pick_next_hook() will be called again, we're only - * running one hook, so indicate that no more work will be - * done. - */ - hook_cb->run_me = NULL; + /* Get the next entry ready */ + if (hook_cb->run_me->list.next == hook_cb->head) + hook_cb->run_me = NULL; + else + hook_cb->run_me = list_entry(hook_cb->run_me->list.next, + struct hook, list); return 1; } @@ -179,13 +227,9 @@ static int notify_hook_finished(int result, return 0; } -int run_hooks(const char *hook_name, const char *hook_path, - struct run_hooks_opt *options) +int run_hooks(const char *hook_name, struct list_head *hooks, + struct run_hooks_opt *options) { - struct strbuf abs_path = STRBUF_INIT; - struct hook my_hook = { - .hook_path = hook_path, - }; struct hook_cb_data cb_data = { .rc = 0, .hook_name = hook_name, @@ -197,11 +241,9 @@ int run_hooks(const char *hook_name, const char *hook_path, if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); - if (options->absolute_path) { - strbuf_add_absolute_path(&abs_path, hook_path); - my_hook.hook_path = abs_path.buf; - } - cb_data.run_me = &my_hook; + + cb_data.head = hooks; + cb_data.run_me = list_first_entry(hooks, struct hook, list); run_processes_parallel_tr2(jobs, pick_next_hook, @@ -213,18 +255,15 @@ int run_hooks(const char *hook_name, const char *hook_path, "hook", hook_name); - - if (options->absolute_path) - strbuf_release(&abs_path); - free(my_hook.feed_pipe_cb_data); + clear_hook_list(hooks); return cb_data.rc; } int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options) { - const char *hook_path; - int ret; + struct list_head *hooks; + int ret = 0; struct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT; if (!options) @@ -233,14 +272,19 @@ int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options) if (options->path_to_stdin && options->feed_pipe) BUG("choose only one method to populate stdin"); - hook_path = find_hook(hook_name); - if (!hook_path) { - ret = 0; + hooks = list_hooks(hook_name); + + /* + * If you need to act on a missing hook, use run_found_hooks() + * instead + */ + if (list_empty(hooks)) goto cleanup; - } - ret = run_hooks(hook_name, hook_path, options); + ret = run_hooks(hook_name, hooks, options); + cleanup: run_hooks_opt_clear(options); + clear_hook_list(hooks); return ret; } diff --git a/hook.h b/hook.h index 58dfbf474c..12b56616bb 100644 --- a/hook.h +++ b/hook.h @@ -3,6 +3,7 @@ #include "strbuf.h" #include "strvec.h" #include "run-command.h" +#include "list.h" /* * Returns the path to the hook file, or NULL if the hook is missing @@ -17,6 +18,7 @@ const char *find_hook(const char *name); int hook_exists(const char *hookname); struct hook { + struct list_head list; /* The path to the hook */ const char *hook_path; @@ -27,6 +29,12 @@ struct hook { void *feed_pipe_cb_data; }; +/* + * Provides a linked list of 'struct hook' detailing commands which should run + * in response to the 'hookname' event, in execution order. + */ +struct list_head *list_hooks(const char *hookname); + struct run_hooks_opt { /* Environment vars to be set for each hook */ @@ -97,6 +105,7 @@ struct hook_cb_data { /* rc reflects the cumulative failure state */ int rc; const char *hook_name; + struct list_head *head; struct hook *run_me; struct run_hooks_opt *options; int *invoked_hook; @@ -110,8 +119,8 @@ void run_hooks_opt_clear(struct run_hooks_opt *o); * * See run_hooks_oneshot() for the simpler one-shot API. */ -int run_hooks(const char *hookname, const char *hook_path, - struct run_hooks_opt *options); +int run_hooks(const char *hookname, struct list_head *hooks, + struct run_hooks_opt *options); /** * Calls find_hook() on your "hook_name" and runs the hooks (if any) @@ -123,4 +132,7 @@ int run_hooks(const char *hookname, const char *hook_path, */ int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options); +/* Empties the list at 'head', calling 'free_hook()' on each entry */ +void clear_hook_list(struct list_head *head); + #endif From patchwork Thu Aug 19 03:34:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446089 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9E32DC4338F for ; Thu, 19 Aug 2021 03:35:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 82AEC610CE for ; Thu, 19 Aug 2021 03:35:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236289AbhHSDfi (ORCPT ); Wed, 18 Aug 2021 23:35:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33594 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235743AbhHSDff (ORCPT ); Wed, 18 Aug 2021 23:35:35 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9E35DC061764 for ; Wed, 18 Aug 2021 20:34:59 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id d69-20020a25e6480000b02904f4a117bd74so5196760ybh.17 for ; Wed, 18 Aug 2021 20:34:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc:content-transfer-encoding; bh=wA9y4rVq7Y692BXWnOq2NxWoqSRyWczEgp+zizC9618=; b=TWFHeWCp+FMXduJLA85FOsAUiG2GMjAfyp8/G7jeU2MhB9E9QjR8GPjHzmOgrxUZIX WY7AhMQ0o+IYNF74Cis/VYQRwqa8taQWq5+pBq+mUAnfrIFsQfIdNBby8QlM3fAUzk0u tj0WRPTrMx0Pt49KBOEuZYtYMTeKau+ZNjScuSlYaZshxJVJUk8Vn2wxu9+JqfD8noXP siOgwh/3OAtHm0PVMr+L7QlfTajI2TBataCBHSdGNaaPasc8emAIrdGhaTOam9D0Ag3N HHhOAoqKhyYySYVMx87K82VkJeY+1GBSxt/HVE6g9hOlaJwOvYgadL3kBTliWL4C+Cgk gKnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc:content-transfer-encoding; bh=wA9y4rVq7Y692BXWnOq2NxWoqSRyWczEgp+zizC9618=; b=BzVUFbliXrGHIr6W7iJZLlZU3h4P+vbEWZbljJs6fGMIdiifrLNvXSjwvMLz3yFVgg VD8y5QC7msycj7gTTwj4Qt0RSDtulgnChiL1K3wMGprljPQW26uP78MPU6CImNVujB3N wyyqls/jV48c7zvvL/w1UwYaq4ds2rHZnkFCgz4V10TS/iI9wCEXqOi9PkZqhSgwtsS2 HUDIv0S2hIOOXiz5kjpYFau8TEOcL256hc23I5f/1IoWNf0oFdXLSiLU9UH43qBisvhy FP/HTWXVxYVKGF2FkytZ+fkxARlk/Om+LGY/PGlo37IyZzbQVZRyjY+tEgxJj3t/UsZk KyEQ== X-Gm-Message-State: AOAM5322tAMCu+3lvsXcAHk3BJzBrAfsnnsa4YjIavUbStBpwxQ1Hxa5 SlTU1vssam81iyHy0tl4o8cZGACMfDqOsd0sUeCCgNL77nE982DCHb/w78ash0Jrl27yEV9qN0n cmJ3d3GZgXpNRlV9WOzsjOwyUx/mjBR7YrNrmnEIB4rBVyCLPU8eBs7dt/6l4v3/xqZZ/3WYGjA == X-Google-Smtp-Source: ABdhPJwZMXZJ34x2Jm8n8g2XnJV8cSSiX2O6C0+r3pcmUtl8sjxq44ZwjkM2yDFOm+ut26zsut9gsu9oyW7I2UBkXSk= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a25:cb0a:: with SMTP id b10mr15550132ybg.137.1629344098806; Wed, 18 Aug 2021 20:34:58 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:46 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-3-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 2/6] hook: allow parallel hook execution From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer , " =?utf-8?b?w4Z2YXIgQXJuZmo=?= =?utf-8?b?w7Zyw7AgQmphcm1hc29u?= " Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In many cases, there's no reason not to allow hooks to execute in parallel, if more than one was provided. hook.c already calls run_processes_parallel(), so all we need to do is allow the job count we hand to run_processes_parallel() to be greater than 1. If users have specified no alternative, we can use the processor count from online_cpus() to run an efficient number of tasks at once. However, users can also override this number if they want by configuring 'hook.jobs'. To avoid looking up 'hook.jobs' in cases where we don't end up with any hooks to run anyways, teach the hook runner commands to notice if .jobs==0 and do a config or online_cpus() lookup if so, when we already know we have jobs to run. Serial execution can still be performed by setting .jobs == 1. Signed-off-by: Emily Shaffer Helped-by: Ævar Arnfjörð Bjarmason --- Documentation/config/hook.txt | 4 ++++ Documentation/git-hook.txt | 17 ++++++++++++++++- builtin/am.c | 4 ++-- builtin/checkout.c | 2 +- builtin/clone.c | 2 +- builtin/hook.c | 4 +++- builtin/merge.c | 2 +- builtin/rebase.c | 2 +- builtin/receive-pack.c | 9 +++++---- builtin/worktree.c | 2 +- commit.c | 2 +- hook.c | 36 +++++++++++++++++++++++++++++++---- hook.h | 24 ++++++++++++++++++----- read-cache.c | 2 +- refs.c | 2 +- reset.c | 3 ++- sequencer.c | 4 ++-- transport.c | 2 +- 18 files changed, 94 insertions(+), 29 deletions(-) create mode 100644 Documentation/config/hook.txt diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt new file mode 100644 index 0000000000..96d3d6572c --- /dev/null +++ b/Documentation/config/hook.txt @@ -0,0 +1,4 @@ +hook.jobs:: + Specifies how many hooks can be run simultaneously during parallelized + hook execution. If unspecified, defaults to the number of processors on + the current system. diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index fa68c1f391..8bf82b5dd4 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -8,7 +8,8 @@ git-hook - run git hooks SYNOPSIS -------- [verse] -'git hook' run [--to-stdin=] [--ignore-missing] [-- ] +'git hook' run [--to-stdin=] [--ignore-missing] [(-j|--jobs) ] + [-- ] DESCRIPTION ----------- @@ -42,6 +43,20 @@ OPTIONS tools that want to do a blind one-shot run of a hook that may or may not be present. +-j:: +--jobs:: + Only valid for `run`. ++ +Specify how many hooks to run simultaneously. If this flag is not specified, use +the value of the `hook.jobs` config. If the config is not specified, use the +number of CPUs on the current system. Some hooks may be ineligible for +parallelization: for example, 'commit-msg' intends hooks modify the commit +message body and cannot be parallelized. + +CONFIGURATION +------------- +include::config/hook.txt[] + SEE ALSO -------- linkgit:githooks[5] diff --git a/builtin/am.c b/builtin/am.c index 9e3d4d9ab4..c7ffc7eafc 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -446,7 +446,7 @@ static void am_destroy(const struct am_state *state) static int run_applypatch_msg_hook(struct am_state *state) { int ret; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; assert(state->msg); strvec_push(&opt.args, am_path(state, "final-commit")); @@ -467,7 +467,7 @@ static int run_applypatch_msg_hook(struct am_state *state) */ static int run_post_rewrite_hook(const struct am_state *state) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; strvec_push(&opt.args, "rebase"); opt.path_to_stdin = am_path(state, "rewritten"); diff --git a/builtin/checkout.c b/builtin/checkout.c index 6d69b4c011..2cb5c67f64 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -107,7 +107,7 @@ struct branch_info { static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit, int changed) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; /* "new_commit" can be NULL when checking out from the index before a commit exists. */ diff --git a/builtin/clone.c b/builtin/clone.c index 27fc05ee51..986c3b1932 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -776,7 +776,7 @@ static int checkout(int submodule_progress) struct tree *tree; struct tree_desc t; int err = 0; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_SERIAL; if (option_no_checkout) return 0; diff --git a/builtin/hook.c b/builtin/hook.c index e18357ba1f..4dd3617876 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -22,7 +22,7 @@ static const char * const builtin_hook_run_usage[] = { static int run(int argc, const char **argv, const char *prefix) { int i; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; int ignore_missing = 0; const char *hook_name; struct list_head *hooks; @@ -32,6 +32,8 @@ static int run(int argc, const char **argv, const char *prefix) N_("exit quietly with a zero exit code if the requested hook cannot be found")), OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), N_("file to read into hooks' stdin")), + OPT_INTEGER('j', "jobs", &opt.jobs, + N_("run up to hooks simultaneously")), OPT_END(), }; int ret; diff --git a/builtin/merge.c b/builtin/merge.c index 9bd4a2532c..81d7ebbbf6 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -448,7 +448,7 @@ static void finish(struct commit *head_commit, const struct object_id *new_head, const char *msg) { struct strbuf reflog_message = STRBUF_INIT; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; const struct object_id *head = &head_commit->object.oid; if (!msg) diff --git a/builtin/rebase.c b/builtin/rebase.c index e7c668c99b..ad118c983c 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1314,7 +1314,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) char *squash_onto_name = NULL; int reschedule_failed_exec = -1; int allow_preemptive_ff = 1; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_PARALLEL; struct option builtin_rebase_options[] = { OPT_STRING(0, "onto", &options.onto_name, N_("revision"), diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ebec6f3bb1..7460124b74 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -909,7 +909,7 @@ static int run_receive_hook(struct command *commands, int skip_broken, const struct string_list *push_options) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; struct receive_hook_feed_context ctx; struct command *iter = commands; @@ -948,7 +948,7 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; strvec_pushl(&opt.args, cmd->ref_name, @@ -1432,7 +1432,8 @@ static const char *push_to_checkout(unsigned char *hash, struct strvec *env, const char *work_tree) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; + opt.invoked_hook = invoked_hook; strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); @@ -1628,7 +1629,7 @@ static const char *update(struct command *cmd, struct shallow_info *si) static void run_update_post_hook(struct command *commands) { struct command *cmd; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) diff --git a/builtin/worktree.c b/builtin/worktree.c index 330867c19b..3090506790 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -382,7 +382,7 @@ static int add_worktree(const char *path, const char *refname, * is_junk is cleared, but do return appropriate code when hook fails. */ if (!ret && opts->checkout) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); strvec_pushl(&opt.args, diff --git a/commit.c b/commit.c index 842e47beae..a38bd04752 100644 --- a/commit.c +++ b/commit.c @@ -1700,7 +1700,7 @@ int run_commit_hook(int editor_is_used, const char *index_file, int *invoked_hook, const char *name, ...) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_SERIAL; va_list args; const char *arg; diff --git a/hook.c b/hook.c index 333cfd633a..b8420353fa 100644 --- a/hook.c +++ b/hook.c @@ -227,6 +227,28 @@ static int notify_hook_finished(int result, return 0; } +/* + * Determines how many jobs to use after we know we want to parallelize. First + * priority is the config 'hook.jobs' and second priority is the number of CPUs. + */ +static int configured_hook_jobs(void) +{ + /* + * The config and the CPU count probably won't change during the process + * lifetime, so cache the result in case we invoke multiple hooks during + * one process. + */ + static int jobs = 0; + if (jobs) + return jobs; + + if (git_config_get_int("hook.jobs", &jobs)) + /* if the config isn't set, fall back to CPU count. */ + jobs = online_cpus(); + + return jobs; +} + int run_hooks(const char *hook_name, struct list_head *hooks, struct run_hooks_opt *options) { @@ -236,16 +258,18 @@ int run_hooks(const char *hook_name, struct list_head *hooks, .options = options, .invoked_hook = options->invoked_hook, }; - int jobs = 1; if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); - cb_data.head = hooks; cb_data.run_me = list_first_entry(hooks, struct hook, list); - run_processes_parallel_tr2(jobs, + /* INIT_PARALLEL sets jobs to 0, so go look up how many to use. */ + if (!options->jobs) + options->jobs = configured_hook_jobs(); + + run_processes_parallel_tr2(options->jobs, pick_next_hook, notify_start_failure, options->feed_pipe, @@ -264,7 +288,11 @@ int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options) { struct list_head *hooks; int ret = 0; - struct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT; + /* + * Turn on parallelism by default. Hooks which don't want it should + * specify their options accordingly. + */ + struct run_hooks_opt hook_opt_scratch = RUN_HOOKS_OPT_INIT_PARALLEL; if (!options) options = &hook_opt_scratch; diff --git a/hook.h b/hook.h index 12b56616bb..cd3123d290 100644 --- a/hook.h +++ b/hook.h @@ -43,6 +43,13 @@ struct run_hooks_opt /* Args to be passed to each hook */ struct strvec args; + /* + * Number of threads to parallelize across. Set to 0 to use the + * 'hook.jobs' config or, if that config is unset, the number of cores + * on the system. + */ + int jobs; + /* Resolve and run the "absolute_path(hook)" instead of * "hook". Used for "git worktree" hooks */ @@ -85,11 +92,6 @@ struct run_hooks_opt int *invoked_hook; }; -#define RUN_HOOKS_OPT_INIT { \ - .env = STRVEC_INIT, \ - .args = STRVEC_INIT, \ -} - /* * To specify a 'struct string_list', set 'run_hooks_opt.feed_pipe_ctx' to the * string_list and set 'run_hooks_opt.feed_pipe' to 'pipe_from_string_list()'. @@ -111,6 +113,18 @@ struct hook_cb_data { int *invoked_hook; }; +#define RUN_HOOKS_OPT_INIT_SERIAL { \ + .jobs = 1, \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ +} + +#define RUN_HOOKS_OPT_INIT_PARALLEL { \ + .jobs = 0, \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ +} + void run_hooks_opt_clear(struct run_hooks_opt *o); /** diff --git a/read-cache.c b/read-cache.c index 90099ca14d..f157e62531 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3068,7 +3068,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l { int ret; int was_full = !istate->sparse_index; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_PARALLEL; ret = convert_to_sparse(istate); diff --git a/refs.c b/refs.c index 73d4a93926..5543b8cdab 100644 --- a/refs.c +++ b/refs.c @@ -2062,7 +2062,7 @@ int ref_update_reject_duplicates(struct string_list *refnames, static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; struct string_list to_stdin = STRING_LIST_INIT_NODUP; int ret = 0, i; diff --git a/reset.c b/reset.c index 6499bc5127..1941adb771 100644 --- a/reset.c +++ b/reset.c @@ -128,7 +128,8 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action, reflog_head); } if (run_hook) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; + strvec_pushl(&opt.args, oid_to_hex(orig ? orig : null_oid()), oid_to_hex(oid), diff --git a/sequencer.c b/sequencer.c index f451e23d0c..30fbe79b8a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1148,7 +1148,7 @@ int update_head_with_reflog(const struct commit *old_head, static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; struct strbuf tmp = STRBUF_INIT; struct string_list to_stdin = STRING_LIST_INIT_DUP; int code; @@ -4522,7 +4522,7 @@ static int pick_commits(struct repository *r, if (!stat(rebase_path_rewritten_list(), &st) && st.st_size > 0) { struct child_process notes_cp = CHILD_PROCESS_INIT; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT_PARALLEL; notes_cp.in = open(rebase_path_rewritten_list(), O_RDONLY); notes_cp.git_cmd = 1; diff --git a/transport.c b/transport.c index 4ca8fc0391..33da71a108 100644 --- a/transport.c +++ b/transport.c @@ -1204,7 +1204,7 @@ static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { int ret = 0; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_PARALLEL; struct ref *r; struct string_list to_stdin = STRING_LIST_INIT_NODUP; From patchwork Thu Aug 19 03:34:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446091 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0BCA0C432BE for ; Thu, 19 Aug 2021 03:35:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DF9E86044F for ; Thu, 19 Aug 2021 03:35:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236257AbhHSDfk (ORCPT ); Wed, 18 Aug 2021 23:35:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33608 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236268AbhHSDfh (ORCPT ); Wed, 18 Aug 2021 23:35:37 -0400 Received: from mail-qv1-xf49.google.com (mail-qv1-xf49.google.com [IPv6:2607:f8b0:4864:20::f49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BDA34C06179A for ; Wed, 18 Aug 2021 20:35:01 -0700 (PDT) Received: by mail-qv1-xf49.google.com with SMTP id u6-20020ad448660000b02903500bf28866so3624337qvy.23 for ; Wed, 18 Aug 2021 20:35:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=nGUuP5ePDBgJW4EyGrxuJF6GJyeiI/jzzY2/gZmO2XY=; b=kgfnShVtu9qJh1cCXnTWOhuNh3xIslyMDtP9rzs4O+EKjdMaSsFjk/AE7eWHUoJaCS zLqNKYEDxq5dmCC9j8mX/4q/XJQkmlCyYHazrYkrnwBqYEnJMf73ft+idkE/YKafg32D YHan3m6zZYFM+yGgtXEcmcjqswrM8XkaCt42jdjWtWzUW1JFRXcHFLypHO3J3piVpDsw Tb48w5UY9rxGUQNa9P2V1NwpzfPXUninp1eK4E21bFNu+4qg9qdsXZcKzJ/gvNDPp9dj PHmw8/K/MFCjHGiYYZ+QS4uIwiBPEHymfkQk4KBIzTUfWFNAZdoSQmNzcAD2fekGYjHL xScA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=nGUuP5ePDBgJW4EyGrxuJF6GJyeiI/jzzY2/gZmO2XY=; b=ajuNhhF6YMawTBOdqwX0lVEEpn5gu3QbfgAzseTs/31UTs5/S4hkIV0Bew/uoxVSbW 0Wr59lV8Ld1cbFanI7AHNhGLlPgVIMu3M/hqFQOm2fayPJJC1Ylk92m5q0fZMYdTClFu DmrZJf5GTzltm/14RsaYz2fXYWltep8pzGxmYNL0qK/5C0QHzSPGGYyvFBEofYNM76tg UIPEHD9bUHrCYFWa5Q6lV5cIohOSx3K365iAHVRAZhm3VFWUdUBjx+jqVrkG7db9MK8h erhvkcdAJu5jBf1yTh7wcEIENSyGHkxPRFu67zP5qtiyrLi71khpQ0L0mhpmI3T1PrI3 7//A== X-Gm-Message-State: AOAM5325CQhg7BtOZRdlztS47D9TUANI6W+pZr952r1hQ/S/p0n3vB+9 N1A7gL2o1iV25lxO00UhrchhM5zojAltskUtoeUkV7cgsT3AP1Oef9lU+HMHM/q5Z2ezt5uGEdU wmsajwRdwE272A1jCSz8PDeWW2jNEaDNzKvAwl2TRSaxtG7Y6Kw6tgW5NdRgAw1eWpiDYJfXbpg == X-Google-Smtp-Source: ABdhPJwct/bvXRcjrSJ0WLLW4UiRIaUwO6w7iqEQRbOeT7dPE3pbcWSgKrTlUrHkDJAdq8I1/rNBPMtaBvTdnTgAsyg= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a05:6214:3b1:: with SMTP id m17mr12679002qvy.60.1629344100919; Wed, 18 Aug 2021 20:35:00 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:47 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-4-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 3/6] hook: introduce "git hook list" From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org If more than one hook will be run, it may be useful to see a list of which hooks should be run. At very least, it will be useful for us to test the semantics of multihooks ourselves. For now, only list the hooks which will run in the order they will run in; later, it might be useful to include more information like where the hooks were configured and whether or not they will run. Signed-off-by: Emily Shaffer --- Documentation/git-hook.txt | 5 +++++ builtin/hook.c | 46 ++++++++++++++++++++++++++++++++++++++ hook.c | 3 +-- 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 8bf82b5dd4..701ada9fc0 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -10,6 +10,7 @@ SYNOPSIS [verse] 'git hook' run [--to-stdin=] [--ignore-missing] [(-j|--jobs) ] [-- ] +'git hook' list DESCRIPTION ----------- @@ -30,6 +31,10 @@ optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The arguments (if any) differ by hook name, see linkgit:githooks[5] for what those are. +list:: + Print a list of hooks which will be run on `` event. If no + hooks are configured for that event, print nothing and return 1. + OPTIONS ------- diff --git a/builtin/hook.c b/builtin/hook.c index 4dd3617876..d21f303eca 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -8,8 +8,11 @@ #define BUILTIN_HOOK_RUN_USAGE \ N_("git hook run [--ignore-missing] [--to-stdin=] [-- ]") +#define BUILTIN_HOOK_LIST_USAGE \ + N_("git hook list ") static const char * const builtin_hook_usage[] = { + BUILTIN_HOOK_LIST_USAGE, BUILTIN_HOOK_RUN_USAGE, NULL }; @@ -19,6 +22,47 @@ static const char * const builtin_hook_run_usage[] = { NULL }; +static const char *const builtin_hook_list_usage[] = { + BUILTIN_HOOK_LIST_USAGE, + NULL +}; + +static int list(int argc, const char **argv, const char *prefix) +{ + struct list_head *head, *pos; + const char *hookname = NULL; + struct strbuf hookdir_annotation = STRBUF_INIT; + + struct option list_options[] = { + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, list_options, + builtin_hook_list_usage, 0); + + if (argc < 1) + usage_msg_opt(_("You must specify a hook event name to list."), + builtin_hook_list_usage, list_options); + + hookname = argv[0]; + + head = hook_list(hookname); + + if (list_empty(head)) + return 1; + + list_for_each(pos, head) { + struct hook *item = list_entry(pos, struct hook, list); + item = list_entry(pos, struct hook, list); + if (item) + printf("%s\n", item->hook_path); + } + + clear_hook_list(head); + strbuf_release(&hookdir_annotation); + + return 0; +} static int run(int argc, const char **argv, const char *prefix) { int i; @@ -88,6 +132,8 @@ int cmd_hook(int argc, const char **argv, const char *prefix) if (!argc) goto usage; + if (!strcmp(argv[0], "list")) + return list(argc, argv, prefix); if (!strcmp(argv[0], "run")) return run(argc, argv, prefix); diff --git a/hook.c b/hook.c index b8420353fa..b1ea372e15 100644 --- a/hook.c +++ b/hook.c @@ -96,6 +96,7 @@ struct list_head *list_hooks(const char *hookname) { struct list_head *hook_head = xmalloc(sizeof(struct list_head)); + INIT_LIST_HEAD(hook_head); if (!hookname) @@ -103,8 +104,6 @@ struct list_head *list_hooks(const char *hookname) if (have_git_dir()) { const char *hook_path = find_hook(hookname); - - /* Add the hook from the hookdir */ if (hook_path) { struct hook *to_add = xmalloc(sizeof(*to_add)); to_add->hook_path = hook_path; From patchwork Thu Aug 19 03:34:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446093 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22CFDC4338F for ; Thu, 19 Aug 2021 03:35:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0D2E96044F for ; Thu, 19 Aug 2021 03:35:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236365AbhHSDfm (ORCPT ); Wed, 18 Aug 2021 23:35:42 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33614 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236232AbhHSDfj (ORCPT ); Wed, 18 Aug 2021 23:35:39 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C6F0AC061764 for ; Wed, 18 Aug 2021 20:35:03 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id w200-20020a25c7d10000b02905585436b530so5170984ybe.21 for ; Wed, 18 Aug 2021 20:35:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=MhcElDgVfxqxQXJUv/XWqEOJuiEDABBTKckPBWruKLs=; b=fWM2d8Uhkm94TzPlKA9RCffKogIsOe3sKnOqiKiXxckuDwiHAHbZPSe8Xq9aDWE1yJ 04k5aMYbD3kNa7daZTokhUPrVQep1mXNLxPbPW+aEhBzlhRCudlBge4J2wHnxQUHFcEz EUT8/Q45j48m/CEvcxRcjQKT0AxdTiDJUYmghVb9nUWWMpjuNGlv9Sj6WnJe4OVL2F8A SK1Nrgofcc72+gQoLmXLHfg++m3Bpbp3gvdAefNGTXaqbWcl5kMszIoSbp6TeMcbkfAz QK0gyRfBLx/QAihwZW7Qx/1Cmrxe52JaRR7lhpHd2BkH+5CLb3qf/N38F0bzRzUHR7Jp Spvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=MhcElDgVfxqxQXJUv/XWqEOJuiEDABBTKckPBWruKLs=; b=k8sp9+cnnpLKIe3Lo2yPsnXG7uy0gJMIws/YSB0AU1Yw7x63vZHNn0FzgjNo11b/3u ibjxZAYzy1imv/5z6XF3Nt6koXFwZd/57G3O1xJY4A8ahnfESZrWw2YseXIm5/saVkCN fdFHXY4TY3GfQjtJIuHn/zeB4o0RSux10eIrJcy2io9zVVHXdQfEndF+iJyNUUIsXgZl kxkddAg1zVFbL0nBTPbZfZiVDj0Ba04ZbOQKBPWjS6+kyS4PPuyqcw4QzYLK5tP9raEs b6yGlI9ifXgEahdOW2q2H0Cb6biZ7gyTLtZrrXZJb5hTgYNEN/Li59ups+95kw33t88I 1arg== X-Gm-Message-State: AOAM532gSTsyQ8zXLrut1epMFV1leLeIQFt91nkv3WiraKVXms9y7cZu JrKzi47Hus2vxfxCHPE9ypaeUTTzJ5hYOLs1/AF8bEjHvcGcyD+TvkvqAYyzctUQqQ+ZbzILJBI /Z3jK2kGTYhIM09QOpJbvxJnOSaY9p5gOENUaO/ZQKOFyahG6umcGgmO6AsMsGwM6q3c/56s9Jw == X-Google-Smtp-Source: ABdhPJxnJT/xc4ShgNTd+GZHVzinUwT7y/mgnUM5f7UTdd40iJhgqGgF4yeNSgJdVXUdZdtwzVezE6oDXE2db6oYDwo= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a25:824b:: with SMTP id d11mr16210214ybn.361.1629344103032; Wed, 18 Aug 2021 20:35:03 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:48 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-5-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 4/6] hook: allow running non-native hooks From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As the hook architecture and 'git hook run' become more featureful, we may find wrappers wanting to use the hook architecture to run their own hooks, thereby getting nice things like parallelism and idiomatic Git configuration for free. Enable this by letting 'git hook run' bypass the known_hooks() check. We do still want to keep known_hooks() around, though - by die()ing when an internal Git call asks for run_hooks("my-new-hook"), we can remind Git developers to update Documentation/githooks.txt with their new hook, which in turn helps Git users discover this new hook. Signed-off-by: Emily Shaffer --- Documentation/git-hook.txt | 8 ++++++++ builtin/hook.c | 4 ++-- hook.c | 35 +++++++++++++++++++++++++++++++---- hook.h | 14 ++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 701ada9fc0..d1db084e4f 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -19,6 +19,14 @@ This command is an interface to git hooks (see linkgit:githooks[5]). Currently it only provides a convenience wrapper for running hooks for use by git itself. In the future it might gain other functionality. +It's possible to use this command to refer to hooks which are not native to Git, +for example if a wrapper around Git wishes to expose hooks into its own +operation in a way which is already familiar to Git users. However, wrappers +invoking such hooks should be careful to name their hook events something which +Git is unlikely to use for a native hook later on. For example, Git is much less +likely to create a `mytool-validate-commit` hook than it is to create a +`validate-commit` hook. + SUBCOMMANDS ----------- diff --git a/builtin/hook.c b/builtin/hook.c index d21f303eca..80397d39f5 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -46,7 +46,7 @@ static int list(int argc, const char **argv, const char *prefix) hookname = argv[0]; - head = hook_list(hookname); + head = list_hooks_gently(hookname); if (list_empty(head)) return 1; @@ -105,7 +105,7 @@ static int run(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); hook_name = argv[0]; - hooks = list_hooks(hook_name); + hooks = list_hooks_gently(hook_name); if (list_empty(hooks)) { /* ... act like run_hooks_oneshot() under --ignore-missing */ if (ignore_missing) diff --git a/hook.c b/hook.c index b1ea372e15..ab1e86ddcf 100644 --- a/hook.c +++ b/hook.c @@ -51,12 +51,21 @@ static int known_hook(const char *name) const char *find_hook(const char *name) { - static struct strbuf path = STRBUF_INIT; + const char *hook_path; if (!known_hook(name)) - die(_("the hook '%s' is not known to git, should be in hook-list.h via githooks(5)"), + BUG(_("the hook '%s' is not known to git, should be in hook-list.h via githooks(5)"), name); + hook_path = find_hook_gently(name); + + return hook_path; +} + +const char *find_hook_gently(const char *name) +{ + static struct strbuf path = STRBUF_INIT; + strbuf_reset(&path); strbuf_git_path(&path, "hooks/%s", name); if (access(path.buf, X_OK) < 0) { @@ -92,10 +101,24 @@ int hook_exists(const char *name) return !list_empty(list_hooks(name)); } +struct hook_config_cb +{ + struct strbuf *hook_key; + struct list_head *list; +}; + struct list_head *list_hooks(const char *hookname) { - struct list_head *hook_head = xmalloc(sizeof(struct list_head)); + if (!known_hook(hookname)) + BUG("Don't recognize hook event '%s'! " + "Is it documented in Documentation/githooks.txt?", + hookname); + return list_hooks_gently(hookname); +} +struct list_head *list_hooks_gently(const char *hookname) +{ + struct list_head *hook_head = xmalloc(sizeof(struct list_head)); INIT_LIST_HEAD(hook_head); @@ -103,7 +126,7 @@ struct list_head *list_hooks(const char *hookname) BUG("null hookname was provided to hook_list()!"); if (have_git_dir()) { - const char *hook_path = find_hook(hookname); + const char *hook_path = find_hook_gently(hookname); if (hook_path) { struct hook *to_add = xmalloc(sizeof(*to_add)); to_add->hook_path = hook_path; @@ -299,6 +322,10 @@ int run_hooks_oneshot(const char *hook_name, struct run_hooks_opt *options) if (options->path_to_stdin && options->feed_pipe) BUG("choose only one method to populate stdin"); + /* + * 'git hooks run ' uses run_hooks, so we don't need to + * allow unknown hooknames here. + */ hooks = list_hooks(hook_name); /* diff --git a/hook.h b/hook.h index cd3123d290..6b7b2d14d2 100644 --- a/hook.h +++ b/hook.h @@ -9,8 +9,16 @@ * Returns the path to the hook file, or NULL if the hook is missing * or disabled. Note that this points to static storage that will be * overwritten by further calls to find_hook and run_hook_*. + * + * If the hook is not a native hook (e.g. present in Documentation/githooks.txt) + * find_hook() will BUG(). find_hook_gently() does not consult the native hook + * list and will check for any hook name in the hooks directory regardless of + * whether it is known. find_hook() should be used by internal calls to hooks, + * and find_hook_gently() should only be used when the hookname was provided by + * the user, such as by 'git hook run my-wrapper-hook'. */ const char *find_hook(const char *name); +const char *find_hook_gently(const char *name); /* * A boolean version of find_hook() @@ -32,8 +40,14 @@ struct hook { /* * Provides a linked list of 'struct hook' detailing commands which should run * in response to the 'hookname' event, in execution order. + * + * list_hooks() checks the provided hookname against Documentation/githooks.txt + * and BUG()s if it is not found. list_hooks_gently() allows any hookname. The + * latter should only be used when the hook name is provided by the user, and + * the former should be used by internal callers. */ struct list_head *list_hooks(const char *hookname); +struct list_head *list_hooks_gently(const char *hookname); struct run_hooks_opt { From patchwork Thu Aug 19 03:34:49 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446097 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E7CFBC4338F for ; Thu, 19 Aug 2021 03:35:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CD6DD6044F for ; Thu, 19 Aug 2021 03:35:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236232AbhHSDfr (ORCPT ); Wed, 18 Aug 2021 23:35:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33640 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236321AbhHSDfl (ORCPT ); Wed, 18 Aug 2021 23:35:41 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 31191C061796 for ; Wed, 18 Aug 2021 20:35:06 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id o3-20020a2541030000b0290557cf3415f8so5311417yba.1 for ; Wed, 18 Aug 2021 20:35:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=yyCYb+cdeHW1Namj+5zsdiZ1CGz8jQzWaUSEaZe1y2A=; b=Ep39IN9p7ZiFx5uKMXsYzhtbxUMO3xKxbW1kmlyHO7FXq8Cp7parU4OQ7nU+vr9QUC V1cGVVLKp+qqtagxWEEsaYln+JhDyUmRJY2vpUgfx6GpTQZTnblTzvXIZcLkK+tJouqJ TNCztbV/IvMqgWqmCOyx7OAKz4IFPm1QMX+KQLRPXse7MaBRU/0nAovDKf1vs6X+HyTz NHQZYNZQOJkrkgY2FH/u+Y+JA5Q+f/j++Rk6rg4j2x4GQa13llQWQQ0vZuEDYJOjL7s8 DYY3kNKJXwXl063exPFMk5QIzQXA3B26E6vlObSz80IphErXMbOIAkadzVBKAR/8TRf6 HkFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=yyCYb+cdeHW1Namj+5zsdiZ1CGz8jQzWaUSEaZe1y2A=; b=cgkmooQ91Y/o6fKpS4CrO81Sn6GB2meA1wxzWVkAk9UbN+BeCvzfl2U6Y0hMwPDbEM dT1h2w20YojLLCVXcYp8XSnCzBf9GF/L+gAXVkMwx5UNmAO8fLiu/JtugrxPt5U0SoXa T+SIpWebD+3WW16U4QStc4RDcg6iK6IRqefXyIdhkGPku/t9gVx6kBFgFy+wQxxUwK6e UhI4YyPzKf7hwpjqq76HM4ijvRuBUYDR6QKzwtYkUYxRc9aidKNUpUrv4gfaWORAa2Hs EDNfWs+WcYdEAxv4Go3VVkyoX+7DcwMLOTnR/G1LZC8cLXRS7e4qrmUKMgpipxrX/L1D bDbg== X-Gm-Message-State: AOAM533DlqTh5WczIYAnPZAXgXcWdfDuWn0hCGtArS76uW+jCpA3UN4s /FZa3MXeWYTGSrNny9tVUBSBFlrOR/TZYP/eTb/7BGjeIYpTs990UIg/ad9lFth3KK8/PpCi+UY fT30sV7OcS29CipFpr1D52horfuVx8wMfV6ZWS2EFqTjhmCkV7/vAnwDZcz0MN3nIBHV9asfDUA == X-Google-Smtp-Source: ABdhPJzJ59RI+qqbakaG9UaE8ej8jIvpd7KUD6amJdTsM2STcyYVDxn1byjHeKdNUdSalZ78VIQ7WkDY8KerDCOLH50= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a25:4c07:: with SMTP id z7mr16844611yba.350.1629344105234; Wed, 18 Aug 2021 20:35:05 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:49 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-6-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 5/6] hook: include hooks from the config From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Teach the hook.[hc] library to parse configs to populare the list of hooks to run for a given event. Multiple commands can be specified for a given hook by providing multiple "hook..command = " and "hook..event = " lines. Hooks will be run in config order of the "hook..event" lines. For example: $ git config --list | grep ^hook hook.bar.command=~/bar.sh hook.bar.event=pre-commit $ git hook run # Runs ~/bar.sh # Runs .git/hooks/pre-commit Signed-off-by: Emily Shaffer --- Documentation/config/hook.txt | 18 ++++ Documentation/git-hook.txt | 57 ++++++++++++- builtin/hook.c | 3 +- hook.c | 153 ++++++++++++++++++++++++++++++---- hook.h | 7 +- t/t1800-hook.sh | 141 ++++++++++++++++++++++++++++++- 6 files changed, 357 insertions(+), 22 deletions(-) diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt index 96d3d6572c..c394756328 100644 --- a/Documentation/config/hook.txt +++ b/Documentation/config/hook.txt @@ -1,3 +1,21 @@ +hook..command:: + A command to execute whenever `hook.` is invoked. `` should + be a unique "friendly" name which you can use to identify this hook + command. (You can specify when to invoke this command with + `hook..event`.) The value can be an executable on your device or a + oneliner for your shell. If more than one value is specified for the + same ``, the last value parsed will be the only command executed. + See linkgit:git-hook[1]. + +hook..event:: + The hook events which should invoke `hook.`. `` should be a + unique "friendly" name which you can use to identify this hook. The + value should be the name of a hook event, like "pre-commit" or "update". + (See linkgit:githooks[5] for a complete list of hooks Git knows about.) + On the specified event, the associated `hook..command` will be + executed. More than one event can be specified if you wish for + `hook.` to execute on multiple events. See linkgit:git-hook[1]. + hook.jobs:: Specifies how many hooks can be run simultaneously during parallelized hook execution. If unspecified, defaults to the number of processors on diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index d1db084e4f..9c6cbdc2eb 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -27,12 +27,65 @@ Git is unlikely to use for a native hook later on. For example, Git is much less likely to create a `mytool-validate-commit` hook than it is to create a `validate-commit` hook. +This command parses the default configuration files for pairs of configs like +so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --c + +In this example, `[hook "linter"]` represents one script - `~/bin/linter --c` - +which can be shared by many repos, and even by many hook events, if appropriate. + +Commands are run in the order Git encounters their associated +`hook..event` configs during the configuration parse (see +linkgit:git-config[1]). Although multiple `hook.linter.event` configs can be +added, only one `hook.linter.command` event is valid - Git uses "last-one-wins" +to determine which command to run. + +So if you wanted your linter to run when you commit as well as when you push, +you would configure it like so: + + [hook "linter"] + event = pre-commit + event = pre-push + command = ~/bin/linter --c + +With this config, `~/bin/linter --c` would be run by Git before a commit is +generated (during `pre-commit`) as well as before a push is performed (during +`pre-push`). + +And if you wanted to run your linter as well as a secret-leak detector during +only the "pre-commit" hook event, you would configure it instead like so: + + [hook "linter"] + event = pre-commit + command = ~/bin/linter --c + [hook "no-leaks"] + event = pre-commit + command = ~/bin/leak-detector + +With this config, before a commit is generated (during `pre-commit`), Git would +first start `~/bin/linter --c` and second start `~/bin/leak-detector`. It would +evaluate the output of each when deciding whether to proceed with the commit. + +For a full list of hook events which you can set your `hook..event` to, +and how hooks are invoked during those events, see linkgit:githooks[5]. + +In general, when instructions suggest adding a script to +`.git/hooks/`, you can specify it in the config instead by running +`git config --add hook..command && git config --add +hook..event ` - this way you can share the script between +multiple repos. That is, `cp ~/my-script.sh ~/project/.git/hooks/pre-commit` +would become `git config --add hook.my-script.command ~/my-script.sh && git +config --add hook.my-script.event pre-commit`. + SUBCOMMANDS ----------- run:: - Run the `` hook. See linkgit:githooks[5] for - the hook names we support. + Runs hooks configured for ``, in the order they are + discovered during the config parse. + Any positional arguments to the hook should be passed after an optional `--` (or `--end-of-options`, see linkgit:gitcli[7]). The diff --git a/builtin/hook.c b/builtin/hook.c index 80397d39f5..50233366a8 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -55,7 +55,8 @@ static int list(int argc, const char **argv, const char *prefix) struct hook *item = list_entry(pos, struct hook, list); item = list_entry(pos, struct hook, list); if (item) - printf("%s\n", item->hook_path); + printf("%s\n", item->name ? item->name + : _("hook from hookdir")); } clear_hook_list(head); diff --git a/hook.c b/hook.c index ab1e86ddcf..581d87cbbd 100644 --- a/hook.c +++ b/hook.c @@ -11,6 +11,50 @@ static void free_hook(struct hook *ptr) free(ptr); } +/* + * Walks the linked list at 'head' to check if any hook named 'name' + * already exists. Returns a pointer to that hook if so, otherwise returns NULL. + */ +static struct hook *find_hook_by_name(struct list_head *head, + const char *name) +{ + struct list_head *pos = NULL, *tmp = NULL; + struct hook *found = NULL; + + list_for_each_safe(pos, tmp, head) { + struct hook *it = list_entry(pos, struct hook, list); + if (!strcmp(it->name, name)) { + list_del(pos); + found = it; + break; + } + } + return found; +} + +/* + * Adds a hook if it's not already in the list, or moves it to the tail of the + * list if it was already there. name == NULL indicates it's from the hookdir; + * just append it in this case. + */ +static void append_or_move_hook(struct list_head *head, const char *name) +{ + struct hook *to_add = NULL; + + /* if it's not from hookdir, check if the hook is already in the list */ + if (name) + to_add = find_hook_by_name(head, name); + + if (!to_add) { + /* adding a new hook, not moving an old one */ + to_add = xmalloc(sizeof(*to_add)); + to_add->name = name; + to_add->feed_pipe_cb_data = NULL; + } + + list_add_tail(&to_add->list, head); +} + static void remove_hook(struct list_head *to_remove) { struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); @@ -103,10 +147,50 @@ int hook_exists(const char *name) struct hook_config_cb { - struct strbuf *hook_key; + const char *hook_event; struct list_head *list; }; +/* + * Callback for git_config which adds configured hooks to a hook list. Hooks + * can be configured by specifying both hook..command = and + * hook..event = . + */ +static int hook_config_lookup(const char *key, const char *value, void *cb_data) +{ + struct hook_config_cb *data = cb_data; + const char *subsection, *parsed_key; + size_t subsection_len = 0; + struct strbuf subsection_cpy = STRBUF_INIT; + + /* + * Don't bother doing the expensive parse if there's no + * chance that the config matches 'hook.myhook.event = hook_event'. + */ + if (!value || strcmp(value, data->hook_event)) + return 0; + + /* Looking for "hook.friendlyname.event = hook_event" */ + if (parse_config_key(key, + "hook", + &subsection, + &subsection_len, + &parsed_key) || + strcmp(parsed_key, "event")) + return 0; + + /* + * 'subsection' is a pointer to the internals of 'key', which we don't + * own the memory for. Copy it away to the hook list. + */ + strbuf_add(&subsection_cpy, subsection, subsection_len); + + append_or_move_hook(data->list, strbuf_detach(&subsection_cpy, NULL)); + + + return 0; +} + struct list_head *list_hooks(const char *hookname) { if (!known_hook(hookname)) @@ -119,21 +203,23 @@ struct list_head *list_hooks(const char *hookname) struct list_head *list_hooks_gently(const char *hookname) { struct list_head *hook_head = xmalloc(sizeof(struct list_head)); + struct hook_config_cb cb_data = { + .hook_event = hookname, + .list = hook_head, + }; INIT_LIST_HEAD(hook_head); if (!hookname) BUG("null hookname was provided to hook_list()!"); - if (have_git_dir()) { - const char *hook_path = find_hook_gently(hookname); - if (hook_path) { - struct hook *to_add = xmalloc(sizeof(*to_add)); - to_add->hook_path = hook_path; - to_add->feed_pipe_cb_data = NULL; - list_add_tail(&to_add->list, hook_head); - } - } + /* Add the hooks from the config, e.g. hook.myhook.event = pre-commit */ + git_config(hook_config_lookup, &cb_data); + + /* Add the hook from the hookdir. The placeholder makes it easier to + * allocate work in pick_next_hook. */ + if (find_hook_gently(hookname)) + append_or_move_hook(hook_head, NULL); return hook_head; } @@ -194,11 +280,43 @@ static int pick_next_hook(struct child_process *cp, cp->trace2_hook_name = hook_cb->hook_name; cp->dir = hook_cb->options->dir; + /* to enable oneliners, let config-specified hooks run in shell. + * config-specified hooks have a name. */ + cp->use_shell = !!run_me->name; + /* add command */ - if (hook_cb->options->absolute_path) - strvec_push(&cp->args, absolute_path(run_me->hook_path)); - else - strvec_push(&cp->args, run_me->hook_path); + if (run_me->name) { + /* ...from config */ + struct strbuf cmd_key = STRBUF_INIT; + char *command = NULL; + + strbuf_addf(&cmd_key, "hook.%s.command", run_me->name); + if (git_config_get_string(cmd_key.buf, &command)) { + /* TODO test me! */ + die(_("'hook.%s.command' must be configured " + "or 'hook.%s.event' must be removed; aborting.\n"), + run_me->name, run_me->name); + } + + strvec_push(&cp->args, command); + } else { + /* ...from hookdir. */ + const char *hook_path = NULL; + /* + * + * At this point we are already running, so don't validate + * whether the hook name is known or not. + */ + hook_path = find_hook_gently(hook_cb->hook_name); + if (!hook_path) + BUG("hookdir hook in hook list but no hookdir hook present in filesystem"); + + if (hook_cb->options->absolute_path) + hook_path = absolute_path(hook_path); + + strvec_push(&cp->args, hook_path); + } + /* * add passed-in argv, without expanding - let the user get back @@ -228,8 +346,11 @@ static int notify_start_failure(struct strbuf *out, hook_cb->rc |= 1; - strbuf_addf(out, _("Couldn't start hook '%s'\n"), - attempted->hook_path); + if (attempted->name) + strbuf_addf(out, _("Couldn't start hook '%s'\n"), + attempted->name); + else + strbuf_addstr(out, _("Couldn't start hook from hooks directory\n")); return 1; } diff --git a/hook.h b/hook.h index 6b7b2d14d2..621bd2cde1 100644 --- a/hook.h +++ b/hook.h @@ -27,8 +27,11 @@ int hook_exists(const char *hookname); struct hook { struct list_head list; - /* The path to the hook */ - const char *hook_path; + /* + * The friendly name of the hook. NULL indicates the hook is from the + * hookdir. + */ + const char *name; /* * Use this to keep state for your feed_pipe_fn if you are using diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index 217db848b3..ef2432f53a 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -1,13 +1,29 @@ #!/bin/bash -test_description='git-hook command' +test_description='git-hook command and config-managed multihooks' . ./test-lib.sh +setup_hooks () { + test_config hook.ghi.event pre-commit --add + test_config hook.ghi.command "/path/ghi" --add + test_config_global hook.def.event pre-commit --add + test_config_global hook.def.command "/path/def" --add +} + +setup_hookdir () { + mkdir .git/hooks + write_script .git/hooks/pre-commit <<-EOF + echo \"Legacy Hook\" + EOF + test_when_finished rm -rf .git/hooks +} + test_expect_success 'git hook usage' ' test_expect_code 129 git hook && test_expect_code 129 git hook run && test_expect_code 129 git hook run -h && + test_expect_code 129 git hook list -h && test_expect_code 129 git hook run --unknown 2>err && grep "unknown option" err ' @@ -153,4 +169,127 @@ test_expect_success 'stdin to hooks' ' test_cmp expect actual ' +test_expect_success 'git hook list orders by config order' ' + setup_hooks && + + cat >expected <<-EOF && + def + ghi + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list reorders on duplicate event declarations' ' + setup_hooks && + + # 'def' is usually configured globally; move it to the end by + # configuring it locally. + test_config hook.def.event "pre-commit" --add && + + cat >expected <<-EOF && + ghi + def + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list shows hooks from the hookdir' ' + setup_hookdir && + + cat >expected <<-EOF && + hook from hookdir + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions execute oneliners' ' + test_config hook.oneliner.event "pre-commit" && + test_config hook.oneliner.command "echo \"Hello World\"" && + + echo "Hello World" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'inline hook definitions resolve paths' ' + write_script sample-hook.sh <<-EOF && + echo \"Sample Hook\" + EOF + + test_when_finished "rm sample-hook.sh" && + + test_config hook.sample-hook.event pre-commit && + test_config hook.sample-hook.command "\"$(pwd)/sample-hook.sh\"" && + + echo \"Sample Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'hookdir hook included in git hook run' ' + setup_hookdir && + + echo \"Legacy Hook\" >expected && + + # hooks are run with stdout_to_stderr = 1 + git hook run pre-commit 2>actual && + test_cmp expected actual +' + +test_expect_success 'stdin to multiple hooks' ' + test_config hook.stdin-a.event "test-hook" --add && + test_config hook.stdin-a.command "xargs -P1 -I% echo a%" --add && + test_config hook.stdin-b.event "test-hook" --add && + test_config hook.stdin-b.command "xargs -P1 -I% echo b%" --add && + + cat >input <<-EOF && + 1 + 2 + 3 + EOF + + cat >expected <<-EOF && + a1 + a2 + a3 + b1 + b2 + b3 + EOF + + git hook run --to-stdin=input test-hook 2>actual && + test_cmp expected actual +' + +test_expect_success 'multiple hooks in series' ' + test_config hook.series-1.event "test-hook" && + test_config hook.series-1.command "echo 1" --add && + test_config hook.series-2.event "test-hook" && + test_config hook.series-2.command "echo 2" --add && + mkdir .git/hooks && + write_script .git/hooks/test-hook <<-EOF && + echo 3 + EOF + + cat >expected <<-EOF && + 1 + 2 + 3 + EOF + + git hook run -j1 test-hook 2>actual && + test_cmp expected actual && + + rm -rf .git/hooks +' test_done From patchwork Thu Aug 19 03:34:50 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 12446095 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2B36EC432BE for ; Thu, 19 Aug 2021 03:35:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0E7A461100 for ; Thu, 19 Aug 2021 03:35:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236358AbhHSDfs (ORCPT ); Wed, 18 Aug 2021 23:35:48 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236250AbhHSDfp (ORCPT ); Wed, 18 Aug 2021 23:35:45 -0400 Received: from mail-yb1-xb4a.google.com (mail-yb1-xb4a.google.com [IPv6:2607:f8b0:4864:20::b4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 49CA1C0617AF for ; Wed, 18 Aug 2021 20:35:08 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id f8-20020a2585480000b02905937897e3daso5218374ybn.2 for ; Wed, 18 Aug 2021 20:35:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=psBwRW3Smh7pS835dsMUT4AYNFbKbJBjRrVf96skj6Y=; b=RzOHyVb/2rOB/xthItDwsmFSh2oQe4Y6HSzzALaxuDPHP5Qf9rFAizYRixPIdbbY5t dVWFdbcporXqvle8TNb/JojzNyQQd1V0H4rrjWIeXpRU90loEbPLMAE2knuOYln7W2uz 2ljpdbSYatcs+EVCdxhS31KNisPmpLxnOhrRCWzRPMMX4IJJdPQGuY9e/OHP1lTQZiZN LJtbCs62BKCoBYCYjYy+VKIaS+ZpE//waDzO5RHKlI3E+TOnUOUHGuRP+5qvA0eTw23O C57e8gxJawqd15FdZRyYKrnUqvjb9MCzJKR1gmQ+Co50v3weVwEGzGOce7DIwiHN27eo dhUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=psBwRW3Smh7pS835dsMUT4AYNFbKbJBjRrVf96skj6Y=; b=uXZY6nmM2eVpKPk505IL7wMPGTCkcLo+4KwUBvT8W9Oq6+Z9GmC77uaMmRWYkGY9NG Kw35x8nUPYe4bAlBm11JRSswhLsfz6jgpKsVXHttbEMWbHgX57gfTUkXTl2LBg7XO0cm dfkZjkKcHl2Ra6c+FvxDJzJMqA11e+az9CH3ypaTzzs3dBZ3S0HabtnCvrSJSZ47r+bA Y20CjtqlA8zentbnZYU026lu4xmUuRDSWtdrFVDuutJGrVXX0+vOBItjBGwc35TYwsuU lM9w3sCa3AHF+8dD4vrnaLK4R+q1O9gIE6tA50PsF1lmGGaXlJrcFNtwhXKKAJnYylGj DyPA== X-Gm-Message-State: AOAM5329Qtc4rEiOXNFL+32v22L/iGJDf73pD/jorm6rW5USRIOO2srf 5G0jB0+qHKuGbei25B3Bur/XR9YJupzl5Zh6zdiT5XAD8bPoKnF6k2Xf1WCDNQ6NEH7lwAFygOI P/w5vZkKi+sphCXXtB7b/y9To6pgf2Q7TZ0TMeAHPRinDCK2xSLlMx+W1MA5UGPQMG1o+32mw9w == X-Google-Smtp-Source: ABdhPJxrafV9AQsJy5GggvEStq0XG6Sr6iCaTaC2MXOzt72JDWjnHADev+6PSBNV+T3k/Yb5uEvFjQBgCAoYUzyHK2o= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:200:a999:9b6d:55c3:b66c]) (user=emilyshaffer job=sendgmr) by 2002:a25:b5ce:: with SMTP id d14mr16019006ybg.415.1629344107547; Wed, 18 Aug 2021 20:35:07 -0700 (PDT) Date: Wed, 18 Aug 2021 20:34:50 -0700 In-Reply-To: <20210819033450.3382652-1-emilyshaffer@google.com> Message-Id: <20210819033450.3382652-7-emilyshaffer@google.com> Mime-Version: 1.0 References: <20210819033450.3382652-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.33.0.rc2.250.ged5fa647cd-goog Subject: [PATCH v3 6/6] hook: allow out-of-repo 'git hook' invocations From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Since hooks can now be supplied via the config, and a config can be present without a gitdir via the global and system configs, we can start to allow 'git hook run' to occur without a gitdir. This enables us to do things like run sendemail-validate hooks when running 'git send-email' from a nongit directory. It still doesn't make sense to look for hooks in the hookdir in nongit repos, though, as there is no hookdir. Signed-off-by: Emily Shaffer --- Notes: For hookdir hooks, do we want to run them in nongit dir when core.hooksPath is set? For example, if someone set core.hooksPath in their global config and then ran 'git hook run sendemail-validate' in a nongit dir? git.c | 2 +- hook.c | 2 +- t/t1800-hook.sh | 20 +++++++++++++++----- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/git.c b/git.c index 540909c391..39988ee3b0 100644 --- a/git.c +++ b/git.c @@ -538,7 +538,7 @@ static struct cmd_struct commands[] = { { "grep", cmd_grep, RUN_SETUP_GENTLY }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, - { "hook", cmd_hook, RUN_SETUP }, + { "hook", cmd_hook, RUN_SETUP_GENTLY }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, diff --git a/hook.c b/hook.c index 581d87cbbd..2e08156546 100644 --- a/hook.c +++ b/hook.c @@ -218,7 +218,7 @@ struct list_head *list_hooks_gently(const char *hookname) /* Add the hook from the hookdir. The placeholder makes it easier to * allocate work in pick_next_hook. */ - if (find_hook_gently(hookname)) + if (have_git_dir() && find_hook_gently(hookname)) append_or_move_hook(hook_head, NULL); return hook_head; diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh index ef2432f53a..a7e45c0d16 100755 --- a/t/t1800-hook.sh +++ b/t/t1800-hook.sh @@ -118,15 +118,25 @@ test_expect_success 'git hook run -- pass arguments' ' test_cmp expect actual ' -test_expect_success 'git hook run -- out-of-repo runs excluded' ' - write_script .git/hooks/test-hook <<-EOF && - echo Test hook - EOF +test_expect_success 'git hook run: out-of-repo runs execute global hooks' ' + test_config_global hook.global-hook.event test-hook --add && + test_config_global hook.global-hook.command "echo no repo no problems" --add && + + echo "global-hook" >expect && + nongit git hook list test-hook >actual && + test_cmp expect actual && + + echo "no repo no problems" >expect && - nongit test_must_fail git hook run test-hook + nongit git hook run test-hook 2>actual && + test_cmp expect actual ' test_expect_success 'git -c core.hooksPath= hook run' ' + write_script .git/hooks/test-hook <<-EOF && + echo Test hook + EOF + mkdir my-hooks && write_script my-hooks/test-hook <<-\EOF && echo Hook ran $1 >>actual