[v2,0/5] hook API: support stdin, convert post-rewrite

Message

Ævar Arnfjörð Bjarmason Feb. 3, 2023, 12:15 p.m. UTC

As noted in the v1[1] this is the initial part of the greater
"config-based hooks" topic. I believe this iteration addresses all
comments on v1. Changes since then:

* Remove a couple of paragraphs in 1/4 that aren't relevant anymore,
  an already-landed topic addressed those.

* Don't needlessly change "cp->no_stdin = 1" and introduce an
  "else". This refactoring was there because that code eventually
  changes in the full "config-based hooks" topic, but going through
  those future changes I found that it wasn't for a good reason there
  either. We can just keep the "no_stdin = 1" by default, and have
  specific cases override that.

* Elaborate on why we're not converting the last "post-rewrite" hook
  here.

* Mention the future expected use for sendemail-validate in 5/5

The (passing) CI & topic branch for this is at[2].

1. https://lore.kernel.org/git/[e-mail]/
2. https://github.com/avar/git/tree/es-avar/config-based-hooks-the-beginning-2

Emily Shaffer (4):
  run-command: allow stdin for run_processes_parallel
  hook API: support passing stdin to hooks, convert am's 'post-rewrite'
  sequencer: use the new hook API for the simpler "post-rewrite" call
  hook: support a --to-stdin=<path> option

Ævar Arnfjörð Bjarmason (1):
  run-command.c: remove dead assignment in while-loop

 Documentation/git-hook.txt |  7 ++++++-
 builtin/am.c               | 20 ++++----------------
 builtin/hook.c             |  4 +++-
 hook.c                     |  5 +++++
 hook.h                     |  5 +++++
 run-command.c              | 13 +++++++++----
 sequencer.c                | 18 ++++--------------
 t/t1800-hook.sh            | 18 ++++++++++++++++++
 8 files changed, 54 insertions(+), 36 deletions(-)

Range-diff against v1:
1:  351c6a55a41 ! 1:  488b24e1c98 run-command.c: remove dead assignment in while-loop
    @@ Commit message
     
         Remove code that's been unused since it was added in
         c553c72eed6 (run-command: add an asynchronous parallel child
    -    processor, 2015-12-15), the next use of "i" in this function is:
    -
    -            for (i = 0; ...
    -
    -    So we'll always clobber the "i" that's set here. Presumably the "i"
    -    assignment is an artifact of WIP code that made it into our tree.
    -
    -    A subsequent commit will need to adjust the type of the "i" variable
    -    in the otherwise unrelated for-loop, which is why this is being
    -    removed now.
    +    processor, 2015-12-15).
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
     
2:  81eef2f60a0 = 2:  9a178577dcc run-command: allow stdin for run_processes_parallel
3:  c6b9b69c516 ! 3:  3d3dd6b900a hook API: support passing stdin to hooks, convert am's 'post-rewrite'
    @@ builtin/am.c: static int run_applypatch_msg_hook(struct am_state *state)
     
      ## hook.c ##
     @@ hook.c: static int pick_next_hook(struct child_process *cp,
    - 	if (!hook_path)
    - 		return 0;
      
    --	cp->no_stdin = 1;
    + 	cp->no_stdin = 1;
      	strvec_pushv(&cp->env, hook_cb->options->env.v);
     +	/* reopen the file for stdin; run_command closes it. */
     +	if (hook_cb->options->path_to_stdin) {
     +		cp->no_stdin = 0;
     +		cp->in = xopen(hook_cb->options->path_to_stdin, O_RDONLY);
    -+	} else {
    -+		cp->no_stdin = 1;
     +	}
      	cp->stdout_to_stderr = 1;
      	cp->trace2_hook_name = hook_cb->hook_name;
4:  7a55c95f60f ! 4:  b96522d593f sequencer: use the new hook API for the simpler "post-rewrite" call
    @@ Commit message
     
         This leaves the more complex "post-rewrite" invocation added in
         a87a6f3c98e (commit: move post-rewrite code to libgit, 2017-11-17)
    -    here in sequencer.c unconverted. That'll be done in a subsequent
    -    commit.
    +    here in sequencer.c unconverted.
    +
    +    Here we can pass in a file's via the "in" file descriptor, in that
    +    case we don't have a file, but will need to write_in_full() to an "in"
    +    provide by the API. Support for that will be added to the hook API in
    +    the future, but we're not there yet.
     
         Signed-off-by: Emily Shaffer <[e-mail]>
         Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>
5:  cb9ef7a89c4 ! 5:  b4e02f41194 hook: support a --to-stdin=<path> option for testing
    @@ Metadata
     Author: Emily Shaffer <[e-mail]>
     
      ## Commit message ##
    -    hook: support a --to-stdin=<path> option for testing
    +    hook: support a --to-stdin=<path> option
     
         Expose the "path_to_stdin" API added in the preceding commit in the
    -    "git hook run" command. For now we won't be using this command
    -    interface outside of the tests, but exposing this functionality makes
    -    it easier to test the hook API.
    +    "git hook run" command.
    +
    +    For now we won't be using this command interface outside of the tests,
    +    but exposing this functionality makes it easier to test the hook
    +    API. The plan is to use this to extend the "sendemail-validate"
    +    hook[1][2].
    +
    +    1. https://lore.kernel.org/git/[e-mail]
    +    2. https://lore.kernel.org/git/[e-mail]
     
         Signed-off-by: Emily Shaffer <[e-mail]>
         Signed-off-by: Ævar Arnfjörð Bjarmason <[e-mail]>