| Message ID | cover-00.31-00000000000-20210528T110515Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | minimal restart of "config-based-hooks" | expand |
On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: > > After suggesting[1] an another round that the config-based-hook > topic[2] should take a more incremental approach to reach its end goal > I thought I'd try hacking that up. > > So this is a proposed restart of that topic which if the consensus > favors it should replace it, and the config-based hooks topic should > be rebased on top of this. I'm not entirely sure what you're trying to achieve by sending this series. It was my impression that the existing config-based-hooks topic was close to being ready to submit anyway (since Junio mentioned submitting it a couple revisions ago); rather than churning by reviewing a different 31-patch topic, and then re-rolling and re-reviewing a (reduced) config hook topic, wouldn't it be easier on everyone's time to do a final incremental review on the existing topic and then start in on bugfixes/feature patches afterwards? It would have been nice to see a more clear discussion of patch organization sometime much sooner in the past year and a half since the project was proposed[3], like maybe in the few iterations of the design doc which included a rollout plan in July of last year[4]. To me, it seems late to be overhauling the direction like this, especially after I asked for opinions and approval on the direction before I started work in earnest. Anyway, I'd personally rather spend effort getting the existing series the last few yards to the finish line than to head most of the way back to the start. - Emily > 1. https://lore.kernel.org/git/87lf80l1m6.fsf@evledraar.gmail.com/ > 2. https://lore.kernel.org/git/20210527000856.695702-1-emilyshaffer@google.com/ 3. https://lore.kernel.org/git/20191116011125.GG22855@google.com/ 4. https://lore.kernel.org/git/20200728222455.3023400-1-emilyshaffer@google.com/
On 6/1/2021 2:14 PM, Emily Shaffer wrote: > On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> After suggesting[1] an another round that the config-based-hook >> topic[2] should take a more incremental approach to reach its end goal >> I thought I'd try hacking that up. I think sending this complete reorganization of a long-lived topic is not helpful, especially because the end-to-end diff is significant. This series has been extensively tested and incrementally improved for months, and it would be a waste to start over and lose all of that hardening. It's also a but rushed that this comes only a day after the previous message recommending a reorganization. It would be best to at least give the original author an opportunity to comment on your idea before working on this. >> So this is a proposed restart of that topic which if the consensus >> favors it should replace it, and the config-based hooks topic should >> be rebased on top of this. > > I'm not entirely sure what you're trying to achieve by sending this > series. It was my impression that the existing config-based-hooks topic > was close to being ready to submit anyway (since Junio mentioned > submitting it a couple revisions ago); rather than churning by reviewing > a different 31-patch topic, and then re-rolling and re-reviewing a > (reduced) config hook topic, wouldn't it be easier on everyone's time to > do a final incremental review on the existing topic and then start in on > bugfixes/feature patches afterwards? I completely agree here. > It would have been nice to see a more clear discussion of patch > organization sometime much sooner in the past year and a half since the > project was proposed[3], like maybe in the few iterations of the design > doc which included a rollout plan in July of last year[4]. To me, it > seems late to be overhauling the direction like this, especially after I > asked for opinions and approval on the direction before I started work > in earnest. I've also seen messages as early as January where Ævar mentioned wanting to review the series, but not finding the time to do so. It is reasonable to expect that contributors attempt such major reorganizations according to reviewers feedback, as long as the reviewers are timely about delivering that feedback. Thanks, -Stolee
Emily Shaffer wrote: > On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: > > > > After suggesting[1] an another round that the config-based-hook > > topic[2] should take a more incremental approach to reach its end goal > > I thought I'd try hacking that up. > > > > So this is a proposed restart of that topic which if the consensus > > favors it should replace it, and the config-based hooks topic should > > be rebased on top of this. > > I'm not entirely sure what you're trying to achieve by sending this > series. Have a better review? By having a more reviwable series? > It was my impression that the existing config-based-hooks topic > was close to being ready to submit anyway (since Junio mentioned > submitting it a couple revisions ago); rather than churning by reviewing > a different 31-patch topic, and then re-rolling and re-reviewing a > (reduced) config hook topic, wouldn't it be easier on everyone's time to > do a final incremental review on the existing topic and then start in on > bugfixes/feature patches afterwards? Not to me. I tried to review your series and I just couldn't. Maybe it's something about me, but the whole point of a review is to have as many eyes as possible on the code in order to spot any potential issues. If some eyes have trouble parsing the patches, that's not optimal. Ævar's approach is easy for me to follow. > It would have been nice to see a more clear discussion of patch > organization sometime much sooner in the past year and a half since the > project was proposed[3], like maybe in the few iterations of the design > doc which included a rollout plan in July of last year[4]. Not all of us are being paid by a big corporation to work on Git. Some of us are doing the work on our own free time. You can't demand we spend our own free time on a certain patch series as soon as possible because it's more convenient for $corporation. Git is an open source community project. > To me, it seems late to be overhauling the direction like this, > especially after I asked for opinions and approval on the direction > before I started work in earnest. To me it's completely the opposite; it's never too late to overwhaul a patch series. In my opinion you should be thankful that somebody took the time to try to improve your series *for free*. Cheers.
Derrick Stolee wrote: > On 6/1/2021 2:14 PM, Emily Shaffer wrote: > > On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> > >> After suggesting[1] an another round that the config-based-hook > >> topic[2] should take a more incremental approach to reach its end goal > >> I thought I'd try hacking that up. > > I think sending this complete reorganization of a long-lived topic > is not helpful, especially because the end-to-end diff is significant. The end-to-end diff is significant because it's not the full patch series. Give me pointers to the two branches and I'll get you a patch series on top of Ævar's that gets you *exactly* zero end-to-end diff to Emily's series. There's many paths to end up with exactly the same code. I do it all the time. > I've also seen messages as early as January where Ævar mentioned > wanting to review the series, but not finding the time to do so. > It is reasonable to expect that contributors attempt such major > reorganizations according to reviewers feedback, as long as the > reviewers are timely about delivering that feedback. The Git project doesn't have deadlines. Code should be merged when it's ready to be merged, not when it's convenient for $company. I have patches that have been stuck for 7 years. Why should $company get a fast-path, an exception, or preferential treatment? Cheers.
On Wed, Jun 02 2021, Felipe Contreras wrote: > Derrick Stolee wrote: >> On 6/1/2021 2:14 PM, Emily Shaffer wrote: >> > On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> >> >> After suggesting[1] an another round that the config-based-hook >> >> topic[2] should take a more incremental approach to reach its end goal >> >> I thought I'd try hacking that up. >> >> I think sending this complete reorganization of a long-lived topic >> is not helpful, especially because the end-to-end diff is significant. > > The end-to-end diff is significant because it's not the full patch > series. > > Give me pointers to the two branches and I'll get you a patch series on > top of Ævar's that gets you *exactly* zero end-to-end diff to Emily's > series. At https://github.com/avar/git this series is at es-avar/config-based-hooks-2 (there's an unsubmitted *-3 with the minor fix I already noted, pending any other feedback). Then es/config-based-hooks-v9-basis-for-es-avar/config-based-hooks-2 is Emily's v9 as applied by Junio, which is what I started work on. Right now it's identical to Junio's (at https://github.com/gitster/git) gitster/es/config-based-hooks, I just saved it away in case that branch moves. A diff --stat -p between es-avar/config-based-hooks-2~2 and that topic follows, i.e. my topic submitted here minus my last two fixes-on-top patches. I.e. this would be more-or-less the diff we'd apply to get to the end-state (minus reverting a few bugfixes etc. I made along the line, e.g. I forgot to mention that I added the "git hook run --ignore-missing" to fix another TOCTOU race, except one introduced by Emily's series). Documentation/Makefile | 1 + Documentation/config/hook.txt | 27 ++ Documentation/git-hook.txt | 157 +++++++-- Documentation/githooks.txt | 79 ++++- Documentation/technical/api-parse-options.txt | 7 + Documentation/technical/config-based-hooks.txt | 369 +++++++++++++++++++++ builtin/am.c | 27 +- builtin/bugreport.c | 2 +- builtin/checkout.c | 4 +- builtin/clone.c | 3 +- builtin/commit.c | 12 +- builtin/gc.c | 3 +- builtin/hook.c | 182 +++++++++-- builtin/merge.c | 13 +- builtin/pack-objects.c | 174 +--------- builtin/rebase.c | 3 +- builtin/receive-pack.c | 51 ++- builtin/worktree.c | 7 +- commit.c | 10 +- commit.h | 3 +- git-p4.py | 13 +- git-send-email.perl | 35 +- git.c | 2 +- hook.c | 397 +++++++++++++++++++---- hook.h | 109 ++++--- pack-objects.h | 159 +++++++++ parse-options-cb.c | 16 + parse-options.h | 4 + perl/Git.pm | 13 + read-cache.c | 3 +- refs.c | 8 +- reset.c | 4 +- run-command.c | 1 - sequencer.c | 16 +- t/helper/test-parse-options.c | 6 + t/t0040-parse-options.sh | 27 ++ t/t1092-sparse-checkout-compatibility.sh | 6 +- t/t1350-config-hooks-path.sh | 1 - t/t1360-config-based-hooks.sh | 329 +++++++++++++++++++ t/t1416-ref-transaction-hooks.sh | 12 +- t/t1800-hook.sh | 158 --------- t/t2080-parallel-checkout-basics.sh | 2 +- t/t5411/test-0015-too-many-hooks-error.sh | 47 +++ t/t6500-gc.sh | 46 --- t/t7503-pre-commit-and-pre-merge-commit-hooks.sh | 17 +- t/t9001-send-email.sh | 16 +- transport.c | 6 +- 47 files changed, 1941 insertions(+), 646 deletions(-) diff --git a/Documentation/Makefile b/Documentation/Makefile index 2aae4c9cbb3..5d19eddb0eb 100644 --- a/Documentation/Makefile +++ b/Documentation/Makefile @@ -90,6 +90,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/config/hook.txt b/Documentation/config/hook.txt new file mode 100644 index 00000000000..4f66bb35cf8 --- /dev/null +++ b/Documentation/config/hook.txt @@ -0,0 +1,27 @@ +hook.<command>.command:: + A command to execute during the <command> 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.<name>.command:: + A command to execute during a hook for which <name> 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]. + +hookcmd.<name>.skip:: + Specify this boolean to remove a command from earlier in the execution + order. Useful if you want to make a single repo an exception to hook + configured at the system or global scope. If there is no hookcmd + specified for the command you want to skip, you can use the value of + `hook.<command>.command` as <name> as a shortcut. The "skip" setting + must be specified after the "hook.<command>.command" to have an effect. + +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 816b3eda460..24e00a6f4af 100644 --- a/Documentation/git-hook.txt +++ b/Documentation/git-hook.txt @@ -3,46 +3,159 @@ git-hook(1) NAME ---- -git-hook - run git hooks +git-hook - Manage configured hooks SYNOPSIS -------- [verse] -'git hook' run [--to-stdin=<path>] [--ignore-missing] <hook-name> [-- <hook-args>] +'git hook' list <hook-name> +'git hook' run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] + [(-j|--jobs) <n>] <hook-name> DESCRIPTION ----------- +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 is an interface to git hooks (see linkgit:githooks[5]). -Currently it only provides a convenience wrapper for running hooks for -use by git itself. In the future it might gain other functionality. +In general, when instructions suggest adding a script to +`.git/hooks/<something>`, you can specify it in the config instead by running +`git config --add hook.<something>.command <path-to-script>` - this way you can +share the script between multiple repos. That is, `cp ~/my-script.sh +~/project/.git/hooks/pre-commit` would become `git config --add +hook.pre-commit.command ~/my-script.sh`. -SUBCOMMANDS ------------ +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 +---- + +If there is a command you wish to run in most cases but have one or two +exceptional repos where it should be skipped, you can use specify +`hookcmd.<name>.skip`, for example: + +System config +---- + [hook "pre-commit"] + command = check-for-secrets + + [hookcmd "check-for-secrets"] + command = /bin/secret-checker --aggressive +---- -run:: +Local config +---- + [hookcmd "check-for-secrets"] + skip = true + # This works for inlined hook commands, too: + [hookcmd "~/typocheck.sh"] + skip = true +---- - Run the `<hook-name>` hook. Any positional arguments to the - hook should be passed after an optional "--" (or - "--end-of-options"). See "OPTIONS" below for the arguments - this accepts. +After these configs are added, the hook list becomes: + +---- +$ git hook list "post-commit" +global: /bin/linter --c +local: python ~/run-test-suite.py + +$ git hook list "pre-commit" +no commands configured for hook 'pre-commit' +---- + +COMMANDS +-------- + +list `<hook-name>`:: + +List the hooks which have been configured for `<hook-name>`. Hooks appear +in the order they should be run, and print the config scope where the relevant +`hook.<hook-name>.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)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] [(-j|--jobs)<n>] `<hook-name>`:: + +Runs hooks configured for `<hook-name>`, in the same order displayed by `git +hook list`. Hooks configured this way may be 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. --to-stdin:: - For "run"; Specify a file which will be streamed into the - hook's stdin. The hook will receive the entire file from - beginning to EOF. + 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. ---ignore-missing:: - Ignore any missing hook by quietly returning zero. Used for - tools that want to do a blind one-shot run of a hook that may - or may not be present. +-j:: +--jobs:: + Only valid for `run`. ++ +Specify how many hooks to run simultaneously. If this flag is not specified, use +the value of the `hook.jobs` config. If the config is not specified, use the +number of CPUs on the current system. Some hooks may be ineligible for +parallelization: for example, 'commit-msg' intends hooks modify the commit +message body and cannot be parallelized. -SEE ALSO --------- -linkgit:githooks[5] +HOOKS +----- +For a list of hooks which can be configured and how they work, see +linkgit:githooks[5]. + +CONFIGURATION +------------- +include::config/hook.txt[] GIT --- diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt index a16e62bc8c8..d780cb3b18d 100644 --- a/Documentation/githooks.txt +++ b/Documentation/githooks.txt @@ -7,15 +7,16 @@ githooks - Hooks used by Git SYNOPSIS -------- +'git hook' $GIT_DIR/hooks/* (or \`git config core.hooksPath`/*) DESCRIPTION ----------- -Hooks are programs you can place in a hooks directory to trigger -actions at certain points in git's execution. Hooks that don't have -the executable bit set are ignored. +Hooks are programs you can specify in your config (see linkgit:git-hook[1]) or +place in a hooks directory to trigger actions at certain points in git's +execution. Hooks that don't have the executable bit set are ignored. By default the hooks directory is `$GIT_DIR/hooks`, but that can be changed via the `core.hooksPath` configuration variable (see @@ -58,6 +59,9 @@ the message file. The default 'applypatch-msg' hook, when enabled, runs the 'commit-msg' hook, if the latter is enabled. +Hooks run during 'applypatch-msg' will not be parallelized, because hooks are +expected to edit the file holding the commit log message. + pre-applypatch ~~~~~~~~~~~~~~ @@ -73,6 +77,9 @@ make a commit if it does not pass certain test. The default 'pre-applypatch' hook, when enabled, runs the 'pre-commit' hook, if the latter is enabled. +Hooks run during 'pre-applypatch' will be run in parallel, unless hook.jobs is +configured to 1. + post-applypatch ~~~~~~~~~~~~~~~ @@ -82,6 +89,9 @@ and is invoked after the patch is applied and a commit is made. This hook is meant primarily for notification, and cannot affect the outcome of `git am`. +Hooks run during 'post-applypatch' will be run in parallel, unless hook.jobs is +configured to 1. + pre-commit ~~~~~~~~~~ @@ -103,6 +113,8 @@ The default 'pre-commit' hook, when enabled--and with the `hooks.allownonascii` config option unset or set to false--prevents the use of non-ASCII filenames. +Hooks executed during 'pre-commit' will not be parallelized. + pre-merge-commit ~~~~~~~~~~~~~~~~ @@ -125,6 +137,8 @@ need to be resolved and the result committed separately (see linkgit:git-merge[1]). At that point, this hook will not be executed, but the 'pre-commit' hook will, if it is enabled. +Hooks executed during 'pre-merge-commit' will not be parallelized. + prepare-commit-msg ~~~~~~~~~~~~~~~~~~ @@ -150,6 +164,9 @@ be used as replacement for pre-commit hook. The sample `prepare-commit-msg` hook that comes with Git removes the help message found in the commented portion of the commit template. +Hooks executed during 'prepare-commit-msg' will not be parallelized, because +hooks are expected to edit the file containing the commit log message. + commit-msg ~~~~~~~~~~ @@ -166,6 +183,9 @@ file. The default 'commit-msg' hook, when enabled, detects duplicate `Signed-off-by` trailers, and aborts the commit if one is found. +Hooks executed during 'commit-msg' will not be parallelized, because hooks are +expected to edit the file containing the proposed commit log message. + post-commit ~~~~~~~~~~~ @@ -175,6 +195,9 @@ invoked after a commit is made. This hook is meant primarily for notification, and cannot affect the outcome of `git commit`. +Hooks executed during 'post-commit' will run in parallel, unless hook.jobs is +configured to 1. + pre-rebase ~~~~~~~~~~ @@ -184,6 +207,9 @@ two parameters. The first parameter is the upstream from which the series was forked. The second parameter is the branch being rebased, and is not set when rebasing the current branch. +Hooks executed during 'pre-rebase' will run in parallel, unless hook.jobs is +configured to 1. + post-checkout ~~~~~~~~~~~~~ @@ -206,6 +232,8 @@ This hook can be used to perform repository validity checks, auto-display differences from the previous HEAD if different, or set working dir metadata properties. +Hooks executed during 'post-checkout' will not be parallelized. + post-merge ~~~~~~~~~~ @@ -220,6 +248,9 @@ save and restore any form of metadata associated with the working tree (e.g.: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl for an example of how to do this. +Hooks executed during 'post-merge' will run in parallel, unless hook.jobs is +configured to 1. + pre-push ~~~~~~~~ @@ -249,6 +280,9 @@ If this hook exits with a non-zero status, `git push` will abort without pushing anything. Information about why the push is rejected may be sent to the user by writing to standard error. +Hooks executed during 'pre-push' will run in parallel, unless hook.jobs is +configured to 1. + [[pre-receive]] pre-receive ~~~~~~~~~~~ @@ -290,6 +324,8 @@ will be set to zero, `GIT_PUSH_OPTION_COUNT=0`. See the section on "Quarantine Environment" in linkgit:git-receive-pack[1] for some caveats. +Hooks executed during 'pre-receive' will not be parallelized. + [[update]] update ~~~~~~ @@ -335,6 +371,9 @@ The default 'update' hook, when enabled--and with `hooks.allowunannotated` config option unset or set to false--prevents unannotated tags to be pushed. +Hooks executed during 'update' are run in parallel, unless hook.jobs is +configured to 1. + [[proc-receive]] proc-receive ~~~~~~~~~~~~ @@ -397,6 +436,10 @@ the input. The exit status of the 'proc-receive' hook only determines the success or failure of the group of commands sent to it, unless atomic push is in use. +It is forbidden to specify more than one hook for 'proc-receive'. If a +globally-configured 'proc-receive' must be overridden, use +'hookcmd.<global-hook>.skip = true' to ignore it. + [[post-receive]] post-receive ~~~~~~~~~~~~ @@ -436,6 +479,9 @@ environment variables will not be set. If the client selects to use push options, but doesn't transmit any, the count variable will be set to zero, `GIT_PUSH_OPTION_COUNT=0`. +Hooks executed during 'post-receive' are run in parallel, unless hook.jobs is +configured to 1. + [[post-update]] post-update ~~~~~~~~~~~ @@ -468,6 +514,9 @@ Both standard output and standard error output are forwarded to `git send-pack` on the other end, so you can simply `echo` messages for the user. +Hooks run during 'post-update' will be run in parallel, unless hook.jobs is +configured to 1. + reference-transaction ~~~~~~~~~~~~~~~~~~~~~ @@ -506,6 +555,9 @@ The exit status of the hook is ignored for any state except for the cause the transaction to be aborted. The hook will not be called with "aborted" state in that case. +Hooks run during 'reference-transaction' will be run in parallel, unless +hook.jobs is configured to 1. + push-to-checkout ~~~~~~~~~~~~~~~~ @@ -536,6 +588,7 @@ that switches branches while keeping the local changes in the working tree that do not interfere with the difference between the branches. +Hooks executed during 'push-to-checkout' will not be parallelized. pre-auto-gc ~~~~~~~~~~~ @@ -544,6 +597,9 @@ This hook is invoked by `git gc --auto` (see linkgit:git-gc[1]). It takes no parameter, and exiting with non-zero status from this script causes the `git gc --auto` to abort. +Hooks run during 'pre-auto-gc' will be run in parallel, unless hook.jobs is +configured to 1. + post-rewrite ~~~~~~~~~~~~ @@ -569,6 +625,9 @@ The hook always runs after the automatic note copying (see "notes.rewrite.<command>" in linkgit:git-config[1]) has happened, and thus has access to these notes. +Hooks run during 'post-rewrite' will be run in parallel, unless hook.jobs is +configured to 1. + The following command-specific comments apply: rebase:: @@ -591,9 +650,12 @@ e-mails. fsmonitor-watchman ~~~~~~~~~~~~~~~~~~ -This hook is invoked when the configuration option `core.fsmonitor` is -set to `.git/hooks/fsmonitor-watchman` or `.git/hooks/fsmonitor-watchmanv2` -depending on the version of the hook to use. +This hook is invoked when the configuration option `core.fsmonitor` is set to a +path containing an executable. It *cannot* be specified via the usual +`hook.fsmonitor-watchman.command` configuration or by providing an executable +in `.git/hooks/fsmonitor-watchman`. The arguments provided to the hook are +determined by the value of the `core.fsmonitorHookVersion` configuration +option. Version 1 takes two arguments, a version (1) and the time in elapsed nanoseconds since midnight, January 1, 1970. @@ -698,9 +760,8 @@ and "0" meaning they were not. Only one parameter should be set to "1" when the hook runs. The hook running passing "1", "1" should not be possible. -SEE ALSO --------- -linkgit:git-hook[1] +Hooks run during 'post-index-change' will be run in parallel, unless hook.jobs +is configured to 1. GIT --- diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 5a60bbfa7f4..f79b17e7fcd 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -173,6 +173,13 @@ 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, meant to be specified + multiple times. + The string argument is stored as an element in `strvec`, and later + arguments are added to the same `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/Documentation/technical/config-based-hooks.txt b/Documentation/technical/config-based-hooks.txt new file mode 100644 index 00000000000..1f973117e44 --- /dev/null +++ b/Documentation/technical/config-based-hooks.txt @@ -0,0 +1,369 @@ +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 - hooks can be easily shared +between multiple Git repos. + +Redefine "hook" as an event rather than a single script, allowing users to +perform multiple unrelated actions on a single event. + +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 +<<per-hook-event-settings,the section in Future Work>> for more details. + +Also contains top-level hook execution settings, for example, `hook.runHookDir`. +(These settings are described more in <<library,Library>>.) + +---- +[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 <args passed by Git>`, but for repos which +include this config, the hook command will be skipped for all events. +Theoretically, the last line could be used to "un-skip" the hook command for +`pre-commit` hooks, but this hasn't been scoped or implemented yet. + +---- +[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, run, 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. Modifier commands (`edit` and `add`) have not been +implemented yet and may not be if manually editing the config proves usable +enough. + +*`git hook list <hook-event>`* + +*`git hook run <hook-event> [-a <arg>]... [-e <env-var>]...`* + +*`git hook edit <hook-event>`* + +*`git hook add <hook-command> <hook-event> <options...>`* + +[[hook-editor]] +=== Hook editor + +The tool which is presented by `git hook edit <hook-command>`. 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`. This has not been designed or implemented yet and may not be if +the config proves usable enough. + +[[implementation]] +== Implementation + +[[library]] +=== Library + +`hook.c` and `hook.h` are responsible for interacting with the config files. The +hook library provides a basic API to call all hooks in config order with more +complex options passed via `struct run_hooks_opt`: + +*`int run_hooks(const char *hookname, struct run_hooks_opt *options)`* + +`struct run_hooks_opt` allows callers to set: + +- environment variables +- command-line arguments +- behavior for the hook command provided by `run-command.h:find_hook()` (see + below) +- a method to provide stdin to each hook, either via a file containing stdin, a + `struct string_list` containing a list of lines to print, or a callback + function to allow the caller to populate stdin manually +- a method to process stdout from each hook, e.g. for printing to sideband + during a network operation +- parallelism +- a custom working directory for hooks to execute in + +And this struct can be extended with more options as necessary in the future. + +The "legacy" hook provided by `run-command.h:find_hook()` - that is, the hook +present in `.git/hooks/<hookname>` or +`$(git config --get core.hooksPath)/<hookname>` - can be handled in a number of +ways, providing an avenue to deprecate these "legacy" hooks if desired. The +handling is based on a config `hook.runHookDir`, which is checked against a +number of cases: + +- "no": the legacy hook will not be run +- "error": Git will print a warning to stderr before ignoring the legacy hook +- "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" (but print a +warning to the user first to let them know their value is wrong). + +`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. + +By default, hook parallelism is chosen based on the semantics of each hook; +callsites initialize their `struct run_hooks_opt` via one of two macros, +`RUN_HOOKS_OPT_INIT_SYNC` or `RUN_HOOKS_OPT_INIT_ASYNC`. The default number of +jobs can be configured in `hook.jobs`; this config applies across all hook +events. If unset, the value of `online_cpus()` (equivalent to `nproc`) is used. + +[[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 <hook-event>` is implemented. `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-2]] +==== Stage 2 + +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-3]] +==== Stage 3 + +`.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. This +assumption was made to avoid overcomplicating the design. So, this series +doesn't particularly improve security or resistance to zip attacks. + +[[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] + +The table below shows a number of goals and how they might be achieved with +config-based hooks, by implementing directory support (i.e. +'.git/hooks/pre-commit.d'), or as hooks are run today. + +.Comparison of alternatives +|=== +|Feature |Config-based hooks |Hook directories |Status quo + +|Supports multiple hooks +|Natively +|Natively +|With user effort + +|Supports parallelization +|Natively +|Natively +|No (user's multihook trampoline script would need to handle parallelism) + +|Safer for zipped repos +|A little +|No +|No + +|Previous hooks just work +|If configured +|Yes +|Yes + +|Can install one hook to many repos +|Yes +|With symlinks or core.hooksPath +|With symlinks or core.hooksPath + +|Discoverability +|Findable with 'git help git' or tab-completion via 'git hook' subcommand +|Findable via improved documentation +|Same as before + +|Hard to run unexpected hook +|If configured +|Could be made to warn or look for a config +|No +|=== + +[[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/<hookname>.d` and execute those hooks in alphabetical order. + +[[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 with dependencies + +Currently hooks use a naive parallelization scheme or are run in series. But 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. + +[[nontrivial-hooks]] +=== Multihooks and nontrivial output + +Some hooks - like 'proc-receive' - don't lend themselves well to multihooks at +all. In the case of 'proc-receive', for now, multiple hook definitions are +disallowed. In the future we might be able to conceive a better approach, for +example, running the hooks in series and using the output from one hook as the +input to the next. + +[[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. diff --git a/builtin/am.c b/builtin/am.c index 6e4f9c80360..d2534f9a1ff 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -11,7 +11,6 @@ #include "parse-options.h" #include "dir.h" #include "run-command.h" -#include "hook.h" #include "quote.h" #include "tempfile.h" #include "lockfile.h" @@ -34,6 +33,7 @@ #include "string-list.h" #include "packfile.h" #include "repository.h" +#include "hook.h" /** * Returns the length of the first line of msg. @@ -445,7 +445,9 @@ static void am_destroy(const struct am_state *state) static int run_applypatch_msg_hook(struct am_state *state) { int ret; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; + + run_hooks_opt_init_sync(&opt); assert(state->msg); strvec_push(&opt.args, am_path(state, "final-commit")); @@ -467,9 +469,11 @@ static int run_applypatch_msg_hook(struct am_state *state) */ static int run_post_rewrite_hook(const struct am_state *state) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; int ret; + run_hooks_opt_init_async(&opt); + strvec_push(&opt.args, "rebase"); opt.path_to_stdin = am_path(state, "rewritten"); @@ -1602,14 +1606,17 @@ static void do_commit(const struct am_state *state) struct commit_list *parents = NULL; const char *reflog_msg, *author, *committer = NULL; struct strbuf sb = STRBUF_INIT; - struct run_hooks_opt hook_opt_pre = RUN_HOOKS_OPT_INIT; - struct run_hooks_opt hook_opt_post = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; + + run_hooks_opt_init_async(&hook_opt); - if (run_hooks("pre-applypatch", &hook_opt_pre)) { - run_hooks_opt_clear(&hook_opt_pre); + if (run_hooks("pre-applypatch", &hook_opt)) { + run_hooks_opt_clear(&hook_opt); exit(1); } + run_hooks_opt_clear(&hook_opt); + if (write_cache_as_tree(&tree, 0, NULL)) die(_("git write-tree failed to write a tree")); @@ -1659,10 +1666,10 @@ static void do_commit(const struct am_state *state) fclose(fp); } - run_hooks("post-applypatch", &hook_opt_post); + run_hooks_opt_init_async(&hook_opt); + run_hooks("post-applypatch", &hook_opt); - run_hooks_opt_clear(&hook_opt_pre); - run_hooks_opt_clear(&hook_opt_post); + run_hooks_opt_clear(&hook_opt); strbuf_release(&sb); } diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 941c8d5e270..190272ba70f 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -82,7 +82,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) } for (i = 0; i < ARRAY_SIZE(hook); i++) - if (hook_exists(hook[i])) + if (hook_exists(hook[i], HOOKDIR_USE_CONFIG)) strbuf_addf(hook_info, "%s\n", hook[i]); } diff --git a/builtin/checkout.c b/builtin/checkout.c index 6205ace09f6..1797f05a50e 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -107,9 +107,11 @@ struct branch_info { static int post_checkout_hook(struct commit *old_commit, struct commit *new_commit, int changed) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; int rc; + run_hooks_opt_init_sync(&opt); + /* "new_commit" can be NULL when checking out from the index before a commit exists. */ strvec_pushl(&opt.args, diff --git a/builtin/clone.c b/builtin/clone.c index 6687025bea5..2a2a03bf768 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -776,7 +776,7 @@ static int checkout(int submodule_progress) struct tree *tree; struct tree_desc t; int err = 0; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; if (option_no_checkout) return 0; @@ -822,6 +822,7 @@ static int checkout(int submodule_progress) if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) die(_("unable to write new index file")); + run_hooks_opt_init_sync(&hook_opt); strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid), "1", NULL); err |= run_hooks("post-checkout", &hook_opt); run_hooks_opt_clear(&hook_opt); diff --git a/builtin/commit.c b/builtin/commit.c index dad4e565443..7e01802961f 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -19,7 +19,6 @@ #include "revision.h" #include "wt-status.h" #include "run-command.h" -#include "hook.h" #include "refs.h" #include "log-tree.h" #include "strbuf.h" @@ -37,6 +36,7 @@ #include "help.h" #include "commit-reach.h" #include "commit-graph.h" +#include "hook.h" static const char * const builtin_commit_usage[] = { N_("git commit [<options>] [--] <pathspec>..."), @@ -729,7 +729,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); - if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL)) + if (!no_verify && run_commit_hook(use_editor, 0, index_file, "pre-commit", NULL)) return 0; if (squash_message) { @@ -1045,7 +1045,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (!no_verify && hook_exists("pre-commit")) { + if (!no_verify && hook_exists("pre-commit", HOOKDIR_USE_CONFIG)) { /* * Re-read the index as pre-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke @@ -1060,7 +1060,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (run_commit_hook(use_editor, index_file, "prepare-commit-msg", + if (run_commit_hook(use_editor, 0, index_file, "prepare-commit-msg", git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL)) return 0; @@ -1077,7 +1077,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, } if (!no_verify && - run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) { + run_commit_hook(use_editor, 0, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) { return 0; } @@ -1830,7 +1830,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix) repo_rerere(the_repository, 0); run_auto_maintenance(quiet); - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); + run_commit_hook(use_editor, 1, get_index_file(), "post-commit", NULL); if (amend && !no_post_rewrite) { commit_post_rewrite(the_repository, current_head, &oid); } diff --git a/builtin/gc.c b/builtin/gc.c index a12641a691d..16890b097cd 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -349,7 +349,7 @@ static void add_repack_incremental_option(void) static int need_to_gc(void) { - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; /* * Setting gc.auto to 0 or negative can disable the @@ -397,6 +397,7 @@ static int need_to_gc(void) else return 0; + run_hooks_opt_init_async(&hook_opt); if (run_hooks("pre-auto-gc", &hook_opt)) { run_hooks_opt_clear(&hook_opt); return 0; diff --git a/builtin/hook.c b/builtin/hook.c index baaef4dce49..c79a961e801 100644 --- a/builtin/hook.c +++ b/builtin/hook.c @@ -7,53 +7,133 @@ #include "strvec.h" static const char * const builtin_hook_usage[] = { - N_("git hook run [--to-stdin=<path>] <hook-name> [-- <hook-args>]"), + N_("git hook list <hookname>"), + N_("git hook run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...]" + "[--to-stdin=<path>] [(-j|--jobs) <count>] <hookname>"), NULL }; +static enum hookdir_opt should_run_hookdir; + +static int list(int argc, const char **argv, const char *prefix) +{ + struct list_head *head, *pos; + const char *hookname = NULL; + struct strbuf hookdir_annotation = STRBUF_INIT; + + struct option list_options[] = { + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, list_options, + builtin_hook_usage, 0); + + if (argc < 1) { + usage_msg_opt(_("You must specify a hook event name to list."), + builtin_hook_usage, list_options); + } + + hookname = argv[0]; + + head = hook_list(hookname); + + if (list_empty(head)) { + printf(_("no commands configured for hook '%s'\n"), + hookname); + return 0; + } + + list_for_each(pos, head) { + struct hook *item = list_entry(pos, struct hook, list); + item = list_entry(pos, struct hook, list); + if (item) { + if (item->from_hookdir) { + /* + * TRANSLATORS: do not translate 'hookdir' as + * it matches the config setting. + */ + switch (should_run_hookdir) { + case HOOKDIR_NO: + printf(_("hookdir: %s (will not run)\n"), + item->command.buf); + break; + case HOOKDIR_ERROR: + printf(_("hookdir: %s (will error and not run)\n"), + item->command.buf); + break; + case HOOKDIR_INTERACTIVE: + printf(_("hookdir: %s (will prompt)\n"), + item->command.buf); + break; + case HOOKDIR_WARN: + printf(_("hookdir: %s (will warn but run)\n"), + item->command.buf); + break; + case HOOKDIR_YES: + /* + * The default behavior should agree with + * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just + * do the default behavior. + */ + case HOOKDIR_UNKNOWN: + default: + printf(_("hookdir: %s\n"), + item->command.buf); + break; + } + } else { + /* + * TRANSLATORS: "<config scope>: <path>". Both fields + * should be left untranslated; config scope matches the + * output of 'git config --show-scope'. Marked for + * translation to provide better RTL support later. + */ + printf(_("%s: %s\n"), + config_scope_name(item->origin), + item->command.buf); + } + } + } + + clear_hook_list(head); + strbuf_release(&hookdir_annotation); + + return 0; +} + static int run(int argc, const char **argv, const char *prefix) { - int i; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct strbuf hookname = STRBUF_INIT; + struct run_hooks_opt opt; int rc = 0; - int ignore_missing = 0; - const char *hook_name; - const char *hook_path; struct option run_options[] = { - OPT_BOOL(0, "ignore-missing", &ignore_missing, - N_("exit quietly with a zero exit code if the requested hook cannot be found")), + 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_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 <n> hooks simultaneously")), OPT_END(), }; + run_hooks_opt_init_async(&opt); + argc = parse_options(argc, argv, prefix, run_options, - builtin_hook_usage, PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH); - - if (argc > 2) { - if (strcmp(argv[2], "--") && - strcmp(argv[2], "--end-of-options")) - /* Having a -- for "run" is mandatory */ - usage_with_options(builtin_hook_usage, run_options); - /* Add our arguments, start after -- */ - for (i = 3 ; i < argc; i++) - strvec_push(&opt.args, argv[i]); - } + builtin_hook_usage, 0); - /* Need to take into account core.hooksPath */ - git_config(git_default_config, NULL); + if (argc < 1) + usage_msg_opt(_("You must specify a hook event to run."), + builtin_hook_usage, run_options); - hook_name = argv[1]; - hook_path = find_hook(hook_name); - if (!hook_path) { - if (ignore_missing) - return 0; - error("cannot find a hook named %s", hook_name); - return 1; - } - rc = run_found_hooks(hook_name, hook_path, &opt); + 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; @@ -61,12 +141,50 @@ static int run(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 (!strcmp(argv[1], "run")) + argc = parse_options(argc, argv, prefix, builtin_hook_options, + builtin_hook_usage, PARSE_OPT_KEEP_UNKNOWN); + + /* after the parse, we should have "<command> <hookname> <args...>" */ + if (argc < 2) + usage_with_options(builtin_hook_usage, builtin_hook_options); + + git_config(git_default_config, NULL); + + + /* argument > config */ + if (run_hookdir) + if (!strcmp(run_hookdir, "no")) + should_run_hookdir = HOOKDIR_NO; + else if (!strcmp(run_hookdir, "error")) + should_run_hookdir = HOOKDIR_ERROR; + 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 + /* + * TRANSLATORS: leave "yes/warn/interactive/no" + * untranslated; the strings are compared literally. + */ + 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); + if (!strcmp(argv[0], "run")) return run(argc, argv, prefix); + usage_with_options(builtin_hook_usage, builtin_hook_options); - return 1; } diff --git a/builtin/merge.c b/builtin/merge.c index a9363b94065..7a524cb3e35 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -13,7 +13,6 @@ #include "builtin.h" #include "lockfile.h" #include "run-command.h" -#include "hook.h" #include "diff.h" #include "diff-merges.h" #include "refs.h" @@ -44,6 +43,7 @@ #include "commit-reach.h" #include "wt-status.h" #include "commit-graph.h" +#include "hook.h" #define DEFAULT_TWOHEAD (1<<0) #define DEFAULT_OCTOPUS (1<<1) @@ -448,7 +448,7 @@ static void finish(struct commit *head_commit, const struct object_id *new_head, const char *msg) { struct strbuf reflog_message = STRBUF_INIT; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; const struct object_id *head = &head_commit->object.oid; if (!msg) @@ -490,6 +490,7 @@ static void finish(struct commit *head_commit, } /* Run a post-merge hook */ + run_hooks_opt_init_async(&opt); strvec_push(&opt.args, squash ? "1" : "0"); run_hooks("post-merge", &opt); run_hooks_opt_clear(&opt); @@ -845,14 +846,14 @@ static void prepare_to_commit(struct commit_list *remoteheads) struct strbuf msg = STRBUF_INIT; const char *index_file = get_index_file(); - if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL)) + if (!no_verify && run_commit_hook(0 < option_edit, 0, index_file, "pre-merge-commit", NULL)) abort_commit(remoteheads, NULL); /* * Re-read the index as pre-merge-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke * the editor and after we invoke run_status above. */ - if (hook_exists("pre-merge-commit")) + if (hook_exists("pre-merge-commit", HOOKDIR_USE_CONFIG)) discard_cache(); read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); @@ -873,7 +874,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0); write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len); - if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", + if (run_commit_hook(0 < option_edit, 0, get_index_file(), "prepare-commit-msg", git_path_merge_msg(the_repository), "merge", NULL)) abort_commit(remoteheads, NULL); if (0 < option_edit) { @@ -881,7 +882,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) abort_commit(remoteheads, NULL); } - if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(), + if (!no_verify && run_commit_hook(0 < option_edit, 0, get_index_file(), "commit-msg", git_path_merge_msg(the_repository), NULL)) abort_commit(remoteheads, NULL); diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index de00adbb9e0..6ded130e45b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -37,134 +37,6 @@ #include "shallow.h" #include "promisor-remote.h" -/* - * Objects we are going to pack are collected in the `to_pack` structure. - * It contains an array (dynamically expanded) of the object data, and a map - * that can resolve SHA1s to their position in the array. - */ -static struct packing_data to_pack; - -static inline struct object_entry *oe_delta( - const struct packing_data *pack, - const struct object_entry *e) -{ - if (!e->delta_idx) - return NULL; - if (e->ext_base) - return &pack->ext_bases[e->delta_idx - 1]; - else - return &pack->objects[e->delta_idx - 1]; -} - -static inline unsigned long oe_delta_size(struct packing_data *pack, - const struct object_entry *e) -{ - if (e->delta_size_valid) - return e->delta_size_; - - /* - * pack->delta_size[] can't be NULL because oe_set_delta_size() - * must have been called when a new delta is saved with - * oe_set_delta(). - * If oe_delta() returns NULL (i.e. default state, which means - * delta_size_valid is also false), then the caller must never - * call oe_delta_size(). - */ - return pack->delta_size[e - pack->objects]; -} - -unsigned long oe_get_size_slow(struct packing_data *pack, - const struct object_entry *e); - -static inline unsigned long oe_size(struct packing_data *pack, - const struct object_entry *e) -{ - if (e->size_valid) - return e->size_; - - return oe_get_size_slow(pack, e); -} - -static inline void oe_set_delta(struct packing_data *pack, - struct object_entry *e, - struct object_entry *delta) -{ - if (delta) - e->delta_idx = (delta - pack->objects) + 1; - else - e->delta_idx = 0; -} - -static inline struct object_entry *oe_delta_sibling( - const struct packing_data *pack, - const struct object_entry *e) -{ - if (e->delta_sibling_idx) - return &pack->objects[e->delta_sibling_idx - 1]; - return NULL; -} - -static inline struct object_entry *oe_delta_child( - const struct packing_data *pack, - const struct object_entry *e) -{ - if (e->delta_child_idx) - return &pack->objects[e->delta_child_idx - 1]; - return NULL; -} - -static inline void oe_set_delta_child(struct packing_data *pack, - struct object_entry *e, - struct object_entry *delta) -{ - if (delta) - e->delta_child_idx = (delta - pack->objects) + 1; - else - e->delta_child_idx = 0; -} - -static inline void oe_set_delta_sibling(struct packing_data *pack, - struct object_entry *e, - struct object_entry *delta) -{ - if (delta) - e->delta_sibling_idx = (delta - pack->objects) + 1; - else - e->delta_sibling_idx = 0; -} - -static inline void oe_set_size(struct packing_data *pack, - struct object_entry *e, - unsigned long size) -{ - if (size < pack->oe_size_limit) { - e->size_ = size; - e->size_valid = 1; - } else { - e->size_valid = 0; - if (oe_get_size_slow(pack, e) != size) - BUG("'size' is supposed to be the object size!"); - } -} - -static inline void oe_set_delta_size(struct packing_data *pack, - struct object_entry *e, - unsigned long size) -{ - if (size < pack->oe_delta_size_limit) { - e->delta_size_ = size; - e->delta_size_valid = 1; - } else { - packing_data_lock(pack); - if (!pack->delta_size) - ALLOC_ARRAY(pack->delta_size, pack->nr_alloc); - packing_data_unlock(pack); - - pack->delta_size[e - pack->objects] = size; - e->delta_size_valid = 0; - } -} - #define IN_PACK(obj) oe_in_pack(&to_pack, obj) #define SIZE(obj) oe_size(&to_pack, obj) #define SET_SIZE(obj,size) oe_set_size(&to_pack, obj, size) @@ -184,6 +56,13 @@ static const char *pack_usage[] = { NULL }; +/* + * Objects we are going to pack are collected in the `to_pack` structure. + * It contains an array (dynamically expanded) of the object data, and a map + * that can resolve SHA1s to their position in the array. + */ +static struct packing_data to_pack; + static struct pack_idx_entry **written_list; static uint32_t nr_result, nr_written, nr_seen; static struct bitmap_index *bitmap_git; @@ -422,17 +301,6 @@ static void copy_pack_data(struct hashfile *f, } } -static inline int oe_size_greater_than(struct packing_data *pack, - const struct object_entry *lhs, - unsigned long rhs) -{ - if (lhs->size_valid) - return lhs->size_ > rhs; - if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */ - return 1; - return oe_get_size_slow(pack, lhs) > rhs; -} - /* Return 0 if we will bust the pack-size limit */ static unsigned long write_no_reuse_object(struct hashfile *f, struct object_entry *entry, unsigned long limit, int usable_delta) @@ -774,14 +642,6 @@ static int mark_tagged(const char *path, const struct object_id *oid, int flag, return 0; } -static inline unsigned char oe_layer(struct packing_data *pack, - struct object_entry *e) -{ - if (!pack->layer) - return 0; - return pack->layer[e - pack->objects]; -} - static inline void add_to_write_order(struct object_entry **wo, unsigned int *endp, struct object_entry *e) @@ -2371,26 +2231,6 @@ static pthread_mutex_t progress_mutex; * progress_mutex for protection. */ -static inline int oe_size_less_than(struct packing_data *pack, - const struct object_entry *lhs, - unsigned long rhs) -{ - if (lhs->size_valid) - return lhs->size_ < rhs; - if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */ - return 0; - return oe_get_size_slow(pack, lhs) < rhs; -} - -static inline void oe_set_tree_depth(struct packing_data *pack, - struct object_entry *e, - unsigned int tree_depth) -{ - if (!pack->tree_depth) - CALLOC_ARRAY(pack->tree_depth, pack->nr_alloc); - pack->tree_depth[e - pack->objects] = tree_depth; -} - /* * Return the size of the object without doing any delta * reconstruction (so non-deltas are true object sizes, but deltas diff --git a/builtin/rebase.c b/builtin/rebase.c index 2081f6fa8db..fe9f144cad6 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -1314,7 +1314,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) char *squash_onto_name = NULL; int reschedule_failed_exec = -1; int allow_preemptive_ff = 1; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; struct option builtin_rebase_options[] = { OPT_STRING(0, "onto", &options.onto_name, N_("revision"), @@ -2024,6 +2024,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) } /* If a hook exists, give it a chance to interrupt*/ + run_hooks_opt_init_async(&hook_opt); strvec_pushl(&hook_opt.args, options.upstream_arg, argc ? argv[0] : NULL, NULL); if (!ok_to_skip_pre_rebase && run_hooks("pre-rebase", &hook_opt)) { diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index ec90e10477a..f44b58e456d 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -7,7 +7,6 @@ #include "pkt-line.h" #include "sideband.h" #include "run-command.h" -#include "hook.h" #include "exec-cmd.h" #include "commit.h" #include "object.h" @@ -30,6 +29,7 @@ #include "commit-reach.h" #include "worktree.h" #include "shallow.h" +#include "hook.h" static const char * const receive_pack_usage[] = { N_("git receive-pack <git-dir>"), @@ -910,11 +910,13 @@ static int run_receive_hook(struct command *commands, int skip_broken, const struct string_list *push_options) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; struct receive_hook_feed_context ctx; int rc; struct command *iter = commands; + run_hooks_opt_init_async(&opt); + /* if there are no valid commands, don't invoke the hook at all. */ while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) iter = iter->next; @@ -956,9 +958,11 @@ static int run_receive_hook(struct command *commands, static int run_update_hook(struct command *cmd) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; int code; + run_hooks_opt_init_async(&opt); + strvec_pushl(&opt.args, cmd->ref_name, oid_to_hex(&cmd->old_oid), @@ -1128,11 +1132,38 @@ static int run_proc_receive_hook(struct command *commands, int version = 0; int code; - argv[0] = find_hook("proc-receive"); - if (!argv[0]) { + struct hook *proc_receive = NULL; + struct list_head *pos, *hooks; + + hooks = hook_list("proc-receive"); + + list_for_each(pos, hooks) { + if (proc_receive) { + rp_error("only one 'proc-receive' hook can be specified"); + return -1; + } + proc_receive = list_entry(pos, struct hook, list); + /* check if the hookdir hook should be ignored */ + if (proc_receive->from_hookdir) { + switch (configured_hookdir_opt()) { + case HOOKDIR_INTERACTIVE: + case HOOKDIR_NO: + proc_receive = NULL; + break; + default: + break; + } + } + + } + + if (!proc_receive) { rp_error("cannot find hook 'proc-receive'"); return -1; } + + + argv[0] = proc_receive->command.buf; argv[1] = NULL; proc.argv = argv; @@ -1442,7 +1473,9 @@ static const char *push_to_checkout(unsigned char *hash, struct strvec *env, const char *work_tree) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; + + run_hooks_opt_init_sync(&opt); strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); strvec_pushv(&opt.env, env->v); @@ -1477,7 +1510,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w strvec_pushf(&env, "GIT_DIR=%s", absolute_path(git_dir)); - if (!hook_exists(push_to_checkout_hook)) + if (!hook_exists(push_to_checkout_hook, HOOKDIR_USE_CONFIG)) retval = push_to_deploy(sha1, &env, work_tree); else retval = push_to_checkout(sha1, &env, work_tree); @@ -1640,7 +1673,9 @@ static const char *update(struct command *cmd, struct shallow_info *si) static void run_update_post_hook(struct command *commands) { struct command *cmd; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; + + run_hooks_opt_init_async(&opt); for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) diff --git a/builtin/worktree.c b/builtin/worktree.c index 2ad26a76f4c..017b2cfcb58 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -8,12 +8,12 @@ #include "branch.h" #include "refs.h" #include "run-command.h" -#include "hook.h" #include "sigchain.h" #include "submodule.h" #include "utf8.h" #include "worktree.h" #include "quote.h" +#include "hook.h" static const char * const worktree_usage[] = { N_("git worktree add [<options>] <path> [<commit-ish>]"), @@ -382,7 +382,9 @@ static int add_worktree(const char *path, const char *refname, * is_junk is cleared, but do return appropriate code when hook fails. */ if (!ret && opts->checkout) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; + + run_hooks_opt_init_sync(&opt); strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); strvec_pushl(&opt.args, @@ -391,7 +393,6 @@ static int add_worktree(const char *path, const char *refname, "1", NULL); opt.dir = path; - opt.absolute_path = 1; ret = run_hooks("post-checkout", &opt); diff --git a/commit.c b/commit.c index e8147a88fc6..0da5b7e7f1a 100644 --- a/commit.c +++ b/commit.c @@ -1696,13 +1696,19 @@ size_t ignore_non_trailer(const char *buf, size_t len) return boc ? len - boc : len - cutoff; } -int run_commit_hook(int editor_is_used, const char *index_file, +int run_commit_hook(int editor_is_used, int parallelize, const char *index_file, const char *name, ...) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; va_list args; const char *arg; int ret; + + run_hooks_opt_init_sync(&opt); + + if (parallelize) + opt.jobs = configured_hook_jobs(); + strvec_pushf(&opt.env, "GIT_INDEX_FILE=%s", index_file); /* diff --git a/commit.h b/commit.h index df42eb434f3..a90c094ec27 100644 --- a/commit.h +++ b/commit.h @@ -363,7 +363,8 @@ int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused) int compare_commits_by_gen_then_commit_date(const void *a_, const void *b_, void *unused); LAST_ARG_MUST_BE_NULL -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...); +int run_commit_hook(int editor_is_used, int parallelize, const char *index_file, + const char *name, ...); /* Sign a commit or tag buffer, storing the result in a header. */ int sign_with_header(struct strbuf *buf, const char *keyid); diff --git a/git-p4.py b/git-p4.py index e76d8df3139..7d770957719 100755 --- a/git-p4.py +++ b/git-p4.py @@ -207,12 +207,15 @@ def decode_path(path): return path def run_git_hook(cmd, param=[]): + """Execute a hook if the hook exists.""" + if not cmd: + return True + """args are specified with -a <arg> -a <arg> -a <arg>""" - args = ['git', 'hook', 'run', '--ignore-missing', cmd] - if param: - args.append("--") - for p in param: - args.append(p) + args = (['git', 'hook', 'run'] + + ["-a" + arg for arg in param] + + [cmd]) + return subprocess.call(args) == 0 def write_pipe(c, stdin): diff --git a/git-send-email.perl b/git-send-email.perl index 2ab8dfdbded..b55687453e0 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -213,13 +213,13 @@ sub format_2822_time { my $editor; sub system_or_msg { - my ($args, $msg, $cmd_name) = @_; + my ($args, $msg) = @_; system(@$args); my $signalled = $? & 127; my $exit_code = $? >> 8; return unless $signalled or $exit_code; - my @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code); + my @sprintf_args = ($args->[0], $exit_code); if (defined $msg) { # Quiet the 'redundant' warning category, except we # need to support down to Perl 5.8, so we can't do a @@ -1958,31 +1958,12 @@ sub unique_email_list { sub validate_patch { my ($fn, $xfer_encoding) = @_; - if ($repo) { - my $hook_name = 'sendemail-validate'; - my $hooks_path = $repo->command_oneline('rev-parse', '--git-path', 'hooks'); - my $validate_hook = catfile($hooks_path, $hook_name); - my $hook_error; - if (-x $validate_hook) { - my $target = abs_path($fn); - # The hook needs a correct cwd and GIT_DIR. - my $cwd_save = cwd(); - chdir($repo->wc_path() or $repo->repo_path()) - or die("chdir: $!"); - local $ENV{"GIT_DIR"} = $repo->repo_path(); - my @validate_hook = ("git", "hook", "run", "--ignore-missing", $hook_name, "--", $target); - $hook_error = system_or_msg(\@validate_hook, undef, - "git hook run $hook_name -- <patch>"); - chdir($cwd_save) or die("chdir: $!"); - } - if ($hook_error) { - $hook_error = sprintf(__("fatal: %s: rejected by %s hook\n" . - $hook_error . "\n" . - "warning: no patches were sent\n"), - $fn, $hook_name); - die $hook_error; - } - } + my $target = abs_path($fn); + + system_or_die(["git", "hook", "run", "sendemail-validate", "-j1", "-a", $target], + sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" . + "warning: no patches were sent\n"), + $fn)); # Any long lines will be automatically fixed if we use a suitable transfer # encoding. diff --git a/git.c b/git.c index 540909c391f..39988ee3b02 100644 --- a/git.c +++ b/git.c @@ -538,7 +538,7 @@ static struct cmd_struct commands[] = { { "grep", cmd_grep, RUN_SETUP_GENTLY }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, - { "hook", cmd_hook, RUN_SETUP }, + { "hook", cmd_hook, RUN_SETUP_GENTLY }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, diff --git a/hook.c b/hook.c index 17ae65eca31..ff80e52eddd 100644 --- a/hook.c +++ b/hook.c @@ -1,8 +1,224 @@ #include "cache.h" + #include "hook.h" +#include "config.h" #include "run-command.h" +#include "prompt.h" + +void free_hook(struct hook *ptr) +{ + if (ptr) { + strbuf_release(&ptr->command); + free(ptr->feed_pipe_cb_data); + } + free(ptr); +} + +static struct hook * find_hook_by_command(struct list_head *head, const char *command) +{ + struct list_head *pos = NULL, *tmp = NULL; + struct hook *found = NULL; + + list_for_each_safe(pos, tmp, head) { + struct hook *it = list_entry(pos, struct hook, list); + if (!strcmp(it->command.buf, command)) { + list_del(pos); + found = it; + break; + } + } + 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 */ + to_add = xmalloc(sizeof(*to_add)); + 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 */ + to_add->origin = current_config_scope(); + + list_add_tail(&to_add->list, head); +} + +static void remove_hook(struct list_head *to_remove) +{ + struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); + 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; + 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. If it doesn't, + * 'git_config_get_value()' is documented not to touch &command, + * so we don't need to do anything. + */ + strbuf_reset(&hookcmd_name); + 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) + */ + + 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); + } + + 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, "error")) + return HOOKDIR_ERROR; + + if (!strcmp(key, "yes")) + return HOOKDIR_YES; + + if (!strcmp(key, "warn")) + return HOOKDIR_WARN; + + if (!strcmp(key, "interactive")) + return HOOKDIR_INTERACTIVE; + + 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; + /* + * 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_ERROR: + fprintf(stderr, _("Skipping legacy hook at '%s'\n"), + path); + /* FALLTHROUGH */ + case HOOKDIR_NO: + return 0; + 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'? [Y/n] "), path); + git_read_line_interactively(&prompt); + /* + * In case of prompt = '' - that is, user hit enter, + * saying "yes I want the default" - strncasecmp will + * return 0 regardless. So list the default first. + * + * Case insensitively, accept "y", "ye", or "yes" as + * "yes"; accept "n" or "no" as "no". + */ + if (!strncasecmp(prompt.buf, "yes", prompt.len)) { + strbuf_release(&prompt); + return 1; + } else if (!strncasecmp(prompt.buf, "no", prompt.len)) { + strbuf_release(&prompt); + return 0; + } + /* otherwise, we didn't understand the input */ + } while (prompt.len); /* an empty reply means default (yes) */ + return 1; + /* + * HOOKDIR_UNKNOWN should match the default behavior, but let's + * give a heads up to the user. + */ + case HOOKDIR_UNKNOWN: + fprintf(stderr, + _("Unrecognized value for 'hook.runHookDir'. " + "Is there a typo? ")); + /* FALLTHROUGH */ + case HOOKDIR_YES: + default: + return 1; + } +} -const char *find_hook(const char *name) +static const char *find_legacy_hook(const char *name) { static struct strbuf path = STRBUF_INIT; @@ -36,9 +252,77 @@ const char *find_hook(const char *name) return path.buf; } -int hook_exists(const char *name) + +struct list_head* hook_list(const char* 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); + + git_config(hook_config_lookup, &cb_data); + + if (have_git_dir()) { + const char *legacy_hook_path = find_legacy_hook(hookname); + + /* 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; +} + +void run_hooks_opt_init_sync(struct run_hooks_opt *o) { - return !!find_hook(name); + strvec_init(&o->env); + strvec_init(&o->args); + o->path_to_stdin = NULL; + o->run_hookdir = configured_hookdir_opt(); + o->jobs = 1; + o->dir = NULL; + o->feed_pipe = NULL; + o->feed_pipe_ctx = NULL; + o->consume_sideband = NULL; +} + +void run_hooks_opt_init_async(struct run_hooks_opt *o) +{ + run_hooks_opt_init_sync(o); + o->jobs = configured_hook_jobs(); +} + +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; + + if (should_run_hookdir == HOOKDIR_USE_CONFIG) + should_run_hookdir = configured_hookdir_opt(); + + could_run_hookdir = (should_run_hookdir == HOOKDIR_INTERACTIVE || + should_run_hookdir == HOOKDIR_WARN || + should_run_hookdir == HOOKDIR_YES) + && !!find_legacy_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) @@ -51,8 +335,7 @@ 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 hook_cb_data *hook_cb = pp_cb; - struct string_list *to_pipe = hook_cb->options->feed_pipe_ctx; + struct string_list *to_pipe = ((struct hook_cb_data*)pp_cb)->options->feed_pipe_ctx; /* Bootstrap the state manager if necessary. */ if (!ctx->feed_pipe_cb_data) { @@ -76,10 +359,10 @@ static int pick_next_hook(struct child_process *cp, void **pp_task_cb) { struct hook_cb_data *hook_cb = pp_cb; - struct hook *run_me = hook_cb->run_me; + struct hook *hook = hook_cb->run_me; - if (!run_me) - BUG("did we not return 1 in notify_hook_finished?"); + if (!hook) + return 0; /* reopen the file for stdin; run_command closes it. */ if (hook_cb->options->path_to_stdin) { @@ -92,13 +375,20 @@ static int pick_next_hook(struct child_process *cp, } else { cp->no_stdin = 1; } + cp->env = hook_cb->options->env.v; cp->stdout_to_stderr = 1; - cp->trace2_hook_name = hook_cb->hook_name; + cp->trace2_hook_name = hook_cb->hookname; cp->dir = hook_cb->options->dir; + /* + * Commands from the config could be oneliners, but we know + * for certain that hookdir commands are not. + */ + cp->use_shell = !hook->from_hookdir; + /* add command */ - strvec_push(&cp->args, run_me->hook_path); + strvec_push(&cp->args, hook->command.buf); /* * add passed-in argv, without expanding - let the user get back @@ -107,7 +397,14 @@ static int pick_next_hook(struct child_process *cp, strvec_pushv(&cp->args, hook_cb->options->args.v); /* Provide context for errors if necessary */ - *pp_task_cb = run_me; + *pp_task_cb = hook; + + /* Get the next entry ready */ + if (hook_cb->run_me->list.next == hook_cb->head) + hook_cb->run_me = NULL; + else + hook_cb->run_me = list_entry(hook_cb->run_me->list.next, + struct hook, list); return 1; } @@ -122,10 +419,13 @@ static int notify_start_failure(struct strbuf *out, /* |= rc in cb */ hook_cb->rc |= 1; - strbuf_addf(out, _("Couldn't start hook '%s'\n"), - attempted->hook_path); + strbuf_addf(out, _("Couldn't start '%s', configured in '%s'\n"), + attempted->command.buf, + attempted->from_hookdir ? "hookdir" + : config_scope_name(attempted->origin)); - return 1; + /* NEEDSWORK: if halt_on_error is desired, do it here. */ + return 0; } static int notify_hook_finished(int result, @@ -138,29 +438,36 @@ static int notify_hook_finished(int result, /* |= rc in cb */ hook_cb->rc |= result; - return 1; + /* NEEDSWORK: if halt_on_error is desired, do it here. */ + return 0; } -int run_found_hooks(const char *hook_name, const char *hook_path, - struct run_hooks_opt *options) -{ - struct strbuf abs_path = STRBUF_INIT; - struct hook my_hook = { - .hook_path = hook_path, - }; - struct hook_cb_data cb_data = { - .rc = 0, - .hook_name = hook_name, - .options = options, - }; - if (options->absolute_path) { - strbuf_add_absolute_path(&abs_path, hook_path); - my_hook.hook_path = abs_path.buf; +int run_hooks(const char *hookname, struct run_hooks_opt *options) +{ + struct list_head *to_run, *pos = NULL, *tmp = NULL; + struct hook_cb_data cb_data = { 0, hookname, NULL, NULL, options }; + + if (!options) + BUG("a struct run_hooks_opt must be provided to run_hooks"); + + if (options->path_to_stdin && options->feed_pipe) + BUG("choose only one method to populate stdin"); + + to_run = hook_list(hookname); + + list_for_each_safe(pos, tmp, to_run) { + struct hook *hook = list_entry(pos, struct hook, list); + + if (hook->from_hookdir && + !should_include_hookdir(hook->command.buf, options->run_hookdir)) + list_del(pos); } - cb_data.run_me = &my_hook; - if (options->jobs != 1) - BUG("we do not handle %d or any other != 1 job number yet", options->jobs); + if (list_empty(to_run)) + return 0; + + cb_data.head = to_run; + cb_data.run_me = list_entry(to_run->next, struct hook, list); run_processes_parallel_tr2(options->jobs, pick_next_hook, @@ -170,29 +477,7 @@ int run_found_hooks(const char *hook_name, const char *hook_path, notify_hook_finished, &cb_data, "hook", - hook_name); - if (options->absolute_path) - strbuf_release(&abs_path); + hookname); return cb_data.rc; } - -int run_hooks(const char *hook_name, struct run_hooks_opt *options) -{ - const char *hook_path; - int ret; - if (!options) - BUG("a struct run_hooks_opt must be provided to run_hooks"); - - if (options->path_to_stdin && options->feed_pipe) - BUG("choose only one method to populate stdin"); - - hook_path = find_hook(hook_name); - - /* Care about nonexistence? Use run_found_hooks() */ - if (!hook_path) - return 0; - - ret = run_found_hooks(hook_name, hook_path, options); - return ret; -} diff --git a/hook.h b/hook.h index 5f895032341..f32189380a8 100644 --- a/hook.h +++ b/hook.h @@ -1,12 +1,19 @@ -#ifndef HOOK_H -#define HOOK_H +#include "config.h" +#include "list.h" #include "strbuf.h" #include "strvec.h" #include "run-command.h" struct hook { - /* The path to the hook */ - const char *hook_path; + struct list_head list; + /* + * Config file which holds the hook.*.command definition. + * (This has nothing to do with the hookcmd.<name>.* configs.) + */ + enum config_scope origin; + /* The literal command to run. */ + struct strbuf command; + unsigned from_hookdir : 1; /* * Use this to keep state for your feed_pipe_fn if you are using @@ -15,6 +22,32 @@ struct hook { void *feed_pipe_cb_data; }; +/* + * Provides a linked list of 'struct hook' detailing commands which should run + * in response to the 'hookname' event, in execution order. + */ +struct list_head* hook_list(const char *hookname); + +enum hookdir_opt +{ + HOOKDIR_USE_CONFIG, + HOOKDIR_NO, + HOOKDIR_ERROR, + 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); + +/* 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 */ @@ -23,20 +56,15 @@ struct run_hooks_opt /* Args to be passed to each hook */ struct strvec args; - /* Number of threads to parallelize across */ - int jobs; - - /* Resolve and run the "absolute_path(hook)" instead of - * "hook". Used for "git worktree" hooks + /* + * 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. */ - int absolute_path; - - /* Path to initial working directory for subprocess */ - const char *dir; + enum hookdir_opt run_hookdir; /* Path to file which should be piped to stdin for each hook */ const char *path_to_stdin; - /* * Callback and state pointer to ask for more content to pipe to stdin. * Will be called repeatedly, for each hook. See @@ -57,13 +85,14 @@ struct run_hooks_opt * for an example. */ consume_sideband_fn consume_sideband; -}; -#define RUN_HOOKS_OPT_INIT { \ - .jobs = 1, \ - .env = STRVEC_INIT, \ - .args = STRVEC_INIT, \ -} + /* Number of threads to parallelize across */ + int jobs; + + /* Path to initial working directory for subprocess */ + const char *dir; + +}; /* * To specify a 'struct string_list', set 'run_hooks_opt.feed_pipe_ctx' to the @@ -78,35 +107,33 @@ int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb); */ struct hook_cb_data { int rc; - const char *hook_name; + const char *hookname; + struct list_head *head; struct hook *run_me; struct run_hooks_opt *options; }; -/* - * Returns the path to the hook file, or NULL if the hook is missing - * or disabled. Note that this points to static storage that will be - * overwritten by further calls to find_hook and run_hook_*. - */ -const char *find_hook(const char *name); - -/* - * A boolean version of find_hook() - */ -int hook_exists(const char *hookname); - +void run_hooks_opt_init_sync(struct run_hooks_opt *o); +void run_hooks_opt_init_async(struct run_hooks_opt *o); void run_hooks_opt_clear(struct run_hooks_opt *o); /* - * Calls find_hook(hookname) and runs the hooks (if any) with - * run_found_hooks(). + * 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 run_hooks(const char *hook_name, struct run_hooks_opt *options); +int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir); /* - * Takes an already resolved hook and runs it. Internally the simpler - * run_hooks() will call this. + * Runs all hooks associated to the 'hookname' event in order. Each hook will be + * passed 'env' and 'args'. The file at 'stdin_path' will be closed and reopened + * for each hook that runs. */ -int run_found_hooks(const char *hookname, const char *hook_path, - struct run_hooks_opt *options); -#endif +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 */ +void clear_hook_list(struct list_head *head); diff --git a/pack-objects.h b/pack-objects.h index dca2351ef94..9d88e3e518f 100644 --- a/pack-objects.h +++ b/pack-objects.h @@ -268,10 +268,152 @@ static inline void oe_set_in_pack(struct packing_data *pack, pack->in_pack[e - pack->objects] = p; } +static inline struct object_entry *oe_delta( + const struct packing_data *pack, + const struct object_entry *e) +{ + if (!e->delta_idx) + return NULL; + if (e->ext_base) + return &pack->ext_bases[e->delta_idx - 1]; + else + return &pack->objects[e->delta_idx - 1]; +} + +static inline void oe_set_delta(struct packing_data *pack, + struct object_entry *e, + struct object_entry *delta) +{ + if (delta) + e->delta_idx = (delta - pack->objects) + 1; + else + e->delta_idx = 0; +} + void oe_set_delta_ext(struct packing_data *pack, struct object_entry *e, const struct object_id *oid); +static inline struct object_entry *oe_delta_child( + const struct packing_data *pack, + const struct object_entry *e) +{ + if (e->delta_child_idx) + return &pack->objects[e->delta_child_idx - 1]; + return NULL; +} + +static inline void oe_set_delta_child(struct packing_data *pack, + struct object_entry *e, + struct object_entry *delta) +{ + if (delta) + e->delta_child_idx = (delta - pack->objects) + 1; + else + e->delta_child_idx = 0; +} + +static inline struct object_entry *oe_delta_sibling( + const struct packing_data *pack, + const struct object_entry *e) +{ + if (e->delta_sibling_idx) + return &pack->objects[e->delta_sibling_idx - 1]; + return NULL; +} + +static inline void oe_set_delta_sibling(struct packing_data *pack, + struct object_entry *e, + struct object_entry *delta) +{ + if (delta) + e->delta_sibling_idx = (delta - pack->objects) + 1; + else + e->delta_sibling_idx = 0; +} + +unsigned long oe_get_size_slow(struct packing_data *pack, + const struct object_entry *e); +static inline unsigned long oe_size(struct packing_data *pack, + const struct object_entry *e) +{ + if (e->size_valid) + return e->size_; + + return oe_get_size_slow(pack, e); +} + +static inline int oe_size_less_than(struct packing_data *pack, + const struct object_entry *lhs, + unsigned long rhs) +{ + if (lhs->size_valid) + return lhs->size_ < rhs; + if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */ + return 0; + return oe_get_size_slow(pack, lhs) < rhs; +} + +static inline int oe_size_greater_than(struct packing_data *pack, + const struct object_entry *lhs, + unsigned long rhs) +{ + if (lhs->size_valid) + return lhs->size_ > rhs; + if (rhs < pack->oe_size_limit) /* rhs < 2^x <= lhs ? */ + return 1; + return oe_get_size_slow(pack, lhs) > rhs; +} + +static inline void oe_set_size(struct packing_data *pack, + struct object_entry *e, + unsigned long size) +{ + if (size < pack->oe_size_limit) { + e->size_ = size; + e->size_valid = 1; + } else { + e->size_valid = 0; + if (oe_get_size_slow(pack, e) != size) + BUG("'size' is supposed to be the object size!"); + } +} + +static inline unsigned long oe_delta_size(struct packing_data *pack, + const struct object_entry *e) +{ + if (e->delta_size_valid) + return e->delta_size_; + + /* + * pack->delta_size[] can't be NULL because oe_set_delta_size() + * must have been called when a new delta is saved with + * oe_set_delta(). + * If oe_delta() returns NULL (i.e. default state, which means + * delta_size_valid is also false), then the caller must never + * call oe_delta_size(). + */ + return pack->delta_size[e - pack->objects]; +} + +static inline void oe_set_delta_size(struct packing_data *pack, + struct object_entry *e, + unsigned long size) +{ + if (size < pack->oe_delta_size_limit) { + e->delta_size_ = size; + e->delta_size_valid = 1; + } else { + packing_data_lock(pack); + if (!pack->delta_size) + ALLOC_ARRAY(pack->delta_size, pack->nr_alloc); + packing_data_unlock(pack); + + pack->delta_size[e - pack->objects] = size; + e->delta_size_valid = 0; + } +} + static inline unsigned int oe_tree_depth(struct packing_data *pack, struct object_entry *e) { @@ -280,6 +422,23 @@ static inline unsigned int oe_tree_depth(struct packing_data *pack, return pack->tree_depth[e - pack->objects]; } +static inline void oe_set_tree_depth(struct packing_data *pack, + struct object_entry *e, + unsigned int tree_depth) +{ + if (!pack->tree_depth) + CALLOC_ARRAY(pack->tree_depth, pack->nr_alloc); + pack->tree_depth[e - pack->objects] = tree_depth; +} + +static inline unsigned char oe_layer(struct packing_data *pack, + struct object_entry *e) +{ + if (!pack->layer) + return 0; + return pack->layer[e - pack->objects]; +} + static inline void oe_set_layer(struct packing_data *pack, struct object_entry *e, unsigned char layer) diff --git a/parse-options-cb.c b/parse-options-cb.c index 3c811e1e4a7..8227499eb6f 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 a845a9d9527..fcb0f1f31eb 100644 --- a/parse-options.h +++ b/parse-options.h @@ -178,6 +178,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) \ @@ -297,6 +300,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 *, diff --git a/perl/Git.pm b/perl/Git.pm index 02eacef0c2a..73ebbf80cc6 100644 --- a/perl/Git.pm +++ b/perl/Git.pm @@ -619,6 +619,19 @@ sub _prompt { sub repo_path { $_[0]->{opts}->{Repository} } +=item hooks_path () + +Return path to the hooks directory. Must be called on a repository instance. + +=cut + +sub hooks_path { + my ($self) = @_; + + my $dir = $self->command_oneline('rev-parse', '--git-path', 'hooks'); + my $abs = abs_path($dir); + return $abs; +} =item wc_path () diff --git a/read-cache.c b/read-cache.c index a17bc30f870..ebb9c190562 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3132,7 +3132,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l { int ret; int was_full = !istate->sparse_index; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; ret = convert_to_sparse(istate); @@ -3161,6 +3161,7 @@ static int do_write_locked_index(struct index_state *istate, struct lock_file *l else ret = close_lock_file_gently(lock); + run_hooks_opt_init_async(&hook_opt); strvec_pushl(&hook_opt.args, istate->updated_workdir ? "1" : "0", istate->updated_skipworktree ? "1" : "0", diff --git a/refs.c b/refs.c index 1149e7e7dcb..32e993aaff3 100644 --- a/refs.c +++ b/refs.c @@ -10,7 +10,6 @@ #include "refs.h" #include "refs/refs-internal.h" #include "run-command.h" -#include "hook.h" #include "object-store.h" #include "object.h" #include "tag.h" @@ -19,6 +18,7 @@ #include "strvec.h" #include "repository.h" #include "sigchain.h" +#include "hook.h" /* * List of all available backends @@ -2063,12 +2063,14 @@ static int run_transaction_hook(struct ref_transaction *transaction, const char *state) { struct strbuf buf = STRBUF_INIT; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; struct string_list to_stdin = STRING_LIST_INIT_DUP; int ret = 0, i; char o[GIT_MAX_HEXSZ + 1], n[GIT_MAX_HEXSZ + 1]; - if (!hook_exists("reference-transaction")) + run_hooks_opt_init_async(&opt); + + if (!hook_exists("reference-transaction", HOOKDIR_USE_CONFIG)) return ret; strvec_push(&opt.args, state); diff --git a/reset.c b/reset.c index e6af33b901c..48d45f5b792 100644 --- a/reset.c +++ b/reset.c @@ -128,7 +128,9 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action, reflog_head); } if (run_hook) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; + + run_hooks_opt_init_sync(&opt); strvec_pushl(&opt.args, oid_to_hex(orig ? orig : null_oid()), oid_to_hex(oid), diff --git a/run-command.c b/run-command.c index 4a1a7a10820..2ff76f3c2f1 100644 --- a/run-command.c +++ b/run-command.c @@ -8,7 +8,6 @@ #include "string-list.h" #include "quote.h" #include "config.h" -#include "hook.h" void child_process_init(struct child_process *child) { diff --git a/sequencer.c b/sequencer.c index ec2761e47d9..3fa76687639 100644 --- a/sequencer.c +++ b/sequencer.c @@ -8,7 +8,6 @@ #include "sequencer.h" #include "tag.h" #include "run-command.h" -#include "hook.h" #include "exec-cmd.h" #include "utf8.h" #include "cache-tree.h" @@ -35,6 +34,7 @@ #include "commit-reach.h" #include "rebase-interactive.h" #include "reset.h" +#include "hook.h" #include "string-list.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ -1148,11 +1148,13 @@ int update_head_with_reflog(const struct commit *old_head, static int run_rewrite_hook(const struct object_id *oldoid, const struct object_id *newoid) { - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; struct strbuf tmp = STRBUF_INIT; struct string_list to_stdin = STRING_LIST_INIT_DUP; int code; + run_hooks_opt_init_async(&opt); + strvec_push(&opt.args, "amend"); strbuf_addf(&tmp, @@ -1204,7 +1206,7 @@ static int run_prepare_commit_msg_hook(struct repository *r, } else { arg1 = "message"; } - if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name, + if (run_commit_hook(0, 0, r->index_file, "prepare-commit-msg", name, arg1, arg2, NULL)) ret = error(_("'prepare-commit-msg' hook failed")); @@ -1442,7 +1444,7 @@ static int try_to_commit(struct repository *r, } } - if (hook_exists("prepare-commit-msg")) { + if (hook_exists("prepare-commit-msg", HOOKDIR_USE_CONFIG)) { res = run_prepare_commit_msg_hook(r, msg, hook_commit); if (res) goto out; @@ -1534,7 +1536,7 @@ static int try_to_commit(struct repository *r, goto out; } - run_commit_hook(0, r->index_file, "post-commit", NULL); + run_commit_hook(0, 1, r->index_file, "post-commit", NULL); if (flags & AMEND_MSG) commit_post_rewrite(r, current_head, oid); @@ -4524,7 +4526,9 @@ static int pick_commits(struct repository *r, if (!stat(rebase_path_rewritten_list(), &st) && st.st_size > 0) { struct child_process notes_cp = CHILD_PROCESS_INIT; - struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt hook_opt; + + run_hooks_opt_init_async(&hook_opt); notes_cp.in = open(rebase_path_rewritten_list(), O_RDONLY); notes_cp.git_cmd = 1; diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c index 2051ce57db7..922af561567 100644 --- a/t/helper/test-parse-options.c +++ b/t/helper/test-parse-options.c @@ -2,6 +2,7 @@ #include "cache.h" #include "parse-options.h" #include "string-list.h" +#include "strvec.h" #include "trace2.h" static int boolean = 0; @@ -15,6 +16,7 @@ static char *string = NULL; static char *file = NULL; static int ambiguous; static struct string_list list = STRING_LIST_INIT_NODUP; +static struct strvec vector = STRVEC_INIT; static struct { int called; @@ -133,6 +135,7 @@ int cmd__parse_options(int argc, const char **argv) OPT_STRING('o', NULL, &string, "str", "get another string"), OPT_NOOP_NOARG(0, "obsolete"), OPT_STRING_LIST(0, "list", &list, "str", "add str to list"), + OPT_STRVEC(0, "vector", &vector, "str", "add str to strvec"), OPT_GROUP("Magic arguments"), OPT_ARGUMENT("quux", NULL, "means --quux"), OPT_NUMBER_CALLBACK(&integer, "set integer to NUM", @@ -183,6 +186,9 @@ int cmd__parse_options(int argc, const char **argv) for (i = 0; i < list.nr; i++) show(&expect, &ret, "list: %s", list.items[i].string); + for (i = 0; i < vector.nr; i++) + show(&expect, &ret, "vector: %s", vector.v[i]); + for (i = 0; i < argc; i++) show(&expect, &ret, "arg %02d: %s", i, argv[i]); diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index ad4746d899a..485e0170bff 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -35,6 +35,7 @@ String options --st <st> get another string (pervert ordering) -o <str> get another string --list <str> add str to list + --vector <str> add str to strvec Magic arguments --quux means --quux @@ -386,6 +387,32 @@ test_expect_success '--no-list resets list' ' test_cmp expect output ' +cat >expect <<\EOF +boolean: 0 +integer: 0 +magnitude: 0 +timestamp: 0 +string: (not set) +abbrev: 7 +verbose: -1 +quiet: 0 +dry run: no +file: (not set) +vector: foo +vector: bar +vector: baz +EOF +test_expect_success '--vector keeps list of strings' ' + test-tool parse-options --vector foo --vector=bar --vector=baz >output && + test_cmp expect output +' + +test_expect_success '--no-vector resets list' ' + test-tool parse-options --vector=other --vector=irrelevant --vector=options \ + --no-vector --vector=foo --vector=bar --vector=baz >output && + test_cmp expect output +' + test_expect_success 'multiple quiet levels' ' test-tool parse-options --expect="quiet: 3" -q -q -q ' diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index e9a815ca7aa..12e6c453024 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -106,18 +106,18 @@ init_repos () { run_on_sparse () { ( cd sparse-checkout && - GIT_PROGRESS_DELAY=100000 "$@" >../sparse-checkout-out 2>../sparse-checkout-err + "$@" >../sparse-checkout-out 2>../sparse-checkout-err ) && ( cd sparse-index && - GIT_PROGRESS_DELAY=100000 "$@" >../sparse-index-out 2>../sparse-index-err + "$@" >../sparse-index-out 2>../sparse-index-err ) } run_on_all () { ( cd full-checkout && - GIT_PROGRESS_DELAY=100000 "$@" >../full-checkout-out 2>../full-checkout-err + "$@" >../full-checkout-out 2>../full-checkout-err ) && run_on_sparse "$@" } diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh index fa9647a7c0b..f1f9aee9f5d 100755 --- a/t/t1350-config-hooks-path.sh +++ b/t/t1350-config-hooks-path.sh @@ -5,7 +5,6 @@ test_description='Test the core.hooksPath configuration variable' . ./test-lib.sh test_expect_success 'set up a pre-commit hook in core.hooksPath' ' - >actual && mkdir -p .git/custom-hooks .git/hooks && write_script .git/custom-hooks/pre-commit <<-\EOF && echo CUSTOM >>actual diff --git a/t/t1360-config-based-hooks.sh b/t/t1360-config-based-hooks.sh new file mode 100755 index 00000000000..43917172d74 --- /dev/null +++ b/t/t1360-config-based-hooks.sh @@ -0,0 +1,329 @@ +#!/bin/bash + +test_description='config-managed multihooks, including git-hook command' + +. ./test-lib.sh + +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 +} + +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 +' + + +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_expect_success 'git hook list shows hooks from the hookdir' ' + setup_hookdir && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit + EOF + + git hook list pre-commit >actual && + test_cmp expected actual +' + +test_expect_success 'hook.runHookDir = no is respected by list' ' + setup_hookdir && + + test_config hook.runHookDir "no" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit (will not run) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_cmp expected actual && + + git hook run pre-commit 2>actual && + test_must_be_empty actual +' + +test_expect_success 'hook.runHookDir = error is respected by list' ' + setup_hookdir && + + test_config hook.runHookDir "error" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit (will error and not run) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_cmp expected actual && + + cat >expected <<-EOF && + Skipping legacy hook at '\''$(pwd)/.git/hooks/pre-commit'\'' + EOF + + git hook run pre-commit 2>actual && + test_cmp 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 but run) + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_cmp 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_cmp 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_cmp expected actual +' + +test_expect_success 'git hook list ignores skip referring to unused hookcmd' ' + test_config hookcmd.abc.command "/path/abc" --add && + 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_cmp 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 and run' ' + 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_cmp 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 'git hook run can pass args and env vars' ' + write_script sample-hook.sh <<-\EOF && + echo $1 + echo $2 + echo $TEST_ENV_1 + echo $TEST_ENV_2 + EOF + + test_config hook.pre-commit.command "\"$(pwd)/sample-hook.sh\"" && + + cat >expected <<-EOF && + arg1 + arg2 + env1 + env2 + EOF + + git hook run --arg arg1 \ + --env TEST_ENV_1=env1 \ + -a arg2 \ + -e TEST_ENV_2=env2 \ + 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_expect_success 'hook.runHookDir is tolerant to unknown values' ' + setup_hookdir && + + test_config hook.runHookDir "junk" && + + cat >expected <<-EOF && + hookdir: $(pwd)/.git/hooks/pre-commit + EOF + + git hook list pre-commit >actual && + # the hookdir annotation is translated + test_cmp expected actual +' + +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 diff --git a/t/t1416-ref-transaction-hooks.sh b/t/t1416-ref-transaction-hooks.sh index 6c941027a81..0a3c3e4a861 100755 --- a/t/t1416-ref-transaction-hooks.sh +++ b/t/t1416-ref-transaction-hooks.sh @@ -124,12 +124,12 @@ test_expect_success 'interleaving hook calls succeed' ' EOF cat >expect <<-EOF && - hooks/update refs/tags/PRE $ZERO_OID $PRE_OID - hooks/reference-transaction prepared - hooks/reference-transaction committed - hooks/update refs/tags/POST $ZERO_OID $POST_OID - hooks/reference-transaction prepared - hooks/reference-transaction committed + $(pwd)/target-repo.git/hooks/update refs/tags/PRE $ZERO_OID $PRE_OID + $(pwd)/target-repo.git/hooks/reference-transaction prepared + $(pwd)/target-repo.git/hooks/reference-transaction committed + $(pwd)/target-repo.git/hooks/update refs/tags/POST $ZERO_OID $POST_OID + $(pwd)/target-repo.git/hooks/reference-transaction prepared + $(pwd)/target-repo.git/hooks/reference-transaction committed EOF git push ./target-repo.git PRE POST && diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh deleted file mode 100755 index 9e2dd64275c..00000000000 --- a/t/t1800-hook.sh +++ /dev/null @@ -1,158 +0,0 @@ -#!/bin/bash - -test_description='git-hook command' - -. ./test-lib.sh - -test_expect_success 'setup .git/hooks' ' - mkdir .git/hooks -' - -test_expect_success 'git hook run -- nonexistent hook' ' - cat >stderr.expect <<-\EOF && - error: cannot find a hook named does-not-exist - EOF - test_expect_code 1 git hook run does-not-exist 2>stderr.actual && - test_cmp stderr.expect stderr.actual -' - -test_expect_success 'git hook run -- nonexistent hook with --ignore-missing' ' - git hook run --ignore-missing does-not-exist 2>stderr.actual && - test_must_be_empty stderr.actual -' - -test_expect_success 'git hook run -- basic' ' - write_script .git/hooks/test-hook <<-EOF && - echo Test hook - EOF - - cat >expect <<-\EOF && - Test hook - EOF - git hook run test-hook 2>actual && - test_cmp expect actual -' - -test_expect_success 'git hook run -- stdout and stderr handling' ' - write_script .git/hooks/test-hook <<-EOF && - echo >&1 Will end up on stderr - echo >&2 Will end up on stderr - EOF - - cat >stderr.expect <<-\EOF && - Will end up on stderr - Will end up on stderr - EOF - git hook run test-hook >stdout.actual 2>stderr.actual && - test_cmp stderr.expect stderr.actual && - test_must_be_empty stdout.actual -' - -test_expect_success 'git hook run -- exit codes are passed along' ' - write_script .git/hooks/test-hook <<-EOF && - exit 1 - EOF - - test_expect_code 1 git hook run test-hook && - - write_script .git/hooks/test-hook <<-EOF && - exit 2 - EOF - - test_expect_code 2 git hook run test-hook && - - write_script .git/hooks/test-hook <<-EOF && - exit 128 - EOF - - test_expect_code 128 git hook run test-hook && - - write_script .git/hooks/test-hook <<-EOF && - exit 129 - EOF - - test_expect_code 129 git hook run test-hook -' - -test_expect_success 'git hook run arg u ments without -- is not allowed' ' - test_expect_code 129 git hook run test-hook arg u ments -' - -test_expect_success 'git hook run -- pass arguments' ' - write_script .git/hooks/test-hook <<-\EOF && - echo $1 - echo $2 - EOF - - cat >expect <<-EOF && - arg - u ments - EOF - - git hook run test-hook -- arg "u ments" 2>actual && - test_cmp expect actual -' - -test_expect_success 'git hook run -- out-of-repo runs excluded' ' - write_script .git/hooks/test-hook <<-EOF && - echo Test hook - EOF - - nongit test_must_fail git hook run test-hook -' - -test_expect_success 'git -c core.hooksPath=<PATH> hook run' ' - mkdir my-hooks && - write_script my-hooks/test-hook <<-EOF && - echo Hook ran >>actual - EOF - - cat >expect <<-\EOF && - Test hook - Hook ran - Hook ran - Hook ran - Hook ran - EOF - - # Test various ways of specifying the path. See also - # t1350-config-hooks-path.sh - >actual && - git hook run test-hook 2>>actual && - git -c core.hooksPath=my-hooks hook run test-hook 2>>actual && - git -c core.hooksPath=my-hooks/ hook run test-hook 2>>actual && - git -c core.hooksPath="$PWD/my-hooks" hook run test-hook 2>>actual && - git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook 2>>actual && - test_cmp expect actual -' - -test_expect_success 'set up a pre-commit hook in core.hooksPath' ' - >actual && - mkdir -p .git/custom-hooks .git/hooks && - write_script .git/custom-hooks/pre-commit <<-\EOF && - echo CUSTOM >>actual - EOF - write_script .git/hooks/pre-commit <<-\EOF - echo NORMAL >>actual - EOF -' - -test_expect_success 'stdin to hooks' ' - write_script .git/hooks/test-hook <<-\EOF && - echo BEGIN stdin - cat - echo END stdin - EOF - - cat >expect <<-EOF && - BEGIN stdin - hello - END stdin - EOF - - echo hello >input && - git hook run --to-stdin=input test-hook 2>actual && - test_cmp expect actual -' - -test_done diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh index 3e0f8c675f7..7087818550f 100755 --- a/t/t2080-parallel-checkout-basics.sh +++ b/t/t2080-parallel-checkout-basics.sh @@ -114,7 +114,7 @@ do test_expect_success "$mode checkout" ' repo=various_$mode && - cp -R -P various $repo && + cp -R various $repo && # The just copied files have more recent timestamps than their # associated index entries. So refresh the cached timestamps diff --git a/t/t5411/test-0015-too-many-hooks-error.sh b/t/t5411/test-0015-too-many-hooks-error.sh new file mode 100644 index 00000000000..2d645345101 --- /dev/null +++ b/t/t5411/test-0015-too-many-hooks-error.sh @@ -0,0 +1,47 @@ +test_expect_success "setup too many proc-receive hooks (ok, $PROTOCOL)" ' + write_script "proc-receive" <<-EOF && + printf >&2 "# proc-receive hook\n" + test-tool proc-receive -v \ + -r "ok refs/for/main/topic" + EOF + + git -C "$upstream" config --add "hook.proc-receive.command" proc-receive && + cp proc-receive "$upstream/hooks/proc-receive" +' + +# Refs of upstream : main(A) +# Refs of workbench: main(A) tags/v123 +# git push : next(A) refs/for/main/topic(A) +test_expect_success "proc-receive: reject more than one configured hook" ' + test_must_fail git -C workbench push origin \ + HEAD:next \ + HEAD:refs/for/main/topic \ + >out 2>&1 && + make_user_friendly_and_stable_output <out >actual && + cat >expect <<-EOF && + remote: # pre-receive hook + remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/heads/next + remote: pre-receive< <ZERO-OID> <COMMIT-A> refs/for/main/topic + remote: error: only one "proc-receive" hook can be specified + remote: # post-receive hook + remote: post-receive< <ZERO-OID> <COMMIT-A> refs/heads/next + To <URL/of/upstream.git> + * [new branch] HEAD -> next + ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook) + EOF + test_cmp expect actual && + git -C "$upstream" show-ref >out && + make_user_friendly_and_stable_output <out >actual && + cat >expect <<-EOF && + <COMMIT-A> refs/heads/main + <COMMIT-A> refs/heads/next + EOF + test_cmp expect actual +' + +# Refs of upstream : main(A) next(A) +# Refs of workbench: main(A) tags/v123 +test_expect_success "cleanup ($PROTOCOL)" ' + git -C "$upstream" config --unset "hook.proc-receive.command" "proc-receive" && + git -C "$upstream" update-ref -d refs/heads/next +' diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh index 10c7ae7f09c..60d961b5260 100755 --- a/t/t6500-gc.sh +++ b/t/t6500-gc.sh @@ -95,52 +95,6 @@ test_expect_success 'gc --keep-largest-pack' ' ) ' -test_expect_success 'pre-auto-gc hook can stop auto gc' ' - cat >err.expect <<-\EOF && - no gc for you - EOF - - git init pre-auto-gc-hook && - ( - cd pre-auto-gc-hook && - write_script ".git/hooks/pre-auto-gc" <<-\EOF && - echo >&2 no gc for you && - exit 1 - EOF - - git config gc.auto 3 && - git config gc.autoDetach false && - - # We need to create two object whose sha1s start with 17 - # since this is what git gc counts. As it happens, these - # two blobs will do so. - test_commit "$(test_oid obj1)" && - test_commit "$(test_oid obj2)" && - - git gc --auto >../out.actual 2>../err.actual - ) && - test_must_be_empty out.actual && - test_cmp err.expect err.actual && - - cat >err.expect <<-\EOF && - will gc for you - Auto packing the repository for optimum performance. - See "git help gc" for manual housekeeping. - EOF - - ( - cd pre-auto-gc-hook && - write_script ".git/hooks/pre-auto-gc" <<-\EOF && - echo >&2 will gc for you && - exit 0 - EOF - git gc --auto >../out.actual 2>../err.actual - ) && - - test_must_be_empty out.actual && - test_cmp err.expect err.actual -' - test_expect_success 'auto gc with too many loose objects does not attempt to create bitmaps' ' test_config gc.auto 3 && test_config gc.autodetach false && diff --git a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh index 606d8d0f089..e9e37130332 100755 --- a/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh +++ b/t/t7503-pre-commit-and-pre-merge-commit-hooks.sh @@ -8,8 +8,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh HOOKDIR="$(git rev-parse --git-dir)/hooks" -PRECOMMIT="$HOOKDIR/pre-commit" -PREMERGE="$HOOKDIR/pre-merge-commit" +PRECOMMIT="$(pwd)/$HOOKDIR/pre-commit" +PREMERGE="$(pwd)/$HOOKDIR/pre-merge-commit" # Prepare sample scripts that write their $0 to actual_hooks test_expect_success 'sample script setup' ' @@ -106,6 +106,19 @@ test_expect_success 'with succeeding hook' ' test_cmp expected_hooks actual_hooks ' +# NEEDSWORK: when 'git hook add' and 'git hook remove' have been added, use that +# instead +test_expect_success 'with succeeding hook (config-based)' ' + test_when_finished "git config --unset hook.pre-commit.command success.sample" && + test_when_finished "rm -f expected_hooks actual_hooks" && + git config hook.pre-commit.command "$HOOKDIR/success.sample" && + echo "$HOOKDIR/success.sample" >expected_hooks && + echo "more" >>file && + git add file && + git commit -m "more" && + test_cmp expected_hooks actual_hooks +' + test_expect_success 'with succeeding hook (merge)' ' test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" && cp "$HOOKDIR/success.sample" "$PREMERGE" && diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 35b513c015f..bdf64728710 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -539,15 +539,13 @@ test_expect_success $PREREQ "--validate respects relative core.hooksPath path" ' test_path_is_file my-hooks.ran && cat >expect <<-EOF && fatal: longline.patch: rejected by sendemail-validate hook - fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1 warning: no patches were sent EOF test_cmp expect actual ' test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" ' - hooks_path="$(pwd)/my-hooks" && - test_config core.hooksPath "$hooks_path" && + test_config core.hooksPath "$(pwd)/my-hooks" && test_when_finished "rm my-hooks.ran" && test_must_fail git send-email \ --from="Example <nobody@example.com>" \ @@ -558,7 +556,6 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" ' test_path_is_file my-hooks.ran && cat >expect <<-EOF && fatal: longline.patch: rejected by sendemail-validate hook - fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1 warning: no patches were sent EOF test_cmp expect actual @@ -2171,16 +2168,7 @@ test_expect_success $PREREQ 'invoke hook' ' mkdir -p .git/hooks && write_script .git/hooks/sendemail-validate <<-\EOF && - # test that we have the correct environment variable, pwd, and - # argument - case "$GIT_DIR" in - *.git) - true - ;; - *) - false - ;; - esac && + # test that we have the correct argument test -f 0001-add-main.patch && grep "add main" "$1" EOF diff --git a/transport.c b/transport.c index 1146ed3143c..91911076264 100644 --- a/transport.c +++ b/transport.c @@ -2,7 +2,6 @@ #include "config.h" #include "transport.h" #include "run-command.h" -#include "hook.h" #include "pkt-line.h" #include "fetch-pack.h" #include "remote.h" @@ -23,6 +22,7 @@ #include "protocol.h" #include "object-store.h" #include "color.h" +#include "hook.h" static int transport_use_color = -1; static char transport_colors[][COLOR_MAXLEN] = { @@ -1198,11 +1198,13 @@ static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { int ret = 0; - struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct run_hooks_opt opt; struct strbuf tmp = STRBUF_INIT; struct ref *r; struct string_list to_stdin = STRING_LIST_INIT_DUP; + run_hooks_opt_init_async(&opt); + strvec_push(&opt.args, transport->remote->name); strvec_push(&opt.args, transport->url);
On Tue, Jun 01 2021, Emily Shaffer wrote: > On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: >> >> After suggesting[1] an another round that the config-based-hook >> topic[2] should take a more incremental approach to reach its end goal >> I thought I'd try hacking that up. >> >> So this is a proposed restart of that topic which if the consensus >> favors it should replace it, and the config-based hooks topic should >> be rebased on top of this. > > I'm not entirely sure what you're trying to achieve by sending this > series. I'm trying to convince you and others that an approach where we split up the refactoring part of your large series from the behavior changes + new behaviors it introduces is a better approach in getting us to your currently proposed (or some small variation thereof) desired end-state. I think the opening line of your reply doesn't bode for the "convince you" part of that, maybe I'll do better on the "and others" front :) I'm a bit surprised at what seems to be some hostility or annoyance that I submitted this as a set of patches. That's ultimately something that saves everyone involved time (well, except me by coming up with said patches). To borrow some words: "Talk is cheap. Show me the code." ― Linus Torvalds. If I give you feedback suggesting that maybe we should reorganize this thing to split out refactorings from behavior changes I'm asking you to do extra work. Ultimately neither I, you nor anyone else can really know if such a proposed effort is going to be better until it happens. When you get feedback in the form of restructured patches we skip past all that. We know it compiles, passes tests, and we can more concretely evaluate the proposal with diff, range-diff etc. > It was my impression that the existing config-based-hooks topic > was close to being ready to submit anyway (since Junio mentioned > submitting it a couple revisions ago); rather than churning by reviewing > a different 31-patch topic, and then re-rolling and re-reviewing a > (reduced) config hook topic, wouldn't it be easier on everyone's time to > do a final incremental review on the existing topic and then start in on > bugfixes/feature patches afterwards? I think it's fair to say that nobody's read your code in its current state more thoroughly than I have at this point, but still, going back to it and paging through it even now a couple of days after reading the whole thing line-by-line I find myself getting lost again. That's because the whole structure of it is to conflate changing existing behavior with the introduction of new behavior. This makes things *much harder* for reviewers, because they need to be on toes about regressions *and* evaluating the function/viability/sanity of new proposed semantics. I very much would like to see some approximation (sans my outstanding comments, more on that below) of your topic land sooner than later. I think this approach gets us there faster, not slower. How long it takes to review something isn't about the number of patches, it can be harder to review a 10 patch series than a 100 patch series. It's mainly about structuring things in such a way as to make it readable to other people. If you've followed any of my own topics you'll probably correctly form the opinion that that the last few paragraphs at best amount to throwing some rather large pebbles from a glass house, which you'd be right about :) It's can be really hard to see how/where to split things when you're the author of the code. It's really hard in the "theory of mind" sense of things to explain an idea to someone who doesn't have the information you have. Still, I do think there's a near-universal rules of thumb that the structure of your series thoroughly runs afoul of, i.e. changing existing behavior at the same time as introducing new behavior. We should split such changes up whenever possible. This alternate approach shows that's it's possible. > their would have been nice to see a more clear discussion of patch > organization sometime much sooner in the past year and a half since the > project was proposed[3], like maybe in the few iterations of the design > doc which included a rollout plan in July of last year[4]. To me, it > seems late to be overhauling the direction like this, especially after I > asked for opinions and approval on the direction before I started work > in earnest. We've had some version of this series going back to at least May last year, and none of it has landed on "master" or "next" yet. To me that's more reason, not less, for it benefiting from a more incremental approach. But yes, I also wish I'd submitted this much earlier. Sorry I didn't have time to review in this detail until now. I think Felipe's downthread "never too late" comments apply here though[Æ.1] I've had outstanding significant feedback on the "function/viability/sanity of new proposed semantics" part of this almost 3 months ago that you still haven't addressed in any way[Æ.2] except this comment[Æ.3] around a month ago on v8 (which I took to mean that you would). So the lateness of us discussing this is at least partially a two-way street. There was also the "why do we need strbuf here?" feedback[Æ.4] around the same time from me, and as shown in the diff between our two versions things like your run_hooks_opt_init_async() being better done as a "struct ... = *_INIT" idiom. Small issues, but especially the latter to me suggests that your v9 is too big a chunk for reviewers to consider. I daresay if this was a smaller series there's no way that wouldn't have been pointed out already, ditto your "git hook run" introducing an "--env" which never gets used etc. > Anyway, I'd personally rather spend effort getting the existing series > the last few yards to the finish line than to head most of the way back > to the start. I think I've made a good argument above for why this takes you a step forward, not backwards, I'm hoping despite this initial reply that you'll come to agree on that. In any case, writing code is hard, but splitting it up like I've done here is rather easily done. It took me about a day with waiting for "rebase -i --exec='make test'" equivalent, and that's from being mostly unfamiliar with the code in question beforehand. If this alternate series were to go in ahead of yours that doesn't land you back on square one, you still have a version you could relatively easily rebase on top. The patches here are mostly smaller versions (no hook config changes) of corresponding patches of yours. If anything I don't see how it doesn't make things much easier for you, if you do get around to replying to my outstanding feedback in [Æ.1] that'll involve rebasing/commit message rewording of patches that now conflate significant refactoring and behavior changes. You'll instead cleanly have just the behavior changes, which at that point will be both easier to justify in a briefer commit message, and easier for others to review. >> 1. https://lore.kernel.org/git/87lf80l1m6.fsf@evledraar.gmail.com/ >> 2. https://lore.kernel.org/git/20210527000856.695702-1-emilyshaffer@google.com/ > 3. https://lore.kernel.org/git/20191116011125.GG22855@google.com/ > 4. https://lore.kernel.org/git/20200728222455.3023400-1-emilyshaffer@google.com/ Æ.1. https://lore.kernel.org/git/60b71788c0e6d_67d0208d4@natae.notmuch/ Æ.2. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/ Æ.3. https://lore.kernel.org/git/YJBdbi50Hz+ekOtt@google.com/ Æ.4. https://lore.kernel.org/git/87pn04g0r1.fsf@evledraar.gmail.com/
On Tue, Jun 01 2021, Derrick Stolee wrote: > On 6/1/2021 2:14 PM, Emily Shaffer wrote: >> On Fri, May 28, 2021 at 02:11:02PM +0200, Ævar Arnfjörð Bjarmason wrote: >>> >>> After suggesting[1] an another round that the config-based-hook >>> topic[2] should take a more incremental approach to reach its end goal >>> I thought I'd try hacking that up. > > I think sending this complete reorganization of a long-lived topic > is not helpful, especially because the end-to-end diff is significant. > This series has been extensively tested and incrementally improved > for months, and it would be a waste to start over and lose all of that > hardening. Felipe already commented downthread on the end-to-end diff aspect of this. I replied in http://lore.kernel.org/git/871r9k8zui.fsf@evledraar.gmail.com with the diff you're probably more interested in looking at. > It's also a but rushed that this comes only a day after the previous > message recommending a reorganization. It would be best to at least > give the original author an opportunity to comment on your idea before > working on this. I replied toth is as "When you get feedback in the form of restructured patches[...]" in https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/ As noted there I really don't see how sending "here's patches that implement my suggestion" as opposed to "maybe xyz would be better" is worse for anyone. >>> So this is a proposed restart of that topic which if the consensus >>> favors it should replace it, and the config-based hooks topic should >>> be rebased on top of this. >> >> I'm not entirely sure what you're trying to achieve by sending this >> series. It was my impression that the existing config-based-hooks topic >> was close to being ready to submit anyway (since Junio mentioned >> submitting it a couple revisions ago); rather than churning by reviewing >> a different 31-patch topic, and then re-rolling and re-reviewing a >> (reduced) config hook topic, wouldn't it be easier on everyone's time to >> do a final incremental review on the existing topic and then start in on >> bugfixes/feature patches afterwards? > > I completely agree here. What do you think of the issues I raised in [1] ? Emily's series isn't a pure internal refactoring of existing functionality, but also an opinionated introduction of new user-facing functionality. Now, you may be 100% in agreement with the "opinionated" or "user-facing" part of that, I'm personally somewhere around 80%. But to me that clearly makes it a case where we need to as much as possible get it right *before* it lands on master, because we can't just incrementally tweak it after it lands without backwards compatibility concerns. Soon after landing it'll be in a release, have users in the wild etc. By default that turns any suggestion of tweaking existing behavior from a "oh that makes sense, let's fix it" into "well, that's unfortunate, but it's existing/documented functionality used in the wild". [2] goes on to mention a couple of other such issues (e.g. the --env switch to "git hook run"), and this time around I've just been narrowly focusing on any such issues in the not-config-base-hooks part of this topic. I.e. the "no user-facing behavior changes yet" part I carved out into a separate topic. I'm not telling you which issues those are because I'd like you to tell me, and then we'll compare notes afterwards. I bet <insert round of beers at git dev summit or equivalent here> that they're not the same list. Anyway, I don't think you will, and I'm not entirely serious. But it's a real rhetorical point about us being unlikely to come up with the same result, and thus about the outstanding v9 series being too large to be readily understood. >> It would have been nice to see a more clear discussion of patch >> organization sometime much sooner in the past year and a half since the >> project was proposed[3], like maybe in the few iterations of the design >> doc which included a rollout plan in July of last year[4]. To me, it >> seems late to be overhauling the direction like this, especially after I >> asked for opinions and approval on the direction before I started work >> in earnest. > > I've also seen messages as early as January where Ævar mentioned > wanting to review the series, but not finding the time to do so. > It is reasonable to expect that contributors attempt such major > reorganizations according to reviewers feedback, as long as the > reviewers are timely about delivering that feedback. I made a case in [2] for it not being too late. For what it's worth (and some of which is noted in [2]) that's partially because I for one have found it really hard to keep track of this series. Multiple re-rolls (including v8->v9) have some outstanding discussion/feedback, some of which is addressed in a re-roll, but other parts (as noted in my [2]) which aren't and are silently omitted. I'm not blaming Emily for that, I think it's a rather inevitable result of this thing just being too big to begin with and needing to be split up. But that's at least part of the story of feedback at such a late time. Whenever I've looked at this topic I've spent quite a lot of time doing that re-reading of past discussions / comparing with the newest cover letter and noting any omissions etc. myself before even getting to the point of reading the first patch, and I've sometimes just given up. 1. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/ 2. https://lore.kernel.org/git/87y2bs7gyc.fsf@evledraar.gmail.com/
On Wed, Jun 02 2021, Ævar Arnfjörð Bjarmason wrote: > [...] > If anything I don't see how it doesn't make things much easier for you, > if you do get around to replying to my outstanding feedback in [Æ.1] Hopefully obvious from context, but I meant [Æ.2] here. > [...] > Æ.1. https://lore.kernel.org/git/60b71788c0e6d_67d0208d4@natae.notmuch/ > Æ.2. https://lore.kernel.org/git/87mtv8fww3.fsf@evledraar.gmail.com/ i.e. this. > Æ.3. https://lore.kernel.org/git/YJBdbi50Hz+ekOtt@google.com/ > Æ.4. https://lore.kernel.org/git/87pn04g0r1.fsf@evledraar.gmail.com/
Ævar Arnfjörð Bjarmason wrote: > I'm a bit surprised at what seems to be some hostility or annoyance that > I submitted this as a set of patches. That's ultimately something that > saves everyone involved time (well, except me by coming up with said > patches). To borrow some words: > > "Talk is cheap. Show me the code." ― Linus Torvalds. Words I live by. > If I give you feedback suggesting that maybe we should reorganize this > thing to split out refactorings from behavior changes I'm asking you to > do extra work. Ultimately neither I, you nor anyone else can really know > if such a proposed effort is going to be better until it happens. Indeed. That's why a lot of time instead of simply replying to a mail with an idea, I actually attempt to code the idea, and only then send te reply. I would say 90% of the time what I originally thought changes once I actually try to implement it. > It's can be really hard to see how/where to split things when you're the > author of the code. It's really hard in the "theory of mind" sense of > things to explain an idea to someone who doesn't have the information > you have. More like impossible. No comic simply writes an act and goes to an auditorium like Carnegie Hall to simply present it knowing full well how people are going to react to it. You never really know how other people are going to react, so it's better to not make assumptions, just try and find out. > I think I've made a good argument above for why this takes you a step > forward, not backwards, I'm hoping despite this initial reply that > you'll come to agree on that. Not to mention that the goal is not to land Emily's patches, the goal is to improve the code while minimizing the potential breakage. For that we need as many eyes as possible, and in my opinion your reorganization patches totally help in that regard. The fact that this makes it easier to land Emily's patches is an added benefit. > In any case, writing code is hard, but splitting it up like I've done > here is rather easily done. It took me about a day with waiting for > "rebase -i --exec='make test'" equivalent, and that's from being mostly > unfamiliar with the code in question beforehand. It's easy if you are an expert at rebasing, which not many people are. But you can't become an expert if you don't do it, over and over. So it's usually better to not think too much about it and simply do it. Cheers.
After suggesting[1] an another round that the config-based-hook topic[2] should take a more incremental approach to reach its end goal I thought I'd try hacking that up. So this is a proposed restart of that topic which if the consensus favors it should replace it, and the config-based hooks topic should be rebased on top of this. As noted in [1] there are no user-visible behavior changes here. All that's being done is to consolidate hook dispatch into the new hook.[ch] library, and the "git hook run" command (used by git-send-email and git-p4). The range-diff below is scary, I recommend just reading this from scratch. Some incomplete notes on changed things from Emily's v9: * First off, we've gone from: 54 files changed, 2689 insertions(+), 713 deletions(-) to: 41 files changed, 1293 insertions(+), 555 deletions(-) So while there's a similar number of patches the change is much smaller. * There's no config via hooks, design doc etc. in this series. It's meant as a basis on which to build that. We still only support running .git/hook/<hook>, and we run those with .jobs=1 by definition. * There's absolutely no behavior change in how we run these hooks, unlike in Emily's version. E.g. in hers sometimes we'd always pass absolute paths when before we didn't, now we just do that for "git worktree" via a flag as on the current "master". There were also some other subtle changes as seen in the tests. Now the only test changes are the addition of more and missing tests. Some of this (e.g. git-send-email's insistance on GIT_DIR being set) is something we'll probably need to change sooner than later, but for now we've got bug-for-bug compatibility. * This has been re-arranged as much as possible to start using minimal bits of the library as soon as we can. E.g. in Emily's the whole of hook.c is implemented before any hook is migrated over to it. In this version we barely know how to do anything yet (no stdin handling etc.) when we can move the pre-auto-gc hook over, we then do a few more hooks and add stdin support, migrate the hooks that need that etc. * In Emily's there's a mid-series switch from the run_command() API to run_processes_parallel_tr2() as we learn parallel execution. I thought this was more complex to read and just reflected the past evolution of the topic. That's now squashed together, we always have the parallel API use, we just have .jobs=1 throughout. * I tried as much as possible to similarly squash together e.g. removing an unused API along with the commit that removes the last user of it, not introducing APIs without simultaneously introducing users of them etc. * The "git hook run" command now takes arguments as: git hook run a-hook -- some arg u ments Not: git hook run -a some -a arg -a u -a ments a-hook The "-e" for "pass this env variable" is also gone, nothing actually made use of it. If we need it in the future surely we can just set them in the environment instead, and ask the relevant command run APIs not to clobber things. I think the -- (or --end-of-options) is easier to work with. Changing this allowed removing the whole parse-options-strvec part of Emily's series. It's now replaced with PARSE_OPT_KEEP_DASHDASH + 2-4 lines of code around that in builtin/hook.c. * Even though this extracted the "config based hooks" part, and .jobs != 1 support I've tried as much as possible to keep the same code layout, to the point where hooks e.g. set .jobs=1 still if they insist that one job should be run. It should thus be fairly drop-in compatible for building .jobs > 1 support on top, config-based hooks etc. * The two last patches are new, a couple of minor bugfixes of mine that I noticed while hacking on this. One solves a long-standing TOCTOU in run_a_hook() and then have_hook() to see if we ran it (we now just remember if we ran it), and there's now a generated and canonical hook-list.h similar to config-list.h, except this one's more strict. We'll die on unknown hooks unless they're in Documentation/githooks.txt. 1. https://lore.kernel.org/git/87lf80l1m6.fsf@evledraar.gmail.com/ 2. https://lore.kernel.org/git/20210527000856.695702-1-emilyshaffer@google.com/ Emily Shaffer (25): hook: add 'run' subcommand hook.c: add a hook_exists() wrapper and use it in bugreport.c gc: use hook library for pre-auto-gc hook rebase: teach pre-rebase to use hook.h am: convert applypatch hooks to use config hooks: convert 'post-checkout' hook to hook library merge: use config-based hooks for post-merge hook send-email: use 'git hook run' for 'sendemail-validate' git-p4: use 'git hook' to run hooks commit: use hook.h to execute hooks read-cache: convert post-index-change hook to use config receive-pack: convert push-to-checkout hook to hook.h run-command: allow stdin for run_processes_parallel hook: support passing stdin to hooks am: convert 'post-rewrite' hook to hook.h run-command: add stdin callback for parallelization hook: provide stdin by string_list or callback hook: convert 'post-rewrite' hook in sequencer.c to hook.h transport: convert pre-push hook to use config reference-transaction: use hook.h to run hooks run-command: allow capturing of collated output hooks: allow callers to capture output receive-pack: convert 'update' hook to hook.h post-update: use hook.h library receive-pack: convert receive hooks to hook.h Ævar Arnfjörð Bjarmason (6): hooks tests: don't leave "actual" nonexisting on failure gc tests: add a test for the "pre-auto-gc" hook run-command.h: move find_hook() to hook.h git hook run: add an --ignore-missing flag hooks: fix a TOCTOU in "did we run a hook?" heuristic hook-list.h: add a generated list of hooks, like config-list.h .gitignore | 2 + Documentation/git-hook.txt | 49 ++++++ Documentation/githooks.txt | 4 + Makefile | 16 +- builtin.h | 1 + builtin/am.c | 34 ++-- builtin/bugreport.c | 46 ++---- builtin/checkout.c | 17 +- builtin/clone.c | 7 +- builtin/commit.c | 19 ++- builtin/fetch.c | 1 + builtin/gc.c | 8 +- builtin/hook.c | 72 +++++++++ builtin/merge.c | 22 ++- builtin/rebase.c | 9 +- builtin/receive-pack.c | 299 ++++++++++++++++++----------------- builtin/submodule--helper.c | 2 +- builtin/worktree.c | 31 ++-- command-list.txt | 1 + commit.c | 17 +- commit.h | 3 +- generate-hooklist.sh | 20 +++ git-p4.py | 72 +-------- git-send-email.perl | 20 ++- git.c | 1 + hook.c | 224 ++++++++++++++++++++++++++ hook.h | 122 ++++++++++++++ read-cache.c | 12 +- refs.c | 43 ++--- reset.c | 15 +- run-command.c | 157 +++++++++--------- run-command.h | 55 ++++--- sequencer.c | 88 +++++------ submodule.c | 1 + t/helper/test-run-command.c | 46 +++++- t/t0061-run-command.sh | 37 +++++ t/t1350-config-hooks-path.sh | 1 + t/t1800-hook.sh | 166 +++++++++++++++++++ t/t6500-gc.sh | 46 ++++++ t/t9001-send-email.sh | 4 +- transport.c | 58 ++----- 41 files changed, 1293 insertions(+), 555 deletions(-) create mode 100644 Documentation/git-hook.txt create mode 100644 builtin/hook.c create mode 100755 generate-hooklist.sh create mode 100644 hook.c create mode 100644 hook.h create mode 100755 t/t1800-hook.sh Range-diff: 1: 9540c006dc0 < -: ----------- doc: propose hooks managed by the config -: ----------- > 1: 8ac2efc71a0 hooks tests: don't leave "actual" nonexisting on failure -: ----------- > 2: eb37693f7dc gc tests: add a test for the "pre-auto-gc" hook 2: 2f50cfe7b92 ! 3: 1ad4e69f7da hook: introduce git-hook subcommand @@ Metadata Author: Emily Shaffer <emilyshaffer@google.com> ## Commit message ## - hook: introduce git-hook subcommand + hook: add 'run' subcommand - Add 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. + 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. - Start with 'git hook list <hookname>', 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.<hookname>.command = <path-to-hook>" 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.<hookname>.command = <hookcmd-name>", as well as a "[hookcmd - <hookcmd-name>]" subsection; this subsection should contain a - "hookcmd.<hookcmd-name>.command = <path-to-hook>" 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 + Most of our hooks require more complex functionality than this, but + let's start with the bare minimum required to support our simplest + hooks. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## .gitignore ## @@ @@ .gitignore /git-http-fetch /git-http-push - ## Documentation/config/hook.txt (new) ## -@@ -+hook.<command>.command:: -+ A command to execute during the <command> 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.<name>.command:: -+ A command to execute during a hook for which <name> 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]. - ## Documentation/git-hook.txt (new) ## @@ +git-hook(1) @@ Documentation/git-hook.txt (new) + +NAME +---- -+git-hook - Manage configured hooks ++git-hook - run git hooks + +SYNOPSIS +-------- +[verse] -+'git hook' list <hook-name> ++'git hook' run <hook-name> [-- <hook-args>] + +DESCRIPTION +----------- -+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" -+---- ++This command is an interface to git hooks (see linkgit:githooks[5]). ++Currently it only provides a convenience wrapper for running hooks for ++use by git itself. In the future it might gain other functionality. + -+Local config -+---- -+ [hook "prepare-commit-msg"] -+ command = "linter" -+ [hook "post-commit"] -+ command = "python ~/run-test-suite.py" -+---- ++SUBCOMMANDS ++----------- + -+With these configs, you'd then see: ++run:: + -+---- -+$ git hook list "post-commit" -+global: /bin/linter --c -+global: ~/typocheck.sh -+local: python ~/run-test-suite.py ++ Run the `<hook-name>` hook. Any positional arguments to the ++ hook should be passed after an optional "--" (or ++ "--end-of-options"). See "OPTIONS" below for the arguments ++ this accepts. + -+$ git hook list "prepare-commit-msg" -+local: /bin/linter --c -+---- -+ -+COMMANDS ++SEE ALSO +-------- -+ -+list `<hook-name>`:: -+ -+List the hooks which have been configured for `<hook-name>`. Hooks appear -+in the order they should be run, and print the config scope where the relevant -+`hook.<hook-name>.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[] ++linkgit:githooks[5] + +GIT +--- +Part of the linkgit:git[1] suite + ## Documentation/githooks.txt ## +@@ Documentation/githooks.txt: and "0" meaning they were not. + Only one parameter should be set to "1" when the hook runs. The hook + running passing "1", "1" should not be possible. + ++SEE ALSO ++-------- ++linkgit:git-hook[1] ++ + GIT + --- + Part of the linkgit:git[1] suite + ## Makefile ## @@ Makefile: LIB_OBJS += hash-lookup.o LIB_OBJS += hashmap.o @@ builtin/hook.c (new) +#include "hook.h" +#include "parse-options.h" +#include "strbuf.h" ++#include "strvec.h" + +static const char * const builtin_hook_usage[] = { -+ N_("git hook list <hookname>"), ++ N_("git hook run <hook-name> [-- <hook-args>]"), + NULL +}; + -+static int list(int argc, const char **argv, const char *prefix) ++static int run(int argc, const char **argv, const char *prefix) +{ -+ struct list_head *head, *pos; -+ const char *hookname = NULL; ++ int i; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; ++ int rc = 0; ++ const char *hook_name; ++ const char *hook_path; + -+ struct option list_options[] = { ++ struct option run_options[] = { + OPT_END(), + }; + -+ 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); ++ argc = parse_options(argc, argv, prefix, run_options, ++ builtin_hook_usage, PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH); ++ ++ if (argc > 2) { ++ if (strcmp(argv[2], "--") && ++ strcmp(argv[2], "--end-of-options")) ++ /* Having a -- for "run" is mandatory */ ++ usage_with_options(builtin_hook_usage, run_options); ++ /* Add our arguments, start after -- */ ++ for (i = 3 ; i < argc; i++) ++ strvec_push(&opt.args, argv[i]); + } + -+ hookname = argv[0]; -+ -+ head = hook_list(hookname); -+ -+ if (list_empty(head)) { -+ printf(_("no commands configured for hook '%s'\n"), -+ hookname); -+ return 0; -+ } ++ /* Need to take into account core.hooksPath */ ++ git_config(git_default_config, NULL); + -+ list_for_each(pos, head) { -+ struct hook *item = list_entry(pos, struct hook, list); -+ if (item) -+ printf("%s: %s\n", -+ config_scope_name(item->origin), -+ item->command.buf); ++ hook_name = argv[1]; ++ hook_path = find_hook(hook_name); ++ if (!hook_path) { ++ error("cannot find a hook named %s", hook_name); ++ return 1; + } ++ rc = run_found_hooks(hook_name, hook_path, &opt); + -+ clear_hook_list(head); ++ run_hooks_opt_clear(&opt); + -+ return 0; ++ return rc; +} + +int cmd_hook(int argc, const char **argv, const char *prefix) @@ builtin/hook.c (new) + 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); + ++ if (!strcmp(argv[1], "run")) ++ return run(argc, argv, prefix); + usage_with_options(builtin_hook_usage, builtin_hook_options); ++ return 1; +} ## command-list.txt ## @@ git.c: 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 }, ++ { "hook", cmd_hook, RUN_SETUP }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, { "init-db", cmd_init_db }, @@ git.c: static struct cmd_struct commands[] = { ## hook.c (new) ## @@ +#include "cache.h" -+ +#include "hook.h" -+#include "config.h" ++#include "run-command.h" + -+void free_hook(struct hook *ptr) ++void run_hooks_opt_clear(struct run_hooks_opt *o) +{ -+ if (ptr) { -+ strbuf_release(&ptr->command); -+ free(ptr); -+ } ++ strvec_clear(&o->env); ++ strvec_clear(&o->args); +} + -+static void append_or_move_hook(struct list_head *head, const char *command) ++static int pick_next_hook(struct child_process *cp, ++ struct strbuf *out, ++ void *pp_cb, ++ void **pp_task_cb) +{ -+ struct list_head *pos = NULL, *tmp = NULL; -+ struct hook *to_add = NULL; ++ struct hook_cb_data *hook_cb = pp_cb; ++ struct hook *run_me = hook_cb->run_me; ++ ++ if (!run_me) ++ BUG("did we not return 1 in notify_hook_finished?"); ++ ++ cp->no_stdin = 1; ++ cp->env = hook_cb->options->env.v; ++ cp->stdout_to_stderr = 1; ++ cp->trace2_hook_name = hook_cb->hook_name; ++ ++ /* add command */ ++ strvec_push(&cp->args, run_me->hook_path); + + /* -+ * remove the prior entry with this command; we'll replace it at the -+ * end. ++ * add passed-in argv, without expanding - let the user get back ++ * exactly what they put in + */ -+ 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; -+ break; -+ } -+ } -+ -+ if (!to_add) { -+ /* adding a new hook, not moving an old one */ -+ to_add = xmalloc(sizeof(*to_add)); -+ strbuf_init(&to_add->command, 0); -+ strbuf_addstr(&to_add->command, command); -+ } ++ strvec_pushv(&cp->args, hook_cb->options->args.v); + -+ /* re-set the scope so we show where an override was specified */ -+ to_add->origin = current_config_scope(); ++ /* Provide context for errors if necessary */ ++ *pp_task_cb = run_me; + -+ list_add_tail(&to_add->list, head); ++ return 1; +} + -+static void remove_hook(struct list_head *to_remove) ++static int notify_start_failure(struct strbuf *out, ++ void *pp_cb, ++ void *pp_task_cp) +{ -+ struct hook *hook_to_remove = list_entry(to_remove, struct hook, list); -+ list_del(to_remove); -+ free_hook(hook_to_remove); -+} ++ struct hook_cb_data *hook_cb = pp_cb; ++ struct hook *attempted = pp_task_cp; + -+void clear_hook_list(struct list_head *head) -+{ -+ struct list_head *pos, *tmp; -+ list_for_each_safe(pos, tmp, head) -+ remove_hook(pos); ++ /* |= rc in cb */ ++ hook_cb->rc |= 1; ++ ++ strbuf_addf(out, _("Couldn't start hook '%s'\n"), ++ attempted->hook_path); ++ ++ return 1; +} + -+struct hook_config_cb ++static int notify_hook_finished(int result, ++ struct strbuf *out, ++ void *pp_cb, ++ void *pp_task_cb) +{ -+ struct strbuf *hookname; -+ struct list_head *list; -+}; ++ struct hook_cb_data *hook_cb = pp_cb; + -+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. If it doesn't, -+ * 'git_config_get_value()' is documented not to touch &command, -+ * so we don't need to do anything. -+ */ -+ 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); -+ } ++ /* |= rc in cb */ ++ hook_cb->rc |= result; + -+ return 0; ++ return 1; +} + -+struct list_head* hook_list(const char* hookname) ++ ++int run_found_hooks(const char *hook_name, const char *hook_path, ++ struct run_hooks_opt *options) +{ -+ 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 }; ++ struct hook my_hook = { ++ .hook_path = hook_path, ++ }; ++ struct hook_cb_data cb_data = { ++ .rc = 0, ++ .hook_name = hook_name, ++ .options = options, ++ }; ++ cb_data.run_me = &my_hook; ++ ++ if (options->jobs != 1) ++ BUG("we do not handle %d or any other != 1 job number yet", options->jobs); + -+ INIT_LIST_HEAD(hook_head); ++ run_processes_parallel_tr2(options->jobs, ++ pick_next_hook, ++ notify_start_failure, ++ notify_hook_finished, ++ &cb_data, ++ "hook", ++ hook_name); + -+ if (!hookname) -+ return NULL; ++ return cb_data.rc; ++} ++ ++int run_hooks(const char *hook_name, struct run_hooks_opt *options) ++{ ++ const char *hook_path; ++ int ret; ++ if (!options) ++ BUG("a struct run_hooks_opt must be provided to run_hooks"); + -+ strbuf_addf(&hook_key, "hook.%s.command", hookname); ++ hook_path = find_hook(hook_name); + -+ git_config(hook_config_lookup, &cb_data); ++ /* Care about nonexistence? Use run_found_hooks() */ ++ if (!hook_path) ++ return 0; + -+ strbuf_release(&hook_key); -+ return hook_head; ++ ret = run_found_hooks(hook_name, hook_path, options); ++ return ret; +} ## hook.h (new) ## @@ -+#include "config.h" -+#include "list.h" ++#ifndef HOOK_H ++#define HOOK_H +#include "strbuf.h" ++#include "strvec.h" ++#include "run-command.h" + +struct hook { -+ struct list_head list; -+ /* -+ * Config file which holds the hook.*.command definition. -+ * (This has nothing to do with the hookcmd.<name>.* configs.) -+ */ -+ enum config_scope origin; -+ /* The literal command to run. */ -+ struct strbuf command; ++ /* The path to the hook */ ++ const char *hook_path; ++}; ++ ++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; ++ ++ /* Number of threads to parallelize across */ ++ int jobs; +}; + ++#define RUN_HOOKS_OPT_INIT { \ ++ .jobs = 1, \ ++ .env = STRVEC_INIT, \ ++ .args = STRVEC_INIT, \ ++} ++ +/* -+ * Provides a linked list of 'struct hook' detailing commands which should run -+ * in response to the 'hookname' event, in execution order. ++ * Callback provided to feed_pipe_fn and consume_sideband_fn. + */ -+struct list_head* hook_list(const char *hookname); ++struct hook_cb_data { ++ int rc; ++ const char *hook_name; ++ struct hook *run_me; ++ 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 */ -+void clear_hook_list(struct list_head *head); ++void run_hooks_opt_clear(struct run_hooks_opt *o); ++ ++/* ++ * Calls find_hook(hookname) and runs the hooks (if any) with ++ * run_found_hooks(). ++ */ ++int run_hooks(const char *hook_name, struct run_hooks_opt *options); ++ ++/* ++ * Takes an already resolved hook and runs it. Internally the simpler ++ * run_hooks() will call this. ++ */ ++int run_found_hooks(const char *hookname, const char *hook_path, ++ struct run_hooks_opt *options); ++#endif - ## t/t1360-config-based-hooks.sh (new) ## + ## t/t1800-hook.sh (new) ## @@ +#!/bin/bash + -+test_description='config-managed multihooks, including git-hook command' ++test_description='git-hook command' + +. ./test-lib.sh + -+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 -+} ++test_expect_success 'setup .git/hooks' ' ++ mkdir .git/hooks ++' + -+setup_hookcmd () { -+ test_config hook.pre-commit.command "abc" --add -+ test_config_global hookcmd.abc.command "/path/abc" --add -+} ++test_expect_success 'git hook run -- nonexistent hook' ' ++ cat >stderr.expect <<-\EOF && ++ error: cannot find a hook named does-not-exist ++ EOF ++ test_expect_code 1 git hook run does-not-exist 2>stderr.actual && ++ test_cmp stderr.expect stderr.actual ++' + -+test_expect_success 'git hook rejects commands without a mode' ' -+ test_must_fail git hook pre-commit ++test_expect_success 'git hook run -- basic' ' ++ write_script .git/hooks/test-hook <<-EOF && ++ echo Test hook ++ EOF ++ ++ cat >expect <<-\EOF && ++ Test hook ++ EOF ++ git hook run test-hook 2>actual && ++ test_cmp expect actual +' + ++test_expect_success 'git hook run -- stdout and stderr handling' ' ++ write_script .git/hooks/test-hook <<-EOF && ++ echo >&1 Will end up on stderr ++ echo >&2 Will end up on stderr ++ EOF + -+test_expect_success 'git hook rejects commands without a hookname' ' -+ test_must_fail git hook list ++ cat >stderr.expect <<-\EOF && ++ Will end up on stderr ++ Will end up on stderr ++ EOF ++ git hook run test-hook >stdout.actual 2>stderr.actual && ++ test_cmp stderr.expect stderr.actual && ++ test_must_be_empty stdout.actual +' + -+test_expect_success 'git hook runs outside of a repo' ' -+ setup_hooks && ++test_expect_success 'git hook run -- exit codes are passed along' ' ++ write_script .git/hooks/test-hook <<-EOF && ++ exit 1 ++ EOF ++ ++ test_expect_code 1 git hook run test-hook && + -+ cat >expected <<-EOF && -+ global: $ROOT/path/def ++ write_script .git/hooks/test-hook <<-EOF && ++ exit 2 + EOF + -+ nongit git config --list --global && ++ test_expect_code 2 git hook run test-hook && + -+ nongit git hook list pre-commit >actual && -+ test_cmp expected actual -+' ++ write_script .git/hooks/test-hook <<-EOF && ++ exit 128 ++ EOF + -+test_expect_success 'git hook list orders by config order' ' -+ setup_hooks && ++ test_expect_code 128 git hook run test-hook && + -+ cat >expected <<-EOF && -+ global: $ROOT/path/def -+ local: $ROOT/path/ghi ++ write_script .git/hooks/test-hook <<-EOF && ++ exit 129 + EOF + -+ git hook list pre-commit >actual && -+ test_cmp expected actual ++ test_expect_code 129 git hook run test-hook +' + -+test_expect_success 'git hook list dereferences a hookcmd' ' -+ setup_hooks && -+ setup_hookcmd && ++test_expect_success 'git hook run arg u ments without -- is not allowed' ' ++ test_expect_code 129 git hook run test-hook arg u ments ++' ++ ++test_expect_success 'git hook run -- pass arguments' ' ++ write_script .git/hooks/test-hook <<-\EOF && ++ echo $1 ++ echo $2 ++ EOF + -+ cat >expected <<-EOF && -+ global: $ROOT/path/def -+ local: $ROOT/path/ghi -+ local: $ROOT/path/abc ++ cat >expect <<-EOF && ++ arg ++ u ments + EOF + -+ git hook list pre-commit >actual && -+ test_cmp expected actual ++ git hook run test-hook -- arg "u ments" 2>actual && ++ test_cmp expect actual +' + -+test_expect_success 'git hook list reorders on duplicate commands' ' -+ setup_hooks && ++test_expect_success 'git hook run -- out-of-repo runs excluded' ' ++ write_script .git/hooks/test-hook <<-EOF && ++ echo Test hook ++ EOF + -+ test_config hook.pre-commit.command "/path/def" --add && ++ nongit test_must_fail git hook run test-hook ++' + -+ cat >expected <<-EOF && -+ local: $ROOT/path/ghi -+ local: $ROOT/path/def ++test_expect_success 'git -c core.hooksPath=<PATH> hook run' ' ++ mkdir my-hooks && ++ write_script my-hooks/test-hook <<-EOF && ++ echo Hook ran >>actual + EOF + -+ git hook list pre-commit >actual && -+ test_cmp expected actual ++ cat >expect <<-\EOF && ++ Test hook ++ Hook ran ++ Hook ran ++ Hook ran ++ Hook ran ++ EOF ++ ++ # Test various ways of specifying the path. See also ++ # t1350-config-hooks-path.sh ++ >actual && ++ git hook run test-hook 2>>actual && ++ git -c core.hooksPath=my-hooks hook run test-hook 2>>actual && ++ git -c core.hooksPath=my-hooks/ hook run test-hook 2>>actual && ++ git -c core.hooksPath="$PWD/my-hooks" hook run test-hook 2>>actual && ++ git -c core.hooksPath="$PWD/my-hooks/" hook run test-hook 2>>actual && ++ test_cmp expect actual ++' ++ ++test_expect_success 'set up a pre-commit hook in core.hooksPath' ' ++ >actual && ++ mkdir -p .git/custom-hooks .git/hooks && ++ write_script .git/custom-hooks/pre-commit <<-\EOF && ++ echo CUSTOM >>actual ++ EOF ++ write_script .git/hooks/pre-commit <<-\EOF ++ echo NORMAL >>actual ++ EOF +' + +test_done 3: faa4a655183 < -: ----------- hook: include hookdir hook in list 4: c43a7e0dd52 < -: ----------- hook: teach hook.runHookDir 5: 81e453baea2 < -: ----------- hook: implement hookcmd.<name>.skip 6: 71f9ee9ab82 < -: ----------- parse-options: parse into strvec 7: e43a7a94163 < -: ----------- hook: add 'run' subcommand 8: 807ad9cf2f3 < -: ----------- hook: introduce hook_exists() 35: 43d2383af49 ! 4: 1a67a1cc065 run-command: stop thinking about hooks @@ ## Metadata ## -Author: Emily Shaffer <emilyshaffer@google.com> +Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - run-command: stop thinking about hooks + run-command.h: move find_hook() to hook.h - hook.h has replaced all run-command.h hook-related functionality. - run-command.h:run_hooks_le/ve and find_hook are no longer used anywhere - in the codebase. So, let's delete the dead code - or, in the one case - where it's still needed, move it to an internal function in hook.c. + Move the find_hook() command to hook.h. Eventually all the hook + related code will live there, let's move this function over as-is. - Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> + + ## builtin/am.c ## +@@ + #include "parse-options.h" + #include "dir.h" + #include "run-command.h" ++#include "hook.h" + #include "quote.h" + #include "tempfile.h" + #include "lockfile.h" + + ## builtin/bugreport.c ## +@@ + #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) + + ## builtin/commit.c ## +@@ + #include "revision.h" + #include "wt-status.h" + #include "run-command.h" ++#include "hook.h" + #include "refs.h" + #include "log-tree.h" + #include "strbuf.h" + + ## builtin/merge.c ## +@@ + #include "builtin.h" + #include "lockfile.h" + #include "run-command.h" ++#include "hook.h" + #include "diff.h" + #include "diff-merges.h" + #include "refs.h" + + ## builtin/receive-pack.c ## +@@ + #include "pkt-line.h" + #include "sideband.h" + #include "run-command.h" ++#include "hook.h" + #include "exec-cmd.h" + #include "commit.h" + #include "object.h" + + ## builtin/worktree.c ## +@@ + #include "branch.h" + #include "refs.h" + #include "run-command.h" ++#include "hook.h" + #include "sigchain.h" + #include "submodule.h" + #include "utf8.h" ## hook.c ## -@@ hook.c: static int should_include_hookdir(const char *path, enum hookdir_opt cfg) - } - } +@@ + #include "hook.h" + #include "run-command.h" -+static const char *find_legacy_hook(const char *name) ++const char *find_hook(const char *name) +{ + static struct strbuf path = STRBUF_INIT; + @@ hook.c: static int should_include_hookdir(const char *path, enum hookdir_opt cfg +} + + - struct list_head* hook_list(const char* hookname) ++ + void run_hooks_opt_clear(struct run_hooks_opt *o) { - struct strbuf hook_key = STRBUF_INIT; -@@ hook.c: struct list_head* hook_list(const char* hookname) - git_config(hook_config_lookup, &cb_data); - - if (have_git_dir()) { -- const char *legacy_hook_path = find_hook(hookname); -+ const char *legacy_hook_path = find_legacy_hook(hookname); - - /* Unconditionally add legacy hook, but annotate it. */ - if (legacy_hook_path) { -@@ hook.c: int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir) - could_run_hookdir = (should_run_hookdir == HOOKDIR_INTERACTIVE || - should_run_hookdir == HOOKDIR_WARN || - should_run_hookdir == HOOKDIR_YES) -- && !!find_hook(hookname); -+ && !!find_legacy_hook(hookname); + strvec_clear(&o->env); + + ## hook.h ## +@@ hook.h: struct hook_cb_data { + struct run_hooks_opt *options; + }; - strbuf_addf(&hook_key, "hook.%s.command", hookname); ++/* ++ * Returns the path to the hook file, or NULL if the hook is missing ++ * or disabled. Note that this points to static storage that will be ++ * overwritten by further calls to find_hook and run_hook_*. ++ */ ++const char *find_hook(const char *name); ++ + void run_hooks_opt_clear(struct run_hooks_opt *o); + /* + + ## refs.c ## +@@ + #include "refs.h" + #include "refs/refs-internal.h" + #include "run-command.h" ++#include "hook.h" + #include "object-store.h" + #include "object.h" + #include "tag.h" ## run-command.c ## +@@ + #include "string-list.h" + #include "quote.h" + #include "config.h" ++#include "hook.h" + + void child_process_init(struct child_process *child) + { @@ run-command.c: int async_with_fork(void) #endif } @@ run-command.c: int async_with_fork(void) - return path.buf; -} - --int run_hook_ve(const char *const *env, const char *name, va_list args) --{ -- struct child_process hook = CHILD_PROCESS_INIT; -- const char *p; -- -- p = find_hook(name); -- if (!p) -- return 0; -- -- strvec_push(&hook.args, p); -- while ((p = va_arg(args, const char *))) -- strvec_push(&hook.args, p); -- hook.env = env; -- hook.no_stdin = 1; -- hook.stdout_to_stderr = 1; -- hook.trace2_hook_name = name; -- -- return run_command(&hook); --} -- --int run_hook_le(const char *const *env, const char *name, ...) --{ -- va_list args; -- int ret; -- -- va_start(args, name); -- ret = run_hook_ve(env, name, args); -- va_end(args); -- -- return ret; --} -- - struct io_pump { - /* initialized by caller */ - int fd; + int run_hook_ve(const char *const *env, const char *name, va_list args) + { + struct child_process hook = CHILD_PROCESS_INIT; ## run-command.h ## @@ run-command.h: int finish_command_in_signal(struct child_process *); @@ run-command.h: int finish_command_in_signal(struct child_process *); - */ -const char *find_hook(const char *name); - --/** -- * Run a hook. -- * The first argument is a pathname to an index file, or NULL -- * if the hook uses the default index file or no index is needed. -- * The second argument is the name of the hook. -- * The further arguments correspond to the hook arguments. -- * The last argument has to be NULL to terminate the arguments list. -- * If the hook does not exist or is not executable, the return -- * value will be zero. -- * If it is executable, the hook will be executed and the exit -- * status of the hook is returned. -- * On execution, .stdout_to_stderr and .no_stdin will be set. -- */ --LAST_ARG_MUST_BE_NULL --int run_hook_le(const char *const *env, const char *name, ...); --int run_hook_ve(const char *const *env, const char *name, va_list args); -- - /* - * Trigger an auto-gc - */ + /** + * Run a hook. + * The first argument is a pathname to an index file, or NULL + + ## sequencer.c ## +@@ + #include "sequencer.h" + #include "tag.h" + #include "run-command.h" ++#include "hook.h" + #include "exec-cmd.h" + #include "utf8.h" + #include "cache-tree.h" + + ## transport.c ## +@@ + #include "config.h" + #include "transport.h" + #include "run-command.h" ++#include "hook.h" + #include "pkt-line.h" + #include "fetch-pack.h" + #include "remote.h" 33: 33043b33b04 ! 5: a6f0817ad81 bugreport: use hook_exists instead of find_hook @@ Metadata Author: Emily Shaffer <emilyshaffer@google.com> ## Commit message ## - bugreport: use hook_exists instead of find_hook + hook.c: add a hook_exists() wrapper and use it in bugreport.c - By using the helper in hook.h instead of the one in run-command.h, we - can also check whether a hook exists in the config - not just whether it - exists in the hookdir. + Add a boolean version of the find_hook() function for those callers + who are only interested in checking whether the hook exists, not what + the path to it is. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/bugreport.c ## -@@ - #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) @@ builtin/bugreport.c: 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], HOOKDIR_USE_CONFIG)) ++ if (hook_exists(hook[i])) strbuf_addf(hook_info, "%s\n", hook[i]); } + + ## hook.c ## +@@ hook.c: const char *find_hook(const char *name) + return path.buf; + } + +- ++int hook_exists(const char *name) ++{ ++ return !!find_hook(name); ++} + + void run_hooks_opt_clear(struct run_hooks_opt *o) + { + + ## hook.h ## +@@ hook.h: struct hook_cb_data { + */ + const char *find_hook(const char *name); + ++/* ++ * A boolean version of find_hook() ++ */ ++int hook_exists(const char *hookname); ++ + void run_hooks_opt_clear(struct run_hooks_opt *o); + + /* 20: 2006e2709f6 ! 6: b186fde43e1 gc: use hook library for pre-auto-gc hook @@ Commit message well as in the hookdir. pre-auto-gc is called only from builtin/gc.c. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: This hook is invoked by `git gc --auto` (see linkgit:git-gc[1]). It - takes no parameter, and exiting with non-zero status from this script - causes the `git gc --auto` to abort. - -+Hooks run during 'pre-auto-gc' will be run in parallel, unless hook.jobs is -+configured to 1. -+ - post-rewrite - ~~~~~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/gc.c ## @@ @@ builtin/gc.c: static void add_repack_incremental_option(void) static int need_to_gc(void) { -+ struct run_hooks_opt hook_opt; ++ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + /* * Setting gc.auto to 0 or negative can disable the @@ builtin/gc.c: static int need_to_gc(void) return 0; - if (run_hook_le(NULL, "pre-auto-gc", NULL)) -+ run_hooks_opt_init_async(&hook_opt); + if (run_hooks("pre-auto-gc", &hook_opt)) { + run_hooks_opt_clear(&hook_opt); return 0; 21: 62f6f9ab90d ! 7: 528402fac69 rebase: teach pre-rebase to use hook.h @@ Metadata ## Commit message ## rebase: teach pre-rebase to use hook.h - By using hook.h instead of run-command.h to run hooks, pre-rebase hooks - can now be specified in the config as well as in the hookdir. pre-rebase - is not called anywhere besides builtin/rebase.c. + Move the pre-rebase hook away from run-command.h to and over to the + new hook.h library. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: two parameters. The first parameter is the upstream from which - the series was forked. The second parameter is the branch being - rebased, and is not set when rebasing the current branch. - -+Hooks executed during 'pre-rebase' will run in parallel, unless hook.jobs is -+configured to 1. -+ - post-checkout - ~~~~~~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/rebase.c ## @@ @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix char *squash_onto_name = NULL; int reschedule_failed_exec = -1; int allow_preemptive_ff = 1; -+ struct run_hooks_opt hook_opt; ++ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; struct option builtin_rebase_options[] = { OPT_STRING(0, "onto", &options.onto_name, N_("revision"), @@ builtin/rebase.c: int cmd_rebase(int argc, const char **argv, const char *prefix } /* If a hook exists, give it a chance to interrupt*/ -+ run_hooks_opt_init_async(&hook_opt); + strvec_pushl(&hook_opt.args, options.upstream_arg, argc ? argv[0] : NULL, NULL); if (!ok_to_skip_pre_rebase && - run_hook_le(NULL, "pre-rebase", options.upstream_arg, 18: 744e156ae91 ! 8: 69842c74383 am: convert applypatch hooks to use config @@ Commit message am: convert applypatch hooks to use config Teach pre-applypatch, post-applypatch, and applypatch-msg to use the - hook.h library instead of the run-command.h library. This enables use of - hooks specified in the config, in addition to those in the hookdir. - These three hooks are called only by builtin/am.c. + hook.h library instead of the run-command.h library. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: the message file. - The default 'applypatch-msg' hook, when enabled, runs the - 'commit-msg' hook, if the latter is enabled. - -+Hooks run during 'applypatch-msg' will not be parallelized, because hooks are -+expected to edit the file holding the commit log message. -+ - pre-applypatch - ~~~~~~~~~~~~~~ - -@@ Documentation/githooks.txt: make a commit if it does not pass certain test. - The default 'pre-applypatch' hook, when enabled, runs the - 'pre-commit' hook, if the latter is enabled. - -+Hooks run during 'pre-applypatch' will be run in parallel, unless hook.jobs is -+configured to 1. -+ - post-applypatch - ~~~~~~~~~~~~~~~ - -@@ Documentation/githooks.txt: and is invoked after the patch is applied and a commit is made. - This hook is meant primarily for notification, and cannot affect - the outcome of `git am`. - -+Hooks run during 'post-applypatch' will be run in parallel, unless hook.jobs is -+configured to 1. -+ - pre-commit - ~~~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/am.c ## -@@ - #include "string-list.h" - #include "packfile.h" - #include "repository.h" -+#include "hook.h" - - /** - * Returns the length of the first line of msg. @@ builtin/am.c: static void am_destroy(const struct am_state *state) static int run_applypatch_msg_hook(struct am_state *state) { int ret; -+ struct run_hooks_opt opt; -+ -+ run_hooks_opt_init_sync(&opt); ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; assert(state->msg); - ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL); @@ builtin/am.c: static void do_commit(const struct am_state *state) struct commit_list *parents = NULL; const char *reflog_msg, *author, *committer = NULL; struct strbuf sb = STRBUF_INIT; -+ struct run_hooks_opt hook_opt; -+ -+ run_hooks_opt_init_async(&hook_opt); ++ struct run_hooks_opt hook_opt_pre = RUN_HOOKS_OPT_INIT; ++ struct run_hooks_opt hook_opt_post = RUN_HOOKS_OPT_INIT; - if (run_hook_le(NULL, "pre-applypatch", NULL)) -+ if (run_hooks("pre-applypatch", &hook_opt)) { -+ run_hooks_opt_clear(&hook_opt); ++ if (run_hooks("pre-applypatch", &hook_opt_pre)) { ++ run_hooks_opt_clear(&hook_opt_pre); exit(1); + } -+ -+ run_hooks_opt_clear(&hook_opt); if (write_cache_as_tree(&tree, 0, NULL)) die(_("git write-tree failed to write a tree")); @@ builtin/am.c: static void do_commit(const struct am_state *state) } - run_hook_le(NULL, "post-applypatch", NULL); -+ run_hooks_opt_init_async(&hook_opt); -+ run_hooks("post-applypatch", &hook_opt); ++ run_hooks("post-applypatch", &hook_opt_post); -+ run_hooks_opt_clear(&hook_opt); ++ run_hooks_opt_clear(&hook_opt_pre); ++ run_hooks_opt_clear(&hook_opt_post); strbuf_release(&sb); } 25: a13a690bebb ! 9: 9b32c14669b hooks: convert 'post-checkout' hook to hook library @@ Metadata ## Commit message ## hooks: convert 'post-checkout' hook to hook library - By using the 'hook.h' library, 'post-checkout' hooks can now be - specified in the config as well as in the hook directory. + Move the running of the 'post-checkout' hook away from run-command.h + to the new hook.h library. For "worktree" this requires a change to it + to run the hooks from a given directory. - Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + We could strictly speaking skip the "absolute_path" flag and just + check if "dir" is specified, but let's split them up for clarity, as + well as for any future user who'd like to set "dir" but not implicitly + change the argument to an absolute path. - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: This hook can be used to perform repository validity checks, auto-display - differences from the previous HEAD if different, or set working dir metadata - properties. - -+Hooks executed during 'post-checkout' will not be parallelized. -+ - post-merge - ~~~~~~~~~~ - + Signed-off-by: Emily Shaffer <emilyshaffer@google.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/checkout.c ## @@ @@ builtin/checkout.c: struct branch_info { - oid_to_hex(old_commit ? &old_commit->object.oid : null_oid()), - oid_to_hex(new_commit ? &new_commit->object.oid : null_oid()), - changed ? "1" : "0", NULL); -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + int rc; -+ -+ run_hooks_opt_init_sync(&opt); + /* "new_commit" can be NULL when checking out from the index before a commit exists. */ @@ builtin/clone.c: static int checkout(int submodule_progress) struct tree *tree; struct tree_desc t; int err = 0; -+ struct run_hooks_opt hook_opt; ++ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; if (option_no_checkout) return 0; @@ builtin/clone.c: static int checkout(int submodule_progress) - err |= run_hook_le(NULL, "post-checkout", oid_to_hex(null_oid()), - oid_to_hex(&oid), "1", NULL); -+ run_hooks_opt_init_sync(&hook_opt); + strvec_pushl(&hook_opt.args, oid_to_hex(null_oid()), oid_to_hex(&oid), "1", NULL); + err |= run_hooks("post-checkout", &hook_opt); + run_hooks_opt_clear(&hook_opt); @@ builtin/clone.c: static int checkout(int submodule_progress) struct strvec args = STRVEC_INIT; ## builtin/worktree.c ## -@@ - #include "utf8.h" - #include "worktree.h" - #include "quote.h" -+#include "hook.h" - - static const char * const worktree_usage[] = { - N_("git worktree add [<options>] <path> [<commit-ish>]"), @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refname, * is_junk is cleared, but do return appropriate code when hook fails. */ @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam - "1", NULL); - ret = run_command(&cp); - } -+ struct run_hooks_opt opt; -+ -+ run_hooks_opt_init_sync(&opt); ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + + strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL); + strvec_pushl(&opt.args, @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam + "1", + NULL); + opt.dir = path; ++ opt.absolute_path = 1; + + ret = run_hooks("post-checkout", &opt); + @@ builtin/worktree.c: static int add_worktree(const char *path, const char *refnam strvec_clear(&child_env); + ## hook.c ## +@@ hook.c: 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_cb->hook_name; ++ cp->dir = hook_cb->options->dir; + + /* add command */ + strvec_push(&cp->args, run_me->hook_path); +@@ hook.c: static int notify_hook_finished(int result, + int run_found_hooks(const char *hook_name, const char *hook_path, + struct run_hooks_opt *options) + { ++ struct strbuf abs_path = STRBUF_INIT; + struct hook my_hook = { + .hook_path = hook_path, + }; +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, + .hook_name = hook_name, + .options = options, + }; ++ if (options->absolute_path) { ++ strbuf_add_absolute_path(&abs_path, hook_path); ++ my_hook.hook_path = abs_path.buf; ++ } + cb_data.run_me = &my_hook; + + if (options->jobs != 1) +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, + &cb_data, + "hook", + hook_name); ++ if (options->absolute_path) ++ strbuf_release(&abs_path); + + return cb_data.rc; + } + + ## hook.h ## +@@ hook.h: struct run_hooks_opt + + /* Number of threads to parallelize across */ + int jobs; ++ ++ /* Resolve and run the "absolute_path(hook)" instead of ++ * "hook". Used for "git worktree" hooks ++ */ ++ int absolute_path; ++ ++ /* Path to initial working directory for subprocess */ ++ const char *dir; ++ + }; + + #define RUN_HOOKS_OPT_INIT { \ + ## read-cache.c ## @@ + #include "thread-utils.h" #include "progress.h" #include "sparse-index.h" - #include "hook.h" -->>>>>>> 9524a9d29d (read-cache: convert post-index-change hook to use config) ++#include "hook.h" /* Mask for the name length in ce_flags in the on-disk index */ @@ reset.c: int reset_head(struct repository *r, struct object_id *oid, const char - oid_to_hex(orig ? orig : null_oid()), - oid_to_hex(oid), "1", NULL); + if (run_hook) { -+ struct run_hooks_opt opt; -+ -+ run_hooks_opt_init_sync(&opt); ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + strvec_pushl(&opt.args, + oid_to_hex(orig ? orig : null_oid()), + oid_to_hex(oid), 19: be1d7c7636b ! 10: 201c654bb0c merge: use config-based hooks for post-merge hook @@ Commit message builtin/merge.c. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: save and restore any form of metadata associated with the working tree - (e.g.: permissions/ownership, ACLS, etc). See contrib/hooks/setgitperms.perl - for an example of how to do this. - -+Hooks executed during 'post-merge' will run in parallel, unless hook.jobs is -+configured to 1. -+ - pre-push - ~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/merge.c ## @@ builtin/merge.c: static void finish(struct commit *head_commit, const struct object_id *new_head, const char *msg) { struct strbuf reflog_message = STRBUF_INIT; -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; const struct object_id *head = &head_commit->object.oid; if (!msg) @@ builtin/merge.c: static void finish(struct commit *head_commit, /* Run a post-merge hook */ - run_hook_le(NULL, "post-merge", squash ? "1" : "0", NULL); -+ run_hooks_opt_init_async(&opt); + strvec_push(&opt.args, squash ? "1" : "0"); + run_hooks("post-merge", &opt); + run_hooks_opt_clear(&opt); apply_autostash(git_path_merge_autostash(the_repository)); strbuf_release(&reflog_message); +@@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads) + * and write it out as a tree. We must do this before we invoke + * the editor and after we invoke run_status above. + */ +- if (find_hook("pre-merge-commit")) ++ if (hook_exists("pre-merge-commit")) + discard_cache(); + read_cache_from(index_file); + strbuf_addbuf(&msg, &merge_msg); -: ----------- > 11: e65d8bd6e6f git hook run: add an --ignore-missing flag 34: 832136eb930 ! 12: 8795e9ceab8 git-send-email: use 'git hook run' for 'sendemail-validate' @@ Metadata Author: Emily Shaffer <emilyshaffer@google.com> ## Commit message ## - git-send-email: use 'git hook run' for 'sendemail-validate' + send-email: use 'git hook run' for 'sendemail-validate' - By using the new 'git hook run' subcommand to run 'sendemail-validate', - we can reduce the boilerplate needed to run this hook in perl. Using - config-based hooks also allows us to run 'sendemail-validate' hooks that - were configured globally when running 'git send-email' from outside of a - Git directory, alongside other benefits like multihooks and - parallelization. + Change the "sendmail-validate" hook to be run via the "git hook run" + wrapper instead of via a direct invocation. + + This is the smallest possibly change to get "send-email" using "git + hook run". We still check the hook itself with "-x", and set a + "GIT_DIR" variable, both of which are asserted by our tests. We'll + need to get rid of this special behavior if we start running N hooks, + but for now let's be as close to bug-for-bug compatible as possible. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## git-send-email.perl ## -@@ git-send-email.perl: sub unique_email_list { - sub validate_patch { +@@ git-send-email.perl: sub format_2822_time { + my $editor; + + sub system_or_msg { +- my ($args, $msg) = @_; ++ my ($args, $msg, $cmd_name) = @_; + system(@$args); + my $signalled = $? & 127; + my $exit_code = $? >> 8; + return unless $signalled or $exit_code; + +- my @sprintf_args = ($args->[0], $exit_code); ++ my @sprintf_args = ($cmd_name ? $cmd_name : $args->[0], $exit_code); + if (defined $msg) { + # Quiet the 'redundant' warning category, except we + # need to support down to Perl 5.8, so we can't do a +@@ git-send-email.perl: sub validate_patch { my ($fn, $xfer_encoding) = @_; -- if ($repo) { -- my $validate_hook = catfile($repo->hooks_path(), + if ($repo) { ++ my $hook_name = 'sendemail-validate'; + my $hooks_path = $repo->command_oneline('rev-parse', '--git-path', 'hooks'); +- my $validate_hook = catfile($hooks_path, - 'sendemail-validate'); -- my $hook_error; -- if (-x $validate_hook) { -- my $target = abs_path($fn); -- # The hook needs a correct cwd and GIT_DIR. -- my $cwd_save = cwd(); -- chdir($repo->wc_path() or $repo->repo_path()) -- or die("chdir: $!"); -- local $ENV{"GIT_DIR"} = $repo->repo_path(); ++ my $validate_hook = catfile($hooks_path, $hook_name); + my $hook_error; + if (-x $validate_hook) { + my $target = abs_path($fn); +@@ git-send-email.perl: sub validate_patch { + chdir($repo->wc_path() or $repo->repo_path()) + or die("chdir: $!"); + local $ENV{"GIT_DIR"} = $repo->repo_path(); - $hook_error = system_or_msg([$validate_hook, $target]); -- chdir($cwd_save) or die("chdir: $!"); -- } -- if ($hook_error) { ++ my @validate_hook = ("git", "hook", "run", "--ignore-missing", $hook_name, "--", $target); ++ $hook_error = system_or_msg(\@validate_hook, undef, ++ "git hook run $hook_name -- <patch>"); + chdir($cwd_save) or die("chdir: $!"); + } + if ($hook_error) { - die sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" . - "%s\n" . - "warning: no patches were sent\n"), $fn, $hook_error); -- } -- } -+ my $target = abs_path($fn); -+ -+ system_or_die(["git", "hook", "run", "sendemail-validate", "-j1", "-a", $target], -+ sprintf(__("fatal: %s: rejected by sendemail-validate hook\n" . -+ "warning: no patches were sent\n"), -+ $fn)); ++ $hook_error = sprintf(__("fatal: %s: rejected by %s hook\n" . ++ $hook_error . "\n" . ++ "warning: no patches were sent\n"), ++ $fn, $hook_name); ++ die $hook_error; + } + } - # Any long lines will be automatically fixed if we use a suitable transfer - # encoding. ## t/t9001-send-email.sh ## @@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects relative core.hooksPath path" ' test_path_is_file my-hooks.ran && cat >expect <<-EOF && fatal: longline.patch: rejected by sendemail-validate hook -- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1 +- fatal: command '"'"'my-hooks/sendemail-validate'"'"' died with exit code 1 ++ fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1 warning: no patches were sent EOF test_cmp expect actual @@ t/t9001-send-email.sh: test_expect_success $PREREQ "--validate respects absolute test_path_is_file my-hooks.ran && cat >expect <<-EOF && fatal: longline.patch: rejected by sendemail-validate hook -- fatal: command '"'"'$PWD/my-hooks/sendemail-validate'"'"' died with exit code 1 +- fatal: command '"'"'$hooks_path/sendemail-validate'"'"' died with exit code 1 ++ fatal: command '"'"'git hook run sendemail-validate -- <patch>'"'"' died with exit code 1 warning: no patches were sent EOF test_cmp expect actual -@@ t/t9001-send-email.sh: test_expect_success $PREREQ 'invoke hook' ' - mkdir -p .git/hooks && - - write_script .git/hooks/sendemail-validate <<-\EOF && -- # test that we have the correct environment variable, pwd, and -- # argument -- case "$GIT_DIR" in -- *.git) -- true -- ;; -- *) -- false -- ;; -- esac && -+ # test that we have the correct argument - test -f 0001-add-main.patch && - grep "add main" "$1" - EOF 24: f4c92f0dbc1 ! 13: 03129460fd2 git-p4: use 'git hook' to run hooks @@ Commit message git-p4: use 'git hook' to run hooks Instead of duplicating the behavior of run-command.h:run_hook_le() in - Python, we can directly call 'git hook run'. As a bonus, this means - git-p4 learns how to find hook specifications from the Git config as - well as from the hookdir. + Python, we can directly call 'git hook run'. We emulate the existence + check with the --ignore-missing flag. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## git-p4.py ## @@ git-p4.py: def decode_path(path): + return path def run_git_hook(cmd, param=[]): - """Execute a hook if the hook exists.""" +- """Execute a hook if the hook exists.""" - if verbose: - sys.stderr.write("Looking for hook: %s\n" % cmd) - sys.stderr.flush() @@ git-p4.py: def decode_path(path): - return True - - if not os.path.isfile(hook_file) or not os.access(hook_file, os.X_OK): -+ if not cmd: - return True - +- return True +- - return run_hook_command(hook_file, param) == 0 - -def run_hook_command(cmd, param): @@ git-p4.py: def decode_path(path): - else: - use_shell = True - return subprocess.call(cli, shell=use_shell) +- + """args are specified with -a <arg> -a <arg> -a <arg>""" -+ args = (['git', 'hook', 'run'] + -+ ["-a" + arg for arg in param] + -+ [cmd]) - ++ args = ['git', 'hook', 'run', '--ignore-missing', cmd] ++ if param: ++ args.append("--") ++ for p in param: ++ args.append(p) + return subprocess.call(args) == 0 def write_pipe(c, stdin): -: ----------- > 14: 3f3610f5ed3 commit: use hook.h to execute hooks 22: 0e405276551 ! 15: 6482a3e4cb8 read-cache: convert post-index-change hook to use config @@ Commit message can now be specified in the config in addition to the hookdir. post-index-change is not run anywhere besides in read-cache.c. - Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + This removes the last direct user of run_hook_ve(), so we can make the + function static now. It'll be removed entirely soon. - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: and "0" meaning they were not. - Only one parameter should be set to "1" when the hook runs. The hook - running passing "1", "1" should not be possible. - -+Hooks run during 'post-index-change' will be run in parallel, unless hook.jobs -+is configured to 1. -+ - GIT - --- - Part of the linkgit:git[1] suite + Signed-off-by: Emily Shaffer <emilyshaffer@google.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## read-cache.c ## -@@ - #include "thread-utils.h" - #include "progress.h" - #include "sparse-index.h" -+#include "hook.h" -+>>>>>>> 9524a9d29d (read-cache: convert post-index-change hook to use config) - - /* Mask for the name length in ce_flags in the on-disk index */ - @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struct lock_file *l { int ret; int was_full = !istate->sparse_index; -+ struct run_hooks_opt hook_opt; ++ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; ret = convert_to_sparse(istate); @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struc - run_hook_le(NULL, "post-index-change", - istate->updated_workdir ? "1" : "0", - istate->updated_skipworktree ? "1" : "0", NULL); -+ run_hooks_opt_init_async(&hook_opt); + strvec_pushl(&hook_opt.args, + istate->updated_workdir ? "1" : "0", + istate->updated_skipworktree ? "1" : "0", @@ read-cache.c: static int do_write_locked_index(struct index_state *istate, struc istate->updated_workdir = 0; istate->updated_skipworktree = 0; + + ## run-command.c ## +@@ run-command.c: int async_with_fork(void) + #endif + } + +-int run_hook_ve(const char *const *env, const char *name, va_list args) ++static int run_hook_ve(const char *const *env, const char *name, va_list args) + { + struct child_process hook = CHILD_PROCESS_INIT; + const char *p; + + ## run-command.h ## +@@ run-command.h: int run_command(struct child_process *); + */ + LAST_ARG_MUST_BE_NULL + int run_hook_le(const char *const *env, const char *name, ...); +-int run_hook_ve(const char *const *env, const char *name, va_list args); + + /* + * Trigger an auto-gc 23: a793508373e ! 16: a16163d4fb5 receive-pack: convert push-to-checkout hook to hook.h @@ Commit message hooks can now be specified in the config as well as in the hookdir. push-to-checkout is not called anywhere but in builtin/receive-pack.c. - Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + This is the last user of the run_hook_le() API, so let's remove it + while we're at it, since run_hook_le() itself is the last user of + run_hook_ve() we can remove that too. The last direct user of + run_hook_le() was removed in the commit preceding this one. - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: that switches branches while - keeping the local changes in the working tree that do not interfere - with the difference between the branches. - -+Hooks executed during 'push-to-checkout' will not be parallelized. - - pre-auto-gc - ~~~~~~~~~~~ + Signed-off-by: Emily Shaffer <emilyshaffer@google.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/receive-pack.c ## -@@ - #include "commit-reach.h" - #include "worktree.h" - #include "shallow.h" -+#include "hook.h" - - static const char * const receive_pack_usage[] = { - N_("git receive-pack <git-dir>"), @@ builtin/receive-pack.c: static const char *push_to_checkout(unsigned char *hash, struct strvec *env, const char *work_tree) { -+ struct run_hooks_opt opt; -+ -+ run_hooks_opt_init_sync(&opt); ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); - if (run_hook_le(env->v, push_to_checkout_hook, @@ builtin/receive-pack.c: static const char *update_worktree(unsigned char *sha1, strvec_pushf(&env, "GIT_DIR=%s", absolute_path(git_dir)); - if (!find_hook(push_to_checkout_hook)) -+ if (!hook_exists(push_to_checkout_hook, HOOKDIR_USE_CONFIG)) ++ if (!hook_exists(push_to_checkout_hook)) retval = push_to_deploy(sha1, &env, work_tree); else retval = push_to_checkout(sha1, &env, work_tree); + + ## run-command.c ## +@@ run-command.c: int async_with_fork(void) + #endif + } + +-static int run_hook_ve(const char *const *env, const char *name, va_list args) +-{ +- struct child_process hook = CHILD_PROCESS_INIT; +- const char *p; +- +- p = find_hook(name); +- if (!p) +- return 0; +- +- strvec_push(&hook.args, p); +- while ((p = va_arg(args, const char *))) +- strvec_push(&hook.args, p); +- hook.env = env; +- hook.no_stdin = 1; +- hook.stdout_to_stderr = 1; +- hook.trace2_hook_name = name; +- +- return run_command(&hook); +-} +- +-int run_hook_le(const char *const *env, const char *name, ...) +-{ +- va_list args; +- int ret; +- +- va_start(args, name); +- ret = run_hook_ve(env, name, args); +- va_end(args); +- +- return ret; +-} +- + struct io_pump { + /* initialized by caller */ + int fd; + + ## run-command.h ## +@@ run-command.h: int finish_command_in_signal(struct child_process *); + */ + int run_command(struct child_process *); + +-/** +- * Run a hook. +- * The first argument is a pathname to an index file, or NULL +- * if the hook uses the default index file or no index is needed. +- * The second argument is the name of the hook. +- * The further arguments correspond to the hook arguments. +- * The last argument has to be NULL to terminate the arguments list. +- * If the hook does not exist or is not executable, the return +- * value will be zero. +- * If it is executable, the hook will be executed and the exit +- * status of the hook is returned. +- * On execution, .stdout_to_stderr and .no_stdin will be set. +- */ +-LAST_ARG_MUST_BE_NULL +-int run_hook_le(const char *const *env, const char *name, ...); +- + /* + * Trigger an auto-gc + */ 10: 744437273de ! 17: 7020cf10c8e run-command: allow stdin for run_processes_parallel @@ Commit message on child output in stderr. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## run-command.c ## @@ run-command.c: static int pp_start_one(struct parallel_processes *pp) 9: 3e07045894f ! 18: 4745dcfce49 hook: support passing stdin to hooks @@ Commit message interim file themselves. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Documentation/git-hook.txt ## -@@ Documentation/git-hook.txt: SYNOPSIS +@@ Documentation/git-hook.txt: git-hook - run git hooks + SYNOPSIS -------- [verse] - 'git hook' list <hook-name> --'git hook' run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] <hook-name> -+'git hook' run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] -+ <hook-name> +-'git hook' run [--ignore-missing] <hook-name> [-- <hook-args>] ++'git hook' run [--to-stdin=<path>] [--ignore-missing] <hook-name> [-- <hook-args>] DESCRIPTION ----------- -@@ Documentation/git-hook.txt: in the order they should be run, and print the config scope where the relevant - `hook.<hook-name>.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)=<var>...] [(-a|--arg)=<arg>...] `<hook-name>`:: -+run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] [--to-stdin=<path>] `<hook-name>`:: - - Runs hooks configured for `<hook-name>`, in the same order displayed by `git - hook list`. Hooks configured this way may be run prepended with `sh -c`, so -@@ Documentation/git-hook.txt: Specify arguments to pass to every hook that is run. - + - Specify environment variables to set for every hook that is run. +@@ Documentation/git-hook.txt: run:: + OPTIONS + ------- +--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. ++ For "run"; Specify a file which will be streamed into the ++ hook's stdin. The hook will receive the entire file from ++ beginning to EOF. + - CONFIGURATION - ------------- - include::config/hook.txt[] + --ignore-missing:: + Ignore any missing hook by quietly returning zero. Used for + tools that want to do a blind one-shot run of a hook that may ## builtin/hook.c ## @@ + #include "strvec.h" static const char * const builtin_hook_usage[] = { - N_("git hook list <hookname>"), -- N_("git hook run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] <hookname>"), -+ N_("git hook run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...]" -+ "[--to-stdin=<path>] <hookname>"), +- N_("git hook run <hook-name> [-- <hook-args>]"), ++ N_("git hook run [--to-stdin=<path>] <hook-name> [-- <hook-args>]"), NULL }; @@ builtin/hook.c: 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")), + struct option run_options[] = { + OPT_BOOL(0, "ignore-missing", &ignore_missing, + N_("exit quietly with a zero exit code if the requested hook cannot be found")), + OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"), + N_("file to read into hooks' stdin")), OPT_END(), @@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix) ## hook.c ## -@@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o) - { - strvec_init(&o->env); - strvec_init(&o->args); -+ o->path_to_stdin = NULL; - o->run_hookdir = configured_hookdir_opt(); - } - -@@ hook.c: static void prepare_hook_cp(const char *hookname, struct hook *hook, - if (!hook) - return; +@@ hook.c: static int pick_next_hook(struct child_process *cp, + if (!run_me) + BUG("did we not return 1 in notify_hook_finished?"); - cp->no_stdin = 1; + /* reopen the file for stdin; run_command closes it. */ -+ if (options->path_to_stdin) -+ cp->in = xopen(options->path_to_stdin, O_RDONLY); -+ else ++ 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; -+ - cp->env = options->env.v; ++ } + cp->env = hook_cb->options->env.v; cp->stdout_to_stderr = 1; - cp->trace2_hook_name = hookname; + cp->trace2_hook_name = hook_cb->hook_name; ## hook.h ## @@ hook.h: struct run_hooks_opt - * to be overridden if the user can override it at the command line. - */ - enum hookdir_opt run_hookdir; -+ + /* Path to initial working directory for subprocess */ + const char *dir; + + /* Path to file which should be piped to stdin for each hook */ + const char *path_to_stdin; }; - void run_hooks_opt_init(struct run_hooks_opt *o); -@@ hook.h: 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); - + #define RUN_HOOKS_OPT_INIT { \ - ## t/t1360-config-based-hooks.sh ## -@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir is tolerant to unknown values' ' - test_cmp expected actual + ## t/t1800-hook.sh ## +@@ t/t1800-hook.sh: test_expect_success 'set up a pre-commit hook in core.hooksPath' ' + EOF ' -+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 ++test_expect_success 'stdin to hooks' ' ++ write_script .git/hooks/test-hook <<-\EOF && ++ echo BEGIN stdin ++ cat ++ echo END stdin + EOF + -+ cat >expected <<-EOF && -+ a1 -+ a2 -+ a3 -+ b1 -+ b2 -+ b3 ++ cat >expect <<-EOF && ++ BEGIN stdin ++ hello ++ END stdin + EOF + -+ git hook run --to-stdin=input test 2>actual && -+ test_cmp expected actual ++ echo hello >input && ++ git hook run --to-stdin=input test-hook 2>actual && ++ test_cmp expect actual +' + test_done 11: e6c789629d6 < -: ----------- hook: allow parallel hook execution 12: 26e2d14bc1a < -: ----------- hook: allow specifying working directory for hooks -: ----------- > 19: 986bfd89a54 am: convert 'post-rewrite' hook to hook.h 13: e721d45efc6 ! 20: 756f52af22d run-command: add stdin callback for parallelization @@ Commit message match the rest of the API reduces mental load on the user. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/fetch.c ## @@ builtin/fetch.c: static int fetch_multiple(struct string_list *list, int max_children) @@ builtin/submodule--helper.c: static int update_submodules(struct submodule_updat ## hook.c ## -@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, run_processes_parallel_tr2(options->jobs, pick_next_hook, notify_start_failure, 14: 5d74ea3ed3f ! 21: 3748f128763 hook: provide stdin by string_list or callback @@ Commit message with instead. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## hook.c ## -@@ - - void free_hook(struct hook *ptr) - { -- if (ptr) -+ if (ptr) { - strbuf_release(&ptr->command); -+ free(ptr->feed_pipe_cb_data); -+ } - free(ptr); - } - -@@ hook.c: 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 */ -@@ hook.c: void run_hooks_opt_init_sync(struct run_hooks_opt *o) - o->run_hookdir = configured_hookdir_opt(); - o->jobs = 1; - o->dir = NULL; -+ o->feed_pipe = NULL; -+ o->feed_pipe_ctx = NULL; - } - - void run_hooks_opt_init_async(struct run_hooks_opt *o) @@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o) strvec_clear(&o->args); } @@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o) +{ + int *item_idx; + struct hook *ctx = pp_task_cb; -+ struct string_list *to_pipe = ((struct hook_cb_data*)pp_cb)->options->feed_pipe_ctx; ++ struct hook_cb_data *hook_cb = pp_cb; ++ struct string_list *to_pipe = hook_cb->options->feed_pipe_ctx; + + /* Bootstrap the state manager if necessary. */ + if (!ctx->feed_pipe_cb_data) { @@ hook.c: static int pick_next_hook(struct child_process *cp, } else { cp->no_stdin = 1; } -@@ hook.c: 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->feed_pipe) -+ BUG("choose only one method to populate stdin"); -+ - to_run = hook_list(hookname); +@@ hook.c: static int notify_hook_finished(int result, + return 1; + } - list_for_each_safe(pos, tmp, to_run) { -@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) +- + int run_found_hooks(const char *hook_name, const char *hook_path, + struct run_hooks_opt *options) + { +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, run_processes_parallel_tr2(options->jobs, pick_next_hook, notify_start_failure, @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) notify_hook_finished, &cb_data, "hook", +@@ hook.c: int run_hooks(const char *hook_name, 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->feed_pipe) ++ BUG("choose only one method to populate stdin"); ++ + hook_path = find_hook(hook_name); + + /* Care about nonexistence? Use run_found_hooks() */ ## hook.h ## @@ - #include "list.h" - #include "strbuf.h" - #include "strvec.h" -+#include "run-command.h" - struct hook { - struct list_head list; -@@ hook.h: struct hook { - /* The literal command to run. */ - struct strbuf command; - unsigned from_hookdir : 1; + /* The path to the hook */ + const char *hook_path; + + /* + * Use this to keep state for your feed_pipe_fn if you are using @@ hook.h: struct hook { + void *feed_pipe_cb_data; }; - /* + struct run_hooks_opt @@ hook.h: struct run_hooks_opt /* Path to file which should be piped to stdin for each hook */ const char *path_to_stdin; ++ + /* + * Callback and state pointer to ask for more content to pipe to stdin. + * Will be called repeatedly, for each hook. See @@ hook.h: struct run_hooks_opt + */ + 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; -+ }; + #define RUN_HOOKS_OPT_INIT { \ +@@ hook.h: struct run_hooks_opt + .args = STRVEC_INIT, \ + } + +/* + * To specify a 'struct string_list', set 'run_hooks_opt.feed_pipe_ctx' to the + * string_list and set 'run_hooks_opt.feed_pipe' to 'pipe_from_string_list()'. 26: 6d804ca20fd ! 22: 0cf0b1fea93 hook: convert 'post-rewrite' hook to config @@ Metadata Author: Emily Shaffer <emilyshaffer@google.com> ## Commit message ## - hook: convert 'post-rewrite' hook to config + hook: convert 'post-rewrite' hook in sequencer.c to hook.h By using 'hook.h' for 'post-rewrite', we simplify hook invocations by - not needing to put together our own 'struct child_process' and we also - learn to run hooks specified in the config as well as the hook dir. + not needing to put together our own 'struct child_process'. - The signal handling that's being removed by this commit now takes place - in run-command.h:run_processes_parallel(), so it is OK to remove them - here. + The signal handling that's being removed by this commit now takes + place in run-command.h:run_processes_parallel(), so it is OK to remove + them here. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: The hook always runs after the automatic note copying (see - "notes.rewrite.<command>" in linkgit:git-config[1]) has happened, and - thus has access to these notes. - -+Hooks run during 'post-rewrite' will be run in parallel, unless hook.jobs is -+configured to 1. -+ - The following command-specific comments apply: - - rebase:: - - ## builtin/am.c ## -@@ builtin/am.c: static int run_applypatch_msg_hook(struct am_state *state) - */ - static int run_post_rewrite_hook(const struct am_state *state) - { -- struct child_process cp = CHILD_PROCESS_INIT; -- const char *hook = find_hook("post-rewrite"); -+ struct run_hooks_opt opt; - int ret; - -- if (!hook) -- return 0; -+ run_hooks_opt_init_async(&opt); - -- strvec_push(&cp.args, hook); -- strvec_push(&cp.args, "rebase"); -+ strvec_push(&opt.args, "rebase"); -+ opt.path_to_stdin = am_path(state, "rewritten"); - -- cp.in = xopen(am_path(state, "rewritten"), O_RDONLY); -- cp.stdout_to_stderr = 1; -- cp.trace2_hook_name = "post-rewrite"; -+ ret = run_hooks("post-rewrite", &opt); - -- ret = run_command(&cp); -- -- close(cp.in); -+ run_hooks_opt_clear(&opt); - return ret; - } - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## sequencer.c ## @@ + #include "commit-reach.h" #include "rebase-interactive.h" #include "reset.h" - #include "hook.h" +#include "string-list.h" #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" @@ sequencer.c: int update_head_with_reflog(const struct commit *old_head, { - struct child_process proc = CHILD_PROCESS_INIT; - const char *argv[3]; -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct strbuf tmp = STRBUF_INIT; + struct string_list to_stdin = STRING_LIST_INIT_DUP; int code; @@ sequencer.c: int update_head_with_reflog(const struct commit *old_head, - argv[0] = find_hook("post-rewrite"); - if (!argv[0]) - return 0; -+ run_hooks_opt_init_async(&opt); ++ strvec_push(&opt.args, "amend"); - argv[1] = "amend"; - argv[2] = NULL; @@ sequencer.c: int update_head_with_reflog(const struct commit *old_head, - strbuf_release(&sb); - sigchain_pop(SIGPIPE); - return finish_command(&proc); -+ strvec_push(&opt.args, "amend"); -+ + strbuf_addf(&tmp, + "%s %s", + oid_to_hex(oldoid), @@ sequencer.c: static int pick_commits(struct repository *r, - strvec_push(&child.args, "copy"); - strvec_push(&child.args, "--for-rewrite=rebase"); + struct child_process notes_cp = CHILD_PROCESS_INIT; -+ struct run_hooks_opt hook_opt; -+ -+ run_hooks_opt_init_async(&hook_opt); ++ struct run_hooks_opt hook_opt = RUN_HOOKS_OPT_INIT; + + notes_cp.in = open(rebase_path_rewritten_list(), O_RDONLY); + notes_cp.git_cmd = 1; 27: e761410a1e8 ! 23: c59443a3b05 transport: convert pre-push hook to use config @@ Commit message the config as well as in the hookdir. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: If this hook exits with a non-zero status, `git push` will abort without - pushing anything. Information about why the push is rejected may be sent - to the user by writing to standard error. - -+Hooks executed during 'pre-push' will run in parallel, unless hook.jobs is -+configured to 1. -+ - [[pre-receive]] - pre-receive - ~~~~~~~~~~~ + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## transport.c ## -@@ - #include "protocol.h" - #include "object-store.h" - #include "color.h" -+#include "hook.h" - - static int transport_use_color = -1; - static char transport_colors[][COLOR_MAXLEN] = { @@ transport.c: static void die_with_unpushed_submodules(struct string_list *needs_pushing) static int run_pre_push_hook(struct transport *transport, struct ref *remote_refs) { - int ret = 0, x; + int ret = 0; -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct strbuf tmp = STRBUF_INIT; struct ref *r; - struct child_process proc = CHILD_PROCESS_INIT; @@ transport.c: static void die_with_unpushed_submodules(struct string_list *needs_ - - sigchain_push(SIGPIPE, SIG_IGN); + struct string_list to_stdin = STRING_LIST_INIT_DUP; -+ run_hooks_opt_init_async(&opt); - strbuf_init(&buf, 256); + strvec_push(&opt.args, transport->remote->name); 28: e43f9f93d7b ! 24: f7c8c97cb81 reference-transaction: look for hooks in config @@ Metadata Author: Emily Shaffer <emilyshaffer@google.com> ## Commit message ## - reference-transaction: look for hooks in config + reference-transaction: use hook.h to run hooks - By using the hook.h library, reference-transaction hooks can be - specified in the config instead. - - The expected output of the test is not fully updated to reflect the - absolute path of the hook called because the 'update' hook has not yet - been converted to use hook.h. + By using the hook.h library, we get closer to removing the hook code + in run-command.c. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: The exit status of the hook is ignored for any state except for the - cause the transaction to be aborted. The hook will not be called with - "aborted" state in that case. - -+Hooks run during 'reference-transaction' will be run in parallel, unless -+hook.jobs is configured to 1. -+ - push-to-checkout - ~~~~~~~~~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## refs.c ## -@@ - #include "strvec.h" - #include "repository.h" - #include "sigchain.h" -+#include "hook.h" - - /* - * List of all available backends @@ refs.c: int ref_update_reject_duplicates(struct string_list *refnames, static int run_transaction_hook(struct ref_transaction *transaction, const char *state) @@ refs.c: int ref_update_reject_duplicates(struct string_list *refnames, - struct child_process proc = CHILD_PROCESS_INIT; struct strbuf buf = STRBUF_INIT; - const char *hook; -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct string_list to_stdin = STRING_LIST_INIT_DUP; int ret = 0, i; + char o[GIT_MAX_HEXSZ + 1], n[GIT_MAX_HEXSZ + 1]; - hook = find_hook("reference-transaction"); - if (!hook) -- return ret; -- ++ if (!hook_exists("reference-transaction")) + return ret; + - strvec_pushl(&proc.args, hook, state, NULL); - proc.in = -1; - proc.stdout_to_stderr = 1; - proc.trace2_hook_name = "reference-transaction"; -+ run_hooks_opt_init_async(&opt); - +- - ret = start_command(&proc); - if (ret) -+ if (!hook_exists("reference-transaction", HOOKDIR_USE_CONFIG)) - return ret; - +- return ret; +- - sigchain_push(SIGPIPE, SIG_IGN); + strvec_push(&opt.args, state); @@ refs.c: int ref_update_reject_duplicates(struct string_list *refnames, return ret; } - - ## t/t1416-ref-transaction-hooks.sh ## -@@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls succeed' ' - - cat >expect <<-EOF && - hooks/update refs/tags/PRE $ZERO_OID $PRE_OID -- hooks/reference-transaction prepared -- hooks/reference-transaction committed -+ $(pwd)/target-repo.git/hooks/reference-transaction prepared -+ $(pwd)/target-repo.git/hooks/reference-transaction committed - hooks/update refs/tags/POST $ZERO_OID $POST_OID -- hooks/reference-transaction prepared -- hooks/reference-transaction committed -+ $(pwd)/target-repo.git/hooks/reference-transaction prepared -+ $(pwd)/target-repo.git/hooks/reference-transaction committed - EOF - - git push ./target-repo.git PRE POST && - - ## transport.c ## -@@ transport.c: static int run_pre_push_hook(struct transport *transport, - struct strbuf tmp = STRBUF_INIT; - struct ref *r; - struct string_list to_stdin = STRING_LIST_INIT_DUP; -+ - run_hooks_opt_init_async(&opt); - - strvec_push(&opt.args, transport->remote->name); 15: 84fe7a21976 ! 25: f240a51ec4e run-command: allow capturing of collated output @@ Commit message instead hand it to the caller. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/fetch.c ## @@ builtin/fetch.c: static int fetch_multiple(struct string_list *list, int max_children) @@ builtin/submodule--helper.c: static int update_submodules(struct submodule_updat ## hook.c ## -@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, pick_next_hook, notify_start_failure, options->feed_pipe, 16: f23bcba44a0 ! 26: 7f10efb7858 hooks: allow callers to capture output @@ Commit message sideband instead of printing directly to stderr. Expose that capability. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## hook.c ## -@@ hook.c: void run_hooks_opt_init_sync(struct run_hooks_opt *o) - o->dir = NULL; - o->feed_pipe = NULL; - o->feed_pipe_ctx = NULL; -+ o->consume_sideband = NULL; - } - - void run_hooks_opt_init_async(struct run_hooks_opt *o) -@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, pick_next_hook, notify_start_failure, options->feed_pipe, @@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options) ## hook.h ## @@ hook.h: 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 @@ hook.h: struct run_hooks_opt + * for an example. + */ + consume_sideband_fn consume_sideband; -+ - /* Number of threads to parallelize across */ - int jobs; + }; + #define RUN_HOOKS_OPT_INIT { \ 29: 7931831dc6e ! 27: c39c608e5cc receive-pack: convert 'update' hook to hook.h @@ Metadata ## Commit message ## receive-pack: convert 'update' hook to hook.h - By using hook.h to invoke the 'update' hook, now hooks can be specified - in the config in addition to the hookdir. + By using hook.h to invoke the 'update' hook we closer to removing the + hooks code in run-command.c. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: The default 'update' hook, when enabled--and with - `hooks.allowunannotated` config option unset or set to false--prevents - unannotated tags to be pushed. - -+Hooks executed during 'update' are run in parallel, unless hook.jobs is -+configured to 1. -+ - [[proc-receive]] - proc-receive - ~~~~~~~~~~~~ + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/receive-pack.c ## @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands, return status; } +-static int run_update_hook(struct command *cmd) +static void hook_output_to_sideband(struct strbuf *output, void *cb_data) -+{ + { +- const char *argv[5]; +- struct child_process proc = CHILD_PROCESS_INIT; +- int code; + int keepalive_active = 0; -+ + +- argv[0] = find_hook("update"); +- if (!argv[0]) +- return 0; + if (keepalive_in_sec <= 0) + use_keepalive = KEEPALIVE_NEVER; + if (use_keepalive == KEEPALIVE_ALWAYS) + keepalive_active = 1; -+ + +- argv[1] = cmd->ref_name; +- argv[2] = oid_to_hex(&cmd->old_oid); +- argv[3] = oid_to_hex(&cmd->new_oid); +- argv[4] = NULL; + /* send a keepalive if there is no data to write */ + if (keepalive_active && !output->len) { + static const char buf[] = "0005\1"; + write_or_die(1, buf, sizeof(buf) - 1); + return; + } -+ + +- proc.no_stdin = 1; +- proc.stdout_to_stderr = 1; +- proc.err = use_sideband ? -1 : 0; +- proc.argv = argv; +- proc.trace2_hook_name = "update"; + if (use_keepalive == KEEPALIVE_AFTER_NUL && !keepalive_active) { + const char *first_null = memchr(output->buf, '\0', output->len); + if (first_null) { @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands, + send_sideband(1, 2, output->buf, output->len, use_sideband); +} + - static int run_update_hook(struct command *cmd) - { -- const char *argv[5]; -- struct child_process proc = CHILD_PROCESS_INIT; -+ struct run_hooks_opt opt; - int code; - -- argv[0] = find_hook("update"); -- if (!argv[0]) -- return 0; -+ run_hooks_opt_init_async(&opt); - -- argv[1] = cmd->ref_name; -- argv[2] = oid_to_hex(&cmd->old_oid); -- argv[3] = oid_to_hex(&cmd->new_oid); -- argv[4] = NULL; ++static int run_update_hook(struct command *cmd) ++{ ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; ++ int code; ++ + strvec_pushl(&opt.args, + cmd->ref_name, + oid_to_hex(&cmd->old_oid), + oid_to_hex(&cmd->new_oid), + NULL); -- proc.no_stdin = 1; -- proc.stdout_to_stderr = 1; -- proc.err = use_sideband ? -1 : 0; -- proc.argv = argv; -- proc.trace2_hook_name = "update"; -- - code = start_command(&proc); - if (code) - return code; @@ builtin/receive-pack.c: static int run_receive_hook(struct command *commands, } static struct command *find_command_by_refname(struct command *list, - - ## t/t1416-ref-transaction-hooks.sh ## -@@ t/t1416-ref-transaction-hooks.sh: test_expect_success 'interleaving hook calls succeed' ' - EOF - - cat >expect <<-EOF && -- hooks/update refs/tags/PRE $ZERO_OID $PRE_OID -+ $(pwd)/target-repo.git/hooks/update refs/tags/PRE $ZERO_OID $PRE_OID - $(pwd)/target-repo.git/hooks/reference-transaction prepared - $(pwd)/target-repo.git/hooks/reference-transaction committed -- hooks/update refs/tags/POST $ZERO_OID $POST_OID -+ $(pwd)/target-repo.git/hooks/update refs/tags/POST $ZERO_OID $POST_OID - $(pwd)/target-repo.git/hooks/reference-transaction prepared - $(pwd)/target-repo.git/hooks/reference-transaction committed - EOF 31: efbe55fcb13 ! 28: 3519068a634 post-update: use hook.h library @@ Commit message be specified in the config as well as the hookdir. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: Both standard output and standard error output are forwarded to - `git send-pack` on the other end, so you can simply `echo` messages - for the user. - -+Hooks run during 'post-update' will be run in parallel, unless hook.jobs is -+configured to 1. -+ - reference-transaction - ~~~~~~~~~~~~~~~~~~~~~ - + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/receive-pack.c ## @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct shallow_info *si) @@ builtin/receive-pack.c: static const char *update(struct command *cmd, struct sh struct command *cmd; - struct child_process proc = CHILD_PROCESS_INIT; - const char *hook; -+ struct run_hooks_opt opt; - +- - hook = find_hook("post-update"); - if (!hook) - return; -+ run_hooks_opt_init_async(&opt); ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; for (cmd = commands; cmd; cmd = cmd->next) { if (cmd->error_string || cmd->did_not_exist) 32: 26e08bd5257 ! 29: 3466f17af08 receive-pack: convert receive hooks to hook.h @@ Metadata ## Commit message ## receive-pack: convert receive hooks to hook.h - By using the hook.h library to run receive hooks, they can be specified - in the config as well as in the hookdir. + By using the hook.h library to run receive hooks we get closer to + deleting the hook functions in run-command.c Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> - - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: will be set to zero, `GIT_PUSH_OPTION_COUNT=0`. - See the section on "Quarantine Environment" in - linkgit:git-receive-pack[1] for some caveats. - -+Hooks executed during 'pre-receive' will not be parallelized. -+ - [[update]] - update - ~~~~~~ -@@ Documentation/githooks.txt: environment variables will not be set. If the client selects - to use push options, but doesn't transmit any, the count variable - will be set to zero, `GIT_PUSH_OPTION_COUNT=0`. - -+Hooks executed during 'post-receive' are run in parallel, unless hook.jobs is -+configured to 1. -+ - [[post-update]] - post-update - ~~~~~~~~~~~ + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/receive-pack.c ## @@ builtin/receive-pack.c: static int check_cert_push_options(const struct string_list *push_options) @@ builtin/receive-pack.c: static void hook_output_to_sideband(struct strbuf *outpu + int skip_broken, + const struct string_list *push_options) +{ -+ struct run_hooks_opt opt; ++ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; + struct receive_hook_feed_context ctx; + int rc; + struct command *iter = commands; + -+ run_hooks_opt_init_async(&opt); -+ + /* if there are no valid commands, don't invoke the hook at all. */ + while (iter && skip_broken && (iter->error_string || iter->did_not_exist)) + iter = iter->next; @@ builtin/receive-pack.c: static void hook_output_to_sideband(struct strbuf *outpu + static int run_update_hook(struct command *cmd) { - struct run_hooks_opt opt; + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; 17: edb22d675b7 ! 30: d93bdc0c294 commit: use config-based hooks @@ ## Metadata ## -Author: Emily Shaffer <emilyshaffer@google.com> +Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## Commit message ## - commit: use config-based hooks + hooks: fix a TOCTOU in "did we run a hook?" heuristic - As part of the adoption of config-based hooks, teach run_commit_hook() - to call hook.h instead of run-command.h. This covers 'pre-commit', - 'commit-msg', and 'prepare-commit-msg'. Additionally, ask the hook - library - not run-command - whether any hooks will be run, as it's - possible hooks may exist in the config but not the hookdir. + Fix a race in code added in 680ee550d72 (commit: skip discarding the + index if there is no pre-commit hook, 2017-08-14) by changing the + hook.c API to optionally indicate whether or not the requested hook + ran or not. This was suggested in the discussion around + 680ee550d72[1]. - Because all but 'post-commit' hooks are expected to make some state - change, force all but 'post-commit' hook to run in series. 'post-commit' - "is meant primarily for notification, and cannot affect the outcome of - `git commit`," so it is fine to run in parallel. + Let's also change this for the pre-merge-commit hook, see + 6098817fd7f (git-merge: honor pre-merge-commit hook, 2019-08-07) for + the introduction of the previous behavior. - Signed-off-by: Emily Shaffer <emilyshaffer@google.com> - Signed-off-by: Junio C Hamano <gitster@pobox.com> + Let's also change this for the push-to-checkout hook. Now instead of + checking if the hook exists and either doing a push to checkout or a + push to deploy we'll always attempt a push to checkout. If the hook + doesn't exist we'll fall back on push to deploy. The same behavior as + before, without the TOCTOU race. See 0855331941b (receive-pack: + support push-to-checkout hook, 2014-12-01) for the introduction of the + previous behavior. - ## Documentation/githooks.txt ## -@@ Documentation/githooks.txt: The default 'pre-commit' hook, when enabled--and with the - `hooks.allownonascii` config option unset or set to false--prevents - the use of non-ASCII filenames. - -+Hooks executed during 'pre-commit' will not be parallelized. -+ - pre-merge-commit - ~~~~~~~~~~~~~~~~ - -@@ Documentation/githooks.txt: need to be resolved and the result committed separately (see - linkgit:git-merge[1]). At that point, this hook will not be executed, - but the 'pre-commit' hook will, if it is enabled. - -+Hooks executed during 'pre-merge-commit' will not be parallelized. -+ - prepare-commit-msg - ~~~~~~~~~~~~~~~~~~ - -@@ Documentation/githooks.txt: be used as replacement for pre-commit hook. - The sample `prepare-commit-msg` hook that comes with Git removes the - help message found in the commented portion of the commit template. - -+Hooks executed during 'prepare-commit-msg' will not be parallelized, because -+hooks are expected to edit the file containing the commit log message. -+ - commit-msg - ~~~~~~~~~~ - -@@ Documentation/githooks.txt: file. - The default 'commit-msg' hook, when enabled, detects duplicate - `Signed-off-by` trailers, and aborts the commit if one is found. - -+Hooks executed during 'commit-msg' will not be parallelized, because hooks are -+expected to edit the file containing the proposed commit log message. -+ - post-commit - ~~~~~~~~~~~ - -@@ Documentation/githooks.txt: invoked after a commit is made. - This hook is meant primarily for notification, and cannot affect - the outcome of `git commit`. - -+Hooks executed during 'post-commit' will run in parallel, unless hook.jobs is -+configured to 1. -+ - pre-rebase - ~~~~~~~~~~ - + This leaves uses of hook_exists() in two places that matter. The + "reference-transaction" check in refs.c, see 67541597670 (refs: + implement reference transaction hook, 2020-06-19), and the + prepare-commit-msg hook, see 66618a50f9c (sequencer: run + 'prepare-commit-msg' hook, 2018-01-24). + + In both of those cases we're saving ourselves CPU time by not + preparing data for the hook that we'll then do nothing with if we + don't have the hook, so using this "invoked_hook" pattern doesn't make + sense there purely for optimization purposes. + + More importantly, in those cases the worst we'll do is miss that we + "should" run the hook because a new hook appeared, whereas in the + pre-commit and pre-merge-commit cases we'll skip an important + discard_cache() on the bases of our faulty guess. + + I do think none of these races really matter in practice. It would be + some one-off issue as a hook was added or removed. I did think it was + stupid that we didn't pass a "did this run?" flag instead of doing + this guessing at a distance though, so now we're not guessing anymore. + + 1. https://lore.kernel.org/git/20170810191613.kpmhzg4seyxy3cpq@sigill.intra.peff.net/ + + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> ## builtin/commit.c ## -@@ - #include "help.h" - #include "commit-reach.h" - #include "commit-graph.h" -+#include "hook.h" - - static const char * const builtin_commit_usage[] = { - N_("git commit [<options>] [--] <pathspec>..."), @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix, + int clean_message_contents = (cleanup_mode != COMMIT_MSG_CLEANUP_NONE); + int old_display_comment_prefix; + int merge_contains_scissors = 0; ++ int invoked_hook = 0; + /* This checks and barfs if author is badly specified */ determine_author_info(author_ident); - if (!no_verify && run_commit_hook(use_editor, index_file, "pre-commit", NULL)) -+ if (!no_verify && run_commit_hook(use_editor, 0, index_file, "pre-commit", NULL)) ++ if (!no_verify && run_commit_hook(use_editor, index_file, &invoked_hook, ++ "pre-commit", NULL)) return 0; if (squash_message) { @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha return 0; } -- if (!no_verify && find_hook("pre-commit")) { -+ if (!no_verify && hook_exists("pre-commit", HOOKDIR_USE_CONFIG)) { +- if (!no_verify && hook_exists("pre-commit")) { ++ if (!no_verify && invoked_hook) { /* - * Re-read the index as pre-commit hook could have updated it, - * and write it out as a tree. We must do this before we invoke +- * Re-read the index as pre-commit hook could have updated it, +- * and write it out as a tree. We must do this before we invoke ++ * Re-read the index as the pre-commit-commit hook was invoked ++ * and could have updated it. We must do this before we invoke + * the editor and after we invoke run_status above. + */ + discard_cache(); @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (run_commit_hook(use_editor, index_file, "prepare-commit-msg", -+ if (run_commit_hook(use_editor, 0, index_file, "prepare-commit-msg", ++ if (run_commit_hook(use_editor, index_file, NULL, "prepare-commit-msg", git_path_commit_editmsg(), hook_arg1, hook_arg2, NULL)) return 0; @@ builtin/commit.c: static int prepare_to_commit(const char *index_file, const cha if (!no_verify && - run_commit_hook(use_editor, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) { -+ run_commit_hook(use_editor, 0, index_file, "commit-msg", git_path_commit_editmsg(), NULL)) { ++ run_commit_hook(use_editor, index_file, NULL, "commit-msg", ++ git_path_commit_editmsg(), NULL)) { return 0; } @@ builtin/commit.c: int cmd_commit(int argc, const char **argv, const char *prefix repo_rerere(the_repository, 0); run_auto_maintenance(quiet); - run_commit_hook(use_editor, get_index_file(), "post-commit", NULL); -+ run_commit_hook(use_editor, 1, get_index_file(), "post-commit", NULL); ++ run_commit_hook(use_editor, get_index_file(), NULL, "post-commit", ++ NULL); if (amend && !no_post_rewrite) { commit_post_rewrite(the_repository, current_head, &oid); } ## builtin/merge.c ## -@@ - #include "commit-reach.h" - #include "wt-status.h" - #include "commit-graph.h" -+#include "hook.h" - - #define DEFAULT_TWOHEAD (1<<0) - #define DEFAULT_OCTOPUS (1<<1) @@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads) + { struct strbuf msg = STRBUF_INIT; const char *index_file = get_index_file(); ++ int invoked_hook = 0; - if (!no_verify && run_commit_hook(0 < option_edit, index_file, "pre-merge-commit", NULL)) -+ if (!no_verify && run_commit_hook(0 < option_edit, 0, index_file, "pre-merge-commit", NULL)) ++ if (!no_verify && run_commit_hook(0 < option_edit, index_file, ++ &invoked_hook, "pre-merge-commit", ++ NULL)) abort_commit(remoteheads, NULL); /* - * Re-read the index as pre-merge-commit hook could have updated it, - * and write it out as a tree. We must do this before we invoke +- * Re-read the index as pre-merge-commit hook could have updated it, +- * and write it out as a tree. We must do this before we invoke ++ * Re-read the index as the pre-merge-commit hook was invoked ++ * and could have updated it. We must do this before we invoke * the editor and after we invoke run_status above. */ -- if (find_hook("pre-merge-commit")) -+ if (hook_exists("pre-merge-commit", HOOKDIR_USE_CONFIG)) +- if (hook_exists("pre-merge-commit")) ++ if (invoked_hook) discard_cache(); read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); @@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads) write_merge_heads(remoteheads); write_file_buf(git_path_merge_msg(the_repository), msg.buf, msg.len); - if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg", -+ if (run_commit_hook(0 < option_edit, 0, get_index_file(), "prepare-commit-msg", ++ if (run_commit_hook(0 < option_edit, get_index_file(), NULL, ++ "prepare-commit-msg", git_path_merge_msg(the_repository), "merge", NULL)) abort_commit(remoteheads, NULL); if (0 < option_edit) { @@ builtin/merge.c: static void prepare_to_commit(struct commit_list *remoteheads) - abort_commit(remoteheads, NULL); } -- if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(), -+ if (!no_verify && run_commit_hook(0 < option_edit, 0, get_index_file(), - "commit-msg", + if (!no_verify && run_commit_hook(0 < option_edit, get_index_file(), +- "commit-msg", ++ NULL, "commit-msg", git_path_merge_msg(the_repository), NULL)) abort_commit(remoteheads, NULL); - - ## commit.c ## -@@ - #include "commit-reach.h" - #include "run-command.h" - #include "shallow.h" -+#include "hook.h" - static struct commit_extra_header *read_commit_extra_header_lines(const char *buf, size_t len, const char **); - -@@ commit.c: size_t ignore_non_trailer(const char *buf, size_t len) - return boc ? len - boc : len - cutoff; - } + + ## builtin/receive-pack.c ## +@@ builtin/receive-pack.c: static const char *push_to_deploy(unsigned char *sha1, + static const char *push_to_checkout_hook = "push-to-checkout"; + + static const char *push_to_checkout(unsigned char *hash, ++ int *invoked_hook, + struct strvec *env, + const char *work_tree) + { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; ++ opt.invoked_hook = invoked_hook; --int run_commit_hook(int editor_is_used, const char *index_file, -+int run_commit_hook(int editor_is_used, int parallelize, const char *index_file, - const char *name, ...) + strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree)); + strvec_pushv(&opt.env, env->v); +@@ builtin/receive-pack.c: static const char *update_worktree(unsigned char *sha1, const struct worktree *w { -- struct strvec hook_env = STRVEC_INIT; -+ struct run_hooks_opt opt; - va_list args; -+ const char *arg; - int ret; + const char *retval, *work_tree, *git_dir = NULL; + struct strvec env = STRVEC_INIT; ++ int invoked_hook = 0; -- strvec_pushf(&hook_env, "GIT_INDEX_FILE=%s", index_file); -+ run_hooks_opt_init_sync(&opt); -+ -+ if (parallelize) -+ opt.jobs = configured_hook_jobs(); -+ -+ strvec_pushf(&opt.env, "GIT_INDEX_FILE=%s", index_file); + if (worktree && worktree->path) + work_tree = worktree->path; +@@ builtin/receive-pack.c: static const char *update_worktree(unsigned char *sha1, const struct worktree *w - /* - * Let the hook know that no editor will be launched. - */ - if (!editor_is_used) -- strvec_push(&hook_env, "GIT_EDITOR=:"); -+ strvec_push(&opt.env, "GIT_EDITOR=:"); + strvec_pushf(&env, "GIT_DIR=%s", absolute_path(git_dir)); - va_start(args, name); -- ret = run_hook_ve(hook_env.v, name, args); -+ while ((arg = va_arg(args, const char *))) -+ strvec_push(&opt.args, arg); - va_end(args); -- strvec_clear(&hook_env); -+ -+ ret = run_hooks(name, &opt); -+ run_hooks_opt_clear(&opt); +- if (!hook_exists(push_to_checkout_hook)) ++ retval = push_to_checkout(sha1, &invoked_hook, &env, work_tree); ++ if (!invoked_hook) + retval = push_to_deploy(sha1, &env, work_tree); +- else +- retval = push_to_checkout(sha1, &env, work_tree); - return ret; + strvec_clear(&env); + return retval; + + ## commit.c ## +@@ commit.c: size_t ignore_non_trailer(const char *buf, size_t len) } + + int run_commit_hook(int editor_is_used, const char *index_file, ++ int *invoked_hook, + const char *name, ...) + { + struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT; ## commit.h ## @@ commit.h: int compare_commits_by_commit_date(const void *a_, const void *b_, void *unused) @@ commit.h: int compare_commits_by_commit_date(const void *a_, const void *b_, voi LAST_ARG_MUST_BE_NULL -int run_commit_hook(int editor_is_used, const char *index_file, const char *name, ...); -+int run_commit_hook(int editor_is_used, int parallelize, const char *index_file, -+ const char *name, ...); ++int run_commit_hook(int editor_is_used, const char *index_file, ++ int *invoked_hook, const char *name, ...); /* Sign a commit or tag buffer, storing the result in a header. */ int sign_with_header(struct strbuf *buf, const char *keyid); - ## sequencer.c ## -@@ - #include "commit-reach.h" - #include "rebase-interactive.h" - #include "reset.h" -+#include "hook.h" + ## hook.c ## +@@ hook.c: static int notify_hook_finished(int result, + /* |= rc in cb */ + hook_cb->rc |= result; - #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION" ++ if (hook_cb->invoked_hook) ++ *hook_cb->invoked_hook = 1; ++ + return 1; + } +@@ hook.c: int run_found_hooks(const char *hook_name, const char *hook_path, + .rc = 0, + .hook_name = hook_name, + .options = options, ++ .invoked_hook = options->invoked_hook, + }; + if (options->absolute_path) { + strbuf_add_absolute_path(&abs_path, hook_path); + + ## hook.h ## +@@ hook.h: struct run_hooks_opt + * for an example. + */ + consume_sideband_fn consume_sideband; ++ ++ /* ++ * A pointer which if provided will be set to 1 or 0 depending ++ * on if a hook was invoked (i.e. existed), regardless of ++ * whether or not that was successful. Used for avoiding ++ * TOCTOU races in code that would otherwise call hook_exist() ++ * after a "maybe hook run" to see if a hook was invoked. ++ */ ++ int *invoked_hook; + }; + + #define RUN_HOOKS_OPT_INIT { \ +@@ hook.h: struct hook_cb_data { + const char *hook_name; + struct hook *run_me; + struct run_hooks_opt *options; ++ int *invoked_hook; + }; + + /* + + ## sequencer.c ## @@ sequencer.c: static int run_prepare_commit_msg_hook(struct repository *r, } else { arg1 = "message"; } - if (run_commit_hook(0, r->index_file, "prepare-commit-msg", name, -+ if (run_commit_hook(0, 0, r->index_file, "prepare-commit-msg", name, ++ if (run_commit_hook(0, r->index_file, NULL, "prepare-commit-msg", name, arg1, arg2, NULL)) ret = error(_("'prepare-commit-msg' hook failed")); -@@ sequencer.c: static int try_to_commit(struct repository *r, - } - } - -- if (find_hook("prepare-commit-msg")) { -+ if (hook_exists("prepare-commit-msg", HOOKDIR_USE_CONFIG)) { - res = run_prepare_commit_msg_hook(r, msg, hook_commit); - if (res) - goto out; @@ sequencer.c: static int try_to_commit(struct repository *r, goto out; } - run_commit_hook(0, r->index_file, "post-commit", NULL); -+ run_commit_hook(0, 1, r->index_file, "post-commit", NULL); ++ run_commit_hook(0, r->index_file, NULL, "post-commit", NULL); if (flags & AMEND_MSG) commit_post_rewrite(r, current_head, oid); - - ## t/t7503-pre-commit-and-pre-merge-commit-hooks.sh ## -@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME - . ./test-lib.sh - - HOOKDIR="$(git rev-parse --git-dir)/hooks" --PRECOMMIT="$HOOKDIR/pre-commit" --PREMERGE="$HOOKDIR/pre-merge-commit" -+PRECOMMIT="$(pwd)/$HOOKDIR/pre-commit" -+PREMERGE="$(pwd)/$HOOKDIR/pre-merge-commit" - - # Prepare sample scripts that write their $0 to actual_hooks - test_expect_success 'sample script setup' ' -@@ t/t7503-pre-commit-and-pre-merge-commit-hooks.sh: test_expect_success 'with succeeding hook' ' - test_cmp expected_hooks actual_hooks - ' - -+# NEEDSWORK: when 'git hook add' and 'git hook remove' have been added, use that -+# instead -+test_expect_success 'with succeeding hook (config-based)' ' -+ test_when_finished "git config --unset hook.pre-commit.command success.sample" && -+ test_when_finished "rm -f expected_hooks actual_hooks" && -+ git config hook.pre-commit.command "$HOOKDIR/success.sample" && -+ echo "$HOOKDIR/success.sample" >expected_hooks && -+ echo "more" >>file && -+ git add file && -+ git commit -m "more" && -+ test_cmp expected_hooks actual_hooks -+' -+ - test_expect_success 'with succeeding hook (merge)' ' - test_when_finished "rm -f \"$PREMERGE\" expected_hooks actual_hooks" && - cp "$HOOKDIR/success.sample" "$PREMERGE" && 30: c9d8c1581de < -: ----------- proc-receive: acquire hook list from hook.h 36: 4caea3e6805 < -: ----------- doc: clarify fsmonitor-watchman specification 37: f2e1003b62a < -: ----------- docs: link githooks and git-hook manpages -: ----------- > 31: 896956250f6 hook-list.h: add a generated list of hooks, like config-list.h