From patchwork Wed Oct 14 23:24:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838393 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 D7E2DC433E7 for ; Thu, 15 Oct 2020 01:54:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 74D8922258 for ; Thu, 15 Oct 2020 01:54:45 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="ln/uqDl5" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732405AbgJOByo (ORCPT ); Wed, 14 Oct 2020 21:54:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53100 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732440AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -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 65836C08EA7D for ; Wed, 14 Oct 2020 16:24:58 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id k7so1099917ybm.13 for ; Wed, 14 Oct 2020 16:24:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=uR5HrcXSisXMvvqDXNi5q8WCr427ELsXrwW13HxSY20=; b=ln/uqDl5aF/XjcShfDyta/QKnl3Ql1cC6vsTMPLFLcFkfrfhPVsgARbUj0DavNttAc TNZkoNO0n/ro+zEWy/HA/qpy7Hyrm0znMTCpHo2Knh4sJbbyWhKCeormT468lMBxiVhR AA/CbOgFw15t6iGskxWh//EOl5MjWntjYhviCIZqPluogwbAoUUih8uAGmAhZKFnMuoA MY0t24Z2QBiAxzmSYOS3d3XJMC2m5O4U2TrBrDe0ZnnuGO/8qxBDtVHlCrDCgkm7RsDU xfyKRjRe/gsG/NIovaVtBqzoA+DPlAoYaBrMukeU7SgS9IYp/nr+aD35i9GZBOS+TFnu XWkQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=uR5HrcXSisXMvvqDXNi5q8WCr427ELsXrwW13HxSY20=; b=dYOJrOWZeTssmuoFzMk2yQeEay2giabuMg8wL2TS0HfIY2WAe3gGUmaiJXPexPOI1f 2foDW2SClxTucaFvODsusbm4SEQUDQBhkGrmte2CDVkDra22kPvjaZluz+a++O3bDQYZ AnPAKu/slZv3U5P9dld6Tpb/mhQQruyECklzpx2zjzlSPCgkjg/Vxh6i3H4Dp/EvFUhZ qKk7yIxXNbKjC5rfINedO/XJXAadQFWvaVz8Rp4s2fDqBIKbyzv/gBuzB7OVUwZFL6cN 6C7R8I6mU/uVcXRFgGBpd+kovE+V6lvTgluYBjEC+malJFyprMx8wtTPlAGiY1Mu327O 270w== X-Gm-Message-State: AOAM533tXzdrHJsLA+xRV5G6VzjiTkoCWz/teU0Tj21+uybZ2Yk8ItjK p0GCnfCeFDF/UDgrbwS14u4CEK1J2b9ZemBBUVWrMl/KzV26Qm7VTH9ZnF2oJfcIm0IDyNR05bV I20Eb2Vyvh1WkjAjMEmKpviHVlP9eyLDhSu7h17z58UPqzTp+Fio4fwCyKGV7pZTyneJxThUnvQ == X-Google-Smtp-Source: ABdhPJyKuxDSoragbvj4ZvzEan+Zz0mSwRKhbjcVF7DXRavuDGj61uOU7/ieMbF4oJSoUXl77W+MDP3NWPXoW7n47AE= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:db4d:: with SMTP id g74mr1527317ybf.51.1602717897509; Wed, 14 Oct 2020 16:24:57 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:40 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-2-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 1/8] doc: propose hooks managed by the config From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Begin a design document for config-based hooks, managed via git-hook. Focus on an overview of the implementation and motivation for design decisions. Briefly discuss the alternatives considered before this point. Also, attempt to redefine terms to fit into a multihook world. Signed-off-by: Emily Shaffer --- Notes: Since v4, addressed comments from Jonathan Tan about wording. Documentation/Makefile | 1 + .../technical/config-based-hooks.txt | 367 ++++++++++++++++++ 2 files changed, 368 insertions(+) create mode 100644 Documentation/technical/config-based-hooks.txt diff --git a/Documentation/Makefile b/Documentation/Makefile index 80d1908a44..58d6b3acbe 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -81,6 +81,7 @@ SP_ARTICLES += $(API_DOCS) TECH_DOCS += MyFirstContribution TECH_DOCS += MyFirstObjectWalk TECH_DOCS += SubmittingPatches +TECH_DOCS += technical/config-based-hooks TECH_DOCS += technical/hash-function-transition TECH_DOCS += technical/http-protocol TECH_DOCS += technical/index-format diff --git a/Documentation/technical/config-based-hooks.txt b/Documentation/technical/config-based-hooks.txt new file mode 100644 index 0000000000..dac391f505 --- /dev/null +++ b/Documentation/technical/config-based-hooks.txt @@ -0,0 +1,367 @@ +Configuration-based hook management +=================================== +:sectanchors: + +[[motivation]] +== Motivation + +Replace the .git/hook/hookname path as the only source of hooks to execute; +allow users to define hooks using config files, in a way which is friendly to +users with multiple repos which have similar needs. + +Redefine "hook" as an event rather than a single script, allowing users to +perform unrelated actions on a single event. + +Take a step closer to safety when copying zipped Git repositories from untrusted +users by making it more apparent to users which scripts will be run during +normal Git operations. + +Make it easier for users to discover Git's hook feature and automate their +workflows. + +[[user-interfaces]] +== User interfaces + +[[config-schema]] +=== Config schema + +Hooks can be introduced by editing the configuration manually. There are two new +sections added, `hook` and `hookcmd`. + +[[config-schema-hook]] +==== `hook` + +Primarily contains subsections for each hook event. The order of variables in +these subsections defines the hook command execution order; hook commands can be +specified by setting the value directly to the command if no additional +configuration is needed, or by setting the value as the name of a `hookcmd`. If +Git does not find a `hookcmd` whose subsection matches the value of the given +command string, Git will try to execute the string directly. Hooks are executed +by passing the resolved command string to the shell. In the future, hook event +subsections could also contain per-hook-event settings; see +<> for more details. + +Also contains top-level hook execution settings, for example, `hook.runHookDir` +or `hook.disableAll`. (These settings are described more in +<>.) + +---- +[hook "pre-commit"] + command = perl-linter + command = /usr/bin/git-secrets --pre-commit + +[hook "pre-applypatch"] + command = perl-linter + # for illustration purposes; error behavior isn't planned yet + error = ignore + +[hook] + runHookDir = interactive +---- + +[[config-schema-hookcmd]] +==== `hookcmd` + +Defines a hook command and its attributes, which will be used when a hook event +occurs. Unqualified attributes are assumed to apply to this hook during all hook +events, but event-specific attributes can also be supplied. The example runs +`/usr/bin/lint-it --language=perl `, but for repos which +include this config, the hook command will be skipped for all events to which +it's normally subscribed _except_ `pre-commit`. + +---- +[hookcmd "perl-linter"] + command = /usr/bin/lint-it --language=perl + skip = true + # for illustration purposes; below hasn't been defined yet + pre-commit-skip = false +---- + +[[command-line-api]] +=== Command-line API + +Users should be able to view, reorder, and create hook commands via the command +line. External tools should be able to view a list of hooks in the correct order +to run. + +*`git hook list `* + +*`git hook list (--system|--global|--local|--worktree)`* + +*`git hook edit `* + +*`git hook add `* + +[[hook-editor]] +=== Hook editor + +The tool which is presented by `git hook edit `. Ideally, this +tool should be easier to use than manually editing the config, and then produce +a concise config afterwards. It may take a form similar to `git rebase +--interactive`. + +[[implementation]] +== Implementation + +[[library]] +=== Library + +`hook.c` and `hook.h` are responsible for interacting with the config files. In +the case when the code generating a hook event doesn't have special concerns +about how to run the hooks, the hook library will provide a basic API to call +all hooks in config order with an `strvec` provided by the code which +generates the hook event: + +*`int run_hooks(const char *hookname, struct strvec *args)`* + +This call includes the hook command provided by `run-command.h:find_hook()`; +eventually, this legacy hook will be gated by a config `hook.runHookDir`. The +config is checked against a number of cases: + +- "no": the legacy hook will not be run +- "interactive": Git will prompt the user before running the legacy hook +- "warn": Git will print a warning to stderr before running the legacy hook +- "yes" (default): Git will silently run the legacy hook + +In case this list is expanded in the future, if a value for `hook.runHookDir` is +given which Git does not recognize, Git should discard that config entry. For +example, if "warn" was specified at system level and "junk" was specified at +global level, Git would resolve the value to "warn"; if the only time the config +was set was to "junk", Git would use the default value of "yes". + +If the caller wants to do something more complicated, the hook library can also +provide a callback API: + +*`int for_each_hookcmd(const char *hookname, hookcmd_function *cb)`* + +Finally, to facilitate the builtin, the library will also provide the following +APIs to interact with the config: + +---- +int set_hook_commands(const char *hookname, struct string_list *commands, + enum config_scope scope); +int set_hookcmd(const char *hookcmd, struct hookcmd options); + +int list_hook_commands(const char *hookname, struct string_list *commands); +int list_hooks_in_scope(enum config_scope scope, struct string_list *commands); +---- + +`struct hookcmd` is expected to grow in size over time as more functionality is +added to hooks; so that other parts of the code don't need to understand the +config schema, `struct hookcmd` should contain logical values instead of string +pairs. + +---- +struct hookcmd { + const char *name; + const char *command; + + /* for illustration only; not planned at present */ + int parallelizable; + const char *hookcmd_before; + const char *hookcmd_after; + enum recovery_action on_fail; +} +---- + +[[builtin]] +=== Builtin + +`builtin/hook.c` is responsible for providing the frontend. It's responsible for +formatting user-provided data and then calling the library API to set the +configs as appropriate. The builtin frontend is not responsible for calling the +config directly, so that other areas of Git can rely on the hook library to +understand the most recent config schema for hooks. + +[[migration]] +=== Migration path + +[[stage-0]] +==== Stage 0 + +Hooks are called by running `run-command.h:find_hook()` with the hookname and +executing the result. The hook library and builtin do not exist. Hooks only +exist as specially named scripts within `.git/hooks/`. + +[[stage-1]] +==== Stage 1 + +`git hook list --porcelain ` is implemented. Users can replace their +`.git/hooks/` scripts with a trampoline based on `git hook list`'s +output. Modifier commands like `git hook add` and `git hook edit` can be +implemented around this time as well. + +[[stage-2]] +==== Stage 2 + +`hook.h:run_hooks()` is taught to include `run-command.h:find_hook()` at the +end; calls to `find_hook()` are replaced with calls to `run_hooks()`. Users can +opt-in to config-based hooks simply by creating some in their config; otherwise +users should remain unaffected by the change. + +[[stage-3]] +==== Stage 3 + +The call to `find_hook()` inside of `run_hooks()` learns to check for a config, +`hook.runHookDir`. Users can opt into managing their hooks completely via the +config this way. + +[[stage-4]] +==== Stage 4 + +`.git/hooks` is removed from the template and the hook directory is considered +deprecated. To avoid breaking older repos, the default of `hook.runHookDir` is +not changed, and `find_hook()` is not removed. + +[[caveats]] +== Caveats + +[[security]] +=== Security and repo config + +Part of the motivation behind this refactor is to mitigate hooks as an attack +vector;footnote:[https://lore.kernel.org/git/20171002234517.GV19555@aiede.mtv.corp.google.com/] +however, as the design stands, users can still provide hooks in the repo-level +config, which is included when a repo is zipped and sent elsewhere. The +security of the repo-level config is still under discussion; this design +generally assumes the repo-level config is secure, which is not true yet. The +goal is to avoid an overcomplicated design to work around a problem which has +ceased to exist. + +[[ease-of-use]] +=== Ease of use + +The config schema is nontrivial; that's why it's important for the `git hook` +modifier commands to be usable. Contributors with UX expertise are encouraged to +share their suggestions. + +[[alternatives]] +== Alternative approaches + +A previous summary of alternatives exists in the +archives.footnote:[https://lore.kernel.org/git/20191116011125.GG22855@google.com] + +[[status-quo]] +=== Status quo + +Today users can implement multihooks themselves by using a "trampoline script" +as their hook, and pointing that script to a directory or list of other scripts +they wish to run. + +[[hook-directories]] +=== Hook directories + +Other contributors have suggested Git learn about the existence of a directory +such as `.git/hooks/.d` and execute those hooks in alphabetical order. + +[[comparison]] +=== Comparison table + +.Comparison of alternatives +|=== +|Feature |Config-based hooks |Hook directories |Status quo + +|Supports multiple hooks +|Natively +|Natively +|With user effort + +|Safer for zipped repos +|A little +|No +|No + +|Previous hooks just work +|If configured +|Yes +|Yes + +|Can install one hook to many repos +|Yes +|No +|No + +|Discoverability +|Better (in `git help git`) +|Same as before +|Same as before + +|Hard to run unexpected hook +|If configured +|No +|No +|=== + +[[future-work]] +== Future work + +[[execution-ordering]] +=== Execution ordering + +We may find that config order is insufficient for some users; for example, +config order makes it difficult to add a new hook to the system or global config +which runs at the end of the hook list. A new ordering schema should be: + +1) Specified by a `hook.order` config, so that users will not unexpectedly see +their order change; + +2) Either dependency or numerically based. + +Dependency-based ordering is prone to classic linked-list problems, like a +cycles and handling of missing dependencies. But, it paves the way for enabling +parallelization if some tasks truly depend on others. + +Numerical ordering makes it tricky for Git to generate suggested ordering +numbers for each command, but is easy to determine a definitive order. + +[[parallelization]] +=== Parallelization + +Users with many hooks might want to run them simultaneously, if the hooks don't +modify state; if one hook depends on another's output, then users will want to +specify those dependencies. If we decide to solve this problem, we may want to +look to modern build systems for inspiration on how to manage dependencies and +parallel tasks. + +[[securing-hookdir-hooks]] +=== Securing hookdir hooks + +With the design as written in this doc, it's still possible for a malicious user +to modify `.git/config` to include `hook.pre-receive.command = rm -rf /`, then +zip their repo and send it to another user. It may be necessary to teach Git to +only allow inlined hooks like this if they were configured outside of the local +scope (in other words, only run hookcmds, and only allow hookcmds to be +configured in global or system scope); or another approach, like a list of safe +projects, might be useful. It may also be sufficient (or at least useful) to +teach a `hook.disableAll` config or similar flag to the Git executable. + +[[submodule-inheritance]] +=== Submodule inheritance + +It's possible some submodules may want to run the identical set of hooks that +their superrepo runs. While a globally-configured hook set is helpful, it's not +a great solution for users who have multiple repos-with-submodules under the +same user. It would be useful for submodules to learn how to run hooks from +their superrepo's config, or inherit that hook setting. + +[[per-hook-event-settings]] +=== Per-hook-event settings + +It might be desirable to keep settings specifically for some hook events, but +not for others - for example, a user may wish to disable hookdir hooks for all +events but pre-commit, which they haven't had time to convert yet; or, a user +may wish for execution order settings to differ based on hook event. In that +case, it would be useful to set something like `hook.pre-commit.executionOrder` +which would not apply to the 'prepare-commit-msg' hook, for example. + +[[glossary]] +== Glossary + +*hook event* + +A point during Git's execution where user scripts may be run, for example, +_prepare-commit-msg_ or _pre-push_. + +*hook command* + +A user script or executable which will be run on one or more hook events. From patchwork Wed Oct 14 23:24:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838407 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 764D6C433E7 for ; Thu, 15 Oct 2020 01:55:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 20DDD22255 for ; Thu, 15 Oct 2020 01:55:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="C5hgez7a" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732450AbgJOBzL (ORCPT ); Wed, 14 Oct 2020 21:55:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53116 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732444AbgJOByk (ORCPT ); Wed, 14 Oct 2020 21:54:40 -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 59377C08EA7E for ; Wed, 14 Oct 2020 16:25:00 -0700 (PDT) Received: by mail-qt1-x849.google.com with SMTP id d22so797662qtn.0 for ; Wed, 14 Oct 2020 16:25:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=/7ik1i6dyouAjukNwAvYq0ihQd6vAXvoJNVKvCCMTuo=; b=C5hgez7aqMtsUBMRWo7yXWrQIJEJI7OJ/KpzNwPbQSEJc0++Sd5MyfJUn0l3x2tqqS dinbyAGhmXbVvwclQWAMAeAlDm8Np46/CtSk0yJXwhjVcnwwEvOXtT2UMxFzkVfGBz1I V/LVQHbmdEG6CTFK5WxO0rddNO+djyCQ2nLwDAL6PDTzwmBz6o6dUTgEoPFpXwdlnSoN RMd6miE6XqwH4jLeiJVcVcHqeCTUsEpgC46DiKye9eIlo2QthzJL4f53UARAmxHpxvGQ 4SdoaV+77NjDj91eRLp5VtRylQYJ0yRPOeB6lq6xDY8lxAAUm9MEkVEUnwit/uMovitw Tgvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=/7ik1i6dyouAjukNwAvYq0ihQd6vAXvoJNVKvCCMTuo=; b=GcvDIobAULt1o7DVdrSrJR9kX8EGK6lKx0JdviE9j2lYJ5DZr6OIFLRfCmBzGe7TCs 6P+dGO4bhHdpkKQla6jbFdoUYH4WSdPIBKZ0C2DTKbqw9eZFjkwjZEGhnL4F9RUQOOSD awVWVexI7pcAQO4mID8d28EaV8IaBs2fgEZxTFMlUK5dn51o89EjIvhXKJSywj0mXNJj o2Gw8UIscRPgCG0i121VACokdNtS8qQwnIOfo7aHb+U1WkSABynnNObZ8zJSOdGqCuJN m1xFgXQgXjj8OBCbPBWRuvEbDIcOGubX0eIk2YVWoJVf4rDIaiAOCvkZXiCoz3zBVeuV LHWA== X-Gm-Message-State: AOAM532df6MxaeehFnLIRPKda4AB4W0FgP6rdRc5Cqf13y9h3npyx2TI PIwEC+AMgH7I6uk6olnuT3m88/lZTGczdvuTq92cJ4Ob1lhKrSAQR5+KOlrSuv1zyTYF/z3wfQz QsXpqT674odQ0kz+3i9IdvsrNX1ErQOlGgrM8IKVRpZWT3Q6W3k1lIWwepKcRSr4Kv5Ak2wynCg == X-Google-Smtp-Source: ABdhPJwAY00nkB9DeUnPTZcv15cORU/7PcJ1BBcs5zgocMm0NQoBjbkrXFiNwRN/joGUpMK+UEedI+5SvpIKHJX3JNI= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a05:6214:11b3:: with SMTP id u19mr1993403qvv.51.1602717899344; Wed, 14 Oct 2020 16:24:59 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:41 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-3-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 2/8] hook: scaffolding for git-hook subcommand From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Introduce infrastructure for a new subcommand, git-hook, which will be used to ease config-based hook management. This command will handle parsing configs to compose a list of hooks to run for a given event, as well as adding or modifying hook configs in an interactive fashion. Signed-off-by: Emily Shaffer --- Notes: Since v4, mainly changed to RUN_SETUP_GENTLY so that 'git hook list' can be executed outside of a repo. .gitignore | 1 + Documentation/git-hook.txt | 20 ++++++++++++++++++++ Makefile | 1 + builtin.h | 1 + builtin/hook.c | 21 +++++++++++++++++++++ git.c | 1 + t/t1360-config-based-hooks.sh | 11 +++++++++++ 7 files changed, 56 insertions(+) create mode 100644 Documentation/git-hook.txt create mode 100644 builtin/hook.c create mode 100755 t/t1360-config-based-hooks.sh diff --git a/.gitignore b/.gitignore index 6232d33924..432e0b11cb 100644 --- a/.gitignore +++ b/.gitignore @@ -75,6 +75,7 @@ /git-grep /git-hash-object /git-help +/git-hook /git-http-backend /git-http-fetch /git-http-push diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt new file mode 100644 index 0000000000..9eeab0009d --- /dev/null +++ b/Documentation/git-hook.txt @@ -0,0 +1,20 @@ +git-hook(1) +=========== + +NAME +---- +git-hook - Manage configured hooks + +SYNOPSIS +-------- +[verse] +'git hook' + +DESCRIPTION +----------- +A placeholder command. Later, you will be able to list, add, and modify hooks +with this command. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 5311b1d2c4..9152f6d7c8 100644 --- a/Makefile +++ b/Makefile @@ -1095,6 +1095,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o BUILTIN_OBJS += builtin/grep.o BUILTIN_OBJS += builtin/hash-object.o BUILTIN_OBJS += builtin/help.o +BUILTIN_OBJS += builtin/hook.o BUILTIN_OBJS += builtin/index-pack.o BUILTIN_OBJS += builtin/init-db.o BUILTIN_OBJS += builtin/interpret-trailers.o diff --git a/builtin.h b/builtin.h index 53fb290963..3b20689d1a 100644 --- a/builtin.h +++ b/builtin.h @@ -162,6 +162,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix); int cmd_grep(int argc, const char **argv, const char *prefix); int cmd_hash_object(int argc, const char **argv, const char *prefix); int cmd_help(int argc, const char **argv, const char *prefix); +int cmd_hook(int argc, const char **argv, const char *prefix); int cmd_index_pack(int argc, const char **argv, const char *prefix); int cmd_init_db(int argc, const char **argv, const char *prefix); int cmd_interpret_trailers(int argc, const char **argv, const char *prefix); diff --git a/builtin/hook.c b/builtin/hook.c new file mode 100644 index 0000000000..b2bbc84d4d --- /dev/null +++ b/builtin/hook.c @@ -0,0 +1,21 @@ +#include "cache.h" + +#include "builtin.h" +#include "parse-options.h" + +static const char * const builtin_hook_usage[] = { + N_("git hook"), + NULL +}; + +int cmd_hook(int argc, const char **argv, const char *prefix) +{ + struct option builtin_hook_options[] = { + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, builtin_hook_options, + builtin_hook_usage, 0); + + return 0; +} diff --git a/git.c b/git.c index f1e8b56d99..caad1c877f 100644 --- a/git.c +++ b/git.c @@ -524,6 +524,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_GENTLY }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh new file mode 100755 index 0000000000..34b0df5216 --- /dev/null +++ b/t/t1360-config-based-hooks.sh @@ -0,0 +1,11 @@ +#!/bin/bash + +test_description='config-managed multihooks, including git-hook command' + +. ./test-lib.sh + +test_expect_success 'git hook command does not crash' ' + git hook +' + +test_done From patchwork Wed Oct 14 23:24:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838395 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 A518BC433E7 for ; Thu, 15 Oct 2020 01:54:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3695022255 for ; Thu, 15 Oct 2020 01:54:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="OKfo68EM" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732446AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53108 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732406AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -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 2B15BC08EA7F for ; Wed, 14 Oct 2020 16:25:02 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id x1so1138150ybi.4 for ; Wed, 14 Oct 2020 16:25:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=Hd4prjUmfg3BvzSKHm9kGV5CdPLwm6rG1IUZtHBeGvA=; b=OKfo68EMqIF0zIem0pznXbY4wDle28OmloEsZDokn8ovC67w3PX/b6AzYAxbu0ibOb OWgPZriM/L+Gyw/WGD2EVJ+Sstps+J9Q3PQY1ur/XbDbPuiioUY4dkMwgDkJdvXB97FC CQ1SSTI0I0qDcp2Km1N3SqyyssytsnWxCamXUmPJNDeEt8rJDtiIFaKqHE+KCrxe57UD CkflRq08v27m0h8tC7csgAa0l+SH5U5AYrKARXlolfejFCO1MEENDrthappA9wG+p0x7 62DFS/fZPPL7jtWobL4aqnKjhowFAYORS+3Vx4f0gEAcQhJ6AlTnRRPTG/XAM18xkAmb hi9w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=Hd4prjUmfg3BvzSKHm9kGV5CdPLwm6rG1IUZtHBeGvA=; b=CBOzKMKS74IyhA9o4G58EDJVYEMA/Ffc/i/WwyxdfNjl93LIZYRzo6ZGQNSqPgc/2/ mcx5PaTp40FiG31y6SKasQtLJkii6WrQH/viLoAPhSC2/XQFljYrcyOvYsYwBfJ8D8Kr D9ZvnDCSENXTvg6XYLvhLGe65rp+CehMR4aCh2Fl1AATfNwnkuPclmt6giyXmsP4SIi4 PbKmDJqRtiIFfXa48aRpcE1J/YKzgqUtAKeJ81ABgnIgUcgmzWfTMNwetREj7MYZGsG+ G3qiyux/APHwL8l/5kHjYxjRt0JARCaYq+EptO7r7Ycz4/VNP4jH5nI111QiLVQ4kJuy 0rAQ== X-Gm-Message-State: AOAM530hTQf0Lgua4pqOJw23zSgfe4xmNNIDyiwydYRsQz21MT/w7xwz HvkXKl5k/J9zLhKgcC4loVr7tYOuZR3/hM+JtWWnOHLZuI5/HiwJXmJgydz9Nbl81pBWd2zfMhA WCmsdUvcSrBPkeIgbusJ7HhSYaa1ePuUrC+RdIeBK4iFLllVE7lwY6blhnjzOpCgM8oVF05dLWA == X-Google-Smtp-Source: ABdhPJxLm3CTX4YFHjnokWiQWoT9/hCVntI23MHtkrelWES1R2ezrFn0dYOj9nHNcUe1Ixt/BerYBO1xCtr2/33p7/k= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:aaae:: with SMTP id t43mr1615289ybi.404.1602717901290; Wed, 14 Oct 2020 16:25:01 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:42 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-4-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 3/8] hook: add list command From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Teach 'git hook list ', which checks the known configs in order to create an ordered list of hooks to run on a given hook event. Multiple commands can be specified for a given hook by providing multiple "hook..command = " lines. Hooks will be run in config order. If more properties need to be set on a given hook in the future, commands can also be specified by providing "hook..command = ", as well as a "[hookcmd ]" subsection; at minimum, this subsection must contain a "hookcmd..command = " line. For example: $ git config --list | grep ^hook hook.pre-commit.command=baz hook.pre-commit.command=~/bar.sh hookcmd.baz.command=~/baz/from/hookcmd.sh $ git hook list pre-commit global: ~/baz/from/hookcmd.sh local: ~/bar.sh Signed-off-by: Emily Shaffer --- Notes: Since v4, more work on the documentation. Also a slight change to the output format (space instead of tab). Documentation/config/hook.txt | 9 +++ Documentation/git-hook.txt | 59 ++++++++++++++++- Makefile | 1 + builtin/hook.c | 56 +++++++++++++++-- hook.c | 115 ++++++++++++++++++++++++++++++++++ hook.h | 26 ++++++++ t/t1360-config-based-hooks.sh | 81 +++++++++++++++++++++++- 7 files changed, 338 insertions(+), 9 deletions(-) create mode 100644 Documentation/config/hook.txt create mode 100644 hook.c create mode 100644 hook.h diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt new file mode 100644 index 0000000000..71449ecbc7 --- /dev/null +++ b/Documentation/config/hook.txt @@ -0,0 +1,9 @@ +hook..command:: + A command to execute during the hook event. This can be an + executable on your device, a oneliner for your shell, or the name of a + hookcmd. See linkgit:git-hook[1]. + +hookcmd..command:: + A command to execute during a hook for which has been specified + as a command. This can be an executable on your device or a oneliner for + your shell. See linkgit:git-hook[1]. diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 9eeab0009d..f19875ed68 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -8,12 +8,65 @@ git-hook - Manage configured hooks SYNOPSIS -------- [verse] -'git hook' +'git hook' list DESCRIPTION ----------- -A placeholder command. Later, you will be able to list, add, and modify hooks -with this command. +You can list configured hooks with this command. Later, you will be able to run, +add, and modify hooks with this command. + +This command parses the default configuration files for sections `hook` and +`hookcmd`. `hook` is used to describe the commands which will be run during a +particular hook event; commands are run in the order Git encounters them during +the configuration parse (see linkgit:git-config[1]). `hookcmd` is used to +describe attributes of a specific command. If additional attributes don't need +to be specified, a command to run can be specified directly in the `hook` +section; if a `hookcmd` by that name isn't found, Git will attempt to run the +provided value directly. For example: + +Global config +---- + [hook "post-commit"] + command = "linter" + command = "~/typocheck.sh" + + [hookcmd "linter"] + command = "/bin/linter --c" +---- + +Local config +---- + [hook "prepare-commit-msg"] + command = "linter" + [hook "post-commit"] + command = "python ~/run-test-suite.py" +---- + +With these configs, you'd then see: + +---- +$ git hook list "post-commit" +global: /bin/linter --c +global: ~/typocheck.sh +local: python ~/run-test-suite.py + +$ git hook list "prepare-commit-msg" +local: /bin/linter --c +---- + +COMMANDS +-------- + +list ``:: + +List the hooks which have been configured for ``. Hooks appear +in the order they should be run, and print the config scope where the relevant +`hook..command` was specified, not the `hookcmd` (if applicable). +This output is human-readable and the format is subject to change over time. + +CONFIGURATION +------------- +include::config/hook.txt[] GIT --- diff --git a/Makefile b/Makefile index 9152f6d7c8..5cd1486e42 100644 --- a/Makefile +++ b/Makefile @@ -902,6 +902,7 @@ LIB_OBJS += grep.o LIB_OBJS += hashmap.o LIB_OBJS += help.o LIB_OBJS += hex.o +LIB_OBJS += hook.o LIB_OBJS += ident.o LIB_OBJS += json-writer.o LIB_OBJS += kwset.o diff --git a/builtin/hook.c b/builtin/hook.c index b2bbc84d4d..4d36de52f8 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -1,21 +1,69 @@ #include "cache.h" #include "builtin.h" +#include "config.h" +#include "hook.h" #include "parse-options.h" +#include "strbuf.h" static const char * const builtin_hook_usage[] = { - N_("git hook"), + N_("git hook list "), NULL }; -int cmd_hook(int argc, const char **argv, const char *prefix) +static int list(int argc, const char **argv, const char *prefix) { - struct option builtin_hook_options[] = { + struct list_head *head, *pos; + struct hook *item; + struct strbuf hookname = STRBUF_INIT; + + struct option list_options[] = { OPT_END(), }; - argc = parse_options(argc, argv, prefix, builtin_hook_options, + argc = parse_options(argc, argv, prefix, list_options, builtin_hook_usage, 0); + if (argc < 1) { + usage_msg_opt(_("You must specify a hook event name to list."), + builtin_hook_usage, list_options); + } + + strbuf_addstr(&hookname, argv[0]); + + head = hook_list(&hookname); + + if (list_empty(head)) { + printf(_("no commands configured for hook '%s'\n"), + hookname.buf); + strbuf_release(&hookname); + return 0; + } + + list_for_each(pos, head) { + item = list_entry(pos, struct hook, list); + if (item) + printf("%s: %s\n", + config_scope_name(item->origin), + item->command.buf); + } + + clear_hook_list(head); + strbuf_release(&hookname); + return 0; } + +int cmd_hook(int argc, const char **argv, const char *prefix) +{ + struct option builtin_hook_options[] = { + OPT_END(), + }; + if (argc < 2) + usage_with_options(builtin_hook_usage, builtin_hook_options); + + if (!strcmp(argv[1], "list")) + return list(argc - 1, argv + 1, prefix); + + usage_with_options(builtin_hook_usage, builtin_hook_options); +} diff --git a/hook.c b/hook.c new file mode 100644 index 0000000000..937dc768c8 --- /dev/null +++ b/hook.c @@ -0,0 +1,115 @@ +#include "cache.h" + +#include "hook.h" +#include "config.h" + +void free_hook(struct hook *ptr) +{ + if (ptr) { + strbuf_release(&ptr->command); + free(ptr); + } +} + +static void append_or_move_hook(struct list_head *head, const char *command) +{ + struct list_head *pos = NULL, *tmp = NULL; + struct hook *to_add = NULL; + + /* + * remove the prior entry with this command; we'll replace it at the + * end. + */ + list_for_each_safe(pos, tmp, head) { + struct hook *it = list_entry(pos, struct hook, list); + if (!strcmp(it->command.buf, command)) { + list_del(pos); + /* we'll simply move the hook to the end */ + to_add = it; + } + } + + if (!to_add) { + /* adding a new hook, not moving an old one */ + to_add = xmalloc(sizeof(struct hook)); + strbuf_init(&to_add->command, 0); + strbuf_addstr(&to_add->command, command); + } + + /* re-set the scope so we show where an override was specified */ + to_add->origin = current_config_scope(); + + list_add_tail(&to_add->list, pos); +} + +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); +} + +struct hook_config_cb +{ + struct strbuf *hookname; + struct list_head *list; +}; + +static int hook_config_lookup(const char *key, const char *value, void *cb_data) +{ + struct hook_config_cb *data = cb_data; + const char *hook_key = data->hookname->buf; + struct list_head *head = data->list; + + if (!strcmp(key, hook_key)) { + const char *command = value; + struct strbuf hookcmd_name = STRBUF_INIT; + + /* Check if a hookcmd with that name exists. */ + strbuf_addf(&hookcmd_name, "hookcmd.%s.command", command); + git_config_get_value(hookcmd_name.buf, &command); + + if (!command) { + strbuf_release(&hookcmd_name); + BUG("git_config_get_value overwrote a string it shouldn't have"); + } + + /* + * TODO: implement an option-getting callback, e.g. + * get configs by pattern hookcmd.$value.* + * for each key+value, do_callback(key, value, cb_data) + */ + + append_or_move_hook(head, command); + + strbuf_release(&hookcmd_name); + } + + return 0; +} + +struct list_head* hook_list(const struct strbuf* hookname) +{ + struct strbuf hook_key = STRBUF_INIT; + struct list_head *hook_head = xmalloc(sizeof(struct list_head)); + struct hook_config_cb cb_data = { &hook_key, hook_head }; + + INIT_LIST_HEAD(hook_head); + + if (!hookname) + return NULL; + + strbuf_addf(&hook_key, "hook.%s.command", hookname->buf); + + git_config(hook_config_lookup, (void*)&cb_data); + + strbuf_release(&hook_key); + return hook_head; +} diff --git a/hook.h b/hook.h new file mode 100644 index 0000000000..8ffc4f14b6 --- /dev/null +++ b/hook.h @@ -0,0 +1,26 @@ +#include "config.h" +#include "list.h" +#include "strbuf.h" + +struct hook +{ + struct list_head list; + /* + * Config file which holds the hook.*.command definition. + * (This has nothing to do with the hookcmd..* configs.) + */ + enum config_scope origin; + /* The literal command to run. */ + struct strbuf command; +}; + +/* + * Provides a linked list of 'struct hook' detailing commands which should run + * in response to the 'hookname' event, in execution order. + */ +struct list_head* hook_list(const struct strbuf *hookname); + +/* Free memory associated with a 'struct hook' */ +void free_hook(struct hook *ptr); +/* Empties the list at 'head', calling 'free_hook()' on each entry */ +void clear_hook_list(struct list_head *head); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 34b0df5216..6e4a3e763f 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -4,8 +4,85 @@ test_description='config-managed multihooks, including git-hook command' . ./test-lib.sh -test_expect_success 'git hook command does not crash' ' - git hook +ROOT= +if test_have_prereq MINGW +then + # In Git for Windows, Unix-like paths work only in shell scripts; + # `git.exe`, however, will prefix them with the pseudo root directory + # (of the Unix shell). Let's accommodate for that. + ROOT="$(cd / && pwd)" +fi + +setup_hooks () { + test_config hook.pre-commit.command "/path/ghi" --add + test_config_global hook.pre-commit.command "/path/def" --add +} + +setup_hookcmd () { + test_config hook.pre-commit.command "abc" --add + test_config_global hookcmd.abc.command "/path/abc" --add +} + +test_expect_success 'git hook rejects commands without a mode' ' + test_must_fail git hook pre-commit +' + + +test_expect_success 'git hook rejects commands without a hookname' ' + test_must_fail git hook list +' + +test_expect_success 'git hook runs outside of a repo' ' + setup_hooks && + + cat >expected <<-EOF && + global: $ROOT/path/def + EOF + + nongit git config --list --global && + + nongit git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list orders by config order' ' + setup_hooks && + + cat >expected <<-EOF && + global: $ROOT/path/def + local: $ROOT/path/ghi + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list dereferences a hookcmd' ' + setup_hooks && + setup_hookcmd && + + cat >expected <<-EOF && + global: $ROOT/path/def + local: $ROOT/path/ghi + local: $ROOT/path/abc + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'git hook list reorders on duplicate commands' ' + setup_hooks && + + test_config hook.pre-commit.command "/path/def" --add && + + cat >expected <<-EOF && + local: $ROOT/path/ghi + local: $ROOT/path/def + EOF + + git hook list pre-commit >actual && + test_cmp expected actual ' test_done From patchwork Wed Oct 14 23:24:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838405 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 A1E29C433E7 for ; Thu, 15 Oct 2020 01:54:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3858122259 for ; Thu, 15 Oct 2020 01:54:56 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="qk2wj5Uk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732449AbgJOByu (ORCPT ); Wed, 14 Oct 2020 21:54:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53098 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732402AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -0400 Received: from mail-pf1-x449.google.com (mail-pf1-x449.google.com [IPv6:2607:f8b0:4864:20::449]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0FE38C08EAC1 for ; Wed, 14 Oct 2020 16:25:04 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id w78so504793pfc.15 for ; Wed, 14 Oct 2020 16:25:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=x1230MfiafhTEHlMUxwGcwVR7XUFArfqKELjukUYBMo=; b=qk2wj5UkKvk4BmwjRAXJGteu1yZS4CB92Rqj423btSnDNl4bK1r7tIlqkegI+7uwoH 9iP2PsHeuS6Eq4lUxMpOJH7Jwr3I1gZvquLqeS6bomuuK4dDD9c6j2LxwFfyKJL7sejh YcWWZzNUWbvIddm5DIrkfbCl8teBRjMQIq2PCSQfPHtEkhAaPtyrDsBtr0uuIo1MWSah dg2KQS7JlS+Jeov9tCquAUZHzT5JMUZSwxJLHqgPEISrHWU1IdBTx2Sjt+23skSJC0K9 QxZzybbbUZa2ObbeSnmOU1VTROjxtTuU7XjSoMXRAC3HIHS8qCYwDgqcQb5ySMTixGNV 1KSA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=x1230MfiafhTEHlMUxwGcwVR7XUFArfqKELjukUYBMo=; b=WQ30vESrfV3uSzsEadjlXHNLW/gfHkPbsWMino7Xudb0DjZ/LfJeKZnYCe2Rws1Xs6 e/leZEYxuaTbMLgellyBSgoY9k757U4INptP8dDsPaRjN/ZKyh0yG9vbI6bZ5NLFz/de Sdqk27co0vn1kK0KyiupUUq/lIwaBRgM/aAxhpRn+16z3qOFpnQrL8HPXOMybJbRZLM7 CSgnwGyCQSVxg2LPGnJGUCrTIeD7N9pDcqyuGsx3qY1v4Oxqy6aPtLwWIu7s0TBVa8+R 7WNRNvgsLl+71Q9LYav3WAPwf4qqm/iB81qrM6vY6FofYtkWmgRqEWfwxWPo/AwgwoLa KgFQ== X-Gm-Message-State: AOAM531/TrWxwxRbGIoRQk4+A1QaGOu+oKvHr9DkLs2SBsqL9pig/ujn 7K4AQ7bTgLLeBA1rDkWtUsUD6gWv9m8VDCigx8uU0wCYNMI9iA+JSHV9ayPC+0ssBO5pNpc+E/e gCxdsWoh/5i8D6IM+qLu8w9tXQqyHdBUYqoEoeT0LzhjrXYfefKqW8qaW4NL6SzFHG5qA9TClAw == X-Google-Smtp-Source: ABdhPJyFw0/ghkbMElbW2kG/g7P4xIDnqkqML5Nny4ejKzIJbEeU1ZDs8SfONzzFSZHVFVEHotI8fIBfrGGvwikRsyU= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:aa7:9501:0:b029:155:3b11:d5c4 with SMTP id b1-20020aa795010000b02901553b11d5c4mr1506192pfp.76.1602717903273; Wed, 14 Oct 2020 16:25:03 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:43 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-5-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 4/8] hook: include hookdir hook in list From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Historically, hooks are declared by placing an executable into $GIT_DIR/hooks/$HOOKNAME (or $HOOKDIR/$HOOKNAME). Although hooks taken from the config are more featureful than hooks placed in the $HOOKDIR, those hooks should not stop working for users who already have them. When we add hooks from $HOOKDIR to the list of all hooks to run, to support paths with spaces in them, quote legacy hook paths. Signed-off-by: Emily Shaffer --- Notes: Newly split into its own commit since v4, and taking place much sooner. An unfortunate side effect of adding this support *before* the hook.runHookDir support is that the labels on the list are not clear - because we aren't yet flagging which hooks are from the hookdir versus the config. I suppose we could move the addition of that field to the struct hook up to this patch, but it didn't make a lot of sense to me to do it just for cosmetic purposes. Documentation/config/hook.txt | 5 +++ builtin/hook.c | 70 +++++++++++++++++++++++++++++++---- hook.c | 36 ++++++++++++++++++ hook.h | 16 ++++++++ t/t1360-config-based-hooks.sh | 62 +++++++++++++++++++++++++++++++ 5 files changed, 182 insertions(+), 7 deletions(-) diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt index 71449ecbc7..75312754ae 100644 --- a/Documentation/config/hook.txt +++ b/Documentation/config/hook.txt @@ -7,3 +7,8 @@ hookcmd..command:: A command to execute during a hook for which has been specified as a command. This can be an executable on your device or a oneliner for your shell. See linkgit:git-hook[1]. + +hook.runHookDir:: + Controls how hooks contained in your hookdir are executed. Can be any of + "yes", "warn", "interactive", or "no". Defaults to "yes". See + linkgit:git-hook[1] and linkgit:git-config[1] "core.hooksPath"). diff --git a/builtin/hook.c b/builtin/hook.c index 4d36de52f8..16324d4195 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -11,11 +11,14 @@ static const char * const builtin_hook_usage[] = { NULL }; +static enum hookdir_opt should_run_hookdir; + static int list(int argc, const char **argv, const char *prefix) { struct list_head *head, *pos; struct hook *item; struct strbuf hookname = STRBUF_INIT; + struct strbuf hookdir_annotation = STRBUF_INIT; struct option list_options[] = { OPT_END(), @@ -40,12 +43,39 @@ static int list(int argc, const char **argv, const char *prefix) return 0; } + switch (should_run_hookdir) { + case hookdir_no: + strbuf_addstr(&hookdir_annotation, _(" (will not run)")); + break; + case hookdir_interactive: + strbuf_addstr(&hookdir_annotation, _(" (will prompt)")); + break; + case hookdir_warn: + case hookdir_unknown: + strbuf_addstr(&hookdir_annotation, _(" (will warn)")); + break; + case hookdir_yes: + /* + * The default behavior should agree with + * hook.c:configured_hookdir_opt(). + */ + default: + break; + } + list_for_each(pos, head) { item = list_entry(pos, struct hook, list); - if (item) - printf("%s: %s\n", - config_scope_name(item->origin), - item->command.buf); + if (item) { + /* Don't translate 'hookdir' - it matches the config */ + printf("%s: %s%s\n", + (item->from_hookdir + ? "hookdir" + : config_scope_name(item->origin)), + item->command.buf, + (item->from_hookdir + ? hookdir_annotation.buf + : "")); + } } clear_hook_list(head); @@ -56,14 +86,40 @@ static int list(int argc, const char **argv, const char *prefix) int cmd_hook(int argc, const char **argv, const char *prefix) { + const char *run_hookdir = NULL; + struct option builtin_hook_options[] = { + OPT_STRING(0, "run-hookdir", &run_hookdir, N_("option"), + N_("what to do with hooks found in the hookdir")), OPT_END(), }; - if (argc < 2) + + argc = parse_options(argc, argv, prefix, builtin_hook_options, + builtin_hook_usage, 0); + + /* after the parse, we should have " " */ + if (argc < 1) usage_with_options(builtin_hook_usage, builtin_hook_options); - if (!strcmp(argv[1], "list")) - return list(argc - 1, argv + 1, prefix); + + /* argument > config */ + if (run_hookdir) + if (!strcmp(run_hookdir, "no")) + should_run_hookdir = hookdir_no; + else if (!strcmp(run_hookdir, "yes")) + should_run_hookdir = hookdir_yes; + else if (!strcmp(run_hookdir, "warn")) + should_run_hookdir = hookdir_warn; + else if (!strcmp(run_hookdir, "interactive")) + should_run_hookdir = hookdir_interactive; + else + die(_("'%s' is not a valid option for --run-hookdir " + "(yes, warn, interactive, no)"), run_hookdir); + else + should_run_hookdir = configured_hookdir_opt(); + + if (!strcmp(argv[0], "list")) + return list(argc, argv, prefix); usage_with_options(builtin_hook_usage, builtin_hook_options); } diff --git a/hook.c b/hook.c index 937dc768c8..340e5a35c8 100644 --- a/hook.c +++ b/hook.c @@ -2,6 +2,7 @@ #include "hook.h" #include "config.h" +#include "run-command.h" void free_hook(struct hook *ptr) { @@ -34,6 +35,7 @@ static void append_or_move_hook(struct list_head *head, const char *command) to_add = xmalloc(sizeof(struct hook)); strbuf_init(&to_add->command, 0); strbuf_addstr(&to_add->command, command); + to_add->from_hookdir = 0; } /* re-set the scope so we show where an override was specified */ @@ -95,11 +97,33 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data) return 0; } +enum hookdir_opt configured_hookdir_opt(void) +{ + const char *key; + if (git_config_get_value("hook.runhookdir", &key)) + return hookdir_yes; /* by default, just run it. */ + + if (!strcmp(key, "no")) + return hookdir_no; + + if (!strcmp(key, "yes")) + return hookdir_yes; + + if (!strcmp(key, "warn")) + return hookdir_warn; + + if (!strcmp(key, "interactive")) + return hookdir_interactive; + + return hookdir_unknown; +} + struct list_head* hook_list(const struct strbuf* hookname) { struct strbuf hook_key = STRBUF_INIT; struct list_head *hook_head = xmalloc(sizeof(struct list_head)); struct hook_config_cb cb_data = { &hook_key, hook_head }; + const char *legacy_hook_path = NULL; INIT_LIST_HEAD(hook_head); @@ -110,6 +134,18 @@ struct list_head* hook_list(const struct strbuf* hookname) git_config(hook_config_lookup, (void*)&cb_data); + if (have_git_dir()) + legacy_hook_path = find_hook(hookname->buf); + + /* Unconditionally add legacy hook, but annotate it. */ + if (legacy_hook_path) { + struct hook *legacy_hook; + + append_or_move_hook(hook_head, absolute_path(legacy_hook_path)); + legacy_hook = list_entry(hook_head->prev, struct hook, list); + legacy_hook->from_hookdir = 1; + } + strbuf_release(&hook_key); return hook_head; } diff --git a/hook.h b/hook.h index 8ffc4f14b6..ca45d388d3 100644 --- a/hook.h +++ b/hook.h @@ -12,6 +12,7 @@ struct hook enum config_scope origin; /* The literal command to run. */ struct strbuf command; + int from_hookdir; }; /* @@ -20,6 +21,21 @@ struct hook */ struct list_head* hook_list(const struct strbuf *hookname); +enum hookdir_opt +{ + hookdir_no, + hookdir_warn, + hookdir_interactive, + hookdir_yes, + hookdir_unknown, +}; + +/* + * Provides the hookdir_opt specified in the config without consulting any + * command line arguments. + */ +enum hookdir_opt configured_hookdir_opt(void); + /* Free memory associated with a 'struct hook' */ void free_hook(struct hook *ptr); /* Empties the list at 'head', calling 'free_hook()' on each entry */ diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 6e4a3e763f..91127a50a4 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -23,6 +23,14 @@ setup_hookcmd () { test_config_global hookcmd.abc.command "/path/abc" --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 rejects commands without a mode' ' test_must_fail git hook pre-commit ' @@ -85,4 +93,58 @@ test_expect_success 'git hook list reorders on duplicate commands' ' test_cmp expected actual ' +test_expect_success 'git hook list shows hooks from the hookdir' ' + setup_hookdir && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'hook.runHookDir = no is respected by list' ' + setup_hookdir && + + test_config hook.runHookDir "no" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit (will not run) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_i18ncmp expected actual +' + +test_expect_success 'hook.runHookDir = warn is respected by list' ' + setup_hookdir && + + test_config hook.runHookDir "warn" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit (will warn) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_i18ncmp expected actual +' + + +test_expect_success 'hook.runHookDir = interactive is respected by list' ' + setup_hookdir && + + test_config hook.runHookDir "interactive" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit (will prompt) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_i18ncmp expected actual +' + test_done From patchwork Wed Oct 14 23:24:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838401 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 ABF7FC4363A for ; Thu, 15 Oct 2020 01:54:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5A12D22257 for ; Thu, 15 Oct 2020 01:54:46 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="IPLqv19l" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732438AbgJOByp (ORCPT ); Wed, 14 Oct 2020 21:54:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53098 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732437AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -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 346AFC08EAC2 for ; Wed, 14 Oct 2020 16:25:06 -0700 (PDT) Received: by mail-qt1-x849.google.com with SMTP id t4so753871qtd.23 for ; Wed, 14 Oct 2020 16:25:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=rgP0+z5JIC8ipIr9bMvY+BnU+XsgUIaNny6z7kLU62g=; b=IPLqv19lVKfYbAE7dCu8BL+w+Rby3kwt2a72S+v30AfSK/YbT+Q3DcUW7FMGU3ZwgI P5mvZCE28HF1bMT5VtHRsPTu4X3mdDb7NtdbCYHXD72FLPJlgNLktRspDsKN434llVjI Is880GTc39b+2onvn7h2clMnfF5hAGEXeJZZtonIJo6BbZ7IyTuM2S6Atxhz2Pxn4/ou qQf85GGJD1Dw286wXqO9jARMYc6Lzs/8UYjSK4xpC1ysDAfNbQbd3AQpUE1TkwAP+igW 8f2hHeB+NoTLwCEfeCny7R12ZYDHKBE6cL91i7mk0O4fAl36HRakPrMXJnIHpeYSVpnt 6+9Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=rgP0+z5JIC8ipIr9bMvY+BnU+XsgUIaNny6z7kLU62g=; b=FKbnI12itr2BOdWNsI2YZJDBwxXyLOyerW/ap5Vv8QMsslTBeMArFobhZHIb677qxU UMS+bCLsVuukdytqmdAuZ40S5PXUnSmQiBvX3gTMeZRolnj58J1dYiB+ZM8k/LdOae+I siQXTBuyUEeA3ByDvwpBBX6w8JXTKMRs/33AfAdEGtK4NnG3KLQBzwwwqCSAKYj+iBMu 4fLB0t904yJvZxpp2p19/hfX7yN2u2tztPkKjSN327aakvhzYxrJAvFM1ySuHBvlmWMG k2/3BmoOmRiDCTtXwYkC8m0fLjif9UCHG0c7ywUYWyI5IaMSCuvxgjPJHncizhxejA7y 8OIQ== X-Gm-Message-State: AOAM533bcyCuqToe253QCB1j0r+8IPENa9GH6BGmmWVfdYc5KYap7Req wctqnFhqpWBPemgux9iaJ5mBZR2PP2DDtQoqDF08XZA0bsiexG/4I/8XPPPOFPvDjMdBOXnIsm5 d7fFsI+UUKbW+GgLdoywOs9vp7zZFy7aa18FoftT+b2nWPhJlNtpssjHIyF9J5Moa7rNE+/YOWw == X-Google-Smtp-Source: ABdhPJxE5f82WzB5V6fSkX6GO8aGgtlyRxgIsx11CWAez3O/kb368pXpJXt7cAJB7PrF/5RmgEcLQVa6wjowcmnpL74= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a0c:a2a6:: with SMTP id g35mr1794550qva.4.1602717905253; Wed, 14 Oct 2020 16:25:05 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:44 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-6-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 5/8] hook: implement hookcmd..skip From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org If a user wants a specific repo to skip execution of a hook which is set at a global or system level, they can now do so by specifying 'skip' in their repo config: ~/.gitconfig [hook.pre-commit] command = skippable-oneliner command = skippable-hookcmd [hookcmd.skippable-hookcmd] command = foo.sh $GIT_DIR/.git/config [hookcmd.skippable-oneliner] skip = true [hookcmd.skippable-hookcmd] skip = true Later it may make sense to add an option like "hookcmd..-skip" - but for simplicity, let's start with a universal skip setting like this. Signed-off-by: Emily Shaffer --- Notes: New since v4. During the Google team's review club I was reminded about this whole 'skip' option I never implemented. It's true that it's impossible to exclude a given hook without this; however, I think I have some more work to do on it, so consider it RFC for now and tell me what you think :) - Emily hook.c | 37 +++++++++++++++++++++++++---------- t/t1360-config-based-hooks.sh | 23 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 10 deletions(-) diff --git a/hook.c b/hook.c index 340e5a35c8..f4084e33c8 100644 --- a/hook.c +++ b/hook.c @@ -12,23 +12,24 @@ void free_hook(struct hook *ptr) } } -static void append_or_move_hook(struct list_head *head, const char *command) +static struct hook* find_hook_by_command(struct list_head *head, const char *command) { struct list_head *pos = NULL, *tmp = NULL; - struct hook *to_add = NULL; + struct hook *found = NULL; - /* - * remove the prior entry with this command; we'll replace it at the - * end. - */ list_for_each_safe(pos, tmp, head) { struct hook *it = list_entry(pos, struct hook, list); if (!strcmp(it->command.buf, command)) { list_del(pos); - /* we'll simply move the hook to the end */ - to_add = it; + found = it; } } + return found; +} + +static void append_or_move_hook(struct list_head *head, const char *command) +{ + struct hook *to_add = find_hook_by_command(head, command); if (!to_add) { /* adding a new hook, not moving an old one */ @@ -41,7 +42,7 @@ static void append_or_move_hook(struct list_head *head, const char *command) /* re-set the scope so we show where an override was specified */ to_add->origin = current_config_scope(); - list_add_tail(&to_add->list, pos); + list_add_tail(&to_add->list, head); } static void remove_hook(struct list_head *to_remove) @@ -73,8 +74,18 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data) if (!strcmp(key, hook_key)) { const char *command = value; struct strbuf hookcmd_name = STRBUF_INIT; + int skip = 0; + + /* + * Check if we're removing that hook instead. Hookcmds are + * removed by name, and inlined hooks are removed by command + * content. + */ + strbuf_addf(&hookcmd_name, "hookcmd.%s.skip", command); + git_config_get_bool(hookcmd_name.buf, &skip); /* Check if a hookcmd with that name exists. */ + strbuf_reset(&hookcmd_name); strbuf_addf(&hookcmd_name, "hookcmd.%s.command", command); git_config_get_value(hookcmd_name.buf, &command); @@ -89,7 +100,13 @@ static int hook_config_lookup(const char *key, const char *value, void *cb_data) * for each key+value, do_callback(key, value, cb_data) */ - append_or_move_hook(head, command); + if (skip) { + struct hook *to_remove = find_hook_by_command(head, command); + if (to_remove) + remove_hook(&(to_remove->list)); + } else { + append_or_move_hook(head, command); + } strbuf_release(&hookcmd_name); } diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 91127a50a4..ebd3bc623f 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -132,6 +132,29 @@ test_expect_success 'hook.runHookDir = warn is respected by list' ' test_i18ncmp expected actual ' +test_expect_success 'git hook list removes skipped hookcmd' ' + setup_hookcmd && + test_config hookcmd.abc.skip "true" --add && + + cat >expected <<-EOF && + no commands configured for hook '\''pre-commit'\'' + EOF + + git hook list pre-commit >actual && + test_i18ncmp expected actual +' + +test_expect_success 'git hook list removes skipped inlined hook' ' + setup_hooks && + test_config hookcmd."$ROOT/path/ghi".skip "true" --add && + + cat >expected <<-EOF && + global: $ROOT/path/def + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' test_expect_success 'hook.runHookDir = interactive is respected by list' ' setup_hookdir && From patchwork Wed Oct 14 23:24:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838397 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,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 02142C433DF for ; Thu, 15 Oct 2020 01:54:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8689B22258 for ; Thu, 15 Oct 2020 01:54:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="kerkV2Ep" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732415AbgJOByj (ORCPT ); Wed, 14 Oct 2020 21:54:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53114 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732412AbgJOByi (ORCPT ); Wed, 14 Oct 2020 21:54:38 -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 04469C08EADA for ; Wed, 14 Oct 2020 16:25:08 -0700 (PDT) Received: by mail-yb1-xb4a.google.com with SMTP id k9so1090145ybf.18 for ; Wed, 14 Oct 2020 16:25:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=tYBIp72At1pPq2NwvWzTuSgSJkDuk44JQzLmJTw7OgE=; b=kerkV2Ep5ZlYTv4FdXs89FdAByqAILzCv9Lo49Seaz85ZOoQ2cLhauSWMVW9NrLg4k jnETXGmQWqGVCkzpqnENdniVImJLWyq1MhekKfo7ZAp0qHQdzlhobqKJM5UlOcsb9TTc iG4yIs3SUVccZX6bV0IcS4XyVHFqFB1kStqQ+DXcA56RBq33ZlkkRXL5Q6i+h6zrLlow QA3XAhtdzNOfZ14/LAkb9wLsZVpJXwgeiaBxcLv6MO4RyaVLhq571nBQS7OqOTi4CeZs yQ9VZiACCNWMvmRo3q7ee3OCmc/X+GAOm2QykVfMgZ+sjNXNyqRU+EtARCOnf+qkSd3K 1kTQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=tYBIp72At1pPq2NwvWzTuSgSJkDuk44JQzLmJTw7OgE=; b=kTn/sIcRV7fiGggr6Nw5/TMIJ4120X3Qiot9mgxu6R3V1jHo9Z3NYJkUdbhRhOL15S GoT79uvNLf6dxN8JQWiuqNnCujePUl6iKqBx4FPfIyHNWkxrvM0pLS6tG/9MLM5h1yPx X+BCgT5TpHgJHKd74JZdVFSSxYP1txLd41kWH8sQyHxiLnQqulLPeq9i97BkHEgAesPe i0TUm1BDwNIWemoL497G9EYLqLykvUC7df0ty6l2W3YljarOQalTxUvyUJQxQQfR2S87 lDuVLF9eLh7rh+kb1SRHAcXCvdqdEMSWGUofKD6RG5DRnGcQ8bF3stz+0VnXqm8Cvp7H C8JA== X-Gm-Message-State: AOAM533rfWCs22Z+Cesi1kgSNJH6embi8MRqIMWJYLn6FD3ARfuTpbdU nSomVDYCzbp+2yjepj9vnmNAJHnss0XVxwmkRx6xUJ5swhrmCuzusIyywwuRWWcjACF5lVOlN7T RPF/LW9ddSGInlgBW3Cyh0zklMfKCiua2Ta5XdHRIKGRD9YhSxbKYsU5ej7DTvfI1rw/P1KUo6w == X-Google-Smtp-Source: ABdhPJzmzCdQqzPYV+sfnJXLlwEG30QyxbBQAB+szCqKPPPpwPf0UFrqowLpETYHtwwk6oKwARRz9Y38efQL8hkZSoY= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:6f89:: with SMTP id k131mr1632826ybc.21.1602717907165; Wed, 14 Oct 2020 16:25:07 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:45 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-7-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 6/8] parse-options: parse into strvec From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org parse-options already knows how to read into a string_list, and it knows how to read into an strvec as a passthrough (that is, including the argument as well as its value). string_list and strvec serve similar purposes but are somewhat painful to convert between; so, let's teach parse-options to read values of string arguments directly into an strvec without preserving the argument name. This is useful if collecting generic arguments to pass through to another command, for example, 'git hook run --arg "--quiet" --arg "--format=pretty" some-hook'. The resulting strvec would contain { "--quiet", "--format=pretty" }. The implementation is based on that of OPT_STRING_LIST. Signed-off-by: Emily Shaffer --- Notes: Since v4, fixed one or two more places where I missed the argv_array->strvec rename. Documentation/technical/api-parse-options.txt | 5 +++++ parse-options-cb.c | 16 ++++++++++++++++ parse-options.h | 4 ++++ 3 files changed, 25 insertions(+) diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 5a60bbfa7f..679bd98629 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -173,6 +173,11 @@ There are some macros to easily define options: The string argument is stored as an element in `string_list`. Use of `--no-option` will clear the list of preceding values. +`OPT_STRVEC(short, long, &struct strvec, arg_str, description)`:: + Introduce an option with a string argument. + The string argument is stored as an element in `strvec`. + Use of `--no-option` will clear the list of preceding values. + `OPT_INTEGER(short, long, &int_var, description)`:: Introduce an option with integer argument. The integer is put into `int_var`. diff --git a/parse-options-cb.c b/parse-options-cb.c index 4542d4d3f9..c2451dfb1b 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -207,6 +207,22 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset) return 0; } +int parse_opt_strvec(const struct option *opt, const char *arg, int unset) +{ + struct strvec *v = opt->value; + + if (unset) { + strvec_clear(v); + return 0; + } + + if (!arg) + return -1; + + strvec_push(v, arg); + return 0; +} + int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) { return 0; diff --git a/parse-options.h b/parse-options.h index 7030d8f3da..75cc8c7c96 100644 --- a/parse-options.h +++ b/parse-options.h @@ -177,6 +177,9 @@ struct option { #define OPT_STRING_LIST(s, l, v, a, h) \ { OPTION_CALLBACK, (s), (l), (v), (a), \ (h), 0, &parse_opt_string_list } +#define OPT_STRVEC(s, l, v, a, h) \ + { OPTION_CALLBACK, (s), (l), (v), (a), \ + (h), 0, &parse_opt_strvec } #define OPT_UYN(s, l, v, h) { OPTION_CALLBACK, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG, &parse_opt_tertiary } #define OPT_EXPIRY_DATE(s, l, v, h) \ @@ -296,6 +299,7 @@ int parse_opt_commits(const struct option *, const char *, int); int parse_opt_commit(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); +int parse_opt_strvec(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, From patchwork Wed Oct 14 23:24:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11838409 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=-20.4 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,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 B805CC43457 for ; Thu, 15 Oct 2020 01:55:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5AB9522255 for ; Thu, 15 Oct 2020 01:55:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="GHEAatLw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732475AbgJOBzM (ORCPT ); Wed, 14 Oct 2020 21:55:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53110 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732445AbgJOByj (ORCPT ); Wed, 14 Oct 2020 21:54:39 -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 0A4E0C08EADB for ; Wed, 14 Oct 2020 16:25:10 -0700 (PDT) Received: by mail-qv1-xf49.google.com with SMTP id dd7so544714qvb.6 for ; Wed, 14 Oct 2020 16:25:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=sender:date:in-reply-to:message-id:mime-version:references:subject :from:to:cc; bh=d+FqIc2lyb1r5z3rvfnilYsPPZMNRaeJTZVBsMYwVVo=; b=GHEAatLwm3w7siSHv9UrMATmJcc85thMgS5IIXBJQ/R7iFle7J2YbWT0HCphICgRjN jd4CIb7KTUo9BPWhiw1ZoJDdqURbMtGEn8CJedr6CfRDQYVSF4m3jATuLrIf29nLo08q T9pPm1WM8h5hzSCoKQ1/h/VDeHiobROXIh+0X5Bs4XMPJCs6y0Q7XwGeOxH1HdHRaAc4 UFl//zT/L8uv5jObpZBlBxTiAcN32wq/cXZoSN3zHXuvsF5UKsxojYjqAW3wkd/zGV7a gITR5D6EEyIbKagK0ilNRs41So2snbc0jZYoR8xcYkEfUlQiR9jVUGIkDCd5u+AetbXo txCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=d+FqIc2lyb1r5z3rvfnilYsPPZMNRaeJTZVBsMYwVVo=; b=jd2PENkV56Q3rvrC9fFy+dTNG+4/dGQbHJpaG7CDyafYr9IrtGYwE7y3ou4PdQcgPJ ukjQFzYUdgYDOSniz81JO1tTMJuL3QBN9Zkhqvt7SChQ7qMU+jv5zBt1mYPkJk2Q7+W0 2ZRcQtvtoHPbWjfji89/IFhXqfAr8a/mtouiix8ndfvQ+DTx0eWCIep5wIDRWHNCNbN2 8OdNw6XDVJv4kUmSgG4DAzzJpmZxum4dlaflSGC5EEwgdWFEgTtMvnUFJmzzTG6X5pBc ONZpbR7d9pAEm1zByvHAGsu/fVdMitqbqm2xFRkbMsPTl986Km3kLDjlDT/CKNx1a76B MVZg== X-Gm-Message-State: AOAM533kGrhsbQm6/W48/pV6vvRlQ7ukJlWy+TwLqTxP1TBecFG3PxdG g33S9LYhapZC/GzJmViOyvA2RJ7HlISps5nwPa14sw7/Fu3X0paZrNAqfB1TdxKS+46b3jPKwuz vtLMN6cvQi/dQMpxaPyJpiUx1VxncUCeoR1gFuc5LmVSa1OcL2um4uxFkimacm4CtwqOMp2H7FA == X-Google-Smtp-Source: ABdhPJy1JDupT4pB/bA34LKWgQuUevZik2uzbuipzByIFb2jtrEeujyV2Rf333C3eR2jZba75RFrNY2Zho5U/+cJZos= Sender: "emilyshaffer via sendgmr" X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:ad4:48c6:: with SMTP id v6mr2053994qvx.11.1602717909143; Wed, 14 Oct 2020 16:25:09 -0700 (PDT) Date: Wed, 14 Oct 2020 16:24:46 -0700 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201014232447.3050579-8-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201014232447.3050579-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH v5 7/8] hook: add 'run' subcommand From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In order to enable hooks to be run as an external process, by a standalone Git command, or by tools which wrap Git, provide an external means to run all configured hook commands for a given hook event. For now, the hook commands will run in config order, in series. As alternate ordering or parallelism is supported in the future, we should add knobs to use those to the command line as well. As with the legacy hook implementation, all stdout generated by hook commands is redirected to stderr. Piping from stdin is not yet supported. Legacy hooks (those present in $GITDIR/hooks) are run at the end of the execution list. For now, there is no way to disable them. Users may wish to provide hook commands like 'git config hook.pre-commit.command "~/linter.sh --pre-commit"'. To enable this, the contents of the 'hook.*.command' and 'hookcmd.*.command' strings are first split by space or quotes into an argv_array, then expanded with 'expand_user_path()'. Signed-off-by: Emily Shaffer --- Notes: Since v4, updated the docs, and did less local application of single quotes. In order for hookdir hooks to run successfully with a space in the path, though, they must not be run with 'sh -c'. So we can treat the hookdir hooks specially, and warn users via doc about special considerations for configured hooks with spaces in their path. Documentation/git-hook.txt | 12 +++- builtin/hook.c | 40 +++++++++++++- hook.c | 100 ++++++++++++++++++++++++++++++++++ hook.h | 7 +++ t/t1360-config-based-hooks.sh | 65 +++++++++++++++++++++- 5 files changed, 218 insertions(+), 6 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index f19875ed68..95d3687905 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -9,11 +9,12 @@ SYNOPSIS -------- [verse] 'git hook' list +'git hook' run DESCRIPTION ----------- -You can list configured hooks with this command. Later, you will be able to run, -add, and modify hooks with this command. +You can list and run configured hooks with this command. Later, you will be able +to add and modify hooks with this command. This command parses the default configuration files for sections `hook` and `hookcmd`. `hook` is used to describe the commands which will be run during a @@ -64,6 +65,13 @@ in the order they should be run, and print the config scope where the relevant `hook..command` was specified, not the `hookcmd` (if applicable). This output is human-readable and the format is subject to change over time. +run ``:: + +Runs hooks configured for ``, in the same order displayed by `git +hook list`. Hooks configured this way are run prepended with `sh -c`, so paths +containing special characters or spaces should be wrapped in single quotes: +`command = '/my/path with spaces/script.sh' some args`. + CONFIGURATION ------------- include::config/hook.txt[] diff --git a/builtin/hook.c b/builtin/hook.c index 16324d4195..64aad28e54 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -5,9 +5,11 @@ #include "hook.h" #include "parse-options.h" #include "strbuf.h" +#include "strvec.h" static const char * const builtin_hook_usage[] = { N_("git hook list "), + N_("git hook run [(-e|--env)=...] [(-a|--arg)=...] "), NULL }; @@ -84,6 +86,40 @@ static int list(int argc, const char **argv, const char *prefix) return 0; } +static int run(int argc, const char **argv, const char *prefix) +{ + struct strbuf hookname = STRBUF_INIT; + struct strvec envs = STRVEC_INIT; + struct strvec args = STRVEC_INIT; + + struct option run_options[] = { + OPT_STRVEC('e', "env", &envs, N_("var"), + N_("environment variables for hook to use")), + OPT_STRVEC('a', "arg", &args, N_("args"), + N_("argument to pass to hook")), + OPT_END(), + }; + + /* + * While it makes sense to list hooks out-of-repo, it doesn't make sense + * to execute them. Hooks usually want to look at repository artifacts. + */ + if (!have_git_dir()) + usage_msg_opt(_("You must be in a Git repo to execute hooks."), + builtin_hook_usage, run_options); + + argc = parse_options(argc, argv, prefix, run_options, + builtin_hook_usage, 0); + + if (argc < 1) + usage_msg_opt(_("You must specify a hook event to run."), + builtin_hook_usage, run_options); + + strbuf_addstr(&hookname, argv[0]); + + return run_hooks(envs.v, hookname.buf, &args, should_run_hookdir); +} + int cmd_hook(int argc, const char **argv, const char *prefix) { const char *run_hookdir = NULL; @@ -98,7 +134,7 @@ int cmd_hook(int argc, const char **argv, const char *prefix) builtin_hook_usage, 0); /* after the parse, we should have " " */ - if (argc < 1) + if (argc < 2) usage_with_options(builtin_hook_usage, builtin_hook_options); @@ -120,6 +156,8 @@ int cmd_hook(int argc, const char **argv, const char *prefix) if (!strcmp(argv[0], "list")) return list(argc, argv, prefix); + if (!strcmp(argv[0], "run")) + return run(argc, argv, prefix); usage_with_options(builtin_hook_usage, builtin_hook_options); } diff --git a/hook.c b/hook.c index f4084e33c8..1494a32c1a 100644 --- a/hook.c +++ b/hook.c @@ -3,6 +3,7 @@ #include "hook.h" #include "config.h" #include "run-command.h" +#include "prompt.h" void free_hook(struct hook *ptr) { @@ -135,6 +136,56 @@ enum hookdir_opt configured_hookdir_opt(void) return hookdir_unknown; } +static int should_include_hookdir(const char *path, enum hookdir_opt cfg) +{ + struct strbuf prompt = STRBUF_INIT; + /* + * If the path doesn't exist, don't bother adding the empty hook and + * don't bother checking the config or prompting the user. + */ + if (!path) + return 0; + + switch (cfg) + { + case hookdir_no: + return 0; + case hookdir_unknown: + fprintf(stderr, + _("Unrecognized value for 'hook.runHookDir'. " + "Is there a typo? ")); + /* FALLTHROUGH */ + case hookdir_warn: + fprintf(stderr, _("Running legacy hook at '%s'\n"), + path); + return 1; + case hookdir_interactive: + do { + /* + * TRANSLATORS: Make sure to include [Y] and [n] + * in your translation. Only English input is + * accepted. Default option is "yes". + */ + fprintf(stderr, _("Run '%s'? [Yn] "), path); + git_read_line_interactively(&prompt); + strbuf_tolower(&prompt); + if (starts_with(prompt.buf, "n")) { + strbuf_release(&prompt); + return 0; + } else if (starts_with(prompt.buf, "y")) { + strbuf_release(&prompt); + return 1; + } + /* otherwise, we didn't understand the input */ + } while (prompt.len); /* an empty reply means "Yes" */ + strbuf_release(&prompt); + return 1; + case hookdir_yes: + default: + return 1; + } +} + struct list_head* hook_list(const struct strbuf* hookname) { struct strbuf hook_key = STRBUF_INIT; @@ -166,3 +217,52 @@ struct list_head* hook_list(const struct strbuf* hookname) strbuf_release(&hook_key); return hook_head; } + + +int run_hooks(const char *const *env, const char *hookname, + const struct strvec *args, enum hookdir_opt run_hookdir) +{ + struct strbuf hookname_str = STRBUF_INIT; + struct list_head *to_run, *pos = NULL, *tmp = NULL; + int rc = 0; + + strbuf_addstr(&hookname_str, hookname); + + to_run = hook_list(&hookname_str); + + list_for_each_safe(pos, tmp, to_run) { + struct child_process hook_proc = CHILD_PROCESS_INIT; + struct hook *hook = list_entry(pos, struct hook, list); + + hook_proc.env = env; + hook_proc.no_stdin = 1; + hook_proc.stdout_to_stderr = 1; + hook_proc.trace2_hook_name = hook->command.buf; + hook_proc.use_shell = 1; + + + if (hook->from_hookdir) { + if (!should_include_hookdir(hook->command.buf, run_hookdir)) + continue; + /* + * Commands from the config could be oneliners, but we know + * for certain that hookdir commands are not. + */ + hook_proc.use_shell = 0; + } + + /* add command */ + strvec_push(&hook_proc.args, hook->command.buf); + + /* + * add passed-in argv, without expanding - let the user get back + * exactly what they put in + */ + if (args) + strvec_pushv(&hook_proc.args, args->v); + + rc |= run_command(&hook_proc); + } + + return rc; +} diff --git a/hook.h b/hook.h index ca45d388d3..6eb1dc99c4 100644 --- a/hook.h +++ b/hook.h @@ -1,6 +1,7 @@ #include "config.h" #include "list.h" #include "strbuf.h" +#include "strvec.h" struct hook { @@ -35,6 +36,12 @@ enum hookdir_opt * command line arguments. */ enum hookdir_opt configured_hookdir_opt(void); +/* + * Runs all hooks associated to the 'hookname' event in order. Each hook will be + * passed 'env' and 'args'. + */ +int run_hooks(const char *const *env, const char *hookname, + const struct strvec *args, enum hookdir_opt run_hookdir); /* Free memory associated with a 'struct hook' */ void free_hook(struct hook *ptr); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index ebd3bc623f..5b3003d59b 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -115,7 +115,10 @@ test_expect_success 'hook.runHookDir = no is respected by list' ' git hook list pre-commit >actual && # the hookdir annotation is translated - test_i18ncmp expected actual + test_i18ncmp expected actual && + + git hook run pre-commit 2>actual && + test_must_be_empty actual ' test_expect_success 'hook.runHookDir = warn is respected by list' ' @@ -129,6 +132,14 @@ test_expect_success 'hook.runHookDir = warn is respected by list' ' git hook list pre-commit >actual && # the hookdir annotation is translated + test_i18ncmp expected actual && + + cat >expected <<-EOF && + Running legacy hook at '\''$(pwd)/.git/hooks/pre-commit'\'' + "Legacy Hook" + EOF + + git hook run pre-commit 2>actual && test_i18ncmp expected actual ' @@ -156,7 +167,7 @@ test_expect_success 'git hook list removes skipped inlined hook' ' test_cmp expected actual ' -test_expect_success 'hook.runHookDir = interactive is respected by list' ' +test_expect_success 'hook.runHookDir = interactive is respected by list and run' ' setup_hookdir && test_config hook.runHookDir "interactive" && @@ -167,7 +178,55 @@ test_expect_success 'hook.runHookDir = interactive is respected by list' ' git hook list pre-commit >actual && # the hookdir annotation is translated - test_i18ncmp expected actual + test_i18ncmp expected actual && + + test_write_lines n | git hook run pre-commit 2>actual && + ! grep "Legacy Hook" actual && + + test_write_lines y | git hook run pre-commit 2>actual && + grep "Legacy Hook" actual +' + +test_expect_success 'inline hook definitions execute oneliners' ' + test_config hook.pre-commit.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.pre-commit.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 'out-of-repo runs excluded' ' + setup_hooks && + + nongit test_must_fail git hook run pre-commit ' test_done