From patchwork Wed Sep 9 00:49:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764767 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A491513B1 for ; Wed, 9 Sep 2020 00:50:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 849A421973 for ; Wed, 9 Sep 2020 00:50:05 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="FKUh63Ho" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729691AbgIIAt7 (ORCPT ); Tue, 8 Sep 2020 20:49:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52814 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729614AbgIIAtw (ORCPT ); Tue, 8 Sep 2020 20:49:52 -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 D2C34C061755 for ; Tue, 8 Sep 2020 17:49:50 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id o184so725000pfb.12 for ; Tue, 08 Sep 2020 17:49:50 -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=FSdhIQDSbEpwwWcFbR7qZ2gy0jA2+EOf+oTCqK9t0kA=; b=FKUh63Ho80we6NOcAwMRW29Ni7o6rwVb5YMOfEDzQ/GTihJnUEYZmfSFUdRR4loFyE Zvq1uavvGRpFqGcVhDBzPgp1J7m7lX1KO34WaSDSdoAtlXb9l86td+vwWhDfnDTzxRT6 26d58M3vM5ZVbXhU6J61uApMgDh1ove99DzRj/GR/oYEGkKKKaG2DC/TWXxSHtW5elZx 7yTTIeY+swr9q8SiQqD/Is9TMDnH9c6h5VdM8QxZuUbV/xpOHK4WqQ77ymLLEB8Pce+O Ya3qb8C1SIIfUyk6Rbqit4VIm/M78Dj8ygYhyNErnPKCPWF+KFrsxMSI+ckB5puzPai4 uHEw== 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=FSdhIQDSbEpwwWcFbR7qZ2gy0jA2+EOf+oTCqK9t0kA=; b=RIIs5m5Qhpn635mkVpQ80Alz1ja/WApb88igrgO5F4b9Z1fV+MmWqNt6u08U2rpE5w w7uUF7YG/IQHRaK8uZMFBXKI8ywgb5MK3rlR8kX0ER/ln5XSUMVtpvvmNjSRDTJRq7gJ CeZQlOrytRJInjhio79N+vQ+0jZmJL/Chww2ke3TL+21HviaDp9Nas8A5IRjmtM+mJ3d rcdBxr1Qutf3KwALYPMVk1X6jJ26qiFa7JcRixGRI3Ax5L7688K1sBuThMIYP7QG6cB6 9D7kx1fR3DMzcrSj5AAkPhclaKUaFUIoZIQXPd0zELxh64IkDRi/h60juJyvaZ1cIqKx mUqA== X-Gm-Message-State: AOAM533HiH2AO9yHVEr7HRl6LE/6OmFHl2njJr4rT65DMdXmJGIO2Jkq luDeoOT6J5NjQgGnIfDckti9z9/ulQcPRyTbHiHbXTICs43X2nkbfRAZ4V6e/hIDlarAdfn7Y8b rAGYEh1PGWj5kz3O9IyHldJazgNQJVEk21BvnXPK6vTQokivka8s/bZiC1EvDOJpOCb6Vvtn6hg == X-Google-Smtp-Source: ABdhPJwvqr54qtx0ww/kLg2KGYT1QO9RdNv9XGrCuwqmJK8PmrKxbNT1kqwDzqaEQDLeDh0kMFjNvZgUUFhrdFXCS4E= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a62:3047:0:b029:13e:d13d:a088 with SMTP id w68-20020a6230470000b029013ed13da088mr1559701pfw.31.1599612590161; Tue, 08 Sep 2020 17:49:50 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:31 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-2-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 1/9] doc: propose hooks managed by the config From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org 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 --- Documentation/Makefile | 1 + .../technical/config-based-hooks.txt | 354 ++++++++++++++++++ 2 files changed, 355 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..c6e762b192 --- /dev/null +++ b/Documentation/technical/config-based-hooks.txt @@ -0,0 +1,354 @@ +Configuration-based hook management +=================================== +:sectanchors: + +[[motivation]] +== Motivation + +Treat hooks as a first-class citizen by replacing the .git/hook/hookname path as +the only source of hooks to execute, 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. These order of 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. Hook event subsections can +also contain per-hook-event settings. + +Also contains top-level hook execution settings, for example, +`hook.warnHookDir`, `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 + 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 + 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 `argv_array` provided by the code which +generates the hook event: + +*`int run_hooks(const char *hookname, struct argv_array *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. + +[[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 Sep 9 00:49:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764769 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 54BE713B1 for ; Wed, 9 Sep 2020 00:50:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 39EB721973 for ; Wed, 9 Sep 2020 00:50:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="WYLmF6O1" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729767AbgIIAuH (ORCPT ); Tue, 8 Sep 2020 20:50:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52822 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728443AbgIIAtx (ORCPT ); Tue, 8 Sep 2020 20:49:53 -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 DD005C061573 for ; Tue, 8 Sep 2020 17:49:52 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id k13so739756pfh.4 for ; Tue, 08 Sep 2020 17:49:52 -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=r6qnFQQAQ0pBsTJaIgLEW50yMm9fukbofv9T2JJc0g8=; b=WYLmF6O1kdA/ZgvFUtFXUIrQRkavA9oo2Op4WxQkwo7rcutHF8s0bN1eD3yklrEezg YnI0YlPTXtS0/iS37A2wOSUlEOWNrfh5oVtxOHGqfc5f0T9W0mbxXtq2Qbr96QW/Dmxm qZzc7IAnJchRG6t2VV6YkGL90UL94nONG5so/1gkRIK299i7Je4hOj6PD/g6TPV2SEqB 7QgFewD7BpRBpA5qePfMwdvbkzbBsgGYT/R7RUcFzwDEHMxPUt9TlSeXGVRwJZgvGPpQ TQKPw224CRFhD8J3WYN8f7MGXwZOIGaySlTcs/uzVdpsodfaoiAsOtRjtAf71Zq0crnI SKoQ== 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=r6qnFQQAQ0pBsTJaIgLEW50yMm9fukbofv9T2JJc0g8=; b=T+FbtTV9G2iyRO593FEjBCJEVY21h3TCONQsHH84AcJim6V3lH/9UImGkw9V2R+rSH fCeQSChdcP4zStnEIinT/oCH0S9YX6qJfpM1XaDJYPCZo8xKgEWfNk5cWd4TkceHgodF eQJu3SsxSjs/2UJw2wcdVIkcXLPqvRAk0ftJ7tb5HoyZrs59y/vD3f2j7SzEO9HD0nvP R/xAQF+vn+653G1zxRcDB/mcSQ4MLUKyV4rfxUnzB17k8O6Oxg2Rlr7zUlq8QBMIPXAH c2q9Ep4EPdtYxvgrZhHkEKv20CNaAS9PnTEwjhwuBmtllM27ZMyyFbXVnkWC9lT4ePix jlLg== X-Gm-Message-State: AOAM531AbPcCDMWcvl4iF2fWGY1V+l4qWSLN74LDVt0048PdZdY6bz0B t/ziwMJl0KTyC+TkleakeSSs0HzU52OyA/fbf7nyJr2cfk5HibA+Abz8n+Xifg8ghsekCAWgdq2 ooBgPawQb8ItcLK4Y1OU/egvtd09PWiqrtti7RrvMTLzDkcM056B/s7vAKYXka58ZPsbWWLyTug == X-Google-Smtp-Source: ABdhPJxI0alYjj2G7pOE5aQko4r+7ImHJnCYA2qq7AghOeQl2WDWgrmPw5+700+/vEmxYccItvoJ3f1Y0lpwOK9L6mg= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a63:374d:: with SMTP id g13mr1028930pgn.383.1599612592110; Tue, 08 Sep 2020 17:49:52 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:32 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-3-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 2/9] hook: scaffolding for git-hook subcommand From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org 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 --- .gitignore | 1 + Documentation/git-hook.txt | 19 +++++++++++++++++++ Makefile | 1 + builtin.h | 1 + builtin/hook.c | 21 +++++++++++++++++++++ git.c | 1 + t/t1360-config-based-hooks.sh | 11 +++++++++++ 7 files changed, 55 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 ee509a2ad2..0694a34884 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..2d50c414cc --- /dev/null +++ b/Documentation/git-hook.txt @@ -0,0 +1,19 @@ +git-hook(1) +=========== + +NAME +---- +git-hook - Manage configured hooks + +SYNOPSIS +-------- +[verse] +'git hook' + +DESCRIPTION +----------- +You can list, add, and modify hooks with this command. + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 65f8cfb236..6eee75555e 100644 --- a/Makefile +++ b/Makefile @@ -1077,6 +1077,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 a5ae15bfe5..4e736499c0 100644 --- a/builtin.h +++ b/builtin.h @@ -157,6 +157,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 8bd1d7551d..1cdb3221a5 100644 --- a/git.c +++ b/git.c @@ -519,6 +519,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 }, { "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 Sep 9 00:49:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764771 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id F3227618 for ; Wed, 9 Sep 2020 00:50:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D415A21973 for ; Wed, 9 Sep 2020 00:50:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="uZwqGo5+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729779AbgIIAuI (ORCPT ); Tue, 8 Sep 2020 20:50:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52828 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729529AbgIIAtz (ORCPT ); Tue, 8 Sep 2020 20:49:55 -0400 Received: from mail-qk1-x74a.google.com (mail-qk1-x74a.google.com [IPv6:2607:f8b0:4864:20::74a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 14D90C061756 for ; Tue, 8 Sep 2020 17:49:55 -0700 (PDT) Received: by mail-qk1-x74a.google.com with SMTP id w126so480951qka.5 for ; Tue, 08 Sep 2020 17:49:55 -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=IOZol2PrmgK69ChF9uhS4Q6VJ8s6MAKp9sU06eeanSs=; b=uZwqGo5+JtuAxgRjXZId5opboSE7Z4bG5DxXDqDQc7vXaMozlOIkJUqqLKyVDZoXV4 EOBRn0BubtXPnp9Sy2V5Ei10fCWJM8sCcc/rggIaAHGmaOTWjU7L45zxPnkBgp7pdxyS 11LBzLt1wF0q+UzMJMuY+x8Iq1FG44Y8b09BzoaFwQUmGhh79576XCur0wwZEgQ4qOrT om+h1EYZ61NlFaEr3ulp/aoQ3qS74JC4q8ksxUMYtKv0JZC+qmpFLwpcChscE6QZ31wl SR/4unkNcW4lrkfRht7A728DyLRkZhWtsHIHOe2sVcz9i3+k5PqTzB7rE42sPKDQB4b+ RSSA== 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=IOZol2PrmgK69ChF9uhS4Q6VJ8s6MAKp9sU06eeanSs=; b=jc4FzQU9vbY4klOMxBUfhST/tTjV3otgO3BN4Ky+ToISTK+S8h/i9Koan2R9ZmM/97 SFafyStjmuAtO84JJ5kVAY0i2XIg8dNUMgEwrUdBS7FJBRTXfGavsL9XmLYiyafDpN7g UQUsBQlTGaIvRdy3OTlFLr5zLiq0Lco4Ycmz4gwidkr163yo4DY/tlzAsWPBUFROMy4Y 14GD4faGaVeoelqXsd2oqg6N4wHdEsOQuJsZNmRy2Cy/DzR5D5PRF50ieT9QCFX6iMMj bqAjwqtpL9xN3iFyCeg59eR42eZW3cKnyWdHV/5LL/lGSTcKc0y0TBXk8tJE9MuVqE32 A/vw== X-Gm-Message-State: AOAM533x3PXHsvDSvWVU8F5Kv9c7okFt0DhcJLzToiEu77MvWd0FsiF7 K8XT3QCfOOZo0DiJGdywv2J0Z7AC+IUEdonaVGKUFhBEIaR9WRlf51i7w0PG8ZXeXvfp6JBiGEh ka0UwcnD/YpOPYgrmV22Hu7Gg+FcGMpGCIPC6QggRiyuL84UjSOfZPo+lRG8ucvOKL9IkFKP7qg == X-Google-Smtp-Source: ABdhPJzXhpFzdQSdmQpDNLU1oWWEhTSH7lYPhwopRkHDnAR2FHJJLBcDCerZ5yP26WWxhap9lLAbmF9SenJh9AFFuTY= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a0c:90c1:: with SMTP id p59mr138267qvp.7.1599612594132; Tue, 08 Sep 2020 17:49:54 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:33 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-4-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 3/9] hook: add list command From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org 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 ~/baz/from/hookcmd.sh ~/bar.sh Signed-off-by: Emily Shaffer --- Documentation/git-hook.txt | 37 +++++++++++- Makefile | 1 + builtin/hook.c | 55 ++++++++++++++++-- hook.c | 102 ++++++++++++++++++++++++++++++++++ hook.h | 15 +++++ t/t1360-config-based-hooks.sh | 68 ++++++++++++++++++++++- 6 files changed, 271 insertions(+), 7 deletions(-) create mode 100644 hook.c create mode 100644 hook.h diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 2d50c414cc..e458586e96 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -8,12 +8,47 @@ git-hook - Manage configured hooks SYNOPSIS -------- [verse] -'git hook' +'git hook' list DESCRIPTION ----------- You can list, 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 config order. "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" +---- + +COMMANDS +-------- + +list :: + +List the hooks which have been configured for . Hooks appear +in the order they should be run, and note the config scope where the relevant +`hook..command` was specified, not the `hookcmd` (if applicable). + GIT --- Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 6eee75555e..804de45b16 100644 --- a/Makefile +++ b/Makefile @@ -890,6 +890,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 += interdiff.o LIB_OBJS += json-writer.o diff --git a/builtin/hook.c b/builtin/hook.c index b2bbc84d4d..a0759a4c26 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -1,21 +1,68 @@ #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("a hookname must be provided to operate on.", + 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); + return 0; + } + + list_for_each(pos, head) { + item = list_entry(pos, struct hook, list); + if (item) + printf("%s:\t%s\n", + config_scope_name(item->origin), + item->command.buf); + } + + clear_hook_list(); + 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..b006950eb8 --- /dev/null +++ b/hook.c @@ -0,0 +1,102 @@ +#include "cache.h" + +#include "hook.h" +#include "config.h" + +/* + * NEEDSWORK: a stateful hook_head means we can't run two hook events in the + * background at the same time - which might be ok, or might not. + * + * Maybe it's better to cache a list head per hookname, since we can probably + * guess that the hook list won't change during a user-initiated operation. For + * now, within list_hooks, call clear_hook_list() at the outset. + */ +static LIST_HEAD(hook_head); + +void free_hook(struct hook *ptr) +{ + if (ptr) { + strbuf_release(&ptr->command); + free(ptr); + } +} + +static void emplace_hook(struct list_head *pos, const char *command) +{ + struct hook *to_add = malloc(sizeof(struct hook)); + to_add->origin = current_config_scope(); + strbuf_init(&to_add->command, 0); + /* even with use_shell, run_command() needs quotes */ + strbuf_addf(&to_add->command, "'%s'", command); + + 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(void) +{ + struct list_head *pos, *tmp; + list_for_each_safe(pos, tmp, &hook_head) + remove_hook(pos); +} + +static int hook_config_lookup(const char *key, const char *value, void *hook_key_cb) +{ + const char *hook_key = hook_key_cb; + + if (!strcmp(key, hook_key)) { + const char *command = value; + struct strbuf hookcmd_name = STRBUF_INIT; + struct list_head *pos = NULL, *tmp = NULL; + + /* 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) + 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) + */ + + list_for_each_safe(pos, tmp, &hook_head) { + struct hook *hook = list_entry(pos, struct hook, list); + /* + * The list of hooks to run can be reordered by being redeclared + * in the config. Options about hook ordering should be checked + * here. + */ + if (0 == strcmp(hook->command.buf, command)) + remove_hook(pos); + } + emplace_hook(pos, command); + } + + return 0; +} + +struct list_head* hook_list(const struct strbuf* hookname) +{ + struct strbuf hook_key = STRBUF_INIT; + + if (!hookname) + return NULL; + + /* hook_head is stateful */ + clear_hook_list(); + + strbuf_addf(&hook_key, "hook.%s.command", hookname->buf); + + git_config(hook_config_lookup, (void*)hook_key.buf); + + return &hook_head; +} diff --git a/hook.h b/hook.h new file mode 100644 index 0000000000..aaf6511cff --- /dev/null +++ b/hook.h @@ -0,0 +1,15 @@ +#include "config.h" +#include "list.h" +#include "strbuf.h" + +struct hook +{ + struct list_head list; + enum config_scope origin; + struct strbuf command; +}; + +struct list_head* hook_list(const struct strbuf *hookname); + +void free_hook(struct hook *ptr); +void clear_hook_list(void); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 34b0df5216..46d1ed354a 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -4,8 +4,72 @@ 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 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 Sep 9 00:49:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764773 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id B4BC1618 for ; Wed, 9 Sep 2020 00:50:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 998D72177B for ; Wed, 9 Sep 2020 00:50:17 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="lmBcbjfz" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726726AbgIIAuQ (ORCPT ); Tue, 8 Sep 2020 20:50:16 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52836 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729663AbgIIAt5 (ORCPT ); Tue, 8 Sep 2020 20:49:57 -0400 Received: from mail-pf1-x44a.google.com (mail-pf1-x44a.google.com [IPv6:2607:f8b0:4864:20::44a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E6961C061755 for ; Tue, 8 Sep 2020 17:49:56 -0700 (PDT) Received: by mail-pf1-x44a.google.com with SMTP id k13so739882pfh.4 for ; Tue, 08 Sep 2020 17:49:56 -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=6OE/uE69yRebrRJPaGprkkwEu88LuiF7HtXoJUEcU5w=; b=lmBcbjfzcpREcxEpKM0AE/wx5vYybx1v3I84ILQt7GbYIutEJWiFoy3w7k+GBduQ5S 5MKCmkVIS9Lpeu/rtt/VIdACpjBPTh2g6HuQqnrL7NPCkaQ56vxPxorUoWkw8Sq3XJM7 e1lJUs+TG7xuoSb8tXxmjJDk36v9Cvd/6dYBDjNNW9bc3PNbv41YD2vfXEh1g9v/7plY U8QAKo4CInSi+sWRX17wQLU/9/Gr/aYgd6Y/G5R9NGBClkY4UK664mMR+iIJi1ssJVLm h4cAzQApSpA2HjRrPTBMd+7GihLvZfnngRDn8IwJSoukNYX4ekWvYicnchGADERscKx7 0gpw== 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=6OE/uE69yRebrRJPaGprkkwEu88LuiF7HtXoJUEcU5w=; b=pFd/+CFuKkU7Kk330sGHQbqEivyuVo0J1w+iQFENQPtjuYKm/udE/Vhsyl0ZxO+aiU 2GUchQmrb1wRh7kisIMuJP3SIpm4cys4zET4TTXEZN6wdr9l3dS1UyB5/mMeNQOqMwj+ bFkPMvskqj5owSfKMl7XuzE3+hYKjVWYXORk0CmVFUcAm6R1Qikv9c6ZwQ8NtfOYCNdG q10wGJKJFQaxhb+boYbrFfePFSHOtorhkOWQa1k5Tp7RDIkZ0DqhsEtegSQ9b0yHHnvn 7g3yeTF52uovxq+ust6RbhCnqB0CahdU7ZczjW3/cSIpHZIaA9L9s7+XqAHmSPZaMpRs l59A== X-Gm-Message-State: AOAM530LnL1QUM+IBZjgwkZ/ahjT98KTSAj47YDSauoC4z8/iQp39H6I R8Ui1hUPdJRN/HnB8vc92P0A4udqyONDgMQMJ8SOTkXLH0T9H5XGrmr1u7kwEL5HS+CQKVls8YS brMcG15/YwKJ5CzJtodZUK6uXekEOeRXu1Y9I75MQqBDMe5obH1iyNlKSpMpP45XqmQl8gED/Xg == X-Google-Smtp-Source: ABdhPJxXYd/rQKPLUicoCrMdUqbvOi67eAWo9NiHlfmlRWoMKJwP0oIV+Domi8kGvobi9vwaZpKn9zuwa7c1wR8ZzII= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a63:2d0:: with SMTP id 199mr1021013pgc.408.1599612596270; Tue, 08 Sep 2020 17:49:56 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:34 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-5-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 4/9] hook: add --porcelain to list command From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Teach 'git hook list --porcelain ', which prints simply the commands to be run in the order suggested by the config. This option is intended for use by user scripts, wrappers, or out-of-process Git commands which still want to execute hooks. For example, the following snippet might be added to git-send-email.perl to introduce a `pre-send-email` hook: sub pre_send_email { open(my $fh, 'git hook list --porcelain pre-send-email |'); chomp(my @hooks = <$fh>); close($fh); foreach $hook (@hooks) { system $hook } Signed-off-by: Emily Shaffer --- Documentation/git-hook.txt | 13 +++++++++++-- builtin/hook.c | 17 +++++++++++++---- t/t1360-config-based-hooks.sh | 12 ++++++++++++ 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index e458586e96..0854035ce2 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -8,7 +8,7 @@ git-hook - Manage configured hooks SYNOPSIS -------- [verse] -'git hook' list +'git hook' list [--porcelain] DESCRIPTION ----------- @@ -43,11 +43,20 @@ Local config COMMANDS -------- -list :: +list [--porcelain] :: List the hooks which have been configured for . Hooks appear in the order they should be run, and note the config scope where the relevant `hook..command` was specified, not the `hookcmd` (if applicable). ++ +If `--porcelain` is specified, instead print the commands alone, separated by +newlines, for easy parsing by a script. + +OPTIONS +------- +--porcelain:: + With `list`, print the commands in the order they should be run, + separated by newlines, for easy parsing by a script. GIT --- diff --git a/builtin/hook.c b/builtin/hook.c index a0759a4c26..0d92124ca6 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -16,8 +16,11 @@ static int list(int argc, const char **argv, const char *prefix) struct list_head *head, *pos; struct hook *item; struct strbuf hookname = STRBUF_INIT; + int porcelain = 0; struct option list_options[] = { + OPT_BOOL(0, "porcelain", &porcelain, + "format for execution by a script"), OPT_END(), }; @@ -29,6 +32,8 @@ static int list(int argc, const char **argv, const char *prefix) builtin_hook_usage, list_options); } + + strbuf_addstr(&hookname, argv[0]); head = hook_list(&hookname); @@ -41,10 +46,14 @@ static int list(int argc, const char **argv, const char *prefix) list_for_each(pos, head) { item = list_entry(pos, struct hook, list); - if (item) - printf("%s:\t%s\n", - config_scope_name(item->origin), - item->command.buf); + if (item) { + if (porcelain) + printf("%s\n", item->command.buf); + else + printf("%s:\t%s\n", + config_scope_name(item->origin), + item->command.buf); + } } clear_hook_list(); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 46d1ed354a..ebf8f38d68 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -72,4 +72,16 @@ test_expect_success 'git hook list reorders on duplicate commands' ' test_cmp expected actual ' +test_expect_success 'git hook list --porcelain prints just the command' ' + setup_hooks && + + cat >expected <<-EOF && + $ROOT/path/def + $ROOT/path/ghi + EOF + + git hook list --porcelain pre-commit >actual && + test_cmp expected actual +' + test_done From patchwork Wed Sep 9 00:49:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764775 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8277B13B1 for ; Wed, 9 Sep 2020 00:50:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 685A021973 for ; Wed, 9 Sep 2020 00:50:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="R9MoF8pB" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729692AbgIIAuU (ORCPT ); Tue, 8 Sep 2020 20:50:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52848 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729717AbgIIAuB (ORCPT ); Tue, 8 Sep 2020 20:50:01 -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 41713C061573 for ; Tue, 8 Sep 2020 17:49:59 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id j20so874239ybt.10 for ; Tue, 08 Sep 2020 17:49:59 -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=nhZYNCfrc7LBG0PG1IIHge72kRRdZrR0w3xUUNmrkO0=; b=R9MoF8pBGF3FCk0DUyQKVgz/HnRa39m2fdOxRkUapW1BNk6vQ4lv9ZRCuHPnCh4Qdz AcXzsEldW1/0OkwiWOKDFTPRp/lM6wKu957tC8Yy6kG0wMV6wW8p9oAHULSU9R+sIrR8 8SE/MoK67NqEZJyfKrS3CYRza2Av+l0591xVaLnRCEqTuv3X9ByqK3ucM2PtpR2tSO2Q w9Z29QNBLs445HXqWmqVwS5Ze+4DcWGqp7VZqgNUHhTpJn6WLhK21kTrjozBJZ+pLzRk /gQ4qYlJA9aFDKJP3CX8c/QSRwC3jhB1ulsZ0DTaIAgX8kGQQk5QIcP8gtt1a+dtWzi7 Wd/Q== 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=nhZYNCfrc7LBG0PG1IIHge72kRRdZrR0w3xUUNmrkO0=; b=txLMXXxA/Lak3tZjS+yeZycBhyo1cUmYBK3tDnv0L/OV2mK+CfYQSA9rSVs7YM3rNh DR6OGBh21bnzYO51TCRiUWhaHL5RCk2p2GE7t8KhwOZ2c9IoUhsRf8OXTNaTAfFfhbbo 746aJXQpg7z9jhbE6ki0IOC+f1AnM//N6tYO4bLB0vjmv7F2aHIhbl6oxnY9RbC7oxan BxAXuOwB0A3nHiCCKB5MLlIcyKdtoNxhX8FjRjt8iJ3dPWTV4i+OSBZBI/QFxW5j/pbV uIqKIFqpIJKw4S6+ErW+WP8WxXGQifqbf4NuxOgrOuZMa2XX1qHa+4e8wtHuow0jpzbZ cz8Q== X-Gm-Message-State: AOAM533OzoRz9dHCNzchazlnOzuTYwfp4ZSOIH3uThEfWTF0mqOeD742 +NfI3htxuKsALW0IaQ3dIYwduEpBrIHLe7vQgzlV9Wf6QCG6N4ihNT9khVF4uPqtYgpsjcIeDpL Lo5Kt1FGsOKks8W/rUfBHg7zAZtdSNEDI1QLs4X/1it5WK2nCQGQMNs/BV/x+pgpLxFWVRYKgYQ == X-Google-Smtp-Source: ABdhPJwUKf3hD+TcsmSPmXi0Ect3wB56VFqkSie0w1Ht72Se/kkNcRy6QTlIMQsgWGYAvUw7gJk6MvcO0vtCITx3G3s= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:2611:: with SMTP id m17mr14666ybm.442.1599612598351; Tue, 08 Sep 2020 17:49:58 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:35 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-6-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 5/9] parse-options: parse into strvec From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org 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 --- 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..b4f1fc4a1a 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_ARGV_ARRAY(short, long, &struct argv_array, arg_str, description)`:: + Introduce an option with a string argument. + The string argument is stored as an element in `argv_array`. + 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 d9d3b0819f..d2b8b7b98a 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -205,6 +205,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 46af942093..177259488b 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 Sep 9 00:49:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764777 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DEDE715E4 for ; Wed, 9 Sep 2020 00:50:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BF67121973 for ; Wed, 9 Sep 2020 00:50:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="NnbZENiS" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729789AbgIIAuX (ORCPT ); Tue, 8 Sep 2020 20:50:23 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52850 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729719AbgIIAuC (ORCPT ); Tue, 8 Sep 2020 20:50:02 -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 F2576C061756 for ; Tue, 8 Sep 2020 17:50:00 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id a6so902793ybr.4 for ; Tue, 08 Sep 2020 17:50: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=MakHIgd9qKExhWlDsWfoivIcevisorK5QVctfKnPPMo=; b=NnbZENiSLp+n0fgna7ArOxYnZuEA7w55zqqPqJquJQR2Ic57kGpTV6sAWOPWvCefzM Y/nQ/msyCyO+qoSMekouoomI1Yk1oLitbrHJXtIs6IOR8s95IxKRaQeuczINWs11rdVB YtUg8i8aiCs/JvETfMjchavR76SeRdoloWYffpOv1QVLgSEamMZxtG6V4Idg/Eh95P4v deadnYBdA1uC0KhOmhnoHLxLJKXc/H5MJClMzoHtvXQlrs0XKOOgcj7lO7X2y86kbcmO 1WOBCGS0jL4djzij9kYGvbf0wx1xvFNAsXFwhdIqsZVCjDFVfTVUgJKaU30PYHiLpMBv hemg== 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=MakHIgd9qKExhWlDsWfoivIcevisorK5QVctfKnPPMo=; b=Hu5Z0QAXy83IflIaGlXnjt/dFXvmQ3jiJC6UFn6oFvhKyYwacVGNYueEnwyRpE7NLP skvae2t5W5ZRtkhoE46aGZX1PrkeYVsGeKjxkBsRYmWr64s2D+MzdADdyXyxOt2qUP6L 93Y8VipTqhNtrgKFFSqmGOeuzyBUIk+NtSyPVPcOG78jFgKv722q5L+GJ0jzL39ByeZL ffDrS7fPLEaiceddz4WWWPGe33jKiSnJciE9Jp27USvpi68JdbWQtr3rm1R8AL8o15Dv FOVBjcnWS/hf6PksxuyfIHQuV1wJ1fFO/TSEw736Jh6DAJwZjEr58hfsTLamd+bGoIbY cyzQ== X-Gm-Message-State: AOAM533/vBpW2NnkPeBL46jw85YA+Jtd95GTAWLbIfs8AOHhLX1jK7RH 2y2mz0Csn1fz6FQXGvgoQLNvZYvL5sNlYYjCeYc3vmX0At1QUwkoj/Zq/wHqxZ/9Ga4X6s+WNnW dQOfFtvA/By8Z47jw1yEkwCu7/IPP0PZO6b9Wy4eboxRYo+gD6BV+eIfz4x1jNoBL3RrDd+asgw == X-Google-Smtp-Source: ABdhPJyy5I9ov5/opipe6xRM7NKDuMuQLA/tzyHWjq72kH6EbHbRjyHjrsuRrj86Jw/23oWC3olsGGydKORJcOMnBcQ= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:7608:: with SMTP id r8mr2546031ybc.518.1599612600114; Tue, 08 Sep 2020 17:50:00 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:36 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-7-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 6/9] hook: add 'run' subcommand From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org 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 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 --- builtin/hook.c | 30 ++++++++++++++++++++ hook.c | 52 ++++++++++++++++++++++++++++++++--- hook.h | 3 ++ t/t1360-config-based-hooks.sh | 28 +++++++++++++++++++ 4 files changed, 109 insertions(+), 4 deletions(-) diff --git a/builtin/hook.c b/builtin/hook.c index 0d92124ca6..a8f8b03699 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 }; @@ -62,6 +64,32 @@ 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(), + }; + + argc = parse_options(argc, argv, prefix, run_options, + builtin_hook_usage, 0); + + if (argc < 1) + usage_msg_opt(_("a hookname must be provided to operate on."), + builtin_hook_usage, run_options); + + strbuf_addstr(&hookname, argv[0]); + + return run_hooks(envs.v, &hookname, &args); +} + int cmd_hook(int argc, const char **argv, const char *prefix) { struct option builtin_hook_options[] = { @@ -72,6 +100,8 @@ int cmd_hook(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], "list")) return list(argc - 1, argv + 1, prefix); + if (!strcmp(argv[1], "run")) + return run(argc - 1, argv + 1, prefix); usage_with_options(builtin_hook_usage, builtin_hook_options); } diff --git a/hook.c b/hook.c index b006950eb8..0dab981681 100644 --- a/hook.c +++ b/hook.c @@ -2,6 +2,7 @@ #include "hook.h" #include "config.h" +#include "run-command.h" /* * NEEDSWORK: a stateful hook_head means we can't run two hook events in the @@ -21,13 +22,15 @@ void free_hook(struct hook *ptr) } } -static void emplace_hook(struct list_head *pos, const char *command) +static void emplace_hook(struct list_head *pos, const char *command, int quoted) { struct hook *to_add = malloc(sizeof(struct hook)); to_add->origin = current_config_scope(); strbuf_init(&to_add->command, 0); - /* even with use_shell, run_command() needs quotes */ - strbuf_addf(&to_add->command, "'%s'", command); + if (quoted) + strbuf_addf(&to_add->command, "'%s'", command); + else + strbuf_addstr(&to_add->command, command); list_add_tail(&to_add->list, pos); } @@ -78,7 +81,7 @@ static int hook_config_lookup(const char *key, const char *value, void *hook_key if (0 == strcmp(hook->command.buf, command)) remove_hook(pos); } - emplace_hook(pos, command); + emplace_hook(pos, command, 0); } return 0; @@ -87,6 +90,7 @@ static int hook_config_lookup(const char *key, const char *value, void *hook_key struct list_head* hook_list(const struct strbuf* hookname) { struct strbuf hook_key = STRBUF_INIT; + const char *legacy_hook_path = NULL; if (!hookname) return NULL; @@ -98,5 +102,45 @@ struct list_head* hook_list(const struct strbuf* hookname) git_config(hook_config_lookup, (void*)hook_key.buf); + legacy_hook_path = find_hook(hookname->buf); + + /* TODO: check hook.runHookDir */ + if (legacy_hook_path) + emplace_hook(&hook_head, legacy_hook_path, 1); + return &hook_head; } + +int run_hooks(const char *const *env, const struct strbuf *hookname, + const struct strvec *args) +{ + struct list_head *to_run, *pos = NULL, *tmp = NULL; + int rc = 0; + + to_run = hook_list(hookname); + + 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); + + /* 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); + + 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; + + rc |= run_command(&hook_proc); + } + + return rc; +} diff --git a/hook.h b/hook.h index aaf6511cff..d020788a6b 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 { @@ -10,6 +11,8 @@ struct hook }; struct list_head* hook_list(const struct strbuf *hookname); +int run_hooks(const char *const *env, const struct strbuf *hookname, + const struct strvec *args); void free_hook(struct hook *ptr); void clear_hook_list(void); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index ebf8f38d68..ee8114250d 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -84,4 +84,32 @@ test_expect_success 'git hook list --porcelain prints just the command' ' test_cmp expected 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' ' + cat >~/sample-hook.sh <<-EOF && + echo \"Sample Hook\" + EOF + + test_when_finished "rm ~/sample-hook.sh" && + + chmod +x ~/sample-hook.sh && + + test_config hook.pre-commit.command "~/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_done From patchwork Wed Sep 9 00:49:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764779 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5F478618 for ; Wed, 9 Sep 2020 00:50:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 44A5B216C4 for ; Wed, 9 Sep 2020 00:50:25 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="HwCXkwKk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729808AbgIIAuY (ORCPT ); Tue, 8 Sep 2020 20:50:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729738AbgIIAuF (ORCPT ); Tue, 8 Sep 2020 20:50:05 -0400 Received: from mail-pj1-x104a.google.com (mail-pj1-x104a.google.com [IPv6:2607:f8b0:4864:20::104a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E7AE6C061757 for ; Tue, 8 Sep 2020 17:50:02 -0700 (PDT) Received: by mail-pj1-x104a.google.com with SMTP id z22so553641pjr.8 for ; Tue, 08 Sep 2020 17:50: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=VF7oE5YMw0/GxqChNnx/6uPxfpcAkfxdnUX1Fq0B3aE=; b=HwCXkwKk1LA0vjcwJEVa1FqIrxlEmpPUQ72elMT5MVFx5YZoY/d/q9Z+QLEr6g0/UZ RbJgvS/nk/3Vh5fhx19QE8etns0HUFEgQEvt1Lns3bBYXxTbvIC/2mst9XV1u92wQFE4 W2pE6wYBxncaZ7rpzrHiYBWzZnRIf1Q4H3CoivLMbdDc2E+MGXsXiDg8YZkwse3emDWy pncD2gYmXF8/fX8RIz7/zyie6WOzNqXtNpscwyE6/6weXjI2JcpcQPdIZQSkWUEaFHDC 50wVKLyJS6OtHaHATQ6NIxLx+LgrhkuDN68eClHEWUMpyPGFkNn1vIY6k/PIGj2cKXjI E3nw== 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=VF7oE5YMw0/GxqChNnx/6uPxfpcAkfxdnUX1Fq0B3aE=; b=kt09wpLjjCNwQ4wPEIPLpZiInNxVMnauOSfco4bE/Bk3hpOLad46YhjgZUEh0F6Nvn CFxGD7R8L+AsrFmWBHiGJR1eqYVJ4trrFjXiPFccOdycqD76mrdps20QnwFFKTbMfYs3 lDeYHz60i9McegrOiiliPziID1S6ZrPK6ZwvA04QUHMRj8zjHI4qcn4FFQiGnn7Lcx1v 8XTevroYbAs+Uxra/SYXO2gaW3sKBZtL5lXJYXGzk1dxCSm0t+HIOXFPzwxDVpOuA8UJ ny4NEZDU5E+KoD6eQPY3vQ+1cjj/FjuD8wdNLHHI/J4yCz6F6zixErXwDvEUtECla7GS BsQw== X-Gm-Message-State: AOAM5335TmCtCXVwtcJ3k9r3bUgzFZG6a9EbzIVDvAp/DztuezEAtmxB JyA8NjZCeQ63SP1ME7nnN8R9VuzK0RN1SOwOSAVjUuTTmBe/1du7FaxWxU/4Q6XiTuvnFLvFPqm Vob504qo9EDavWKK472IyH7gg4c/AdWfuC4X7S1ToRhqw4cMtfAom5rkFL9wEEkoOQ4RLplm6aQ == X-Google-Smtp-Source: ABdhPJxdhwxQ8DmT2IIKYIvwSDpZ4Oup9fu36A0Vm1bHKRCJgIcAk54DAckIJgHXh5M++atW14FxANyV1TjDENYpVj8= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a17:90b:941:: with SMTP id dw1mr145986pjb.1.1599612602109; Tue, 08 Sep 2020 17:50:02 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:37 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-8-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 7/9] hook: replace run-command.h:find_hook From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a helper to easily determine whether any hooks exist for a given hook event. Signed-off-by: Emily Shaffer --- hook.c | 9 +++++++++ hook.h | 1 + 2 files changed, 10 insertions(+) diff --git a/hook.c b/hook.c index 0dab981681..7c7b922369 100644 --- a/hook.c +++ b/hook.c @@ -111,6 +111,15 @@ struct list_head* hook_list(const struct strbuf* hookname) return &hook_head; } +int hook_exists(const char *hookname) +{ + const char *value = NULL; + struct strbuf hook_key = STRBUF_INIT; + strbuf_addf(&hook_key, "hook.%s.command", hookname); + + return (!git_config_get_value(hook_key.buf, &value)) || !!find_hook(hookname); +} + int run_hooks(const char *const *env, const struct strbuf *hookname, const struct strvec *args) { diff --git a/hook.h b/hook.h index d020788a6b..d94511b609 100644 --- a/hook.h +++ b/hook.h @@ -11,6 +11,7 @@ struct hook }; struct list_head* hook_list(const struct strbuf *hookname); +int hook_exists(const char *hookname); int run_hooks(const char *const *env, const struct strbuf *hookname, const struct strvec *args); From patchwork Wed Sep 9 00:49:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764783 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 33BA1618 for ; Wed, 9 Sep 2020 00:50:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 187D72068D for ; Wed, 9 Sep 2020 00:50:31 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="W036QJgU" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729799AbgIIAu2 (ORCPT ); Tue, 8 Sep 2020 20:50:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52864 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729741AbgIIAuG (ORCPT ); Tue, 8 Sep 2020 20:50:06 -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 DFC00C0613ED for ; Tue, 8 Sep 2020 17:50:04 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id e1so882916ybk.14 for ; Tue, 08 Sep 2020 17:50: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=Whgj4+S6hi3Yf6sSoKGg5cKYVxwTioXGs+Po6FqpmeY=; b=W036QJgUPvA2YSfvUMefRaju7hPVEbZmYG0CSFe728Af//Hx/zASiWpviPraUqdZRR fpYPh2wUGoE+akoiAepWrq57A9NHsvB8kZ91NYj6DxPZ0UBsh3FkdXuvViZHY94afvkG e0o+cy/gmAuSJplfmQBzcbtLjXTlTnUu9Lp6x1gKUuc+QQ2uWaJ9fpRrUXxgCa5iWk5M 2UFGLB0gNgT51K/Suge/hebrV5GuQ7HCqdck0O+a70Pydx0U1ZXtGXxgjgRioOtNMSxR hjc1NZOFR8LNyqmooCffMGXvf/LxgQ/m5Ps3RrAQ627gWe4WDdbrDi8RuCIoWjz3Ig/k MeCA== 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=Whgj4+S6hi3Yf6sSoKGg5cKYVxwTioXGs+Po6FqpmeY=; b=jThKYk9VykfzVDS5rRt4D4Lcw19eS+X6sGVAyr+cyv4NLYmF35EQ2QKTdcEHSYya9a djhn8ADzJUagcooBbw0+Ft7hqxHtdRkFhvbE7VuYv50Wt3vPc1PXahO/AfV9nfBqOS6D B2YjlY3UQNYBPZ46970J1MlqH0aPosbbZxfJk6Lc+QIJuXGwy/wDg94j4KfXehrAAFF9 BGVV1uX8DrOBASKi+ZRat7+yII7PDxMGss1xNIgIsvj05ED4iCpCrBa1YslG5Cs1dtao bzDAsspfv1oErnYtr9RNb7+L9uhucOvm8mwc5yhAPeK0eozPRdKkE6ZKRlsaqPLgjZDz HYTA== X-Gm-Message-State: AOAM532AZi7YcIBPoxpCUvzcypXzfjJOnF60RJugi7W7wgqu78Ew8pLu csL08VggKP9lnBniGriq9wGv9/wS1lTvtOMHQ2q3AGK3/6RVsVJSzZxknoddXS1R0hKTJXpKqT6 yJy1LeO/01JrXcPLfrvm+OYMqSmke2CpZvbZPJkEXIBj3uI9UvonxNuUaT57fo+FjfUxUbTqE5w == X-Google-Smtp-Source: ABdhPJyHSp7aDO3LlQzqDbIiRmSRVlPdc41J/JKVj0zfpwgmzials5KO+kwdnvQCAwVSPV6roh1stYclPYb+aRIuzjc= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a25:d98e:: with SMTP id q136mr2414551ybg.354.1599612604038; Tue, 08 Sep 2020 17:50:04 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:38 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-9-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 8/9] commit: use config-based hooks From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org As part of the adoption of config-based hooks, teach run_commit_hook() to call hook.h instead of run-command.h. This covers 'pre-commit', 'commit-msg', and 'prepare-commit-msg'. Additionally, ask the hook library - not run-command - whether any hooks will be run, as it's possible hooks may exist in the config but not the hookdir. Signed-off-by: Emily Shaffer --- builtin/commit.c | 3 ++- builtin/merge.c | 3 ++- commit.c | 13 ++++++++++++- t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 13 +++++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 69ac78d5e5..a19c6478eb 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -36,6 +36,7 @@ #include "help.h" #include "commit-reach.h" #include "commit-graph.h" +#include "hook.h" static const char * const builtin_commit_usage[] = { N_("git commit [] [--] ..."), @@ -985,7 +986,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (!no_verify && find_hook("pre-commit")) { + if (!no_verify && hook_exists("pre-commit")) { /* * Re-read the index as pre-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke diff --git a/builtin/merge.c b/builtin/merge.c index 74829a838e..c1a9d0083d 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -41,6 +41,7 @@ #include "commit-reach.h" #include "wt-status.h" #include "commit-graph.h" +#include "hook.h" #define DEFAULT_TWOHEAD (1<<0) #define DEFAULT_OCTOPUS (1<<1) @@ -829,7 +830,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) * and write it out as a tree. We must do this before we invoke * the editor and after we invoke run_status above. */ - if (find_hook("pre-merge-commit")) + if (hook_exists("pre-merge-commit")) discard_cache(); read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); diff --git a/commit.c b/commit.c index 4ce8cb38d5..c7a243e848 100644 --- a/commit.c +++ b/commit.c @@ -21,6 +21,7 @@ #include "commit-reach.h" #include "run-command.h" #include "shallow.h" +#include "hook.h" static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **); @@ -1632,8 +1633,13 @@ int run_commit_hook(int editor_is_used, const char *index_file, { struct strvec hook_env = STRVEC_INIT; va_list args; + const char *arg; + struct strvec hook_args = STRVEC_INIT; + struct strbuf hook_name = STRBUF_INIT; int ret; + strbuf_addstr(&hook_name, name); + strvec_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file); /* @@ -1643,9 +1649,14 @@ int run_commit_hook(int editor_is_used, const char *index_file, strvec_push(&hook_env, "GIT_EDITOR=:"); va_start(args, name); - ret = run_hook_ve(hook_env.v, name, args); + while ((arg = va_arg(args, const char *))) + strvec_push(&hook_args, arg); va_end(args); + + ret = run_hooks(hook_env.v, &hook_name, &hook_args); strvec_clear(&hook_env); + strvec_clear(&hook_args); + strbuf_release(&hook_name); return ret; } diff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh index b3485450a2..cef8085dcc 100755 --- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh +++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh @@ -103,6 +103,19 @@ test_expect_success 'with succeeding hook' ' test_cmp expected_hooks actual_hooks ' +# NEEDSWORK: when 'git hook add' and 'git hook remove' have been added, use that +# instead +test_expect_success 'with succeeding hook (config-based)' ' + test_when_finished "git config --unset hook.pre-commit.command success.sample" && + test_when_finished "rm -f expected_hooks actual_hooks" && + git config hook.pre-commit.command "$HOOKDIR/success.sample" && + echo "$HOOKDIR/success.sample" >expected_hooks && + echo "more" >>file && + git add file && + git commit -m "more" && + test_cmp expected_hooks actual_hooks +' + test_expect_success 'with succeeding hook (merge)' ' test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" && cp "$HOOKDIR/success.sample" "$PREMERGE" && From patchwork Wed Sep 9 00:49:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11764781 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 87CB9618 for ; Wed, 9 Sep 2020 00:50:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 68E2721973 for ; Wed, 9 Sep 2020 00:50:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=google.com header.i=@google.com header.b="utEvZEUX" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729824AbgIIAu1 (ORCPT ); Tue, 8 Sep 2020 20:50:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52870 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729753AbgIIAuH (ORCPT ); Tue, 8 Sep 2020 20:50:07 -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 CDE14C061786 for ; Tue, 8 Sep 2020 17:50:06 -0700 (PDT) Received: by mail-pf1-x449.google.com with SMTP id t201so731315pfc.13 for ; Tue, 08 Sep 2020 17:50: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=AZ4HpHzxu4QorBGlB69peEQ909CDU/ErYSqOA8fe1rs=; b=utEvZEUXT9mD4jLVfYrrFjNGiyJAFHLp+z90D8qE4cai5xFrqTEtdbiLZuwH3zBRq2 QhB8l5wF8t2dqRgN/wOjswCTaN9yWrudEdPOYnnalUR9hgECcYaT/m0vCGu9osn0XHaC LG9YB3LQ5pWW8xEwEPEgLHpxda0XZyxordFQqbUI5nS52pVUweiVvJFAs6pK0kCGOmDx ibNtb2F3pGlQvmSVCdcTXAs8Fk1yjuWBXc9SriXKpQnCi7hm5gz4nCgLtUUulXFLaSxA CSIlh56d2U1OghlKRAF++ve9rdsVy9zYj5sniyDajRq7EUPnkCEdJsr8b1MUTJEFB+vc p0Hg== 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=AZ4HpHzxu4QorBGlB69peEQ909CDU/ErYSqOA8fe1rs=; b=ZtVdvRBQZ3kIA5wic88qKZo9eKzEwYXh6rdNpvuiI1dDNc+5kJnDHC5crjolpKhRzA ccpks8M1/9Ym5LX0jnbLp57qVW8tgbOVPR5XowH65MHcIFhUZAMlzSkjlKDwiU3WTzAY Fvu0Nl9rJc87TIqfdJpP2xk8cC1M8HSwZMjWugCF2DmEctAe+jkSZgsjnjWjYMKVavV5 KwHu+5mpuWpa01ZbzeWPd0nf95NxpQt7Hksbz5JXp7OgwtMUgA+QVmD56BKrSC+xeQ7n mhfFz7Qus1abjtJ9xblmu5+IK9O23Vwdmd4Y4wYJ/HFP8WrkNbiGoOv619n297ZID1YK 6rfg== X-Gm-Message-State: AOAM532xhOEy05FbbBL6AUjV0Ea3+T17pFRf+0TsuJBhYCQi6DsZfOpS 3d5hwauk5Lub8ih+y6n+VUywYDXyrMLxsR1UVwxMpjkQNSpDiBBsKd4zS/PKfwUHVHV4uRfdMew rKcVD5lzLcNPTgnmzpJF5Tut8fZbc+1ONJiiOWOPP+L90klQBqpsPEbQBrOE95aHC41kveWqKxw == X-Google-Smtp-Source: ABdhPJwbSU/hHSvFVtw6mDo6zjOMYtFFAmW8cEYNjMtdqkQjZSbS49ePw6ZDbmuT/eowkc61ChUXwJ2JbNH81FL7ILI= X-Received: from podkayne.svl.corp.google.com ([2620:15c:2ce:0:1ea0:b8ff:fe77:f690]) (user=emilyshaffer job=sendgmr) by 2002:a62:be0b:0:b029:137:3e38:c5de with SMTP id l11-20020a62be0b0000b02901373e38c5demr1621590pff.0.1599612606187; Tue, 08 Sep 2020 17:50:06 -0700 (PDT) Date: Tue, 8 Sep 2020 17:49:39 -0700 In-Reply-To: <20200909004939.1942347-1-emilyshaffer@google.com> Message-Id: <20200909004939.1942347-10-emilyshaffer@google.com> Mime-Version: 1.0 References: <20200909004939.1942347-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.rc0.142.g3c755180ce-goog Subject: [PATCH v4 9/9] run_commit_hook: take strvec instead of varargs From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Taking varargs in run_commit_hook() led to some bizarre patterns, like callers using two string variables (which may or may not be filled) to express different argument lists for the commit hooks. Because run_commit_hook() no longer needs to call a variadic function for the hook run itself, we can use strvec to make the calling code more conventional. Signed-off-by: Emily Shaffer --- builtin/commit.c | 46 +++++++++++++++++++++++----------------------- builtin/merge.c | 20 ++++++++++++++++---- commit.c | 13 ++----------- commit.h | 5 +++-- sequencer.c | 15 ++++++++------- 5 files changed, 52 insertions(+), 47 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index a19c6478eb..f029d4f5ac 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -691,8 +691,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, struct strbuf committer_ident = STRBUF_INIT; int committable; struct strbuf sb = STRBUF_INIT; - const char *hook_arg1 = NULL; - const char *hook_arg2 = NULL; + struct strvec hook_args = STRVEC_INIT; int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); int old_display_comment_prefix; int merge_contains_scissors = 0; @@ -700,7 +699,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); - if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL)) + if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", + &hook_args)) return 0; if (squash_message) { @@ -722,27 +722,28 @@ static int prepare_to_commit(const char *index_file, const char *prefix, } } + strvec_push(&hook_args, git_path_commit_editmsg()); + if (have_option_m && !fixup_message) { strbuf_addbuf(&sb, &message); - hook_arg1 = "message"; + strvec_push(&hook_args, "message"); } else if (logfile && !strcmp(logfile, "-")) { if (isatty(0)) fprintf(stderr, _("(reading log message from standard input)\n")); if (strbuf_read(&sb, 0, 0) < 0) die_errno(_("could not read log from standard input")); - hook_arg1 = "message"; + strvec_push(&hook_args, "message"); } else if (logfile) { if (strbuf_read_file(&sb, logfile, 0) < 0) die_errno(_("could not read log file '%s'"), logfile); - hook_arg1 = "message"; + strvec_push(&hook_args, "message"); } else if (use_message) { char *buffer; buffer = strstr(use_message_buffer, "\n\n"); if (buffer) strbuf_addstr(&sb, skip_blank_lines(buffer + 2)); - hook_arg1 = "commit"; - hook_arg2 = use_message; + strvec_pushl(&hook_args, "commit", use_message, NULL); } else if (fixup_message) { struct pretty_print_context ctx = {0}; struct commit *commit; @@ -754,7 +755,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, &sb, &ctx); if (have_option_m) strbuf_addbuf(&sb, &message); - hook_arg1 = "message"; + strvec_push(&hook_args, "message"); } else if (!stat(git_path_merge_msg(the_repository), &statbuf)) { size_t merge_msg_start; @@ -765,9 +766,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, if (!stat(git_path_squash_msg(the_repository), &statbuf)) { if (strbuf_read_file(&sb, git_path_squash_msg(the_repository), 0) < 0) die_errno(_("could not read SQUASH_MSG")); - hook_arg1 = "squash"; + strvec_push(&hook_args, "squash"); } else - hook_arg1 = "merge"; + strvec_push(&hook_args, "merge"); merge_msg_start = sb.len; if (strbuf_read_file(&sb, git_path_merge_msg(the_repository), 0) < 0) @@ -781,11 +782,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix, } else if (!stat(git_path_squash_msg(the_repository), &statbuf)) { if (strbuf_read_file(&sb, git_path_squash_msg(the_repository), 0) < 0) die_errno(_("could not read SQUASH_MSG")); - hook_arg1 = "squash"; + strvec_push(&hook_args, "squash"); } else if (template_file) { if (strbuf_read_file(&sb, template_file, 0) < 0) die_errno(_("could not read '%s'"), template_file); - hook_arg1 = "template"; + strvec_push(&hook_args, "template"); clean_message_contents = 0; } @@ -794,11 +795,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix, * just set the argument(s) to the prepare-commit-msg hook. */ else if (whence == FROM_MERGE) - hook_arg1 = "merge"; - else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) { - hook_arg1 = "commit"; - hook_arg2 = "CHERRY_PICK_HEAD"; - } + strvec_push(&hook_args, "merge"); + else if (is_from_cherry_pick(whence) || whence == FROM_REBASE_PICK) + strvec_pushl(&hook_args, "commit", "CHERRY_PICK_HEAD", NULL); if (squash_message) { /* @@ -806,8 +805,8 @@ static int prepare_to_commit(const char *index_file, const char *prefix, * then we're possibly hijacking other commit log options. * Reset the hook args to tell the real story. */ - hook_arg1 = "message"; - hook_arg2 = ""; + strvec_clear(&hook_args); + strvec_pushl(&hook_args, git_path_commit_editmsg(), "message", NULL); } s->fp = fopen_for_writing(git_path_commit_editmsg()); @@ -1001,8 +1000,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (run_commit_hook(use_editor, index_file, "prepare-commit-msg", - git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL)) + if (run_commit_hook(use_editor, index_file, "prepare-commit-msg", &hook_args)) return 0; if (use_editor) { @@ -1017,8 +1015,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix, strvec_clear(&env); } + strvec_clear(&hook_args); + strvec_push(&hook_args, git_path_commit_editmsg()); if (!no_verify && - run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) { + run_commit_hook(use_editor, index_file, "commit-msg", &hook_args)) { return 0; } diff --git a/builtin/merge.c b/builtin/merge.c index c1a9d0083d..863c9039a3 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -821,10 +821,14 @@ static void write_merge_heads(struct commit_list *); static void prepare_to_commit(struct commit_list *remoteheads) { struct strbuf msg = STRBUF_INIT; + struct strvec hook_args = STRVEC_INIT; + struct strbuf hook_name = STRBUF_INIT; const char *index_file = get_index_file(); - if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL)) + if (!no_verify && run_commit_hook(0 < option_edit, index_file, + "pre-merge-commit", &hook_args)) abort_commit(remoteheads, NULL); + /* * Re-read the index as pre-merge-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke @@ -832,6 +836,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) */ if (hook_exists("pre-merge-commit")) discard_cache(); + read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); if (squash) @@ -851,17 +856,22 @@ static void prepare_to_commit(struct commit_list *remoteheads) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len); + + strvec_clear(&hook_args); + strvec_pushl(&hook_args, git_path_merge_msg(the_repository), "merge", NULL); if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", - git_path_merge_msg(the_repository), "merge", NULL)) + &hook_args)) abort_commit(remoteheads, NULL); + if (0 < option_edit) { if (launch_editor(git_path_merge_msg(the_repository), NULL, NULL)) abort_commit(remoteheads, NULL); } + strvec_clear(&hook_args); + strvec_push(&hook_args, git_path_merge_msg(the_repository)); if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(), - "commit-msg", - git_path_merge_msg(the_repository), NULL)) + "commit-msg", &hook_args)) abort_commit(remoteheads, NULL); read_merge_msg(&msg); @@ -871,6 +881,8 @@ static void prepare_to_commit(struct commit_list *remoteheads) strbuf_release(&merge_msg); strbuf_addbuf(&merge_msg, &msg); strbuf_release(&msg); + strbuf_release(&hook_name); + strvec_clear(&hook_args); } static int merge_trivial(struct commit *head, struct commit_list *remoteheads) diff --git a/commit.c b/commit.c index c7a243e848..726407152c 100644 --- a/commit.c +++ b/commit.c @@ -1629,12 +1629,9 @@ size_t ignore_non_trailer(const char *buf, size_t len) } int run_commit_hook(int editor_is_used, const char *index_file, - const char *name, ...) + const char *name, struct strvec *args) { struct strvec hook_env = STRVEC_INIT; - va_list args; - const char *arg; - struct strvec hook_args = STRVEC_INIT; struct strbuf hook_name = STRBUF_INIT; int ret; @@ -1648,14 +1645,8 @@ int run_commit_hook(int editor_is_used, const char *index_file, if (!editor_is_used) strvec_push(&hook_env, "GIT_EDITOR=:"); - va_start(args, name); - while ((arg = va_arg(args, const char *))) - strvec_push(&hook_args, arg); - va_end(args); - - ret = run_hooks(hook_env.v, &hook_name, &hook_args); + ret = run_hooks(hook_env.v, &hook_name, args); strvec_clear(&hook_env); - strvec_clear(&hook_args); strbuf_release(&hook_name); return ret; diff --git a/commit.h b/commit.h index e901538909..978da3c3e0 100644 --- a/commit.h +++ b/commit.h @@ -9,6 +9,7 @@ #include "string-list.h" #include "pretty.h" #include "commit-slab.h" +#include "strvec.h" #define COMMIT_NOT_FROM_GRAPH 0xFFFFFFFF #define GENERATION_NUMBER_INFINITY 0xFFFFFFFF @@ -353,7 +354,7 @@ void verify_merge_signature(struct commit *commit, int verbose, int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused); int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused); -LAST_ARG_MUST_BE_NULL -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...); +int run_commit_hook(int editor_is_used, const char *index_file, + const char *name, struct strvec *args); #endif /* COMMIT_H */ diff --git a/sequencer.c b/sequencer.c index cc3f8fa88e..5dd4b134d6 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1124,22 +1124,23 @@ static int run_prepare_commit_msg_hook(struct repository *r, const char *commit) { int ret = 0; - const char *name, *arg1 = NULL, *arg2 = NULL; + struct strvec args = STRVEC_INIT; + const char *name = git_path_commit_editmsg(); - name = git_path_commit_editmsg(); + strvec_push(&args, name); if (write_message(msg->buf, msg->len, name, 0)) return -1; if (commit) { - arg1 = "commit"; - arg2 = commit; + strvec_push(&args, "commit"); + strvec_push(&args, commit); } else { - arg1 = "message"; + strvec_push(&args, "message"); } - if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name, - arg1, arg2, NULL)) + if (run_commit_hook(0, r->index_file, "prepare-commit-msg", &args)) ret = error(_("'prepare-commit-msg' hook failed")); + strvec_clear(&args); return ret; }