| Message ID | 20210311021037.3001235-18-emilyshaffer@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | config-based hooks | expand |
On Thu, Mar 11 2021, Emily Shaffer wrote: > Some server-side hooks will require capturing output to send over > sideband instead of printing directly to stderr. Expose that capability. So added here in 17/37 and not used until 30/37. As a point on readability (this isn't the first such patch) I think it would be better to just squash those together with some "since we now need access to consume_sideband in hooks, do that ...". If there's a much larger API it makes sense to do it as another step... > hook.c | 3 ++- > hook.h | 8 ++++++++ > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/hook.c b/hook.c > index e16b082cbd..2322720ffe 100644 > --- a/hook.c > +++ b/hook.c > @@ -256,6 +256,7 @@ 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) > @@ -434,7 +435,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) > pick_next_hook, > notify_start_failure, > options->feed_pipe, > - NULL, > + options->consume_sideband, > notify_hook_finished, > &cb_data, > "hook", > diff --git a/hook.h b/hook.h > index ecf0228a46..4ff9999b04 100644 > --- a/hook.h > +++ b/hook.h > @@ -78,6 +78,14 @@ struct run_hooks_opt > feed_pipe_fn feed_pipe; > void *feed_pipe_ctx; > > + /* > + * Populate this to capture output and prevent it from being printed to > + * stderr. This will be passed directly through to > + * run_command:run_parallel_processes(). See t/helper/test-run-command.c > + * for an example. > + */ > + consume_sideband_fn consume_sideband; > + > /* Number of threads to parallelize across */ > int jobs; ...but this scaffolding is rather trivial.
On Fri, Mar 12, 2021 at 10:08:04AM +0100, Ævar Arnfjörð Bjarmason wrote: > > > On Thu, Mar 11 2021, Emily Shaffer wrote: > > > Some server-side hooks will require capturing output to send over > > sideband instead of printing directly to stderr. Expose that capability. > > So added here in 17/37 and not used until 30/37. As a point on > readability (this isn't the first such patch) I think it would be better > to just squash those together with some "since we now need access to > consume_sideband in hooks, do that ...". Yeah. When I was putting together the series I had two thoughts on how best to organize it: 1. Adding functionality just-in-time for the hook that needs it (like you describe) or 2. Implementing the whole utility, then doing hook conversions in a separate chunk or series (what I went with). I chose 2 for a couple reasons: that it would be easier for people who just care "did a hook I use start working differently?" to review only the second chunk of the change, and that it would be easier if we wanted to adopt the library part into the codebase without converting the hooks to use it (this was listed as a step in the design doc, but I think we ended up abandoning it). The differentiation was certainly easier when I had the two "chunks" separated into a part I and part II series, but Junio asked me to combine them starting with this revision so it would be easier to merge to 'seen' (as I understood it). At this point, I'd prefer not to rearrange the series, though. - Emily
diff --git a/hook.c b/hook.c index e16b082cbd..2322720ffe 100644 --- a/hook.c +++ b/hook.c @@ -256,6 +256,7 @@ 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) @@ -434,7 +435,7 @@ int run_hooks(const char *hookname, struct run_hooks_opt *options) pick_next_hook, notify_start_failure, options->feed_pipe, - NULL, + options->consume_sideband, notify_hook_finished, &cb_data, "hook", diff --git a/hook.h b/hook.h index ecf0228a46..4ff9999b04 100644 --- a/hook.h +++ b/hook.h @@ -78,6 +78,14 @@ struct run_hooks_opt feed_pipe_fn feed_pipe; void *feed_pipe_ctx; + /* + * Populate this to capture output and prevent it from being printed to + * stderr. This will be passed directly through to + * run_command:run_parallel_processes(). See t/helper/test-run-command.c + * for an example. + */ + consume_sideband_fn consume_sideband; + /* Number of threads to parallelize across */ int jobs;
Some server-side hooks will require capturing output to send over sideband instead of printing directly to stderr. Expose that capability. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> --- Notes: You can see this in practice in the conversions for some of the push hooks, like 'receive-pack'. hook.c | 3 ++- hook.h | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)