On Wed, Mar 17, 2021 at 11:41:59AM -0700, Emily Shaffer wrote:
>
> On Fri, Mar 12, 2021 at 10:49:38AM +0100, �var Arnfj�r� Bjarmason wrote:
> >
> >
> > On Thu, Mar 11 2021, Emily Shaffer wrote:
> >
> > > Since v7:
> > > - Addressed Jonathan Tan's review of part I
> > > - Addressed Junio's review of part I and II
> > > - Combined parts I and II
> > >
> > > I think the updates to patch 1 between the rest of the work I've been
> > > doing probably have covered �var's comments.
> >
> > A range-diff between iterations of such a large series would be most
> > useful. Do you have a public repo with tags or whatever the different
> > versions, for those who'd like an easier way to follow along the
> > differing versions than scraping the ML archive?
>
> I am really embarrassed to say that I don't have the
> branches/tags/whatever up. I have not succeeded in building that habit
> yet. I'll generate one from my local patches today and send it here.
1: be907f68b9 ! 1: a5e8c233c3 doc: propose hooks managed by the config
@@ Documentation/technical/config-based-hooks.txt (new)
+[[motivation]]
+== Motivation
+
-+Replace the .git/hook/hookname path as the only source of hooks to execute;
++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.
++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.
+
-+Take a step closer to safety when copying zipped Git repositories from untrusted
-+users by making it more apparent to users which scripts will be run during
-+normal Git operations.
-+
+Make it easier for users to discover Git's hook feature and automate their
+workflows.
+
@@ Documentation/technical/config-based-hooks.txt (new)
+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
@@ Documentation/technical/config-based-hooks.txt (new)
+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".
++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
@@ Documentation/technical/config-based-hooks.txt (new)
+=== 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@xxxxxxxxxxxxxxxxxxxxxxxxx/]
-+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
++vector.footnote:[https://lore.kernel.org/git/20171002234517.GV19555@xxxxxxxxxxxxxxxxxxxxxxxxx/]
++However, as the design stands, users can still provide hooks in the repo-level
++config, which is included when a repo is zipped and sent elsewhere. The
+security of the repo-level config is still under discussion; this design
-+generally assumes the repo-level config is secure, which is not true yet. The
-+goal is to avoid an overcomplicated design to work around a problem which has
-+ceased to exist.
++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
@@ Documentation/technical/config-based-hooks.txt (new)
+A previous summary of alternatives exists in the
+archives.footnote:[https://lore.kernel.org/git/20191116011125.GG22855@xxxxxxxxxx]
+
-+[[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.
-+
-+[[comparison]]
-+=== Comparison table
++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
+|===
@@ Documentation/technical/config-based-hooks.txt (new)
+|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
@@ Documentation/technical/config-based-hooks.txt (new)
+
+|Can install one hook to many repos
+|Yes
-+|No
-+|No
++|With symlinks or core.hooksPath
++|With symlinks or core.hooksPath
+
+|Discoverability
-+|Better (in `git help git`)
-+|Same as before
++|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
-+|No
++|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
+
@@ Documentation/technical/config-based-hooks.txt (new)
+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
+
2: b1d37c3911 = 2: a3e858d056 hook: scaffolding for git-hook subcommand
3: fea411c598 ! 3: 60b28a652b hook: add list command
@@ Commit message
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; at minimum, this subsection must contain a
+ <hookcmd-name>]" subsection; this subsection should contain a
"hookcmd.<hookcmd-name>.command = <path-to-hook>" line.
For example:
@@ Makefile: LIB_OBJS += hash-lookup.o
## builtin/hook.c ##
@@
#include "cache.h"
-
+-
#include "builtin.h"
+#include "config.h"
+#include "hook.h"
@@ builtin/hook.c
{
- struct option builtin_hook_options[] = {
+ struct list_head *head, *pos;
-+ struct hook *item;
+ struct strbuf hookname = STRBUF_INIT;
+
+ struct option list_options[] = {
@@ builtin/hook.c
+ }
+
+ list_for_each(pos, head) {
-+ item = list_entry(pos, struct hook, list);
++ struct hook *item = list_entry(pos, struct hook, list);
+ if (item)
+ printf("%s: %s\n",
+ config_scope_name(item->origin),
@@ hook.c (new)
+ 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(struct hook));
++ to_add = xmalloc(sizeof(*to_add));
+ strbuf_init(&to_add->command, 0);
+ strbuf_addstr(&to_add->command, command);
+ }
@@ hook.c (new)
+ /* re-set the scope so we show where an override was specified */
+ to_add->origin = current_config_scope();
+
-+ list_add_tail(&to_add->list, pos);
++ list_add_tail(&to_add->list, head);
+}
+
+static void remove_hook(struct list_head *to_remove)
@@ hook.c (new)
+ const char *command = value;
+ struct strbuf hookcmd_name = STRBUF_INIT;
+
-+ /* Check if a hookcmd with that name exists. */
++ /*
++ * 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);
+
@@ hook.c (new)
+
+ strbuf_addf(&hook_key, "hook.%s.command", hookname->buf);
+
-+ git_config(hook_config_lookup, (void*)&cb_data);
++ git_config(hook_config_lookup, &cb_data);
+
+ strbuf_release(&hook_key);
+ return hook_head;
@@ hook.h (new)
+#include "list.h"
+#include "strbuf.h"
+
-+struct hook
-+{
++struct hook {
+ struct list_head list;
+ /*
+ * Config file which holds the hook.*.command definition.
4: 89f1adf34d ! 4: d8232a8254 hook: include hookdir hook in list
@@ Commit message
$GIT_DIR/hooks/$HOOKNAME (or $HOOKDIR/$HOOKNAME). Although hooks taken
from the config are more featureful than hooks placed in the $HOOKDIR,
those hooks should not stop working for users who already have them.
-
- Legacy hooks should be run directly, not in shell. We know that they are
- a path to an executable, not a oneliner script - and running them
- directly takes care of path quoting concerns for us for free.
+ Let's list them to the user, but instead of displaying a config scope
+ (e.g. "global: blah") we can prefix them with "hookdir:".
Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
## builtin/hook.c ##
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
- struct list_head *head, *pos;
- struct hook *item;
- struct strbuf hookname = STRBUF_INIT;
-+ struct strbuf hookdir_annotation = STRBUF_INIT;
-
- struct option list_options[] = {
- OPT_END(),
-@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
list_for_each(pos, head) {
- item = list_entry(pos, struct hook, list);
+ struct hook *item = list_entry(pos, struct hook, list);
- if (item)
-- printf("%s: %s\n",
-- config_scope_name(item->origin),
-- item->command.buf);
++ item = list_entry(pos, struct hook, list);
+ if (item) {
+ /* Don't translate 'hookdir' - it matches the config */
-+ printf("%s: %s%s\n",
+ printf("%s: %s\n",
+- config_scope_name(item->origin),
+ (item->from_hookdir
+ ? "hookdir"
+ : config_scope_name(item->origin)),
-+ item->command.buf,
-+ (item->from_hookdir
-+ ? hookdir_annotation.buf
-+ : ""));
+ item->command.buf);
+ }
}
@@ hook.c
void free_hook(struct hook *ptr)
{
@@ hook.c: static void append_or_move_hook(struct list_head *head, const char *command)
- to_add = xmalloc(sizeof(struct hook));
+ to_add = xmalloc(sizeof(*to_add));
strbuf_init(&to_add->command, 0);
strbuf_addstr(&to_add->command, command);
+ to_add->from_hookdir = 0;
@@ hook.c: static void append_or_move_hook(struct list_head *head, const char *comm
/* re-set the scope so we show where an override was specified */
@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
- struct strbuf hook_key = STRBUF_INIT;
- struct list_head *hook_head = xmalloc(sizeof(struct list_head));
- struct hook_config_cb cb_data = { &hook_key, hook_head };
-+ const char *legacy_hook_path = NULL;
-
- INIT_LIST_HEAD(hook_head);
-@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
+ git_config(hook_config_lookup, &cb_data);
- git_config(hook_config_lookup, (void*)&cb_data);
-
-+ if (have_git_dir())
-+ legacy_hook_path = find_hook(hookname->buf);
++ if (have_git_dir()) {
++ const char *legacy_hook_path = find_hook(hookname->buf);
+
-+ /* Unconditionally add legacy hook, but annotate it. */
-+ if (legacy_hook_path) {
-+ struct hook *legacy_hook;
++ /* 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;
++ 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);
@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
}
## hook.h ##
-@@ hook.h: struct hook
+@@ hook.h: struct hook {
enum config_scope origin;
/* The literal command to run. */
struct strbuf command;
-+ int from_hookdir;
++ unsigned from_hookdir : 1;
};
/*
5: 723edcd785 ! 5: 96c0a4838f hook: respect hook.runHookDir
@@ Metadata
Author: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
## Commit message ##
- hook: respect hook.runHookDir
+ hook: teach hook.runHookDir
- Include hooks specified in the hook directory in the list of hooks to
- run. These hooks do need to be treated differently from config-specified
- ones - they do not need to run in a shell, and later on may be disabled
- or warned about based on a config setting.
-
- Because they are at least as local as the local config, we'll run them
- last - to keep the hook execution order from global to local.
-
- Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
+ For now, just give a hint about how these hooks will be run in 'git hook
+ list'. Later on, though, we will pay attention to this enum when running
+ the hooks.
## Documentation/config/hook.txt ##
@@ Documentation/config/hook.txt: hookcmd.<name>.command::
@@ builtin/hook.c: static const char * const builtin_hook_usage[] = {
static int list(int argc, const char **argv, const char *prefix)
{
struct list_head *head, *pos;
+ struct strbuf hookname = STRBUF_INIT;
++ struct strbuf hookdir_annotation = STRBUF_INIT;
+
+ struct option list_options[] = {
+ OPT_END(),
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
return 0;
}
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
+ case HOOKDIR_NO:
+ strbuf_addstr(&hookdir_annotation, _(" (will not run)"));
+ break;
++ case HOOKDIR_ERROR:
++ strbuf_addstr(&hookdir_annotation, _(" (will error and not run)"));
++ break;
+ case HOOKDIR_INTERACTIVE:
+ strbuf_addstr(&hookdir_annotation, _(" (will prompt)"));
+ break;
+ case HOOKDIR_WARN:
-+ case HOOKDIR_UNKNOWN:
-+ strbuf_addstr(&hookdir_annotation, _(" (will warn)"));
++ strbuf_addstr(&hookdir_annotation, _(" (will warn but run)"));
+ break;
+ case HOOKDIR_YES:
+ /*
+ * The default behavior should agree with
-+ * hook.c:configured_hookdir_opt().
++ * hook.c:configured_hookdir_opt(). HOOKDIR_UNKNOWN should just
++ * do the default behavior.
+ */
++ case HOOKDIR_UNKNOWN:
+ default:
+ break;
+ }
+
list_for_each(pos, head) {
+ struct hook *item = list_entry(pos, struct hook, list);
item = list_entry(pos, struct hook, list);
if (item) {
+ /* Don't translate 'hookdir' - it matches the config */
+- printf("%s: %s\n",
++ printf("%s: %s%s\n",
+ (item->from_hookdir
+ ? "hookdir"
+ : config_scope_name(item->origin)),
+- item->command.buf);
++ item->command.buf,
++ (item->from_hookdir
++ ? hookdir_annotation.buf
++ : ""));
+ }
+ }
+
+ clear_hook_list(head);
++ strbuf_release(&hookdir_annotation);
+ strbuf_release(&hookname);
+
+ return 0;
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
int cmd_hook(int argc, const char **argv, const char *prefix)
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
+ 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"))
@@ hook.c: static int hook_config_lookup(const char *key, const char *value, void *
+ if (!strcmp(key, "no"))
+ return HOOKDIR_NO;
+
++ if (!strcmp(key, "error"))
++ return HOOKDIR_ERROR;
++
+ if (!strcmp(key, "yes"))
+ return HOOKDIR_YES;
+
@@ hook.c: static int hook_config_lookup(const char *key, const char *value, void *
struct strbuf hook_key = STRBUF_INIT;
## hook.h ##
-@@ hook.h: struct hook
+@@ hook.h: struct hook {
*/
struct list_head* hook_list(const struct strbuf *hookname);
+enum hookdir_opt
+{
+ HOOKDIR_NO,
++ HOOKDIR_ERROR,
+ HOOKDIR_WARN,
+ HOOKDIR_INTERACTIVE,
+ HOOKDIR_YES,
@@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr
+ test_i18ncmp expected 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_i18ncmp expected actual
++'
++
+test_expect_success 'hook.runHookDir = warn is respected by list' '
+ setup_hookdir &&
+
+ test_config hook.runHookDir "warn" &&
+
+ cat >expected <<-EOF &&
-+ hookdir: $(pwd)/.git/hooks/pre-commit (will warn)
++ hookdir: $(pwd)/.git/hooks/pre-commit (will warn but run)
+ EOF
+
+ git hook list pre-commit >actual &&
@@ t/t1360-config-based-hooks.sh: test_expect_success 'git hook list shows hooks fr
+ # the hookdir annotation is translated
+ test_i18ncmp expected actual
+'
++
++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_i18ncmp expected actual
++'
+
test_done
6: 567f6d9d00 ! 6: 9068e11679 hook: implement hookcmd.<name>.skip
@@ Commit message
hook: implement hookcmd.<name>.skip
If a user wants a specific repo to skip execution of a hook which is set
- at a global or system level, they can now do so by specifying 'skip' in
- their repo config:
+ at a global or system level, they will be able to do so by specifying
+ 'skip' in their repo config:
~/.gitconfig
[hook.pre-commit]
@@ Commit message
Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
+ ## Documentation/config/hook.txt ##
+@@ Documentation/config/hook.txt: hookcmd.<name>.command::
+ 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
+
+ ## Documentation/git-hook.txt ##
+@@ Documentation/git-hook.txt: $ 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
++----
++
++Local config
++----
++ [hookcmd "check-for-secrets"]
++ skip = true
++ # This works for inlined hook commands, too:
++ [hookcmd "~/typocheck.sh"]
++ skip = true
++----
++
++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
+ --------
+
+
## hook.c ##
@@ hook.c: void free_hook(struct hook *ptr)
}
}
-static void append_or_move_hook(struct list_head *head, const char *command)
-+static struct hook* find_hook_by_command(struct list_head *head, const char *command)
++static struct hook * find_hook_by_command(struct list_head *head, const char *command)
{
struct list_head *pos = NULL, *tmp = NULL;
- struct hook *to_add = NULL;
@@ hook.c: void free_hook(struct hook *ptr)
- /* we'll simply move the hook to the end */
- to_add = it;
+ found = it;
+ break;
}
}
+ return found;
@@ hook.c: void free_hook(struct hook *ptr)
if (!to_add) {
/* adding a new hook, not moving an old one */
-@@ hook.c: static void append_or_move_hook(struct list_head *head, const char *command)
- /* re-set the scope so we show where an override was specified */
- to_add->origin = current_config_scope();
-
-- list_add_tail(&to_add->list, pos);
-+ list_add_tail(&to_add->list, head);
- }
-
- static void remove_hook(struct list_head *to_remove)
@@ hook.c: static int hook_config_lookup(const char *key, const char *value, void *cb_data)
if (!strcmp(key, hook_key)) {
const char *command = value;
@@ hook.c: static int hook_config_lookup(const char *key, const char *value, void *
+ strbuf_addf(&hookcmd_name, "hookcmd.%s.skip", command);
+ git_config_get_bool(hookcmd_name.buf, &skip);
- /* Check if a hookcmd with that name exists. */
+ /*
+ * 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);
@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is re
+ test_i18ncmp 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_i18ncmp expected actual
++'
++
+test_expect_success 'git hook list removes skipped inlined hook' '
+ setup_hooks &&
+ test_config hookcmd."$ROOT/path/ghi".skip "true" --add &&
7: a1c02b6758 ! 7: a2867ab8c0 parse-options: parse into strvec
@@ Documentation/technical/api-parse-options.txt: There are some macros to easily d
Use of `--no-option` will clear the list of preceding values.
+`OPT_STRVEC(short, long, &struct strvec, arg_str, description)`::
-+ Introduce an option with a string argument.
-+ The string argument is stored as an element in `strvec`.
++ 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)`::
8: d865772ebc ! 8: 8848eeddf2 hook: add 'run' subcommand
@@ Commit message
supported.
Legacy hooks (those present in $GITDIR/hooks) are run at the end of the
- execution list. For now, there is no way to disable them.
+ execution list. They can be disabled, or made to print warnings, or to
+ prompt before running, with the 'hook.runHookDir' config.
Users may wish to provide hook commands like 'git config
hook.pre-commit.command "~/linter.sh --pre-commit"'. To enable this,
@@ Documentation/git-hook.txt: in the order they should be run, and print the confi
+run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] `<hook-name>`::
+
+Runs hooks configured for `<hook-name>`, in the same order displayed by `git
-+hook list`. Hooks configured this way are run prepended with `sh -c`, so paths
-+containing special characters or spaces should be wrapped in single quotes:
-+`command = '/my/path with spaces/script.sh' some args`.
++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
+-------
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
+static int run(int argc, const char **argv, const char *prefix)
+{
+ struct strbuf hookname = STRBUF_INIT;
-+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
++ struct run_hooks_opt opt;
+ int rc = 0;
+
+ struct option run_options[] = {
@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
+ OPT_END(),
+ };
+
-+ /*
-+ * While it makes sense to list hooks out-of-repo, it doesn't make sense
-+ * to execute them. Hooks usually want to look at repository artifacts.
-+ */
-+ if (!have_git_dir())
-+ usage_msg_opt(_("You must be in a Git repo to execute hooks."),
-+ builtin_hook_usage, run_options);
++ run_hooks_opt_init(&opt);
+
+ argc = parse_options(argc, argv, prefix, run_options,
+ builtin_hook_usage, 0);
@@ hook.c: enum hookdir_opt configured_hookdir_opt(void)
+
+ switch (cfg)
+ {
++ case HOOKDIR_ERROR:
++ fprintf(stderr, _("Skipping legacy hook at '%s'\n"),
++ path);
++ /* FALLTHROUGH */
+ case HOOKDIR_NO:
+ return 0;
-+ case HOOKDIR_UNKNOWN:
-+ fprintf(stderr,
-+ _("Unrecognized value for 'hook.runHookDir'. "
-+ "Is there a typo? "));
-+ /* FALLTHROUGH */
+ case HOOKDIR_WARN:
+ fprintf(stderr, _("Running legacy hook at '%s'\n"),
+ path);
@@ hook.c: enum hookdir_opt configured_hookdir_opt(void)
+ } while (prompt.len); /* an empty reply means "Yes" */
+ strbuf_release(&prompt);
+ 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;
@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
+ strvec_clear(&o->args);
+}
+
++static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options,
++ struct child_process *cp)
++{
++ if (!hook)
++ return;
++
++ cp->no_stdin = 1;
++ cp->env = options->env.v;
++ cp->stdout_to_stderr = 1;
++ cp->trace2_hook_name = hook->command.buf;
++
++ /*
++ * 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, hook->command.buf);
++
++ /*
++ * add passed-in argv, without expanding - let the user get back
++ * exactly what they put in
++ */
++ strvec_pushv(&cp->args, options->args.v);
++}
++
+int run_hooks(const char *hookname, struct run_hooks_opt *options)
+{
+ struct strbuf hookname_str = STRBUF_INIT;
@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
+ struct child_process hook_proc = CHILD_PROCESS_INIT;
+ struct hook *hook = list_entry(pos, struct hook, list);
+
-+ hook_proc.env = options->env.v;
-+ hook_proc.no_stdin = 1;
-+ hook_proc.stdout_to_stderr = 1;
-+ hook_proc.trace2_hook_name = hook->command.buf;
-+ hook_proc.use_shell = 1;
-+
-+ if (hook->from_hookdir) {
-+ if (!should_include_hookdir(hook->command.buf, options->run_hookdir))
++ if (hook->from_hookdir &&
++ !should_include_hookdir(hook->command.buf, options->run_hookdir))
+ continue;
-+ /*
-+ * Commands from the config could be oneliners, but we know
-+ * for certain that hookdir commands are not.
-+ */
-+ hook_proc.use_shell = 0;
-+ }
-+
-+ /* add command */
-+ strvec_push(&hook_proc.args, hook->command.buf);
+
-+ /*
-+ * add passed-in argv, without expanding - let the user get back
-+ * exactly what they put in
-+ */
-+ strvec_pushv(&hook_proc.args, options->args.v);
++ prepare_hook_cp(hook, options, &hook_proc);
+
+ rc |= run_command(&hook_proc);
+ }
@@ hook.h
#include "strbuf.h"
+#include "strvec.h"
- struct hook
- {
+ struct hook {
+ struct list_head list;
@@ hook.h: enum hookdir_opt
*/
enum hookdir_opt configured_hookdir_opt(void);
@@ hook.h: enum hookdir_opt
+ enum hookdir_opt run_hookdir;
+};
+
-+#define RUN_HOOKS_OPT_INIT { \
-+ .env = STRVEC_INIT, \
-+ .args = STRVEC_INIT, \
-+ .run_hookdir = configured_hookdir_opt() \
-+}
-+
+void run_hooks_opt_init(struct run_hooks_opt *o);
+void run_hooks_opt_clear(struct run_hooks_opt *o);
+
@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = no is resp
+ test_must_be_empty actual
'
- test_expect_success 'hook.runHookDir = warn is respected by list' '
+ test_expect_success 'hook.runHookDir = error is respected by list' '
+@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = error is respected by list' '
+
+ git hook list pre-commit >actual &&
+ # the hookdir annotation is translated
++ test_i18ncmp expected actual &&
++
++ cat >expected <<-EOF &&
++ Skipping legacy hook at '\''$(pwd)/.git/hooks/pre-commit'\''
++ EOF
++
++ git hook run pre-commit 2>actual &&
+ test_i18ncmp expected actual
+ '
+
@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = warn is respected by list' '
git hook list pre-commit >actual &&
@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir = interactiv
+ nongit test_must_fail git hook run pre-commit
'
- test_done
+ test_expect_success 'hook.runHookDir is tolerant to unknown values' '
9: 53a655ed2c ! 9: 452f7eea89 hook: replace find_hook() with hook_exists()
@@ Metadata
Author: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
## Commit message ##
- hook: replace find_hook() with hook_exists()
+ hook: introduce hook_exists()
Add a helper to easily determine whether any hooks exist for a given
hook event.
@@ Commit message
hook; that check should include the config-based hooks as well. Optimize
by checking the config directly. Since commands which execute hooks
might want to take args to replace 'hook.runHookDir', let
- 'hook_exists()' mirror the behavior of 'hook.runHookDir'.
+ 'hook_exists()' take a hookdir_opt to override that config.
- Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
+ In some cases, external callers today use find_hook() to discover the
+ location of a hook and then run it manually with run-command.h (that is,
+ not with run_hook_le()). Later, those cases will call hook.h:run_hook()
+ directly instead.
- ## 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], configured_hookdir_opt()))
- strbuf_addf(hook_info, "%s\n", hook[i]);
- }
-
+ Once the entire codebase is using hook_exists() instead of find_hook(),
+ find_hook() can be safely rolled into hook_exists() and removed from
+ run-command.h.
+
+ Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
## hook.c ##
@@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o)
@@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o)
+{
+ 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();
+
-+ int could_run_hookdir = (should_run_hookdir == HOOKDIR_INTERACTIVE ||
++ could_run_hookdir = (should_run_hookdir == HOOKDIR_INTERACTIVE ||
+ should_run_hookdir == HOOKDIR_WARN ||
+ should_run_hookdir == HOOKDIR_YES)
+ && !!find_hook(hookname);
@@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o)
strvec_clear(&o->env);
## hook.h ##
+@@ hook.h: struct list_head* hook_list(const struct strbuf *hookname);
+
+ enum hookdir_opt
+ {
++ HOOKDIR_USE_CONFIG,
+ HOOKDIR_NO,
+ HOOKDIR_ERROR,
+ HOOKDIR_WARN,
@@ hook.h: struct run_hooks_opt
void run_hooks_opt_init(struct run_hooks_opt *o);
void run_hooks_opt_clear(struct run_hooks_opt *o);
10: 13abc6ce24 ! 10: e76507b290 hook: support passing stdin to hooks
@@ Documentation/git-hook.txt: in the order they should be run, and print the confi
+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 are run prepended with `sh -c`, so paths
+ 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.
@@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
## hook.c ##
-@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options)
- struct child_process hook_proc = CHILD_PROCESS_INIT;
- struct hook *hook = list_entry(pos, struct hook, list);
+@@ 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(struct hook *hook, struct run_hooks_opt *options,
+ if (!hook)
+ return;
-+ /* reopen the file for stdin; run_command closes it. */
-+ if (options->path_to_stdin)
-+ hook_proc.in = xopen(options->path_to_stdin, O_RDONLY);
-+ else
-+ hook_proc.no_stdin = 1;
+- 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
++ cp->no_stdin = 1;
+
- hook_proc.env = options->env.v;
-- hook_proc.no_stdin = 1;
- hook_proc.stdout_to_stderr = 1;
- hook_proc.trace2_hook_name = hook->command.buf;
- hook_proc.use_shell = 1;
+ cp->env = options->env.v;
+ cp->stdout_to_stderr = 1;
+ cp->trace2_hook_name = hook->command.buf;
## hook.h ##
@@ hook.h: struct run_hooks_opt
@@ hook.h: struct run_hooks_opt
+ const char *path_to_stdin;
};
- #define RUN_HOOKS_OPT_INIT { \
-- .env = STRVEC_INIT, \
-+ .env = STRVEC_INIT, \
- .args = STRVEC_INIT, \
-+ .path_to_stdin = NULL, \
- .run_hookdir = configured_hookdir_opt() \
- }
-
+ void run_hooks_opt_init(struct run_hooks_opt *o);
@@ hook.h: int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir);
/*
@@ hook.h: int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdi
## t/t1360-config-based-hooks.sh ##
-@@ t/t1360-config-based-hooks.sh: test_expect_success 'out-of-repo runs excluded' '
- nongit test_must_fail git hook run pre-commit
+@@ t/t1360-config-based-hooks.sh: test_expect_success 'hook.runHookDir is tolerant to unknown values' '
+ test_i18ncmp expected actual
'
+test_expect_success 'stdin to multiple hooks' '
11: 0465a9ec94 ! 11: 5f41555e49 run-command: allow stdin for run_processes_parallel
@@ run-command.c: static int pp_start_one(struct parallel_processes *pp)
if (i == pp->max_processes)
BUG("bookkeeping is hard");
-+ /* disallow by default, but allow users to set up stdin if they wish */
++ /*
++ * By default, do not inherit stdin from the parent process - otherwise,
++ * all children would share stdin! Users may overwrite this to provide
++ * something to the child's stdin by having their 'get_next_task'
++ * callback assign 0 to .no_stdin and an appropriate integer to .in.
++ */
+ pp->children[i].process.no_stdin = 1;
+
code = pp->get_next_task(&pp->children[i].process,
12: 83eb7805a4 ! 12: a3bf826304 hook: allow parallel hook execution
@@ Documentation/git-hook.txt: in the order they should be run, and print the confi
+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 are run prepended with `sh -c`, so paths
+ hook list`. Hooks configured this way may be run prepended with `sh -c`, so
@@ Documentation/git-hook.txt: Specify environment variables to set for every hook that is run.
Specify a file which will be streamed into stdin for every hook that is run.
Each hook will receive the entire file from beginning to EOF.
@@ builtin/hook.c
NULL
};
-@@ builtin/hook.c: static int list(int argc, const char **argv, const char *prefix)
- static int run(int argc, const char **argv, const char *prefix)
- {
- struct strbuf hookname = STRBUF_INIT;
-- struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
-+ struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT_ASYNC;
- int rc = 0;
-
- struct option run_options[] = {
@@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
N_("argument to pass to hook")),
OPT_STRING(0, "to-stdin", &opt.path_to_stdin, N_("path"),
@@ builtin/hook.c: static int run(int argc, const char **argv, const char *prefix)
OPT_END(),
};
+- run_hooks_opt_init(&opt);
++ run_hooks_opt_init_async(&opt);
+
+ argc = parse_options(argc, argv, prefix, run_options,
+ builtin_hook_usage, 0);
## hook.c ##
@@ hook.c: enum hookdir_opt configured_hookdir_opt(void)
@@ hook.c: enum hookdir_opt configured_hookdir_opt(void)
static int should_include_hookdir(const char *path, enum hookdir_opt cfg)
{
struct strbuf prompt = STRBUF_INIT;
-@@ hook.c: void run_hooks_opt_init(struct run_hooks_opt *o)
+@@ hook.c: struct list_head* hook_list(const struct strbuf* hookname)
+ return hook_head;
+ }
+
+-void run_hooks_opt_init(struct run_hooks_opt *o)
++void run_hooks_opt_init_sync(struct run_hooks_opt *o)
+ {
strvec_init(&o->env);
strvec_init(&o->args);
+ o->path_to_stdin = NULL;
o->run_hookdir = configured_hookdir_opt();
++ o->jobs = 1;
++}
++
++void run_hooks_opt_init_async(struct run_hooks_opt *o)
++{
++ run_hooks_opt_init_sync(o);
+ o->jobs = configured_hook_jobs();
}
@@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o)
strvec_clear(&o->args);
}
-+
+-static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options,
+- struct child_process *cp)
+static int pick_next_hook(struct child_process *cp,
+ struct strbuf *out,
+ void *pp_cb,
+ void **pp_task_cb)
-+{
+ {
+ struct hook_cb_data *hook_cb = pp_cb;
++ struct hook *hook = hook_cb->run_me;
+
-+ struct hook *hook = list_entry(hook_cb->run_me, struct hook, list);
-+
-+ if (hook_cb->head == hook_cb->run_me)
+ if (!hook)
+- return;
+ return 0;
-+
-+ cp->env = hook_cb->options->env.v;
-+ cp->stdout_to_stderr = 1;
-+ cp->trace2_hook_name = hook->command.buf;
-+
-+ /* reopen the file for stdin; run_command closes it. */
+
+ /* 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->no_stdin = 1;
+ }
-+
-+ /*
-+ * Commands from the config could be oneliners, but we know
-+ * for certain that hookdir commands are not.
-+ */
-+ if (hook->from_hookdir)
-+ cp->use_shell = 0;
-+ else
-+ cp->use_shell = 1;
-+
-+ /* add command */
-+ strvec_push(&cp->args, hook->command.buf);
-+
-+ /*
-+ * add passed-in argv, without expanding - let the user get back
-+ * exactly what they put in
-+ */
+
+- cp->env = options->env.v;
++ cp->env = hook_cb->options->env.v;
+ cp->stdout_to_stderr = 1;
+ cp->trace2_hook_name = hook->command.buf;
+
+@@ hook.c: static void prepare_hook_cp(struct hook *hook, struct run_hooks_opt *options,
+ * add passed-in argv, without expanding - let the user get back
+ * exactly what they put in
+ */
+- strvec_pushv(&cp->args, options->args.v);
+ strvec_pushv(&cp->args, hook_cb->options->args.v);
+
+ /* Provide context for errors if necessary */
+ *pp_task_cb = hook;
+
+ /* Get the next entry ready */
-+ hook_cb->run_me = hook_cb->run_me->next;
++ 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;
+}
@@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o)
+ strbuf_addf(out, _("Couldn't start '%s', configured in '%s'\n"),
+ attempted->command.buf,
+ attempted->from_hookdir ? "hookdir"
-+ : config_scope_name(attempted->origin));
++ : config_scope_name(attempted->origin));
+
+ /* NEEDSWORK: if halt_on_error is desired, do it here. */
+ return 0;
@@ hook.c: void run_hooks_opt_clear(struct run_hooks_opt *o)
+
+ /* NEEDSWORK: if halt_on_error is desired, do it here. */
+ return 0;
-+}
-+
+ }
+
int run_hooks(const char *hookname, struct run_hooks_opt *options)
{
struct strbuf hookname_str = STRBUF_INIT;
@@ hook.c: int run_hooks(const char *hookname, struct run_hooks_opt *options)
- struct child_process hook_proc = CHILD_PROCESS_INIT;
struct hook *hook = list_entry(pos, struct hook, list);
-- /* reopen the file for stdin; run_command closes it. */
-- if (options->path_to_stdin)
-- hook_proc.in = xopen(options->path_to_stdin, O_RDONLY);
-- else
-- hook_proc.no_stdin = 1;
--
-- hook_proc.env = options->env.v;
-- hook_proc.stdout_to_stderr = 1;
-- hook_proc.trace2_hook_name = hook->command.buf;
-- hook_proc.use_shell = 1;
--
-- if (hook->from_hookdir) {
-- if (!should_include_hookdir(hook->command.buf, options->run_hookdir))
+ if (hook->from_hookdir &&
+ !should_include_hookdir(hook->command.buf, options->run_hookdir))
- continue;
-- /*
-- * Commands from the config could be oneliners, but we know
-- * for certain that hookdir commands are not.
-- */
-- hook_proc.use_shell = 0;
-- }
--
-- /* add command */
-- strvec_push(&hook_proc.args, hook->command.buf);
-+ if (hook->from_hookdir &&
-+ !should_include_hookdir(hook->command.buf, options->run_hookdir))
+ list_del(pos);
+ }
++
++ if (list_empty(to_run))
++ return 0;
-- /*
-- * add passed-in argv, without expanding - let the user get back
-- * exactly what they put in
-- */
-- strvec_pushv(&hook_proc.args, options->args.v);
+- prepare_hook_cp(hook, options, &hook_proc);
+ cb_data.head = to_run;
-+ cb_data.run_me = to_run->next;
++ cb_data.run_me = list_entry(to_run->next, struct hook, list);
- rc |= run_command(&hook_proc);
- }
@@ hook.h: enum hookdir_opt
/* Environment vars to be set for each hook */
@@ hook.h: struct run_hooks_opt
+ /*
+ * How should the hookdir be handled?
+- * Leave the RUN_HOOKS_OPT_INIT default in most cases; this only needs
++ * Leave the run_hooks_opt_init_*() default in most cases; this only needs
+ * to be overridden if the user can override it at the command line.
+ */
+ enum hookdir_opt run_hookdir;
+
/* Path to file which should be piped to stdin for each hook */
const char *path_to_stdin;
+
+ /* Number of threads to parallelize across */
+ int jobs;
- };
-
--#define RUN_HOOKS_OPT_INIT { \
++};
++
+/*
+ * Callback provided to feed_pipe_fn and consume_sideband_fn.
+ */
+struct hook_cb_data {
+ int rc;
+ struct list_head *head;
-+ struct list_head *run_me;
++ struct hook *run_me;
+ struct run_hooks_opt *options;
-+};
-+
-+#define RUN_HOOKS_OPT_INIT_SYNC { \
- .env = STRVEC_INIT, \
- .args = STRVEC_INIT, \
- .path_to_stdin = NULL, \
-+ .jobs = 1, \
- .run_hookdir = configured_hookdir_opt() \
- }
+ };
-+#define RUN_HOOKS_OPT_INIT_ASYNC { \
-+ .env = STRVEC_INIT, \
-+ .args = STRVEC_INIT, \
-+ .path_to_stdin = NULL, \
-+ .jobs = configured_hook_jobs(), \
-+ .run_hookdir = configured_hookdir_opt() \
-+}
-+
-+
- void run_hooks_opt_init(struct run_hooks_opt *o);
+-void run_hooks_opt_init(struct run_hooks_opt *o);
++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);
+ /*
13: f84c879d5a < -: ---------- hook: allow specifying working directory for hooks
-: ---------- > 13: 0c4add98a4 hook: allow specifying working directory for hooks
14: ac9cec6587 = 14: 1847c4c675 run-command: add stdin callback for parallelization
15: 71fca28ccf ! 15: ab781c94d7 hook: provide stdin by string_list or callback
@@ hook.c: static void append_or_move_hook(struct list_head *head, const char *comm
}
/* 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->env);
strvec_clear(&o->args);
-+ string_list_clear(&o->str_stdin, 0);
}
-
-+static int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb)
++int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb)
+{
+ int *item_idx;
+ struct hook *ctx = pp_task_cb;
-+ struct string_list *to_pipe = &((struct hook_cb_data*)pp_cb)->options->str_stdin;
++ 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) {
@@ 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->str_stdin.nr) ||
-+ (options->path_to_stdin && options->feed_pipe) ||
-+ (options->str_stdin.nr && options->feed_pipe))
++ if (options->path_to_stdin && options->feed_pipe)
+ BUG("choose only one method to populate stdin");
-+
-+ if (options->str_stdin.nr)
-+ options->feed_pipe = &pipe_from_string_list;
+
strbuf_addstr(&hookname_str, hookname);
@@ hook.h
#include "strvec.h"
+#include "run-command.h"
- struct hook
- {
-@@ hook.h: struct hook
+ struct hook {
+ struct list_head list;
+@@ hook.h: struct hook {
/* The literal command to run. */
struct strbuf command;
- int from_hookdir;
+ unsigned from_hookdir : 1;
+
+ /*
+ * Use this to keep state for your feed_pipe_fn if you are using
@@ hook.h: struct run_hooks_opt
/* Path to file which should be piped to stdin for each hook */
const char *path_to_stdin;
-+ /* Pipe each string to stdin, separated by newlines */
-+ struct string_list str_stdin;
+ /*
+ * Callback and state pointer to ask for more content to pipe to stdin.
+ * Will be called repeatedly, for each hook. See
+ * hook.c:pipe_from_stdin() for an example. Keep per-hook state in
+ * hook.feed_pipe_cb_data (per process). Keep initialization context in
+ * feed_pipe_ctx (shared by all processes).
++ *
++ * See 'pipe_from_string_list()' for info about how to specify a
++ * string_list as the stdin input instead of writing your own handler.
+ */
+ feed_pipe_fn feed_pipe;
+ void *feed_pipe_ctx;
@@ hook.h: struct run_hooks_opt
+
};
++/*
++ * 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()'.
++ * This will pipe each string in the list to stdin, separated by newlines. (Do
++ * not inject your own newlines.)
++ */
++int pipe_from_string_list(struct strbuf *pipe, void *pp_cb, void *pp_task_cb);
++
/*
-@@ hook.h: struct hook_cb_data {
- .path_to_stdin = NULL, \
- .jobs = 1, \
- .dir = NULL, \
-+ .str_stdin = STRING_LIST_INIT_DUP, \
-+ .feed_pipe = NULL, \
-+ .feed_pipe_ctx = NULL, \
- .run_hookdir = configured_hookdir_opt() \
- }
-
-@@ hook.h: struct hook_cb_data {
- .path_to_stdin = NULL, \
- .jobs = configured_hook_jobs(), \
- .dir = NULL, \
-+ .str_stdin = STRING_LIST_INIT_DUP, \
-+ .feed_pipe = NULL, \
-+ .feed_pipe_ctx = NULL, \
- .run_hookdir = configured_hookdir_opt() \
- }
-
+ * Callback provided to feed_pipe_fn and consume_sideband_fn.
+ */
16: 98253fa8fd = 16: c51bf46e8d run-command: allow capturing of collated output
17: 9505812b74 ! 17: b90a4ee79b hooks: allow callers to capture output
@@ Commit message
Signed-off-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
## 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)
pick_next_hook,
notify_start_failure,
@@ hook.h: struct run_hooks_opt
/* Number of threads to parallelize across */
int jobs;
-@@ hook.h: struct hook_cb_data {
- .str_stdin = STRING_LIST_INIT_DUP, \
- .feed_pipe = NULL, \
- .feed_pipe_ctx = NULL, \
-+ .consume_sideband = NULL, \
- .run_hookdir = configured_hookdir_opt() \
- }
-
-@@ hook.h: struct hook_cb_data {
- .str_stdin = STRING_LIST_INIT_DUP, \
- .feed_pipe = NULL, \
- .feed_pipe_ctx = NULL, \
-+ .consume_sideband = NULL, \
- .run_hookdir = configured_hookdir_opt() \
- }
-
