From patchwork Sat Dec 5 01:45:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952757 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 446E1C4361A for ; Sat, 5 Dec 2020 01:46:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EBFB422DBF for ; Sat, 5 Dec 2020 01:46:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726708AbgLEBq6 (ORCPT ); Fri, 4 Dec 2020 20:46:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57360 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726210AbgLEBq5 (ORCPT ); Fri, 4 Dec 2020 20:46:57 -0500 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 87A13C061A4F for ; Fri, 4 Dec 2020 17:46:17 -0800 (PST) Received: by mail-qk1-x74a.google.com with SMTP id z129so7046775qkb.13 for ; Fri, 04 Dec 2020 17:46:17 -0800 (PST) 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=6cPPi981+oTQnopSEARAzAyCvS8FWO93GfCv8uq8KzU=; b=ZZ0PVP4SmQQNdtpZ49CAOsXwW7BioUpjKXEM3PPk9xMXgqiiGO2NAKpPvkoyqQUjaA S9UCkJ5pcM5zHPF75NP2Czr5+mUm05PB4Kp4NMwlexSSWfvBtcl+MJxKKw7FpObAXGC3 Hg9LXPVOIp5lg/FHa9xR25CBmXHltqkWSvHBszVV6uNXn9vTLoH9gr+q+oPzKLmT6dXo +JUFbvuWcrPGG/jSdH7IraFR3YibJ/XJhouQ3US3FLZDkVBOC3EIAvI4VOwutv/iS2y9 h9nz3gsdnPcQ+mS9h6L7Q+aNiOxnsbeLY4n5mXvq6In/koW3kGHatSFxA+vmbq2aCd66 /F8A== 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=6cPPi981+oTQnopSEARAzAyCvS8FWO93GfCv8uq8KzU=; b=qdQldsNclXqBA0mNRhnRsTBHo+nXxvAbeGFN8SJVcveNk6FUAjrs58QcOn/bMJ4TUj 8Kxq9UCqy/ekkNT3ujlq2bsTNRWc9lzOqWeUN+9PoC8rKKCDxqGZDpJ/Y4kwR+0IPKUZ Tisg9a6rbfAuYsY2bjdompOosDR7Ce6lrmyMIjfyGsu9+StFwqBH5Tt75i18KjChCUCl 8yXSjq6dO+wUA2gM9oYCyIboJCjq6wZo8WZzvdTo0zt/yqlspiY6kXku5cw1nX3Qt1bz dDby5YgtajEHkGk9wn51wmG/2T2QL949KSm+IbXcgPe8i3/Xw0v2qm1Kua0eTpd2TMZ4 fm5Q== X-Gm-Message-State: AOAM5325YLSttHAlc7s3vu5oO2CcOtatgHvdvXxS2jQcgceXzCaf7GGw c/wFMgdUHdHNJlnUjdw3WIlYF3vJpcPqiwgbuclK+Fb5QCwP4PptV+5qKXCOGC4pTLdAAY5a4Ar +cGSJhLN2jXMmVN7RkJmYQ2I4Iwb8BZjCZCPunEpPgQ6PrYliYl+bFWIApSLjn7sR97BWSHTMKg == X-Google-Smtp-Source: ABdhPJz4qF9Jfx9lYC1VqvxQcaKwNRCbtrwkR8lU8q6uTGwbuN+mZ62NA/IcDZgaZT3RjPqjtJ1VijsX81KrGXva4Wo= 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:7a9:: with SMTP id v9mr9180644qvz.38.1607132776679; Fri, 04 Dec 2020 17:46:16 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:51 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-2-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 01/17] 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. However, I have not addressed AEvar's comments or done a full re-review of this document. I wanted to get the rest of the series out for initial review first. - Emily 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 Sat Dec 5 01:45:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952759 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 0498CC433FE for ; Sat, 5 Dec 2020 01:47:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CD51B22DBF for ; Sat, 5 Dec 2020 01:47:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728117AbgLEBrA (ORCPT ); Fri, 4 Dec 2020 20:47:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57366 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726210AbgLEBq7 (ORCPT ); Fri, 4 Dec 2020 20:46:59 -0500 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 6B161C061A51 for ; Fri, 4 Dec 2020 17:46:19 -0800 (PST) Received: by mail-yb1-xb4a.google.com with SMTP id z83so9291919ybz.2 for ; Fri, 04 Dec 2020 17:46:19 -0800 (PST) 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=m859VmDX9tXDBkLVYCddTqjZNXiDWAKFJHTOT8Y5YGs=; b=ngLgiFlzF+qqIVArT0x51oNYns0yTygBkbzUa5YArn8N7TLKnTyc36SZM0mjpY8pr9 6aLjMnfAXzRQQPSptRS5Az03st1WUfeu68InEr0SQXhRD8dpbIgl8L0y61uENU+Mfc48 61qx84Al7XQCsB+O6Eje9AMHOMbh+JuHUOT/TxgD7bYxYQI+5fG1lYH4UK9wsFwbD1P4 g5FR3NX7zFcptcPcK6r3wi2n+bxllhBqoDR5YKWCUkJMYnv+yGlqEWIfpSkJYpvgb/xy 3iSnU1IYJWnoICjopgAdRtHmGtCAAGtff9hiU/sjgNI00CWmqpLam8YrQP+5Y53BSu9u r6Nw== 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=m859VmDX9tXDBkLVYCddTqjZNXiDWAKFJHTOT8Y5YGs=; b=IkKaVpBg42GNn2bBlpA2mKMVSFCPVMT9RiQaS/smr6Ph7Rl5tQyfFID4R99BYayM89 o6WnXoJUUk/MjkeuDqPsg1PCCS5xNbuIztXLLbjeyFa1yeqflH2VRw/D3MnJL02s46hF CIAnAoNHY9KDCDQMLPRV67lW00jQD4ThSR37/7VwMP7daHv805GnOK/C7XceWIZkbCoC GciKkkIPSXvsmUrAqHdMJ7KhtvkbBN0/aoWzZXKBFxDgarI/GhfwYO5amy/aNZr+aIDt CFQfWyklzoxbft9h8++/0iXsPupTIu+ormCWZM+cFu1UcJMKWfYzVwjpHqWsdrqesSfR g65w== X-Gm-Message-State: AOAM532hj7qrJt+AbAiqI+kpOo7wCKesSaUXn6lP7U6nVCZYYz6jS4nB 50rAeuEkU92zg1O/gJL3yucruz0QoFZNVxXQqLxD7inYA1xkJHNlHQ1dMDm26x30/SUJ/c9TGLi 7ybApF2sBRI8sC6lVSeLEuaHZlAQTFINwoqPYjo8+jlhsSkZFcaOGWcMoOTdseoVK7WUJORQepg == X-Google-Smtp-Source: ABdhPJxkzMRX4VcjLKTrjM6z7U0lsvV1EVByHcErbjOD2bRY136/JAs79d1QRJmdgE4z+hsLreL1GRp7cxLaVqViUeA= 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:a5b:9cf:: with SMTP id y15mr2246691ybq.463.1607132778552; Fri, 04 Dec 2020 17:46:18 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:52 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-3-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 02/17] 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 f22b7a4cf1..094f58a175 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,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 45bce31016..6ef9c0ee4e 100644 --- a/Makefile +++ b/Makefile @@ -1100,6 +1100,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 b6ce981b73..8df1d36a7a 100644 --- a/builtin.h +++ b/builtin.h @@ -163,6 +163,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 4b7bd77b80..8e92b5d3f6 100644 --- a/git.c +++ b/git.c @@ -525,6 +525,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 Sat Dec 5 01:45:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952763 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 C7960C433FE for ; Sat, 5 Dec 2020 01:47:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 94B1F22DBF for ; Sat, 5 Dec 2020 01:47:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728158AbgLEBrH (ORCPT ); Fri, 4 Dec 2020 20:47:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57370 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726210AbgLEBrH (ORCPT ); Fri, 4 Dec 2020 20:47:07 -0500 Received: from mail-qv1-xf4a.google.com (mail-qv1-xf4a.google.com [IPv6:2607:f8b0:4864:20::f4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 254E6C061A52 for ; Fri, 4 Dec 2020 17:46:21 -0800 (PST) Received: by mail-qv1-xf4a.google.com with SMTP id 102so6386223qva.0 for ; Fri, 04 Dec 2020 17:46:21 -0800 (PST) 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=wdTVSjbDCKZ8nZxxBl7Hr1U2I020cecb3VX0fkiEKxE=; b=K89nBb42FEs9Zgn7pX6YYoJZnM+TvrjfbmfLEsRSmN57P8/Puwq7IhRDiJE+U8hsZo HyjI6BQDvwLeeBYOKfVIXkQ4l7in8yCF5LW2yeSlwWlccU09r9mdiMBhDpcCgjJa8vYR pLUBfkKd2WhnIvb3aGGgTdYYMDM/UD+oEBE/adsWuazp/TFy7/ji/Ff6UppJvIE60uVf fz6oEYpxKqffr4SL615bgcoDB0/jkIs9VDhudMn4HB8YFT0GUq6bRyn9+wSfcM+zFPCj 41BfiQGhVCOHLJKw09Uv4uLnPtQuYYG/xeaQtRvY7Z7ubQdgZjVwBNZTAbzOIaxClE9D AOEQ== 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=wdTVSjbDCKZ8nZxxBl7Hr1U2I020cecb3VX0fkiEKxE=; b=TSU1KibUpmDcuaQS6Bfkv/V18Wed9VUR6myY8hpBEF6LC2jHkdYKYG+Cg0tADxB38l uikI3F70nwvbCLcm7l+z8CC7RlJwou/aCOt5Dsv2UlBrcDRgBoqNhuWKKYcB1d1Da1LT 2aqcvhZErpkBNDqWtNVmsI4CTTt6RymqLAt1K5y7jylHPTZs+VQB58qi8CAvxMRvQdV3 4R9s2ZaPaeEt7Z697E9ezFGGwaPZxdq0zXsbd/YiBwcBZ8vNYOuZ1+KXBtFFfzm57ZjY MfCFFY7HeWt0VZxXu0fxFCHo4E24gurC6XJhGibof1kArqYdVHH70hdDEU2dcZNZFew/ S8ZQ== X-Gm-Message-State: AOAM531pk0H3KOfOTTadHBbEpTeYs5hlZAKSUiB3CZDAGJ5iwATeNymP ETpmXU2QZlLhNmh6901s3MGFvFvtXPueraiaNPh2jMFV9kjfOTaDIcFvHmX4CnNUBiAaWnoEjHM nmYTUQ52k8LcCLNw5DffheU5qtWPFs8NU8DwbxzV3O5LzKnbsH8SBjHWHhhEBLJn/shYBpd2Qqg == X-Google-Smtp-Source: ABdhPJwWfQrHp7oX0/nwzDbXA9KRN44vT4RGJaOTRedx5T9HN6hHZG2Ox7vlo5y3igYtVi710iXTfnC5rJSxIzj26W8= 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:5886:: with SMTP id dz6mr9262129qvb.10.1607132780202; Fri, 04 Dec 2020 17:46:20 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:53 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-4-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 03/17] 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, updated the sample in the commit message to reflect reality better. 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 6ef9c0ee4e..4bf158c4f8 100644 --- a/Makefile +++ b/Makefile @@ -903,6 +903,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 Sat Dec 5 01:45:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952765 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 5C90FC4361A for ; Sat, 5 Dec 2020 01:47:10 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2C23522DBF for ; Sat, 5 Dec 2020 01:47:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730890AbgLEBrJ (ORCPT ); Fri, 4 Dec 2020 20:47:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57378 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725300AbgLEBrJ (ORCPT ); Fri, 4 Dec 2020 20:47:09 -0500 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 11131C061A53 for ; Fri, 4 Dec 2020 17:46:23 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id hg11so4206297pjb.2 for ; Fri, 04 Dec 2020 17:46:23 -0800 (PST) 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=Ni9MQpie+amFIefK6KBwScCT4aTJCKDvIMPdrnS1NaM=; b=g+6d1umEjOav+u1xa6D8K2ciKEh1y5yFKIA7cE3O/Gz/wC8G3eHnYGABGa6/lR62et JDhzWRqjnda/81x65Z64P28aRfUuGgtxKcaGAY0z1j+PoEs1/DqHoExXnyJ9Z+hMi4sm lWj0DbIITjTICtq1Ssclk5C9nsoa1Dz1BDrTqYC9Uh2GTixU3P02hgPMXjS+9xACKqby +kYTpNzRc0h6OSmpZxBpdkhTyK6hpKs5e2jAk8nQ1GyBnYu90lJPrALcoH+VjFO/yfcN +z4eMGd3DelFuvM3hBWbgGR/qPqAH0mMVgc8WxGCO8gugD5PDF396QHauvsufyYlD05K HLBQ== 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=Ni9MQpie+amFIefK6KBwScCT4aTJCKDvIMPdrnS1NaM=; b=A3SHgX1iak+vfQQegY36JXVL1F4qKNmqaNFyGxhtlDYEGwivO3jEsacqWP5tpVwmKD W9FkkJwwwqoo+DXPciI/wAKZqk6rR0rZP7FmYHeJYnKwkNfxoWHXe4Eu0EoucROzPZ6g Pp0Lwzk8/Mg23ISbRk82iCk/RnyrSExxeKWwzKz9deGncOW3iltiGnmeU5vquREp49R7 ZjgMmKrLXO6EfMX3Xp8TlvxzwsmIAFb8O2pk6t3KZLwNiI9IddvvPjuXyHQiCt8dcK3h QXDetc2YFnU6FTj5YnT7xjdY+5jmMXmxmVESd+s5LK3PFVbo3WYnqKLzD0obI6z/1kxu fUYg== X-Gm-Message-State: AOAM532xoCwcrHIuIyO7HdEsPVpgi+PdDNUTR4uHESvpRE0wrT5ncIU5 4QTOEaNmn6zzsfgy42sC5IPkw/PNXDJ3x5oCtFdNSCNF/BCEXpv/Q0aWcgCKkYOO7Fv4nUht+EJ CEYd6K1FIz1EJz0by3tQzi8UluVdl1eeOfIevWiCkbtOtVWYwN5c4c6nmRL5CTUtfNQX2gV9WRA == X-Google-Smtp-Source: ABdhPJzI4PXo3NPdnfHb7EH45bHLgVaA4Ksl63mjW8KtQfoviMyrUZPHnKOJRFJEK+TlPByv3oPwWAgRqY9fCaZuKOQ= 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:a63:1e11:: with SMTP id e17mr9935380pge.156.1607132782204; Fri, 04 Dec 2020 17:46:22 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:54 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-5-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 04/17] 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. Legacy hooks should be run directly, not in shell. We know that they are a path to an executable, not a oneliner script - and running them directly takes care of path quoting concerns for us for free. 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. builtin/hook.c | 16 ++++++++++++---- hook.c | 15 +++++++++++++++ hook.h | 1 + t/t1360-config-based-hooks.sh | 19 +++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/builtin/hook.c b/builtin/hook.c index 4d36de52f8..45bbc83b2b 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -16,6 +16,7 @@ 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(), @@ -42,10 +43,17 @@ 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: %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); diff --git a/hook.c b/hook.c index 937dc768c8..ffbdcfd987 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 */ @@ -100,6 +102,7 @@ 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 +113,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..5750634c83 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; }; /* diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 6e4a3e763f..0f12af4659 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,15 @@ 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_done From patchwork Sat Dec 5 01:45:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952767 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 1082EC4361A for ; Sat, 5 Dec 2020 01:47:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D601722DBF for ; Sat, 5 Dec 2020 01:47:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728538AbgLEBri (ORCPT ); Fri, 4 Dec 2020 20:47:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726194AbgLEBri (ORCPT ); Fri, 4 Dec 2020 20:47:38 -0500 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 C8F8BC061A54 for ; Fri, 4 Dec 2020 17:46:24 -0800 (PST) Received: by mail-pf1-x449.google.com with SMTP id q22so4892310pfj.20 for ; Fri, 04 Dec 2020 17:46:24 -0800 (PST) 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=6FkmMIv8T+hhh07ee5LfB9fHE709Ji9MhlDFZfbBFKc=; b=ceBUSwid3oQzBdGynjf39MLwqsga3UYOfJ+2vjWfovVWTo5PQMjBVR26y1531joAea rAcbgQoK304qBcT/R/vTAi7d1lnn8zUsfHiVwq2hzIkolXrETKdeFYvwWzeZmHoCdD31 aTyFvvagJnajvwT7cvJAnBRu95C6V3kAHGddSdL6yuWeo3RmaDU7TtwsVN4gfGPH9yhH +jzYg3CeDokfLlYtXBUbKpWKUh9EQ8POa9Z6xUo4+SDcbOaUiIH9L8MNzKM9COGqKLKC 96KnNHuGQJnQmrIHpaUuof6CTwBpzI2yZB9ivgSJDJ0MotEyxmsLUXQsF6oBO6pVntnZ Qp5w== 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=6FkmMIv8T+hhh07ee5LfB9fHE709Ji9MhlDFZfbBFKc=; b=W44WP9hmxCKVPm9z2azPTIbHtZDyy4DOXbzuzGix6bBH/MoOM406NXP2XSnkHntVli yREAL4oin89+uo2q4GB55tq+dwIEjYJ+99AgRiecSZADUpncU2LWuO6OFoyw+Ik6k+3E vclwD1tdOAAxtFYVOKkaZwf35Wei1jsDXxIjXYCR9wcTkK21ZSMvZBfKkxVMQt2HHk6m wruc8WkxyufQCJ58ZE5uqEIPbmF1sn65h4cegVgkqJWNfdb5fXGqlcd51j09bOSE/wKr xljdlcVoPzM+hua1/OoM3jNxjPOm2MwsNL1NqwZOlftEzWidBYadnuYnEY+oQDBsNMq4 6psg== X-Gm-Message-State: AOAM532Ifs7azmBfS9HyKIUxBWFgud8Rjlmj5pWV62yhWHKhs33mAECw 7dwReMcnWsEf7MxNVmcQK7xdz0m506OpbNC28WwRs9HqG/tY3rewcg0fE13kavzBn+14YK48Mfp 1of9b401CrFtr3Dpz0DEPU4NJAVGP3eYKGicEYY7IKaIufZfq4M4b/RoKj+2bWp7JhFDtZm3xfA == X-Google-Smtp-Source: ABdhPJzUFYiELTmW6fWVDfPj9DKtTC6AH7efnxvjEUc37d+dDA6ZMKOOyMFb5YuJz3Rkoh1tuI2D1+wQtmMm6F4MCfw= 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:a63:5f07:: with SMTP id t7mr9739238pgb.144.1607132784182; Fri, 04 Dec 2020 17:46:24 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:55 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-6-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 05/17] hook: respect hook.runHookDir From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Include hooks specified in the hook directory in the list of hooks to run. These hooks do need to be treated differently from config-specified ones - they do not need to run in a shell, and later on may be disabled or warned about based on a config setting. Because they are at least as local as the local config, we'll run them last - to keep the hook execution order from global to local. 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 | 54 +++++++++++++++++++++++++++++++++-- hook.c | 21 ++++++++++++++ hook.h | 15 ++++++++++ t/t1360-config-based-hooks.sh | 43 ++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 3 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 45bbc83b2b..16324d4195 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -11,6 +11,8 @@ 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; @@ -41,6 +43,26 @@ 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) { @@ -64,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 ffbdcfd987..340e5a35c8 100644 --- a/hook.c +++ b/hook.c @@ -97,6 +97,27 @@ 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; diff --git a/hook.h b/hook.h index 5750634c83..ca45d388d3 100644 --- a/hook.h +++ b/hook.h @@ -21,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 0f12af4659..91127a50a4 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -104,4 +104,47 @@ test_expect_success 'git hook list shows hooks from the hookdir' ' 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 Sat Dec 5 01:45:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952769 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 68D39C433FE for ; Sat, 5 Dec 2020 01:47:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3EA2822DBF for ; Sat, 5 Dec 2020 01:47:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729972AbgLEBri (ORCPT ); Fri, 4 Dec 2020 20:47:38 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727225AbgLEBri (ORCPT ); Fri, 4 Dec 2020 20:47:38 -0500 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 C2424C061A55 for ; Fri, 4 Dec 2020 17:46:26 -0800 (PST) Received: by mail-yb1-xb4a.google.com with SMTP id z29so9213832ybi.23 for ; Fri, 04 Dec 2020 17:46:26 -0800 (PST) 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=wvrSrg5uh1R7ODnVjJP54cmy3dbwabyEERDXi5IHc6c=; b=CAw5mcoiSVWoUfrgi2lDPDdIqgUUim2H5jUw6jgnGbDxuTUMbC2k0xQkZ5NcuUNcUF bffOjQyATiqQHcV/NQkmqo9pymHNPBHU3Ik0UoyIN1HxgCHEUX07H/CxAGI60Wlfl5bL LAkEV+4Q3csGOiMGBMe94I764AIeDJJZYnFdpwFmEdHag9vlMf8YAu5eXjJJyaXDIpRW DGsKvLEDvn4lGwML7RLgd+4rfIAFX1DZ+7SE6Q4dP0hzLawl5vQi2Ahr8YJcInso4w3o Ba2CeIdzpkvMogFbxgGFNLjSkkF3x24tnskrQ+ys2xtw9K1IpaABhtgLL3cjzG2bWG+8 85mA== 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=wvrSrg5uh1R7ODnVjJP54cmy3dbwabyEERDXi5IHc6c=; b=aoOM+UJE1AmW24D1uGHUbC3GtAzWYqxc3eErlt2qW+umhnQptJV7dRFBRBKh22tBxF 105FeZyftW7mpPDaHTpKAs7Eh561FQXTXWP/5oTpmka7YnjFWN7fNWgjIgRQwyA9dTRw 71YbPJ4VSRQMPH0j/dpaPef9A3RVbGBxl6UgKJ5St5VBwB64EFBzOnFOkjnMqux8URFW qVrVg/0KYF0k8Q02yYtz9SvTouwNwbJDbwRHNgGPzj1iox/fiz7NSyRaKezPBSsjfWca O0pIcbwChD/HgsjEewbLSjZ05glI0EfwDnXksw2X75eYcC6Rh/OGEown8tHIvUWHLL9j kzIg== X-Gm-Message-State: AOAM531fLWnHztdIWRaDx6ceeJ30YuZSKwyTrM3JO1JwnnCGRDVXv+Uf UZUfJetc8fkciVLMLTFgOCFItOCUOtSGE/uNwLGqgv5vEOQuzwcjBIqLfIBxTNC+N9bI/IgF/2j oyLy4ugcWfEgGWNFKSdE7/p5TdZDGbqXdsRnHBehDzs7o/h0epUfw90STdvPWVH1JMwrbRE0Cug == X-Google-Smtp-Source: ABdhPJy5F6Gs2F/qNdkRZXXNM5Gfx+X7Z83T7eDKPgwRgrY7bOtPxvL274s3ebiCrdgOuEXRigiSdTOu2mAuQcXNVHc= 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:6902:4a5:: with SMTP id r5mr9131695ybs.443.1607132786032; Fri, 04 Dec 2020 17:46:26 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:56 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-7-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 06/17] 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: In addition to being handy for turning off global hooks one project doesn't care about, this setting will be necessary much later for the 'proc-receive' hook, which can only cope with up to one hook being specified. 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 During the Google team's review club this week 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 we 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 Sat Dec 5 01:45:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952775 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2573CC433FE for ; Sat, 5 Dec 2020 01:47:47 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 035D922DFB for ; Sat, 5 Dec 2020 01:47:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731105AbgLEBrq (ORCPT ); Fri, 4 Dec 2020 20:47:46 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725300AbgLEBrp (ORCPT ); Fri, 4 Dec 2020 20:47:45 -0500 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 BE6F0C061A56 for ; Fri, 4 Dec 2020 17:46:28 -0800 (PST) Received: by mail-qt1-x849.google.com with SMTP id v18so6236886qta.22 for ; Fri, 04 Dec 2020 17:46:28 -0800 (PST) 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=IrR711oKxyB5r8jYdcnsLyLHtTjfJvr79t/+lDexFhPTx1uvAxg3g9350+ZmW2yWa6 wjaclTIaibOZ8gQZgM3rhPPaAq3YTw3mcI+DkDDH2CiIkeq9GZHVM4D6hpIYagFBqdQZ 4mfDUGDXvUoGSBDptapDY/beRz/YltCkP73JgXofPlu9ie+h/HQ6CdtyANKBYHjLHAFf ulr5HUWiOhQ6H44yUo9mt7WdEZXPIvxPSKNz+95W6Jh5c0mi+G6ktgIBtSb6rhxK0o6n 2xgwGkBKfDbq0XSHGgUp9ABkRmujn5Kq4c2O8Nhwt5OOoKkVsWbwCM/e6zDBi9rgcohx UKbA== 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=VDkChuXBy5buVVKBqTbXiiyrHsPjjJPKYTW6bZ0NliPYQvMdph2loPO0y5nh3xqiz9 8I8kRA0fr4aJUADbmZ7wyZIwL4hyJZilE329/umm3HNQ6tROXNk+/fjbVd7f+DWU8NiI 4CBSZHO72joCel91NgudQxe8M5MlOPg6P0Y1HfZorh1eFytCCKHXe6io6iabzXvOhFIb 9LKd/fdmxIyfN4thLMTQ4MSQZWB+mRNbu0q5VLk/E+1d9kzPD1LZ75xnsuzCdOkkduUZ 7YPfT7NrVAxmhlXuxzsMHXS6OhsZkZI0vB/vpftjXy6uKBQg0qT5wlGZpzFmpnZJBJbD ZoWQ== X-Gm-Message-State: AOAM530uDWv3vCdWe69LqTAqeC7itnILQbfQpZB6WDTyDimO9tZMKh7e gxsh/sx1s3slH3IXGj0u3CrnFgymH+A6LsNUzdAzxkdNM6yrEtCkoAisuIwSn1aQ/2Guljeyt6g XveZWaDexPXf7rkCzlHeGf/VBjYCbsnRNDuUTWsv8KoPfbSF588SfhFnbj/ydzidHRCGqpsX5uw == X-Google-Smtp-Source: ABdhPJwrygg3ICRFKoPGO5pzvtUkcLwHntw8LTnha+Wo6ama9Xo1U5q6MvvzichSo9ctPIxeJHKzOK5twok7wYL1ixo= 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:b59a:: with SMTP id g26mr9152078qve.26.1607132787887; Fri, 04 Dec 2020 17:46:27 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:57 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-8-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 07/17] 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 Sat Dec 5 01:45:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952771 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 6E72AC4361B for ; Sat, 5 Dec 2020 01:47:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4331D22DFB for ; Sat, 5 Dec 2020 01:47:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731085AbgLEBrk (ORCPT ); Fri, 4 Dec 2020 20:47:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57478 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725300AbgLEBrk (ORCPT ); Fri, 4 Dec 2020 20:47:40 -0500 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 69D52C08C5F2 for ; Fri, 4 Dec 2020 17:46:30 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id 4so9313191ybv.11 for ; Fri, 04 Dec 2020 17:46:30 -0800 (PST) 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=0SgyFa5iSE9dtTYUMGnI4CHaY0mL5uwWD+0ut/xu9Mo=; b=POuV+2EBv+7USUk7pceVUovnvTJg/gHHZwGm5D5YZxjv5zYYwPlaPwKgM1elF1iLbm Z0SIYOHEeu4E4ocLPvMqTDhYuTdv3Uhrl5fL7c4hrABr7gAC1kN0Uc3kZXvlb6KYoIKR JV4frq8tPTybzNqAF9ROIPIBvoFEY07bTJjRaMy7ra/MlWmyS0OK+bO49B44kWHimEWJ hylxo5TNxzA2vgoAMNNS7m2Zv3RPhXD4/B+qp1gF5kpK916seIsLgyA/xQxxdtTwNifd JHagCX1op/dpa2BmRk7t5oBNhQSqy0JDh62Wi+BlrwHH9YgSjvCqsc9H9vm1B5YjMmcr +hTg== 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=0SgyFa5iSE9dtTYUMGnI4CHaY0mL5uwWD+0ut/xu9Mo=; b=JSmob3j5SqwBF7bQe6qWGB0CsB5YU3vBg2+PpAanrqbSLwV5I96sMgf5DiFtVMw7AD xYZqqEKpldrv65x3Sgj/LrZWdNeHX+pvs7OavdE9ljOZSugp4KLvgv3+snkMjFoI/4JL hrmDGHP5bvhiMcpmV3OQkHKMqGHFChgScBWu6JyHeBfbC1yrtGcWc4KJcAq7JRN6L/a/ ryZEsiy1JpXB+jek3AlldUxB9GOkXGLvg9MxG6u9Kp/5rNogSC3K9junhP43PCaUS736 F+gDSpjQ1DBmWzGOAog91r5xOpNWf1nPbU3MgxRFYUqdF1Dosup6hSgixTSUfUrEqFeN Sn6Q== X-Gm-Message-State: AOAM531tjS40X5cecT69ULlHx3/4H6yyfnnl4qOzUJeWFDErYDgbiiLG 4zqOCUXPiympEceHnXhFPrwkj8z8FEeyhlzaZEwJC/WARg7RYnUA8HVGj2QHv89gRqVvt3NAIdy Wmr0ZaAIJIkabjy0Kp2vq8wvAQpWJj0EO+JkohZ7Za8VTUbu0zRdCWl2zQlExaEOKerE1kskPUA == X-Google-Smtp-Source: ABdhPJzzv8/FVz/NgBfDpAlgbYxQfezhgzd79v4fkvcTEBKiLi2yXJBkHQubg/1zNCI6D15B9F65dIAI2UuHfodymsI= 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:38d5:: with SMTP id f204mr10207194yba.88.1607132789573; Fri, 04 Dec 2020 17:46:29 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:58 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-9-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 08/17] 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 | 31 +++++++++- builtin/hook.c | 48 ++++++++++++++- hook.c | 112 ++++++++++++++++++++++++++++++++++ hook.h | 32 ++++++++++ t/t1360-config-based-hooks.sh | 65 +++++++++++++++++++- 5 files changed, 281 insertions(+), 7 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index f19875ed68..18a817d832 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -9,11 +9,12 @@ SYNOPSIS -------- [verse] 'git hook' list +'git hook' run [(-e|--env)=...] [(-a|--arg)=...] 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,32 @@ 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 [(-e|--env)=...] [(-a|--arg)=...] ``:: + +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`. + +OPTIONS +------- +--run-hookdir:: + Overrides the hook.runHookDir config. Must be 'yes', 'warn', + 'interactive', or 'no'. Specifies how to handle hooks located in the Git + hook directory (core.hooksPath). + +-a:: +--arg:: + Only valid for `run`. ++ +Specify arguments to pass to every hook that is run. + +-e:: +--env:: + Only valid for `run`. ++ +Specify environment variables to set for every hook that is run. + CONFIGURATION ------------- include::config/hook.txt[] diff --git a/builtin/hook.c b/builtin/hook.c index 16324d4195..26f7050387 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,46 @@ 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 run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + int rc = 0; + + struct option run_options[] = { + OPT_STRVEC('e', "env", &opt.env, N_("var"), + N_("environment variables for hook to use")), + OPT_STRVEC('a', "arg", &opt.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]); + opt.run_hookdir = should_run_hookdir; + + rc = run_hooks(hookname.buf, &opt); + + strbuf_release(&hookname); + run_hooks_opt_clear(&opt); + + return rc; +} + int cmd_hook(int argc, const char **argv, const char *prefix) { const char *run_hookdir = NULL; @@ -95,10 +137,10 @@ int cmd_hook(int argc, const char **argv, const char *prefix) }; argc = parse_options(argc, argv, prefix, builtin_hook_options, - builtin_hook_usage, 0); + builtin_hook_usage, PARSE_OPT_KEEP_UNKNOWN); /* after the parse, we should have " " */ - if (argc < 1) + if (argc < 2) usage_with_options(builtin_hook_usage, builtin_hook_options); @@ -120,6 +162,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..c4595a2324 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,64 @@ struct list_head* hook_list(const struct strbuf* hookname) strbuf_release(&hook_key); return hook_head; } + +void run_hooks_opt_init(struct run_hooks_opt *o) +{ + strvec_init(&o->env); + strvec_init(&o->args); + o->run_hookdir = configured_hookdir_opt(); +} + +void run_hooks_opt_clear(struct run_hooks_opt *o) +{ + strvec_clear(&o->env); + strvec_clear(&o->args); +} + +int run_hooks(const char *hookname, struct run_hooks_opt *options) +{ + struct strbuf hookname_str = STRBUF_INIT; + struct list_head *to_run, *pos = NULL, *tmp = NULL; + int rc = 0; + + if (!options) + BUG("a struct run_hooks_opt must be provided to run_hooks"); + + 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 = options->env.v; + 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, options->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 + */ + strvec_pushv(&hook_proc.args, options->args.v); + + rc |= run_command(&hook_proc); + } + + return rc; +} diff --git a/hook.h b/hook.h index ca45d388d3..d1c3d71e82 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 { @@ -36,6 +37,37 @@ enum hookdir_opt */ enum hookdir_opt configured_hookdir_opt(void); +struct run_hooks_opt +{ + /* Environment vars to be set for each hook */ + struct strvec env; + + /* Args to be passed to each hook */ + struct strvec args; + + /* + * How should the hookdir be handled? + * Leave the RUN_HOOKS_OPT_INIT default in most cases; this only needs + * to be overridden if the user can override it at the command line. + */ + enum hookdir_opt run_hookdir; +}; + +#define RUN_HOOKS_OPT_INIT { \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ + .run_hookdir = configured_hookdir_opt() \ +} + +void run_hooks_opt_init(struct run_hooks_opt *o); +void run_hooks_opt_clear(struct run_hooks_opt *o); + +/* + * Runs all hooks associated to the 'hookname' event in order. Each hook will be + * passed 'env' and 'args'. + */ +int run_hooks(const char *hookname, struct run_hooks_opt *options); + /* 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 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 From patchwork Sat Dec 5 01:45:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952773 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 87474C4361A for ; Sat, 5 Dec 2020 01:47:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6129622DBF for ; Sat, 5 Dec 2020 01:47:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731100AbgLEBrm (ORCPT ); Fri, 4 Dec 2020 20:47:42 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725300AbgLEBrm (ORCPT ); Fri, 4 Dec 2020 20:47:42 -0500 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 28982C08E85E for ; Fri, 4 Dec 2020 17:46:32 -0800 (PST) Received: by mail-qv1-xf49.google.com with SMTP id l15so6390665qvu.8 for ; Fri, 04 Dec 2020 17:46:32 -0800 (PST) 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=0i3Af1xw3+WpEJSag78xE+ks79Awiuaj9JxLRcxb80c=; b=JuR8agcc7UWswzxfKJUAgwNRV+fF8tEPvSfwozCP0ql1xCEpWnLpSuC3H4/Ne2utBH XyagHdoLLpfH+LlszqqWnCoMmU77B2SP3XV8pD7Zn0kGvL2J3vzNeif0mXq36f9lm6oL 8qI/eM7Xg+io20K81YdPa0p0GadpcrVkMShLVD5Q5+N34P0PpKFaGHvSf5r0GgDr02Zq aDzIPaEjrJhjPS2bk/91w2xjSnkCDBu6w0MUt7uM1Gu59YzlShp1DW4L7HxA06h8OzDs UaR3v33dlwqx+Bys+TmDOVK+moe3nAQiZnyW4yPoFXL1uP3Po9KgNCcZMG6dByZTZgJn gZkg== 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=0i3Af1xw3+WpEJSag78xE+ks79Awiuaj9JxLRcxb80c=; b=RfjKy61N8/E34Bfk2ty0Oj7JBhNgLOE4AyCx249NhZQr3y5H4mpVYZ+AMLtxNHE3kf Rg2fuCTnTzHv4OBcD6uh8Bu0PNeHcJ24+G+tqsjBtl/jm9rkjF4xN75mjbJaPbjnQvgL F4tyvP82EngXvP7IQjjkeSaY70h33T6aOYKRIP+9oZuZflYbrHxWl15c/CMrM9KUXlnJ rxOdhurZg211dBNvwtQWIpcsN5EpjwgLyE9H1IZVXgduBWFEQ85lup/Pii5pxXeHa36e EupOKT/mjZhq8TBHBGzjq7AQC27NBd/xi9lhmYTV9dHZRT8mGbG0vKDN6WHEaQ2tKmtr xB9w== X-Gm-Message-State: AOAM532nfqrlMXJPy4bOO241DOgKt+7JbT3MLNas/kd7UG66g+b/iRre KyBKNlC5hDnjObyGfOnusMF0+ewkpbqbegEriCcKUf8EjxGYqLVTwqDsNZkMOA0KQaoHgXuWMpf EPgX1Pb88s0pe52UwPVmrV0RtoEExr5A91lcC3j0KeWvPc0+JP2cAoNckF2zOj58Hfe1sKB81mw == X-Google-Smtp-Source: ABdhPJzeZ45C28B0WHbAjjT15JI2nnT+NXd2ClL8BFEHbijJME4qjdygKlb8G8y9pFjh1zpEVS7XERju5EshnpKu75M= 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:55c2:: with SMTP id bt2mr9233080qvb.48.1607132791319; Fri, 04 Dec 2020 17:46:31 -0800 (PST) Date: Fri, 4 Dec 2020 17:45:59 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-10-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 09/17] hook: replace find_hook() with hook_exists() From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer 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. Many callers want to check whether some state could be modified by a hook; that check should include the config-based hooks as well. Optimize by checking the config directly. Since commands which execute hooks might want to take args to replace 'hook.runHookDir', let 'hook_exists()' mirror the behavior of 'hook.runHookDir'. Signed-off-by: Emily Shaffer --- Notes: Since v4, updated this commit to include bugreport as a builtin instead of as a standalone. Since v4, a little more nuance when deciding whether a hookdir hook can happen. builtin/bugreport.c | 4 ++-- hook.c | 15 +++++++++++++++ hook.h | 9 +++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 3ad4b9b62e..11043f4a22 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -3,7 +3,7 @@ #include "strbuf.h" #include "help.h" #include "compat/compiler.h" -#include "run-command.h" +#include "hook.h" static void get_system_info(struct strbuf *sys_info) @@ -82,7 +82,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) } for (i = 0; i < ARRAY_SIZE(hook); i++) - if (find_hook(hook[i])) + if (hook_exists(hook[i], configured_hookdir_opt())) strbuf_addf(hook_info, "%s\n", hook[i]); } diff --git a/hook.c b/hook.c index c4595a2324..a7a4abdcac 100644 --- a/hook.c +++ b/hook.c @@ -225,6 +225,21 @@ void run_hooks_opt_init(struct run_hooks_opt *o) o->run_hookdir = configured_hookdir_opt(); } +int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir) +{ + const char *value = NULL; /* throwaway */ + struct strbuf hook_key = STRBUF_INIT; + + int could_run_hookdir = (should_run_hookdir == hookdir_interactive || + should_run_hookdir == hookdir_warn || + should_run_hookdir == hookdir_yes) + && !!find_hook(hookname); + + strbuf_addf(&hook_key, "hook.%s.command", hookname); + + return (!git_config_get_value(hook_key.buf, &value)) || could_run_hookdir; +} + void run_hooks_opt_clear(struct run_hooks_opt *o) { strvec_clear(&o->env); diff --git a/hook.h b/hook.h index d1c3d71e82..94a25c7cd0 100644 --- a/hook.h +++ b/hook.h @@ -62,6 +62,15 @@ struct run_hooks_opt void run_hooks_opt_init(struct run_hooks_opt *o); void run_hooks_opt_clear(struct run_hooks_opt *o); +/* + * Returns 1 if any hooks are specified in the config or if a hook exists in the + * hookdir. Typically, invoke hook_exsts() like: + * hook_exists(hookname, configured_hookdir_opt()); + * Like with run_hooks, if you take a --run-hookdir flag, reflect that + * user-specified behavior here instead. + */ +int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir); + /* * Runs all hooks associated to the 'hookname' event in order. Each hook will be * passed 'env' and 'args'. From patchwork Sat Dec 5 01:46:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952779 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 0A513C4361B for ; Sat, 5 Dec 2020 01:47:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CFE5922DFB for ; Sat, 5 Dec 2020 01:47:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731125AbgLEBrs (ORCPT ); Fri, 4 Dec 2020 20:47:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57488 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731109AbgLEBrr (ORCPT ); Fri, 4 Dec 2020 20:47:47 -0500 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 EFF66C08E85F for ; Fri, 4 Dec 2020 17:46:33 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id a13so9306733ybj.3 for ; Fri, 04 Dec 2020 17:46:33 -0800 (PST) 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=47/DYeNU6D8st82KWpNBYJdvhFv5Im0BXbxBiJhbb6M=; b=mI0TrZ65mMaQYaw37y8b/kYi7yUCPQapoppj81KFFSddBbylpNNIWe/wSnDLhuulFl 7liRSobSBc8NlpJ4d2GBbxHQdtZXNfdRaUcmKqc+rBBBj513M8S4hu6xqRCjXmUq5ZPG eQb+QKGCre3s2/s+6L1CTzEDMLOGa611546z2QMD34diyu+rZr8g5/IbKohYsHdvtzSB e6WDs5tuzq6t8uziEs70gng2WlfEAmhouJ524XsR9PGJfBwCAGIzVcHgafYdfVyy1TxK ej66tHmtAL2e7IQIfaeTKOx0Dwn54cuaHiu10bI5CMrng74SdlO+w4RUIWdzySwKI1ej HTRQ== 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=47/DYeNU6D8st82KWpNBYJdvhFv5Im0BXbxBiJhbb6M=; b=nQqjZWT9gNx47sLy9GgazjfF5P0BSHI+UFLikhoLgb1l76XfpnU0WTpHAhCfeOa/MN j14B0pheAuxEr99Qr5ewpKKRbxwfAKkQKnQ60bmKx898yFkKZE7QGbeXe+kP9cTlrQ9t 9uvPUKzcfKYskKecZczhDEPVfS9zlyQefP3mikFO30aspKklXjhBIZuSPbvnpt3VodU/ PFZYETMKN2g0191jx/5z+nw9guPWdoquIr3KSYOr8tM6KEs0QyiPCdTUkj7ndEIeEeVn uaMRcymE9NYrk+itskqtHLUDkjqS5SOz8rPb4y+VuWqTvvnw7lb1SynBLDpJ41oOwHvd Hm5w== X-Gm-Message-State: AOAM530gRNlZO3AvdGu2vyfPaHevW7keio84KfSxMPcz2hL9tIUytni6 PI5spD/uuEHEv8qSKZKvZhHHYZA8/W2teRGRtFA5RCBxdgX7Gb7AGkm6Kuh/DStbDYPXdjgitI3 Y0AcXWAUpvUehKlOa1bO7a2kt4dDN+MzlFPBCPfSIHLd6eAzy4smYYVo8CeASMw7pKUunZ6MH9Q == X-Google-Smtp-Source: ABdhPJx0qpDUL31l4Ii15yKAOHkrXccqZNxF5/qFrd5cPTcgHJQj9XRPI3lHITVEQLJd2cpb7ZsSdBTQf+JguLprSFw= 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:b11e:: with SMTP id g30mr10173535ybj.71.1607132793166; Fri, 04 Dec 2020 17:46:33 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:00 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-11-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 10/17] hook: support passing stdin to hooks From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Some hooks (such as post-rewrite) need to take input via stdin. Previously, callers provided stdin to hooks by setting run-command.h:child_process.in, which takes a FD. Callers would open the file in question themselves before calling run-command(). However, since we will now need to seek to the front of the file and read it again for every hook which runs, hook.h:run_command() takes a path and handles FD management itself. Since this file is opened for read only, it should not prevent later parallel execution support. On the frontend, this is supported by asking for a file path, rather than by reading stdin. Reading directly from stdin would involve caching the entire stdin (to memory or to disk) and reading it back from the beginning to each hook. We'd want to support cases like insufficient memory or storage for the file. While this may prove useful later, for now the path of least resistance is to just ask the user to make this interim file themselves. Signed-off-by: Emily Shaffer --- Documentation/git-hook.txt | 11 +++++++++-- builtin/hook.c | 5 ++++- hook.c | 7 ++++++- hook.h | 9 +++++++-- t/t1360-config-based-hooks.sh | 24 ++++++++++++++++++++++++ 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index 18a817d832..cce30a80d0 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -9,7 +9,8 @@ SYNOPSIS -------- [verse] 'git hook' list -'git hook' run [(-e|--env)=...] [(-a|--arg)=...] +'git hook' run [(-e|--env)=...] [(-a|--arg)=...] [--to-stdin=] + DESCRIPTION ----------- @@ -65,7 +66,7 @@ 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 [(-e|--env)=...] [(-a|--arg)=...] ``:: +run [(-e|--env)=...] [(-a|--arg)=...] [--to-stdin=] ``:: 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 @@ -91,6 +92,12 @@ Specify arguments to pass to every hook that is run. + Specify environment variables to set for every hook that is run. +--to-stdin:: + Only valid for `run`. ++ +Specify a file which will be streamed into stdin for every hook that is run. +Each hook will receive the entire file from beginning to EOF. + CONFIGURATION ------------- include::config/hook.txt[] diff --git a/builtin/hook.c b/builtin/hook.c index 26f7050387..e45831e01d 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -9,7 +9,8 @@ static const char * const builtin_hook_usage[] = { N_("git hook list "), - N_("git hook run [(-e|--env)=...] [(-a|--arg)=...] "), + N_("git hook run [(-e|--env)=...] [(-a|--arg)=...]" + "[--to-stdin=] "), NULL }; @@ -97,6 +98,8 @@ static int run(int argc, const char **argv, const char *prefix) N_("environment variables for hook to use")), OPT_STRVEC('a', "arg", &opt.args, N_("args"), N_("argument to pass to hook")), + OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), + N_("file to read into hooks' stdin")), OPT_END(), }; diff --git a/hook.c b/hook.c index a7a4abdcac..c7fdf556fe 100644 --- a/hook.c +++ b/hook.c @@ -263,8 +263,13 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) struct child_process hook_proc = CHILD_PROCESS_INIT; struct hook *hook = list_entry(pos, struct hook, list); + /* reopen the file for stdin; run_command closes it. */ + if (options->path_to_stdin) + hook_proc.in = xopen(options->path_to_stdin, O_RDONLY); + else + hook_proc.no_stdin = 1; + hook_proc.env = options->env.v; - hook_proc.no_stdin = 1; hook_proc.stdout_to_stderr = 1; hook_proc.trace2_hook_name = hook->command.buf; hook_proc.use_shell = 1; diff --git a/hook.h b/hook.h index 94a25c7cd0..5184dcaa5a 100644 --- a/hook.h +++ b/hook.h @@ -51,11 +51,15 @@ struct run_hooks_opt * to be overridden if the user can override it at the command line. */ enum hookdir_opt run_hookdir; + + /* Path to file which should be piped to stdin for each hook */ + const char *path_to_stdin; }; #define RUN_HOOKS_OPT_INIT { \ - .env = STRVEC_INIT, \ + .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ + .path_to_stdin = NULL, \ .run_hookdir = configured_hookdir_opt() \ } @@ -73,7 +77,8 @@ int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir); /* * Runs all hooks associated to the 'hookname' event in order. Each hook will be - * passed 'env' and 'args'. + * passed 'env' and 'args'. The file at 'stdin_path' will be closed and reopened + * for each hook that runs. */ int run_hooks(const char *hookname, struct run_hooks_opt *options); diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh index 5b3003d59b..c672269ee4 100755 --- a/t/t1360-config-based-hooks.sh +++ b/t/t1360-config-based-hooks.sh @@ -229,4 +229,28 @@ test_expect_success 'out-of-repo runs excluded' ' nongit test_must_fail git hook run pre-commit ' +test_expect_success 'stdin to multiple hooks' ' + git config --add hook.test.command "xargs -P1 -I% echo a%" && + git config --add hook.test.command "xargs -P1 -I% echo b%" && + test_when_finished "test_unconfig hook.test.command" && + + cat >input <<-EOF && + 1 + 2 + 3 + EOF + + cat >expected <<-EOF && + a1 + a2 + a3 + b1 + b2 + b3 + EOF + + git hook run --to-stdin=input test 2>actual && + test_cmp expected actual +' + test_done From patchwork Sat Dec 5 01:46:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952777 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9C688C4361A for ; Sat, 5 Dec 2020 01:47:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D0A322DFB for ; Sat, 5 Dec 2020 01:47:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731123AbgLEBrs (ORCPT ); Fri, 4 Dec 2020 20:47:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57504 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725300AbgLEBrr (ORCPT ); Fri, 4 Dec 2020 20:47:47 -0500 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 B9671C08E860 for ; Fri, 4 Dec 2020 17:46:35 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id z29so9214097ybi.23 for ; Fri, 04 Dec 2020 17:46:35 -0800 (PST) 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=xksZ8TN+Bx5zxuwy/jMdxbToJEO3CXMa1+UlyOml024=; b=BBu0jpX+Oxo6lcgFxpiasDApHEuNsnoq4r4VFN8Xl3vxcJZuNiMdfVfN4N0GEBtjoY 3iIMKU54LjRf2N94HbYadA/eTnKwCfJiD53lkYt4ROPfCVP0CclYEcqVKErVlGtXEuos cXqMb77INliFu43biEREGiT5iMO2Tb0p7EKm+SgFX5qQBrNChk6eIEgW2mXMobiji/Ae sQDbSau6WWND0U1kdBLpCsY18GORgGDsR7eOdjP7TNHW8YeF9qii22+HZYhvPKPz+zrr 3m8+PMhDdS7K7cxNKTesuwMMVuf83GI0T+0h7e6DWaQ4kpWsc1zTKgcZbBp0Hq5msecl iTcA== 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=xksZ8TN+Bx5zxuwy/jMdxbToJEO3CXMa1+UlyOml024=; b=pr+xpNeWR1ktZf9FUYM3fd3zeOUpDycqkhyVuP1I62wNG8HVp1/qbOMYI9XCfuu0rZ Gu+I0T3uMZUUw0ULYDEZhngNDgTME8Glom6QkF7qrxHmHvkRRNr6sjPQja6mju7fZhqS 8RBr+T3rH/tBC67FSSAK9TNg/nl5/3PiXzMn6YrkLA6VtKKuM5FWQ6S5eN3VypFnUOqg sVq7I8GSJHN9LI2cRkGAtOnDLRnS5HomxUPN4Le4R1cJrMhQMFiKvLBgpiciLPC59Zuo YY8CLRHJQGq4ZhECnyV+bXEtjxRjpmvhOkzL4HMR5g4sdExwqme/TEjtyoJoehNmMHY/ hB9Q== X-Gm-Message-State: AOAM532tdTyO4bJ9nzVVIu/iOycrZR+7SZPjGjBoumqnTsWoXinsHwcR O6dlLAKJGR/a1Yy29KaPCCAfHrUjZlNW8nqRsg+uGOkEIPovpA89jsGa9skqn8WsqbqtI86DOK+ aWY6e4dxnOiwcWXWtlXy0asoyoV4lqQmGpTpU6VKQaKZUWLayOXejhjwzFFZoHmNs6JWapZlVzQ == X-Google-Smtp-Source: ABdhPJzE2axp34nWc+SsmBAkpIWEAbz/toBmOe2ZCe8J8IykrHeMhhBdWmPQgWfZdvTi6ftbQN/rzA0UuoJom3Ee+do= 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:f512:: with SMTP id a18mr10163204ybe.159.1607132794984; Fri, 04 Dec 2020 17:46:34 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:01 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-12-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 11/17] run-command: allow stdin for run_processes_parallel From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org While it makes sense not to inherit stdin from the parent process to avoid deadlocking, it's not necessary to completely ban stdin to children. An informed user should be able to configure stdin safely. By setting `some_child.process.no_stdin=1` before calling `get_next_task()` we provide a reasonable default behavior but enable users to set up stdin streaming for themselves during the callback. `some_child.process.stdout_to_stderr`, however, remains unmodifiable by `get_next_task()` - the rest of the run_processes_parallel() API depends on child output in stderr. Signed-off-by: Emily Shaffer --- run-command.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index ea4d0fb4b1..80c8c97bc1 100644 --- a/run-command.c +++ b/run-command.c @@ -1683,6 +1683,9 @@ static int pp_start_one(struct parallel_processes *pp) if (i == pp->max_processes) BUG("bookkeeping is hard"); + /* disallow by default, but allow users to set up stdin if they wish */ + pp->children[i].process.no_stdin = 1; + code = pp->get_next_task(&pp->children[i].process, &pp->children[i].err, pp->data, @@ -1694,7 +1697,6 @@ static int pp_start_one(struct parallel_processes *pp) } pp->children[i].process.err = -1; pp->children[i].process.stdout_to_stderr = 1; - pp->children[i].process.no_stdin = 1; if (start_command(&pp->children[i].process)) { code = pp->start_failure(&pp->children[i].err, From patchwork Sat Dec 5 01:46:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952781 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 E1494C4361A for ; Sat, 5 Dec 2020 01:47:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A880922DFB for ; Sat, 5 Dec 2020 01:47:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731139AbgLEBrt (ORCPT ); Fri, 4 Dec 2020 20:47:49 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57506 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731107AbgLEBrs (ORCPT ); Fri, 4 Dec 2020 20:47:48 -0500 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 AD034C08E861 for ; Fri, 4 Dec 2020 17:46:37 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id v12so9330276ybi.6 for ; Fri, 04 Dec 2020 17:46:37 -0800 (PST) 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=o8KZDdUJzRd1GBk+8PwuUt5xSveC2sAaaXwufSZuVz8=; b=jim91O/WxeTKb63gfKQyDJ9r7RwFlQ7SErgkySU5VbIOTlfHK7vnMwkRLWYHm1XFx+ xRO1NG25rlz8mOYzsE2gypRIOegT5BULrLb1OFddq1f8429I9HvH5NeJlXGkj29fyfMG sJF7YyndIgi3MA72hThtiBj1p3chPxzl0liAuGK1x1ermkZKm17SshepPoWrPObgXmNU bPBAZFeEJN6Cj6aamHnfFArfgtnh8LniZBzrwzGk8KsGLFn6cDlw7ffhq16GpjTA5q9z eDmBjSQugAm5TFjzQgSAlnk52sRiJHMGmg8sme9KSY1VHLd+dbi1QW2EcNzMHI0W7Th3 rpcA== 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=o8KZDdUJzRd1GBk+8PwuUt5xSveC2sAaaXwufSZuVz8=; b=ijiEYPy1KyEdCkoViRDZk0owhfNOutixQU9Rs7P5/q5x1JsIQcU5Bs9gqH+pZoSwoN 16Y7DXXNhlDWPt+Z7Bq/dn216kbR5zSXCUu0bFEksk2L5hDSscQVd5/Q+KIQG1Nygbgv 06OAzSdIiU1JWz3GORdEJ5FVjftI1MXqoAEu1nEPPJ87EmBsRBObEgz0WVBhCuNX/gz7 ipOildGka3pJpdKNn3oRpaHC0kouFBzFIbss20j+Y1cxbRI7zUI67RO3q0inhpZ+BtjM qnfRCpS3jIKU43paVaJELxFL6ZYWBZO8qIZdPwMht1NirpfGVlHuGClvCVykSqYi4CxP 16FQ== X-Gm-Message-State: AOAM533Lqo/wyNmH/uwyrZywXHi146dzn2lT/oNl8uwtI/fn1uhj4Pjj II5nEnR9L5C9Ky1PnYMbkwMn21ziBjBNAZhx8uCeuTbORymJR+V7aJyVxiHt6HbQLQUlVtRvwJY 3bGQny27Bkj3isDnxKyPqd7KJ8YFDjo92XrZcoXnSNX+vb0lxTUSY8NbnG1P2t6FDZbEDy6KK6Q == X-Google-Smtp-Source: ABdhPJxx3y2KO84KYYdtGzDyEtlyamkKrvnmYdEF3ibnuf3PMWdsiWy/ZSJNl9l1PqcogOW3kdCcXGdKH4kYUVdQ8ts= 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:37c6:: with SMTP id e189mr8980790yba.89.1607132796896; Fri, 04 Dec 2020 17:46:36 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:02 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-13-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 12/17] hook: allow parallel hook execution From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In many cases, there's no reason not to allow hooks to execute in parallel. run_processes_parallel() is well-suited - it's a task queue that runs its housekeeping in series, which means users don't need to worry about thread safety on their callback data. True multithreaded execution with the async_* functions isn't necessary here. Synchronous hook execution can be achieved by only allowing 1 job to run at a time. Teach run_hooks() to use that function for simple hooks which don't require stdin or capture of stderr. Signed-off-by: Emily Shaffer --- Notes: Per AEvar's request - parallel hook execution on day zero. In most ways run_processes_parallel() worked great for me - but it didn't have great support for hooks where we pipe to and from. I had to add this support later in the series. Since I modified an existing and in-use library I'd appreciate a keen look on these patches. - Emily Documentation/config/hook.txt | 5 ++ Documentation/git-hook.txt | 15 +++- builtin/hook.c | 6 +- hook.c | 142 ++++++++++++++++++++++++++-------- hook.h | 28 ++++++- 5 files changed, 158 insertions(+), 38 deletions(-) diff --git a/Documentation/config/hook.txt b/Documentation/config/hook.txt index 75312754ae..a423d13781 100644 --- a/Documentation/config/hook.txt +++ b/Documentation/config/hook.txt @@ -12,3 +12,8 @@ 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"). + +hook.jobs:: + Specifies how many hooks can be run simultaneously during parallelized + hook execution. If unspecified, defaults to the number of processors on + the current system. diff --git a/Documentation/git-hook.txt b/Documentation/git-hook.txt index cce30a80d0..c2678c61b2 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -10,7 +10,7 @@ SYNOPSIS [verse] 'git hook' list 'git hook' run [(-e|--env)=...] [(-a|--arg)=...] [--to-stdin=] - + [(-j|--jobs) ] DESCRIPTION ----------- @@ -66,7 +66,8 @@ 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 [(-e|--env)=...] [(-a|--arg)=...] [--to-stdin=] ``:: +run [(-e|--env)=...] [(-a|--arg)=...] [--to-stdin=] + [(-j|--jobs)]``:: 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 @@ -98,6 +99,16 @@ Specify environment variables to set for every hook that is run. Specify a file which will be streamed into stdin for every hook that is run. Each hook will receive the entire file from beginning to EOF. +-j:: +--jobs:: + Only valid for `run`. ++ +Specify how many hooks to run simultaneously. If this flag is not specified, use +the value of the `hook.jobs` config. If the config is not specified, use the +number of CPUs on the current system. Some hooks may be ineligible for +parallelization: for example, 'commit-msg' intends hooks modify the commit +message body and cannot be parallelized. + CONFIGURATION ------------- include::config/hook.txt[] diff --git a/builtin/hook.c b/builtin/hook.c index e45831e01d..064a0fea29 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -10,7 +10,7 @@ static const char * const builtin_hook_usage[] = { N_("git hook list "), N_("git hook run [(-e|--env)=...] [(-a|--arg)=...]" - "[--to-stdin=] "), + "[--to-stdin=] [(-j|--jobs) ] "), NULL }; @@ -90,7 +90,7 @@ static int list(int argc, const char **argv, const char *prefix) static int run(int argc, const char **argv, const char *prefix) { struct strbuf hookname = STRBUF_INIT; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_ASYNC; int rc = 0; struct option run_options[] = { @@ -100,6 +100,8 @@ static int run(int argc, const char **argv, const char *prefix) N_("argument to pass to hook")), OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), N_("file to read into hooks' stdin")), + OPT_INTEGER('j', "jobs", &opt.jobs, + N_("run up to hooks simultaneously")), OPT_END(), }; diff --git a/hook.c b/hook.c index c7fdf556fe..edea54f95c 100644 --- a/hook.c +++ b/hook.c @@ -136,6 +136,14 @@ enum hookdir_opt configured_hookdir_opt(void) return hookdir_unknown; } +int configured_hook_jobs(void) +{ + int n = online_cpus(); + git_config_get_int("hook.jobs", &n); + + return n; +} + static int should_include_hookdir(const char *path, enum hookdir_opt cfg) { struct strbuf prompt = STRBUF_INIT; @@ -223,6 +231,7 @@ void run_hooks_opt_init(struct run_hooks_opt *o) strvec_init(&o->env); strvec_init(&o->args); o->run_hookdir = configured_hookdir_opt(); + o->jobs = configured_hook_jobs(); } int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir) @@ -246,11 +255,96 @@ void run_hooks_opt_clear(struct run_hooks_opt *o) strvec_clear(&o->args); } + +static int pick_next_hook(struct child_process *cp, + struct strbuf *out, + void *pp_cb, + void **pp_task_cb) +{ + struct hook_cb_data *hook_cb = pp_cb; + + struct hook *hook = list_entry(hook_cb->run_me, struct hook, list); + + if (hook_cb->head == hook_cb->run_me) + return 0; + + cp->env = hook_cb->options->env.v; + cp->stdout_to_stderr = 1; + cp->trace2_hook_name = hook->command.buf; + + /* reopen the file for stdin; run_command closes it. */ + if (hook_cb->options->path_to_stdin) { + cp->no_stdin = 0; + cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); + } else { + cp->no_stdin = 1; + } + + /* + * Commands from the config could be oneliners, but we know + * for certain that hookdir commands are not. + */ + if (hook->from_hookdir) + cp->use_shell = 0; + else + cp->use_shell = 1; + + /* add command */ + strvec_push(&cp->args, hook->command.buf); + + /* + * add passed-in argv, without expanding - let the user get back + * exactly what they put in + */ + strvec_pushv(&cp->args, hook_cb->options->args.v); + + /* Provide context for errors if necessary */ + *pp_task_cb = hook; + + /* Get the next entry ready */ + hook_cb->run_me = hook_cb->run_me->next; + + return 1; +} + +static int notify_start_failure(struct strbuf *out, + void *pp_cb, + void *pp_task_cp) +{ + struct hook_cb_data *hook_cb = pp_cb; + struct hook *attempted = pp_task_cp; + + /* |= rc in cb */ + hook_cb->rc |= 1; + + strbuf_addf(out, _("Couldn't start '%s', configured in '%s'\n"), + attempted->command.buf, + attempted->from_hookdir ? "hookdir" + : config_scope_name(attempted->origin)); + + /* NEEDSWORK: if halt_on_error is desired, do it here. */ + return 0; +} + +static int notify_hook_finished(int result, + struct strbuf *out, + void *pp_cb, + void *pp_task_cb) +{ + struct hook_cb_data *hook_cb = pp_cb; + + /* |= rc in cb */ + hook_cb->rc |= result; + + /* NEEDSWORK: if halt_on_error is desired, do it here. */ + return 0; +} + int run_hooks(const char *hookname, struct run_hooks_opt *options) { struct strbuf hookname_str = STRBUF_INIT; struct list_head *to_run, *pos = NULL, *tmp = NULL; - int rc = 0; + struct hook_cb_data cb_data = { 0, NULL, NULL, options }; if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); @@ -260,41 +354,23 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) 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); - /* reopen the file for stdin; run_command closes it. */ - if (options->path_to_stdin) - hook_proc.in = xopen(options->path_to_stdin, O_RDONLY); - else - hook_proc.no_stdin = 1; - - hook_proc.env = options->env.v; - 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, options->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); + if (hook->from_hookdir && + !should_include_hookdir(hook->command.buf, options->run_hookdir)) + list_del(pos); + } - /* - * add passed-in argv, without expanding - let the user get back - * exactly what they put in - */ - strvec_pushv(&hook_proc.args, options->args.v); + cb_data.head = to_run; + cb_data.run_me = to_run->next; - rc |= run_command(&hook_proc); - } + run_processes_parallel_tr2(options->jobs, + pick_next_hook, + notify_start_failure, + notify_hook_finished, + &cb_data, + "hook", + hookname); - return rc; + return cb_data.rc; } diff --git a/hook.h b/hook.h index 5184dcaa5a..f54568afe3 100644 --- a/hook.h +++ b/hook.h @@ -37,6 +37,9 @@ enum hookdir_opt */ enum hookdir_opt configured_hookdir_opt(void); +/* Provides the number of threads to use for parallel hook execution. */ +int configured_hook_jobs(void); + struct run_hooks_opt { /* Environment vars to be set for each hook */ @@ -54,15 +57,38 @@ struct run_hooks_opt /* Path to file which should be piped to stdin for each hook */ const char *path_to_stdin; + + /* Number of threads to parallelize across */ + int jobs; }; -#define RUN_HOOKS_OPT_INIT { \ +/* + * Callback provided to feed_pipe_fn and consume_sideband_fn. + */ +struct hook_cb_data { + int rc; + struct list_head *head; + struct list_head *run_me; + struct run_hooks_opt *options; +}; + +#define RUN_HOOKS_OPT_INIT_SYNC { \ .env = STRVEC_INIT, \ .args = STRVEC_INIT, \ .path_to_stdin = NULL, \ + .jobs = 1, \ .run_hookdir = configured_hookdir_opt() \ } +#define RUN_HOOKS_OPT_INIT_ASYNC { \ + .env = STRVEC_INIT, \ + .args = STRVEC_INIT, \ + .path_to_stdin = NULL, \ + .jobs = configured_hook_jobs(), \ + .run_hookdir = configured_hookdir_opt() \ +} + + void run_hooks_opt_init(struct run_hooks_opt *o); void run_hooks_opt_clear(struct run_hooks_opt *o); From patchwork Sat Dec 5 01:46:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952785 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D17C4C4167B for ; Sat, 5 Dec 2020 01:47:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AC9A422DBF for ; Sat, 5 Dec 2020 01:47:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731144AbgLEBru (ORCPT ); Fri, 4 Dec 2020 20:47:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731126AbgLEBrt (ORCPT ); Fri, 4 Dec 2020 20:47:49 -0500 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 B2075C08E862 for ; Fri, 4 Dec 2020 17:46:39 -0800 (PST) Received: by mail-qt1-x849.google.com with SMTP id f11so6253634qth.23 for ; Fri, 04 Dec 2020 17:46:39 -0800 (PST) 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=b0JOpPGS4lpoiIbzKa9QKHaCbedLFF1bN+1kpGq8ChY=; b=pAsJajn7szGg7afDKH4kUEkcN4AlbyobgOH/w4P5fhBCPEvnvEG16UQ4l12bItYT1U OPd1Ukx/y2A6PzDYhtz0gJqICt9yDiNcb17hH98Yrf8Y2UpAA7avz/vHecWUirwhQ/Te o7ECjnCfxVd3au++nga4U/NgZN6ghB5k5Z5aiFrsJE2i0DTfJBGnxtwFqvVLAk/E6PS5 RcAfoA1JkwICmR4Og830DOK8AheTQEjp+2XH65052PkU/a5vKa2XtzKe9rAUAJH2HKDy DkkhXsN/uVYUR8pigpe0/ahG5lM6DI8AKG8TYWSh/0fG3hWgk7U7xhIfGgcBl5lRQWK/ zM6A== 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=b0JOpPGS4lpoiIbzKa9QKHaCbedLFF1bN+1kpGq8ChY=; b=YQHZB2v0S0NnzxvRGzNr9QRITxheffUyyEayaekkYOu1IvfHLg38kj/fcy+OFk7m9F RvhvA6I5/230qJ2wNx3oMcvqSlTeIgg2MdReEmaIuB1yoZa6PgzBrIaGvXConLPhA5B/ bKSgn2JglM8rurJkzYI7vXyoZ7JC0zETF8ml11ly5Ow3qrI2JG1N3VOowXnaWUBPKYQh X/gmUqNlt1tzKHXOvqIaXOA3TbaWcgr58iRlWA7D4v2xRi56XMBJS6s02o4vujzhOAze Ui9q92A30V2KCP5WMvriSyKCjBy70n6fWfm/jj6yY4lUSr0TZw6hpRhJjkk3DDUy5+pl 3W3w== X-Gm-Message-State: AOAM530/qihooIPwlvvWGL0wc7Y3pFPmOmVccW3LIv+6acy7JJ1g8qzI nrI9XANZUsUQtfhoGeNIqFi0Bx/IyEPm7/cfFxp09t9vk2Rm+PKmmSbgIzDV0Ijc2foFEC8rAtP zpEstBZFDWdSuvPSTmdNNyH891aTAFN+88rcBwqQd2Z9GcRDTE7cBFPEyIw80ui0fUzxpnPB+nA == X-Google-Smtp-Source: ABdhPJzbeSZ2VqYJRoA4HeucIFDYoYaNoRzLgjUMnmdGMpfZK/qd1h/5Nz3KMT9QtgaVVe0FrEbxc6nazfs27gcWDq4= 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:4e30:: with SMTP id dm16mr9062164qvb.47.1607132798813; Fri, 04 Dec 2020 17:46:38 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:03 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-14-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 13/17] hook: allow specifying working directory for hooks From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Hooks like "post-checkout" require that hooks have a different working directory than the initial process. Pipe that directly through to struct child_process. Because we can just run 'git -C hook run ...' it shouldn't be necessary to pipe this option through the frontend. In fact, this reduces the possibility of users running hooks which affect some part of the filesystem outside of the repo in question. Signed-off-by: Emily Shaffer --- Notes: Needed later for "post-checkout" conversion. hook.c | 1 + hook.h | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/hook.c b/hook.c index edea54f95c..f0c052d847 100644 --- a/hook.c +++ b/hook.c @@ -271,6 +271,7 @@ static int pick_next_hook(struct child_process *cp, cp->env = hook_cb->options->env.v; cp->stdout_to_stderr = 1; cp->trace2_hook_name = hook->command.buf; + cp->dir = hook_cb->options->dir; /* reopen the file for stdin; run_command closes it. */ if (hook_cb->options->path_to_stdin) { diff --git a/hook.h b/hook.h index f54568afe3..4aae8e2dbb 100644 --- a/hook.h +++ b/hook.h @@ -60,6 +60,9 @@ struct run_hooks_opt /* Number of threads to parallelize across */ int jobs; + + /* Path to initial working directory for subprocess */ + const char *dir; }; /* @@ -77,6 +80,7 @@ struct hook_cb_data { .args = STRVEC_INIT, \ .path_to_stdin = NULL, \ .jobs = 1, \ + .dir = NULL, \ .run_hookdir = configured_hookdir_opt() \ } @@ -85,6 +89,7 @@ struct hook_cb_data { .args = STRVEC_INIT, \ .path_to_stdin = NULL, \ .jobs = configured_hook_jobs(), \ + .dir = NULL, \ .run_hookdir = configured_hookdir_opt() \ } From patchwork Sat Dec 5 01:46:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952783 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 7B723C433FE for ; Sat, 5 Dec 2020 01:47:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 512AC22DFB for ; Sat, 5 Dec 2020 01:47:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731142AbgLEBru (ORCPT ); Fri, 4 Dec 2020 20:47:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57512 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731130AbgLEBrt (ORCPT ); Fri, 4 Dec 2020 20:47:49 -0500 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 9ADF0C08E863 for ; Fri, 4 Dec 2020 17:46:41 -0800 (PST) Received: by mail-qk1-x74a.google.com with SMTP id x196so7058233qkb.12 for ; Fri, 04 Dec 2020 17:46:41 -0800 (PST) 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=oD2rhCpADMStyPd20XMg0pSOsFarv5F7zUMU8LdXwn0=; b=koRT99b9+3RPqtyIZizgGBPVGl1G91zFtGCpg2ZR/uo7CvMWve25JuLREIEUTZEnZD 8DR0lbF9tAGOgrt7txgR16JfRjKkGnxBIB01r9uGlwSVaXxktQDuQYfUlPPxXk7XDLUs Uf/sHZNu11ZuObBAVQmLIvAkn9aI5n6mi0ARWzG42plr7A4CsNoOXjuii7kgYX+qyZkR GsGw6pnf8UoaxZ+8710fFXicHhfkAzGVjMkB+/JN9n/sgggdayw/5FQ1gjV12kf4D1WM t0gP7DI8E5THIHwtBkD175IuA1xtslPvdAaFoyeHF0Wqr5R0hC8k+mbUBuf3iZmBGeXY gXQw== 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=oD2rhCpADMStyPd20XMg0pSOsFarv5F7zUMU8LdXwn0=; b=NyRPRfAQN4nELdle2ClBJ6s05q1oXbMAtPha5a9VFS3V/zyd/pVeWQ2rzdCxgj6pfk FMS74gE+SlcFPRceIxnzefUBZa3dtXCO8hmlXS1C7WluaX3LPNrCFwrRP3l40HNsIJ7h eDe3TxfPaVeH4O4c9pfIDlyNGzAelEhndrUX+Czl9MAsWLNWy+HBAeCHccvL0GrOJsyB 957iVv58aIoADydJIea46NgjZ4CZeyThry/jqJK0LW6rX/1u7fCJ4x5llwcAcYdWGmoB MlgQ5jwOq6ExfLbeOmep4MjPtJ5W2jS6l1qoXkl2uMfJrJ+uyuBJfUh43B3mBl1tE2m3 5YEQ== X-Gm-Message-State: AOAM530SweOj6WyG+6LjPBjaynrdMO1eE7RuJiEE2AJp8qSgmF9g+rFx joAHrwadb0KmORAJGw+dV8Fk/TAlkFMrhH8i4i5fAZE24+/rQSftFo+RToHf92ibQXbzGMRMjyn we9izTAS0jT82KNQvmTgUlQH6vuhOxCxDqblfNMLd7zyzIMOBycRyira62u+sjEIfaaWebftKxA == X-Google-Smtp-Source: ABdhPJwNkkG83i8/n5hFwlYui840TCvrUEKXh6jsLaUu2se/Ek0DM4rIW4A9bGwocf8r0x09pNR08hLYsZUZgY9cSHU= 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:b799:: with SMTP id l25mr8952246qve.25.1607132800629; Fri, 04 Dec 2020 17:46:40 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:04 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-15-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 14/17] run-command: add stdin callback for parallelization 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 of the run_processes_parallel() API wants to pipe a large amount of information to stdin of each parallel command, that information could exceed the buffer of the pipe allocated for that process's stdin. Generally this is solved by repeatedly writing to child_process.in between calls to start_command() and finish_command(); run_processes_parallel() did not provide users an opportunity to access child_process at that time. Because the data might be extremely large (for example, a list of all refs received during a push from a client) simply taking a string_list or strbuf is not as scalable as using a callback; the rest of the run_processes_parallel() API also uses callbacks, so making this feature match the rest of the API reduces mental load on the user. Signed-off-by: Emily Shaffer --- Notes: Since run_processes_parallel() is used elsewhere, I'd appreciate a close look on this patch which modifies it. Thanks :) builtin/fetch.c | 1 + builtin/submodule--helper.c | 2 +- run-command.c | 54 +++++++++++++++++++++++++++++++++++-- run-command.h | 17 +++++++++++- submodule.c | 1 + t/helper/test-run-command.c | 31 ++++++++++++++++++--- t/t0061-run-command.sh | 30 +++++++++++++++++++++ 7 files changed, 128 insertions(+), 8 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index ecf8537605..5e153b5193 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1647,6 +1647,7 @@ static int fetch_multiple(struct string_list *list, int max_children) result = run_processes_parallel_tr2(max_children, &fetch_next_remote, &fetch_failed_to_start, + NULL, &fetch_finished, &state, "fetch", "parallel/fetch"); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index c30896c897..bb623c1852 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2294,7 +2294,7 @@ static int update_submodules(struct submodule_update_clone *suc) int i; run_processes_parallel_tr2(suc->max_jobs, update_clone_get_next_task, - update_clone_start_failure, + update_clone_start_failure, NULL, update_clone_task_finished, suc, "submodule", "parallel/update"); diff --git a/run-command.c b/run-command.c index 80c8c97bc1..7b65c087f8 100644 --- a/run-command.c +++ b/run-command.c @@ -1548,6 +1548,7 @@ struct parallel_processes { get_next_task_fn get_next_task; start_failure_fn start_failure; + feed_pipe_fn feed_pipe; task_finished_fn task_finished; struct { @@ -1575,6 +1576,13 @@ static int default_start_failure(struct strbuf *out, return 0; } +static int default_feed_pipe(struct strbuf *pipe, + void *pp_cb, + void *pp_task_cb) +{ + return 1; +} + static int default_task_finished(int result, struct strbuf *out, void *pp_cb, @@ -1605,6 +1613,7 @@ static void pp_init(struct parallel_processes *pp, int n, get_next_task_fn get_next_task, start_failure_fn start_failure, + feed_pipe_fn feed_pipe, task_finished_fn task_finished, void *data) { @@ -1623,6 +1632,7 @@ static void pp_init(struct parallel_processes *pp, pp->get_next_task = get_next_task; pp->start_failure = start_failure ? start_failure : default_start_failure; + pp->feed_pipe = feed_pipe ? feed_pipe : default_feed_pipe; pp->task_finished = task_finished ? task_finished : default_task_finished; pp->nr_processes = 0; @@ -1715,6 +1725,37 @@ static int pp_start_one(struct parallel_processes *pp) return 0; } +static void pp_buffer_stdin(struct parallel_processes *pp) +{ + int i; + struct strbuf sb = STRBUF_INIT; + + /* Buffer stdin for each pipe. */ + for (i = 0; i < pp->max_processes; i++) { + if (pp->children[i].state == GIT_CP_WORKING && + pp->children[i].process.in > 0) { + int done; + strbuf_reset(&sb); + done = pp->feed_pipe(&sb, pp->data, + pp->children[i].data); + if (sb.len) { + if (write_in_full(pp->children[i].process.in, + sb.buf, sb.len) < 0) { + if (errno != EPIPE) + die_errno("write"); + done = 1; + } + } + if (done) { + close(pp->children[i].process.in); + pp->children[i].process.in = 0; + } + } + } + + strbuf_release(&sb); +} + static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout) { int i; @@ -1779,6 +1820,7 @@ static int pp_collect_finished(struct parallel_processes *pp) pp->nr_processes--; pp->children[i].state = GIT_CP_FREE; pp->pfd[i].fd = -1; + pp->children[i].process.in = 0; child_process_init(&pp->children[i].process); if (i != pp->output_owner) { @@ -1812,6 +1854,7 @@ static int pp_collect_finished(struct parallel_processes *pp) int run_processes_parallel(int n, get_next_task_fn get_next_task, start_failure_fn start_failure, + feed_pipe_fn feed_pipe, task_finished_fn task_finished, void *pp_cb) { @@ -1820,7 +1863,9 @@ int run_processes_parallel(int n, int spawn_cap = 4; struct parallel_processes pp; - pp_init(&pp, n, get_next_task, start_failure, task_finished, pp_cb); + sigchain_push(SIGPIPE, SIG_IGN); + + pp_init(&pp, n, get_next_task, start_failure, feed_pipe, task_finished, pp_cb); while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && @@ -1837,6 +1882,7 @@ int run_processes_parallel(int n, } if (!pp.nr_processes) break; + pp_buffer_stdin(&pp); pp_buffer_stderr(&pp, output_timeout); pp_output(&pp); code = pp_collect_finished(&pp); @@ -1848,11 +1894,15 @@ int run_processes_parallel(int n, } pp_cleanup(&pp); + + sigchain_pop(SIGPIPE); + return 0; } int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task, start_failure_fn start_failure, + feed_pipe_fn feed_pipe, task_finished_fn task_finished, void *pp_cb, const char *tr2_category, const char *tr2_label) { @@ -1862,7 +1912,7 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task, ((n < 1) ? online_cpus() : n)); result = run_processes_parallel(n, get_next_task, start_failure, - task_finished, pp_cb); + feed_pipe, task_finished, pp_cb); trace2_region_leave(tr2_category, tr2_label, NULL); diff --git a/run-command.h b/run-command.h index 6472b38bde..e058c0e2c8 100644 --- a/run-command.h +++ b/run-command.h @@ -436,6 +436,20 @@ typedef int (*start_failure_fn)(struct strbuf *out, void *pp_cb, void *pp_task_cb); +/** + * This callback is called repeatedly on every child process who requests + * start_command() to create a pipe by setting child_process.in < 0. + * + * pp_cb is the callback cookie as passed into run_processes_parallel, and + * pp_task_cb is the callback cookie as passed into get_next_task_fn. + * The contents of 'send' will be read into the pipe and passed to the pipe. + * + * Return nonzero to close the pipe. + */ +typedef int (*feed_pipe_fn)(struct strbuf *pipe, + void *pp_cb, + void *pp_task_cb); + /** * This callback is called on every child process that finished processing. * @@ -470,10 +484,11 @@ typedef int (*task_finished_fn)(int result, int run_processes_parallel(int n, get_next_task_fn, start_failure_fn, + feed_pipe_fn, task_finished_fn, void *pp_cb); int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, - task_finished_fn, void *pp_cb, + feed_pipe_fn, task_finished_fn, void *pp_cb, const char *tr2_category, const char *tr2_label); #endif diff --git a/submodule.c b/submodule.c index b3bb59f066..953f41818c 100644 --- a/submodule.c +++ b/submodule.c @@ -1638,6 +1638,7 @@ int fetch_populated_submodules(struct repository *r, run_processes_parallel_tr2(max_parallel_jobs, get_next_submodule, fetch_start_failure, + NULL, fetch_finish, &spf, "submodule", "parallel/fetch"); diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 7ae03dc712..9348184d30 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -32,8 +32,13 @@ static int parallel_next(struct child_process *cp, return 0; strvec_pushv(&cp->args, d->argv); + cp->in = d->in; + cp->no_stdin = d->no_stdin; strbuf_addstr(err, "preloaded output of a child\n"); number_callbacks++; + + *task_cb = xmalloc(sizeof(int)); + *(int*)(*task_cb) = 2; return 1; } @@ -55,6 +60,17 @@ static int task_finished(int result, return 1; } +static int test_stdin(struct strbuf *pipe, void *cb, void *task_cb) +{ + int *lines_remaining = task_cb; + + if (*lines_remaining) + strbuf_addf(pipe, "sample stdin %d\n", --(*lines_remaining)); + + return !(*lines_remaining); +} + + struct testsuite { struct string_list tests, failed; int next; @@ -185,7 +201,7 @@ static int testsuite(int argc, const char **argv) suite.tests.nr, max_jobs); ret = run_processes_parallel(max_jobs, next_test, test_failed, - test_finished, &suite); + test_stdin, test_finished, &suite); if (suite.failed.nr > 0) { ret = 1; @@ -413,15 +429,22 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command-parallel")) exit(run_processes_parallel(jobs, parallel_next, - NULL, NULL, &proc)); + NULL, NULL, NULL, &proc)); if (!strcmp(argv[1], "run-command-abort")) exit(run_processes_parallel(jobs, parallel_next, - NULL, task_finished, &proc)); + NULL, NULL, task_finished, &proc)); if (!strcmp(argv[1], "run-command-no-jobs")) exit(run_processes_parallel(jobs, no_job, - NULL, task_finished, &proc)); + NULL, NULL, task_finished, &proc)); + + if (!strcmp(argv[1], "run-command-stdin")) { + proc.in = -1; + proc.no_stdin = 0; + exit (run_processes_parallel(jobs, parallel_next, NULL, + test_stdin, NULL, &proc)); + } fprintf(stderr, "check usage\n"); return 1; diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 7d599675e3..3eb572e6cd 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -143,6 +143,36 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai test_cmp expect actual ' +cat >expect <<-EOF +preloaded output of a child +listening for stdin: +sample stdin 1 +sample stdin 0 +preloaded output of a child +listening for stdin: +sample stdin 1 +sample stdin 0 +preloaded output of a child +listening for stdin: +sample stdin 1 +sample stdin 0 +preloaded output of a child +listening for stdin: +sample stdin 1 +sample stdin 0 +EOF + +test_expect_success 'run_command listens to stdin' ' + write_script stdin-script <<-\EOF && + echo "listening for stdin:" + while read line; do + echo "$line" + done actual && + test_cmp expect actual +' + cat >expect <<-EOF preloaded output of a child asking for a quick stop From patchwork Sat Dec 5 01:46:05 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952789 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 76BA2C4361A for ; Sat, 5 Dec 2020 01:48:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5217122DBF for ; Sat, 5 Dec 2020 01:48:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731168AbgLEBr7 (ORCPT ); Fri, 4 Dec 2020 20:47:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57468 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731126AbgLEBr7 (ORCPT ); Fri, 4 Dec 2020 20:47:59 -0500 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 3F986C08E864 for ; Fri, 4 Dec 2020 17:46:43 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id s30so2553917ybe.15 for ; Fri, 04 Dec 2020 17:46:43 -0800 (PST) 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=/k10iA52eh9k+kpE7X1UGt/QRk/HBhUx/d0hlLJ8Hxo=; b=LtMbcRgw32l6I7qtiHbEfj+smfODYLb/iqf5wd98yRlNyQ+ISqP1Z4AIPZNgkgOdn2 9SIb+mxSGWgnMQ98AAEUZ6t9VzE12RPzzvDesOB2Kiho/Vwo6biXkXzdV/oqS9Y7c+yj bT5iPRg3lxCCbC95G1+XSliohTrZ0c8qadGG4J91RuSJO+LkJtfozjeuyMyVXoOlSxlj 4N5MVuTCisb+OZNZQ8xP+HH7GlXQJBwIjxXgdA/x6DOhfpL98pMlh93rINr8s2FQ+2sp fiQXAqDQTIhGtGYKPyyZgW3KF/BO804dZSbWIHrTxAPfcALHX/4GpqTXNxTOfaAFc30r AJLQ== 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=/k10iA52eh9k+kpE7X1UGt/QRk/HBhUx/d0hlLJ8Hxo=; b=QcQyDScky3xWATwP9y/ndTjRSU1e/VicEFqFKMnuf+3GRi8HIz9P/YbzO17dUBTjFy D3GI7Gml6711lKYa+tmz1OFf72kV7ZALWKMGP1mjREUeZYx+2+teK7InmUeBSDEBGnSY j8wbUe+SK7pbTTHoip+SWgiKV/NomjvY+lxE4sIlRv5T2TMI0db3I0VfJwm7EKiwIgvJ bs9Gw5oglnIvkfd7LCiRoq8iyvPSbWrbqDFeIuZfspTnMoYyigQRV7X1mZF+lZpom7EC QJPrsznoHpApArpFPHTPk+xs5J26li1f4ng6Fh6LSN/QRsv1b1vtxFIEyHerJnD5iJR7 SsGQ== X-Gm-Message-State: AOAM532KF9rPCX8xf/LbY/s2coPYLdlf0ZLH0LF97/TZWTwC9y93Dukg vp9j2lGvEln33a3bG9jZKhhyC1yEczp5pzQYrq3wuTP/hayGx9LktNyTb3/50EpP3fMeTYRKShw D+aGMUWNuOBfdfXVz2ihleaMeReHQhUPrK37NcGMCxA2H4MAsffrlzafEQjpr26jVR5URZrUkQQ == X-Google-Smtp-Source: ABdhPJwkDSJrqXXI5RSAT/Y5Rd8ozrR9en1qktz98lh2ffrqt6Rb3ZDvOZkDXEAMaJDJjJZCZBXcf2FqszPwEknNy6c= 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:5b89:: with SMTP id p131mr9381798ybb.310.1607132802491; Fri, 04 Dec 2020 17:46:42 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:05 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-16-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 15/17] hook: provide stdin by string_list or callback From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In cases where a hook requires only a small amount of information via stdin, it should be simple for users to provide a string_list alone. But in more complicated cases where the stdin is too large to hold in memory, let's provide a callback the users can populate line after line with instead. Signed-off-by: Emily Shaffer --- hook.c | 39 +++++++++++++++++++++++++++++++++++++++ hook.h | 25 +++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/hook.c b/hook.c index f0c052d847..fbb49f241d 100644 --- a/hook.c +++ b/hook.c @@ -9,6 +9,8 @@ void free_hook(struct hook *ptr) { if (ptr) { strbuf_release(&ptr->command); + if (ptr->feed_pipe_cb_data) + free(ptr->feed_pipe_cb_data); free(ptr); } } @@ -38,6 +40,7 @@ static void append_or_move_hook(struct list_head *head, const char *command) strbuf_init(&to_add->command, 0); strbuf_addstr(&to_add->command, command); to_add->from_hookdir = 0; + to_add->feed_pipe_cb_data = NULL; } /* re-set the scope so we show where an override was specified */ @@ -253,9 +256,32 @@ void run_hooks_opt_clear(struct run_hooks_opt *o) { strvec_clear(&o->env); strvec_clear(&o->args); + string_list_clear(&o->str_stdin, 0); } +static int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb) +{ + int *item_idx; + struct hook *ctx = pp_task_cb; + struct string_list *to_pipe = &((struct hook_cb_data*)pp_cb)->options->str_stdin; + + /* Bootstrap the state manager if necessary. */ + if (!ctx->feed_pipe_cb_data) { + ctx->feed_pipe_cb_data = xmalloc(sizeof(unsigned int)); + *(int*)ctx->feed_pipe_cb_data = 0; + } + + item_idx = ctx->feed_pipe_cb_data; + + if (*item_idx < to_pipe->nr) { + strbuf_addf(pipe, "%s\n", to_pipe->items[*item_idx].string); + (*item_idx)++; + return 0; + } + return 1; +} + static int pick_next_hook(struct child_process *cp, struct strbuf *out, void *pp_cb, @@ -277,6 +303,10 @@ static int pick_next_hook(struct child_process *cp, if (hook_cb->options->path_to_stdin) { cp->no_stdin = 0; cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY); + } else if (hook_cb->options->feed_pipe) { + /* ask for start_command() to make a pipe for us */ + cp->in = -1; + cp->no_stdin = 0; } else { cp->no_stdin = 1; } @@ -350,6 +380,14 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) if (!options) BUG("a struct run_hooks_opt must be provided to run_hooks"); + if ((options->path_to_stdin && options->str_stdin.nr) || + (options->path_to_stdin && options->feed_pipe) || + (options->str_stdin.nr && options->feed_pipe)) + BUG("choose only one method to populate stdin"); + + if (options->str_stdin.nr) + options->feed_pipe = &pipe_from_string_list; + strbuf_addstr(&hookname_str, hookname); to_run = hook_list(&hookname_str); @@ -368,6 +406,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) run_processes_parallel_tr2(options->jobs, pick_next_hook, notify_start_failure, + options->feed_pipe, notify_hook_finished, &cb_data, "hook", diff --git a/hook.h b/hook.h index 4aae8e2dbb..ace26c637e 100644 --- a/hook.h +++ b/hook.h @@ -2,6 +2,7 @@ #include "list.h" #include "strbuf.h" #include "strvec.h" +#include "run-command.h" struct hook { @@ -14,6 +15,12 @@ struct hook /* The literal command to run. */ struct strbuf command; int from_hookdir; + + /* + * Use this to keep state for your feed_pipe_fn if you are using + * run_hooks_opt.feed_pipe. Otherwise, do not touch it. + */ + void *feed_pipe_cb_data; }; /* @@ -57,12 +64,24 @@ struct run_hooks_opt /* Path to file which should be piped to stdin for each hook */ const char *path_to_stdin; + /* Pipe each string to stdin, separated by newlines */ + struct string_list str_stdin; + /* + * Callback and state pointer to ask for more content to pipe to stdin. + * Will be called repeatedly, for each hook. See + * hook.c:pipe_from_stdin() for an example. Keep per-hook state in + * hook.feed_pipe_cb_data (per process). Keep initialization context in + * feed_pipe_ctx (shared by all processes). + */ + feed_pipe_fn feed_pipe; + void *feed_pipe_ctx; /* Number of threads to parallelize across */ int jobs; /* Path to initial working directory for subprocess */ const char *dir; + }; /* @@ -81,6 +100,9 @@ struct hook_cb_data { .path_to_stdin = NULL, \ .jobs = 1, \ .dir = NULL, \ + .str_stdin = STRING_LIST_INIT_DUP, \ + .feed_pipe = NULL, \ + .feed_pipe_ctx = NULL, \ .run_hookdir = configured_hookdir_opt() \ } @@ -90,6 +112,9 @@ struct hook_cb_data { .path_to_stdin = NULL, \ .jobs = configured_hook_jobs(), \ .dir = NULL, \ + .str_stdin = STRING_LIST_INIT_DUP, \ + .feed_pipe = NULL, \ + .feed_pipe_ctx = NULL, \ .run_hookdir = configured_hookdir_opt() \ } From patchwork Sat Dec 5 01:46:06 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952791 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, 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 9599AC4361A for ; Sat, 5 Dec 2020 01:48:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5C89022DBF for ; Sat, 5 Dec 2020 01:48:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729222AbgLEBsY (ORCPT ); Fri, 4 Dec 2020 20:48:24 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57588 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727225AbgLEBsY (ORCPT ); Fri, 4 Dec 2020 20:48:24 -0500 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 476A2C08E9AA for ; Fri, 4 Dec 2020 17:46:45 -0800 (PST) Received: by mail-yb1-xb49.google.com with SMTP id k196so9284747ybf.9 for ; Fri, 04 Dec 2020 17:46:45 -0800 (PST) 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=a4ISl+AlC5HxlW/Cd9WsvKawkbmJdEAy9Xf+PQ6kfHQ=; b=hW2Xa8mENl5Dmx2gkd4Z4A2gdoNU6aBhF/GtlcR157STwSnERGEiBMlJweZ2WymtnK BQ9v6BiXSZrlMRyir64WGr/oYGl7LIni1N73a5a6hgq4xV4bdpJSvbVEKEviD23Q3pCc XbY1jJt832OxYGJrn7myGjJ0acM1LvbyCbEJ+MDLeicfxYUhK4CzYv6e6RkNQEyNq1q5 6oSRDypGcfG4GECrfskFVmIKXfoS4COOs5llVa/JoSfx72wi3ZdTpw8L2SiL257VcEJx G1Kxam0TXG7NE9vXLcBJzc3R6wrAm+HQT7AaW6qeEDG38j8lYuZKZN6j/mMWdmTqTivF M/Ug== 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=a4ISl+AlC5HxlW/Cd9WsvKawkbmJdEAy9Xf+PQ6kfHQ=; b=Q3cD+fZfS5xOdzBv8vQ5YwPUlPQ4zahP7JZwF6a0Xtr2fF46S+Iwe/UMBNG0UhKqbd 4jnpJb3BV4LCXQGQ7uTrzQ1qHUKfKoRYgIyH1XeH7SetdeHLgXnN3V90bW3SmzUkdNbC N6NUvlAGfc3igBBfwm/iuKGlpYqB7Q1Z2Zy5CwwDTQoo7WXMHMU/UOPm9uRNkukrHKdQ 4T+df3TYA/sZwHylCu6xpdw2QwyDF+evrhkbNnH89ZUQQ+HrOSfQUOkBIdIsRlmHhtvp uBXlPgmOCJsy0wDrAOd4b0Rvj0l7ZR+mj+d20LtK7bQZbSoCMl9rUY6kIPgcl6YPk1EZ ti/w== X-Gm-Message-State: AOAM533IdatqYB0NYpZofmHm8urQlI2J4uGhLDJXUYDs4rOhZNEzuZkK 6eIWWZAOwhOOMOk4RNaEkuBCVINAqZ2IfsfP6pZF0FvW3KaEhiSB6hJ2Bo66za/KBjyZfRZ0niC VfCXNr7bRHPr2GB0DJkHBghpll4hPZ/wzm2Ou5OvTjjk0c+DuacFi8CJKwe+5fBJIYt6hxtYnFw == X-Google-Smtp-Source: ABdhPJysgkWki2bC43R9U+uw8t0KIOJlY0Hlz/Lvq/JPA9rHYgRuYwEgCgAlQFJJmnjxsQBLDC7ovr/CGjE/Sa1SoeI= 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:ce50:: with SMTP id x77mr10624032ybe.38.1607132804518; Fri, 04 Dec 2020 17:46:44 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:06 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-17-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 16/17] run-command: allow capturing of collated output From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Some callers, for example server-side hooks which wish to relay hook output to clients across a transport, want to capture what would normally print to stderr and do something else with it. Allow that via a callback. By calling the callback regardless of whether there's output available, we allow clients to send e.g. a keepalive if necessary. Because we expose a strbuf, not a fd or FILE*, there's no need to create a temporary pipe or similar - we can just skip the print to stderr and instead hand it to the caller. Signed-off-by: Emily Shaffer --- Notes: Originally when writing this patch I attempted to use a pipe in memory - but managing its lifetime was actually pretty tricky, and I found I could achieve the same thing with less code by doing it this way. Critique welcome, including "no, you really need to do it with a pipe". builtin/fetch.c | 2 +- builtin/submodule--helper.c | 2 +- hook.c | 1 + run-command.c | 33 +++++++++++++++++++++++++-------- run-command.h | 18 +++++++++++++++++- submodule.c | 2 +- t/helper/test-run-command.c | 25 ++++++++++++++++++++----- t/t0061-run-command.sh | 7 +++++++ 8 files changed, 73 insertions(+), 17 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 5e153b5193..6a634085d9 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1647,7 +1647,7 @@ static int fetch_multiple(struct string_list *list, int max_children) result = run_processes_parallel_tr2(max_children, &fetch_next_remote, &fetch_failed_to_start, - NULL, + NULL, NULL, &fetch_finished, &state, "fetch", "parallel/fetch"); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index bb623c1852..8c543d33fd 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2294,7 +2294,7 @@ static int update_submodules(struct submodule_update_clone *suc) int i; run_processes_parallel_tr2(suc->max_jobs, update_clone_get_next_task, - update_clone_start_failure, NULL, + update_clone_start_failure, NULL, NULL, update_clone_task_finished, suc, "submodule", "parallel/update"); diff --git a/hook.c b/hook.c index fbb49f241d..1186ee41b3 100644 --- a/hook.c +++ b/hook.c @@ -407,6 +407,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) pick_next_hook, notify_start_failure, options->feed_pipe, + NULL, notify_hook_finished, &cb_data, "hook", diff --git a/run-command.c b/run-command.c index 7b65c087f8..0dce6bec83 100644 --- a/run-command.c +++ b/run-command.c @@ -1549,6 +1549,7 @@ struct parallel_processes { get_next_task_fn get_next_task; start_failure_fn start_failure; feed_pipe_fn feed_pipe; + consume_sideband_fn consume_sideband; task_finished_fn task_finished; struct { @@ -1614,6 +1615,7 @@ static void pp_init(struct parallel_processes *pp, get_next_task_fn get_next_task, start_failure_fn start_failure, feed_pipe_fn feed_pipe, + consume_sideband_fn consume_sideband, task_finished_fn task_finished, void *data) { @@ -1634,6 +1636,7 @@ static void pp_init(struct parallel_processes *pp, pp->start_failure = start_failure ? start_failure : default_start_failure; pp->feed_pipe = feed_pipe ? feed_pipe : default_feed_pipe; pp->task_finished = task_finished ? task_finished : default_task_finished; + pp->consume_sideband = consume_sideband; pp->nr_processes = 0; pp->output_owner = 0; @@ -1670,7 +1673,10 @@ static void pp_cleanup(struct parallel_processes *pp) * When get_next_task added messages to the buffer in its last * iteration, the buffered output is non empty. */ - strbuf_write(&pp->buffered_output, stderr); + if (pp->consume_sideband) + pp->consume_sideband(&pp->buffered_output, pp->data); + else + strbuf_write(&pp->buffered_output, stderr); strbuf_release(&pp->buffered_output); sigchain_pop_common(); @@ -1786,9 +1792,13 @@ static void pp_buffer_stderr(struct parallel_processes *pp, int output_timeout) static void pp_output(struct parallel_processes *pp) { int i = pp->output_owner; + if (pp->children[i].state == GIT_CP_WORKING && pp->children[i].err.len) { - strbuf_write(&pp->children[i].err, stderr); + if (pp->consume_sideband) + pp->consume_sideband(&pp->children[i].err, pp->data); + else + strbuf_write(&pp->children[i].err, stderr); strbuf_reset(&pp->children[i].err); } } @@ -1827,11 +1837,15 @@ static int pp_collect_finished(struct parallel_processes *pp) strbuf_addbuf(&pp->buffered_output, &pp->children[i].err); strbuf_reset(&pp->children[i].err); } else { - strbuf_write(&pp->children[i].err, stderr); + /* Output errors, then all other finished child processes */ + if (pp->consume_sideband) { + pp->consume_sideband(&pp->children[i].err, pp->data); + pp->consume_sideband(&pp->buffered_output, pp->data); + } else { + strbuf_write(&pp->children[i].err, stderr); + strbuf_write(&pp->buffered_output, stderr); + } strbuf_reset(&pp->children[i].err); - - /* Output all other finished child processes */ - strbuf_write(&pp->buffered_output, stderr); strbuf_reset(&pp->buffered_output); /* @@ -1855,6 +1869,7 @@ int run_processes_parallel(int n, get_next_task_fn get_next_task, start_failure_fn start_failure, feed_pipe_fn feed_pipe, + consume_sideband_fn consume_sideband, task_finished_fn task_finished, void *pp_cb) { @@ -1865,7 +1880,7 @@ int run_processes_parallel(int n, sigchain_push(SIGPIPE, SIG_IGN); - pp_init(&pp, n, get_next_task, start_failure, feed_pipe, task_finished, pp_cb); + pp_init(&pp, n, get_next_task, start_failure, feed_pipe, consume_sideband, task_finished, pp_cb); while (1) { for (i = 0; i < spawn_cap && !pp.shutdown && @@ -1903,6 +1918,7 @@ int run_processes_parallel(int n, int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task, start_failure_fn start_failure, feed_pipe_fn feed_pipe, + consume_sideband_fn consume_sideband, task_finished_fn task_finished, void *pp_cb, const char *tr2_category, const char *tr2_label) { @@ -1912,7 +1928,8 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task, ((n < 1) ? online_cpus() : n)); result = run_processes_parallel(n, get_next_task, start_failure, - feed_pipe, task_finished, pp_cb); + feed_pipe, consume_sideband, + task_finished, pp_cb); trace2_region_leave(tr2_category, tr2_label, NULL); diff --git a/run-command.h b/run-command.h index e058c0e2c8..2ad8271f56 100644 --- a/run-command.h +++ b/run-command.h @@ -450,6 +450,20 @@ typedef int (*feed_pipe_fn)(struct strbuf *pipe, void *pp_cb, void *pp_task_cb); +/** + * If this callback is provided, instead of collating process output to stderr, + * they will be collated into a new pipe. consume_sideband_fn will be called + * repeatedly. When output is available on that pipe, it will be contained in + * 'output'. But it will be called with an empty 'output' too, to allow for + * keepalives or similar operations if necessary. + * + * pp_cb is the callback cookie as passed into run_processes_parallel. + * + * Since this callback is provided with the collated output, no task cookie is + * provided. + */ +typedef void (*consume_sideband_fn)(struct strbuf *output, void *pp_cb); + /** * This callback is called on every child process that finished processing. * @@ -485,10 +499,12 @@ int run_processes_parallel(int n, get_next_task_fn, start_failure_fn, feed_pipe_fn, + consume_sideband_fn, task_finished_fn, void *pp_cb); int run_processes_parallel_tr2(int n, get_next_task_fn, start_failure_fn, - feed_pipe_fn, task_finished_fn, void *pp_cb, + feed_pipe_fn, consume_sideband_fn, + task_finished_fn, void *pp_cb, const char *tr2_category, const char *tr2_label); #endif diff --git a/submodule.c b/submodule.c index 953f41818c..215bff22d9 100644 --- a/submodule.c +++ b/submodule.c @@ -1638,7 +1638,7 @@ int fetch_populated_submodules(struct repository *r, run_processes_parallel_tr2(max_parallel_jobs, get_next_submodule, fetch_start_failure, - NULL, + NULL, NULL, fetch_finish, &spf, "submodule", "parallel/fetch"); diff --git a/t/helper/test-run-command.c b/t/helper/test-run-command.c index 9348184d30..d53db6d11c 100644 --- a/t/helper/test-run-command.c +++ b/t/helper/test-run-command.c @@ -51,6 +51,16 @@ static int no_job(struct child_process *cp, return 0; } +static void test_consume_sideband(struct strbuf *output, void *cb) +{ + FILE *sideband; + + sideband = fopen("./sideband", "a"); + + strbuf_write(output, sideband); + fclose(sideband); +} + static int task_finished(int result, struct strbuf *err, void *pp_cb, @@ -201,7 +211,7 @@ static int testsuite(int argc, const char **argv) suite.tests.nr, max_jobs); ret = run_processes_parallel(max_jobs, next_test, test_failed, - test_stdin, test_finished, &suite); + test_stdin, NULL, test_finished, &suite); if (suite.failed.nr > 0) { ret = 1; @@ -429,23 +439,28 @@ int cmd__run_command(int argc, const char **argv) if (!strcmp(argv[1], "run-command-parallel")) exit(run_processes_parallel(jobs, parallel_next, - NULL, NULL, NULL, &proc)); + NULL, NULL, NULL, NULL, &proc)); if (!strcmp(argv[1], "run-command-abort")) exit(run_processes_parallel(jobs, parallel_next, - NULL, NULL, task_finished, &proc)); + NULL, NULL, NULL, task_finished, &proc)); if (!strcmp(argv[1], "run-command-no-jobs")) exit(run_processes_parallel(jobs, no_job, - NULL, NULL, task_finished, &proc)); + NULL, NULL, NULL, task_finished, &proc)); if (!strcmp(argv[1], "run-command-stdin")) { proc.in = -1; proc.no_stdin = 0; exit (run_processes_parallel(jobs, parallel_next, NULL, - test_stdin, NULL, &proc)); + test_stdin, NULL, NULL, &proc)); } + if (!strcmp(argv[1], "run-command-sideband")) + exit(run_processes_parallel(jobs, parallel_next, NULL, NULL, + test_consume_sideband, NULL, + &proc)); + fprintf(stderr, "check usage\n"); return 1; } diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh index 3eb572e6cd..c5a5b6df6c 100755 --- a/t/t0061-run-command.sh +++ b/t/t0061-run-command.sh @@ -143,6 +143,13 @@ test_expect_success 'run_command runs in parallel with more tasks than jobs avai test_cmp expect actual ' +test_expect_success 'run_command can divert output' ' + test_when_finished rm sideband && + test-tool run-command run-command-sideband 3 sh -c "printf \"%s\n%s\n\" Hello World" 2>actual && + test_must_be_empty actual && + test_cmp expect sideband +' + cat >expect <<-EOF preloaded output of a child listening for stdin: From patchwork Sat Dec 5 01:46:07 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emily Shaffer X-Patchwork-Id: 11952787 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-26.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B36E1C4361A for ; Sat, 5 Dec 2020 01:47:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 84E2122DBF for ; Sat, 5 Dec 2020 01:47:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731162AbgLEBrx (ORCPT ); Fri, 4 Dec 2020 20:47:53 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:57470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731126AbgLEBrx (ORCPT ); Fri, 4 Dec 2020 20:47:53 -0500 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 0876BC08ED7E for ; Fri, 4 Dec 2020 17:46:47 -0800 (PST) Received: by mail-qt1-x849.google.com with SMTP id i1so1817521qtw.4 for ; Fri, 04 Dec 2020 17:46:46 -0800 (PST) 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=FyZvN3it5ExyrC1BDLo/CrnbYgEG+KU+/ghE/g+v6so=; b=HSk5L23JSSFAk6lLLRtzDMca34Nt+BpyDbghs8IQYRtKV+PVl3dYwQQY8orX3CRoRM fTcFnD2h4GE9AMa8zhMC3oLmQo646tjY+p9erqoIun6/cij7sSoWrV6xHLC5/ax6B/SU COb070UFR1Jx9mRkXdvjAq5dvWuYbvpwtljrUyj3jNUIIvBfPSwUt+zqX5DCZKcaTSfH RaQh/rftgoBjy1/cbCVrxsUI0z1MYWQJ1rOynK8yGBZ5NlqO0CMqRcUAgMXtr8e40OVQ PPadZ0WE6ox6uS96/6lBN0KjHewUI45i5+zSN7L/osO1Vmz14gcO61g2A57klQMekCgg gTTg== 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=FyZvN3it5ExyrC1BDLo/CrnbYgEG+KU+/ghE/g+v6so=; b=pgc3rTQa7Aiq4YD67CiQvO1tG7xlVkHepb85zCjPA8rcIhyPJsHpxRJktuoe+U7lcM AYOz1rXKw6hWa0MLvMFlfA3sGj/94GndK9LLZwpgkNS7P8gTBbW5OvaPxaQceT7KlFpv XSATgZzhAgHXWeHz/dNS2LmZnksZqCMlDeL8SgtQH2NcDFNX5dLbxk9tnuNmG8vnMiN7 Jg+9B5qYIloWtumB30bptUuCXB2emWDnWQveXmzMgBeRq9r9ost5pUSVD9tVBeRe+9KF URux1+ct2/B0dLXS+PoV3HrLX7nTaHzdvxch95iQacmDUY18SulPJk8WleFHaKYGrPll Qwgg== X-Gm-Message-State: AOAM5308abpoK1kX5VQ1Xwj8M3uSRaezf0GyD7mhZjQmNUBP+RAR/PrS dsq3Uh965y5OBS8ikPPE+89yz12B/9+M7dsYAxejiSqgHi0qS2NPu9Xq0e2TrFAGV7ZFQoicU0Y VnJSjtB73uJZGY3ZuOPHm3zNW7UW6gnZ2+hJMdHmTw5A8QnZICzwGJxAEWIfFj3lJM8ilPndjKw == X-Google-Smtp-Source: ABdhPJztdOwweCLCtFXjKYb1PqeLUBzZJkqgLPQ5p4JqKz1/klY6fZr+6Z/6kxkn9cY+h/4gSuQknbdbmqg/0ZJVn/I= 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:fad1:: with SMTP id p17mr3000419qvo.47.1607132806150; Fri, 04 Dec 2020 17:46:46 -0800 (PST) Date: Fri, 4 Dec 2020 17:46:07 -0800 In-Reply-To: <20201205014607.1464119-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-18-emilyshaffer@google.com> Mime-Version: 1.0 References: <20201205014607.1464119-1-emilyshaffer@google.com> X-Mailer: git-send-email 2.28.0.226.g0268cb6820 Subject: [PATCH 17/17] hooks: allow callers to capture output From: Emily Shaffer To: git@vger.kernel.org Cc: Emily Shaffer Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Some server-side hooks will require capturing output to send over sideband instead of printing directly to stderr. Expose that capability. Signed-off-by: Emily Shaffer --- Notes: You can see this in practice in the conversions for some of the push hooks, like 'receive-pack'. hook.c | 2 +- hook.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/hook.c b/hook.c index 1186ee41b3..78d7721b74 100644 --- a/hook.c +++ b/hook.c @@ -407,7 +407,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) pick_next_hook, notify_start_failure, options->feed_pipe, - NULL, + options->consume_sideband, notify_hook_finished, &cb_data, "hook", diff --git a/hook.h b/hook.h index ace26c637e..7059e0db77 100644 --- a/hook.h +++ b/hook.h @@ -76,6 +76,14 @@ struct run_hooks_opt feed_pipe_fn feed_pipe; void *feed_pipe_ctx; + /* + * Populate this to capture output and prevent it from being printed to + * stderr. This will be passed directly through to + * run_command:run_parallel_processes(). See t/helper/test-run-command.c + * for an example. + */ + consume_sideband_fn consume_sideband; + /* Number of threads to parallelize across */ int jobs; @@ -103,6 +111,7 @@ struct hook_cb_data { .str_stdin = STRING_LIST_INIT_DUP, \ .feed_pipe = NULL, \ .feed_pipe_ctx = NULL, \ + .consume_sideband = NULL, \ .run_hookdir = configured_hookdir_opt() \ } @@ -115,6 +124,7 @@ struct hook_cb_data { .str_stdin = STRING_LIST_INIT_DUP, \ .feed_pipe = NULL, \ .feed_pipe_ctx = NULL, \ + .consume_sideband = NULL, \ .run_hookdir = configured_hookdir_opt() \ }