[v6,00/17] propose config-based hooks (part I)

Message ID	20201205014607.1464119-1-emilyshaffer@google.com (mailing list archive)
Headers	show Return-Path: <git-owner@kernel.org> Sender: "emilyshaffer via sendgmr" <emilyshaffer@podkayne.svl.corp.google.com> Date: Fri, 4 Dec 2020 17:45:50 -0800 In-Reply-To: <20201014232447.3050579-1-emilyshaffer@google.com> Message-Id: <20201205014607.1464119-1-emilyshaffer@google.com> Mime-Version: 1.0 Subject: [PATCH v6 00/17] propose config-based hooks (part I) From: Emily Shaffer <emilyshaffer@google.com> To: git@vger.kernel.org Cc: Emily Shaffer <emilyshaffer@google.com>, Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>, James Ramsay <james@jramsay.com.au>, Jonathan Nieder <jrnieder@gmail.com>, "brian m. carlson" <sandals@crustytoothpaste.net>, " =?utf-8?b?w4Z2YXIgQXJu?= =?utf-8?b?ZmrDtnLDsCBCamFybWFzb24=?= " <avarab@gmail.com>, Phillip Wood <phillip.wood123@gmail.com>, Josh Steadmon <steadmon@google.com>, Johannes Schindelin <Johannes.Schindelin@gmx.de> Content-Type: text/plain; charset="UTF-8" Precedence: bulk
Series	propose config-based hooks (part I) \| expand [v6,00/17] propose config-based hooks (part I) [01/17] doc: propose hooks managed by the config [02/17] hook: scaffolding for git-hook subcommand [03/17] hook: add list command [04/17] hook: include hookdir hook in list [05/17] hook: respect hook.runHookDir [06/17] hook: implement hookcmd.<name>.skip [07/17] parse-options: parse into strvec [08/17] hook: add 'run' subcommand [09/17] hook: replace find_hook() with hook_exists() [10/17] hook: support passing stdin to hooks [11/17] run-command: allow stdin for run_processes_parallel [12/17] hook: allow parallel hook execution [13/17] hook: allow specifying working directory for hooks [14/17] run-command: add stdin callback for parallelization [15/17] hook: provide stdin by string_list or callback [16/17] run-command: allow capturing of collated output [17/17] hooks: allow callers to capture output

Emily Shaffer Dec. 5, 2020, 1:45 a.m. UTC

Hi folks, and thanks for the patience - I ran into many, many last-mile
challenges.

I haven't addressed many comments on the design doc yet - I was keen to get the
"functionally complete" implementation and conversion to the list.

Next on my plate:
 - Update the design doc to make sense with what's in the implementation.
 - A blog post! How to set up new hooks, why they're neat, etc.
 - We seem to have some Googlers interested in trying it out internally, so
   I'm hoping we'll gather and collate feedback from that soon too.
 - And of course addressing comments on this series.

Thanks!
 - Emily

Emily Shaffer (17):
  doc: propose hooks managed by the config
  hook: scaffolding for git-hook subcommand
  hook: add list command
  hook: include hookdir hook in list
  hook: respect hook.runHookDir
  hook: implement hookcmd.<name>.skip
  parse-options: parse into strvec
  hook: add 'run' subcommand
  hook: replace find_hook() with hook_exists()
  hook: support passing stdin to hooks
  run-command: allow stdin for run_processes_parallel
  hook: allow parallel hook execution
  hook: allow specifying working directory for hooks
  run-command: add stdin callback for parallelization
  hook: provide stdin by string_list or callback
  run-command: allow capturing of collated output
  hooks: allow callers to capture output

 .gitignore                                    |   1 +
 Documentation/Makefile                        |   1 +
 Documentation/config/hook.txt                 |  19 +
 Documentation/git-hook.txt                    | 118 +++++
 Documentation/technical/api-parse-options.txt |   5 +
 .../technical/config-based-hooks.txt          | 367 +++++++++++++++
 Makefile                                      |   2 +
 builtin.h                                     |   1 +
 builtin/bugreport.c                           |   4 +-
 builtin/fetch.c                               |   1 +
 builtin/hook.c                                | 174 ++++++++
 builtin/submodule--helper.c                   |   2 +-
 git.c                                         |   1 +
 hook.c                                        | 417 ++++++++++++++++++
 hook.h                                        | 154 +++++++
 parse-options-cb.c                            |  16 +
 parse-options.h                               |   4 +
 run-command.c                                 |  85 +++-
 run-command.h                                 |  31 ++
 submodule.c                                   |   1 +
 t/helper/test-run-command.c                   |  46 +-
 t/t0061-run-command.sh                        |  37 ++
 t/t1360-config-based-hooks.sh                 | 256 +++++++++++
 23 files changed, 1728 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/config/hook.txt
 create mode 100644 Documentation/git-hook.txt
 create mode 100644 Documentation/technical/config-based-hooks.txt
 create mode 100644 builtin/hook.c
 create mode 100644 hook.c
 create mode 100644 hook.h
 create mode 100755 t/t1360-config-based-hooks.sh

Josh Steadmon Dec. 16, 2020, 12:34 a.m. UTC | #1

On 2020.12.04 17:45, Emily Shaffer wrote:
> Hi folks, and thanks for the patience - I ran into many, many last-mile
> challenges.
> 
> I haven't addressed many comments on the design doc yet - I was keen to get the
> "functionally complete" implementation and conversion to the list.
> 
> Next on my plate:
>  - Update the design doc to make sense with what's in the implementation.
>  - A blog post! How to set up new hooks, why they're neat, etc.
>  - We seem to have some Googlers interested in trying it out internally, so
>    I'm hoping we'll gather and collate feedback from that soon too.
>  - And of course addressing comments on this series.
> 
> Thanks!
>  - Emily

This approach looks good to me. I'll look forward to seeing the updated
design and the feedback from the internal tests.

Junio C Hamano Dec. 16, 2020, 12:56 a.m. UTC | #2

Josh Steadmon <steadmon@google.com> writes:

> On 2020.12.04 17:45, Emily Shaffer wrote:
>> Hi folks, and thanks for the patience - I ran into many, many last-mile
>> challenges.
>> 
>> I haven't addressed many comments on the design doc yet - I was keen to get the
>> "functionally complete" implementation and conversion to the list.
>> 
>> Next on my plate:
>>  - Update the design doc to make sense with what's in the implementation.
>>  - A blog post! How to set up new hooks, why they're neat, etc.
>>  - We seem to have some Googlers interested in trying it out internally, so
>>    I'm hoping we'll gather and collate feedback from that soon too.
>>  - And of course addressing comments on this series.
>> 
>> Thanks!
>>  - Emily
>
> This approach looks good to me. I'll look forward to seeing the updated
> design and the feedback from the internal tests.

Thanks.

By the way, es/config-hooks does not seem to pass 5411 (at least)
even as a standalone topic, and has been kicked out of 'seen' for
some time.  Has anybody took a look into the issue?

Emily Shaffer Dec. 16, 2020, 8:16 p.m. UTC | #3

On Tue, Dec 15, 2020 at 04:56:18PM -0800, Junio C Hamano wrote:
> 
> Josh Steadmon <steadmon@google.com> writes:
> 
> > On 2020.12.04 17:45, Emily Shaffer wrote:
> >> Hi folks, and thanks for the patience - I ran into many, many last-mile
> >> challenges.
> >> 
> >> I haven't addressed many comments on the design doc yet - I was keen to get the
> >> "functionally complete" implementation and conversion to the list.
> >> 
> >> Next on my plate:
> >>  - Update the design doc to make sense with what's in the implementation.
> >>  - A blog post! How to set up new hooks, why they're neat, etc.
> >>  - We seem to have some Googlers interested in trying it out internally, so
> >>    I'm hoping we'll gather and collate feedback from that soon too.
> >>  - And of course addressing comments on this series.
> >> 
> >> Thanks!
> >>  - Emily
> >
> > This approach looks good to me. I'll look forward to seeing the updated
> > design and the feedback from the internal tests.
> 
> Thanks.
> 
> By the way, es/config-hooks does not seem to pass 5411 (at least)
> even as a standalone topic, and has been kicked out of 'seen' for
> some time.  Has anybody took a look into the issue?

Yeah, I looked at it today. Looks like an issue with not paying
attention to master->main default config, since I added a new test to
the 5411 suite (which means it wouldn't have made a conflict for someone
to say "ah yes, s/master/main/g"). I am tracking down couple of other CI
errors today and will send a reroll today or tomorrow.

Junio C Hamano Dec. 16, 2020, 11:32 p.m. UTC | #4

Emily Shaffer <emilyshaffer@google.com> writes:

>> By the way, es/config-hooks does not seem to pass 5411 (at least)
>> even as a standalone topic, and has been kicked out of 'seen' for
>> some time.  Has anybody took a look into the issue?
>
> Yeah, I looked at it today. Looks like an issue with not paying
> attention to master->main default config, since I added a new test to
> the 5411 suite (which means it wouldn't have made a conflict for someone
> to say "ah yes, s/master/main/g"). I am tracking down couple of other CI
> errors today and will send a reroll today or tomorrow.

Thanks.

Emily Shaffer Dec. 18, 2020, 2:07 a.m. UTC | #5

On Wed, Dec 16, 2020 at 03:32:46PM -0800, Junio C Hamano wrote:
> 
> Emily Shaffer <emilyshaffer@google.com> writes:
> 
> >> By the way, es/config-hooks does not seem to pass 5411 (at least)
> >> even as a standalone topic, and has been kicked out of 'seen' for
> >> some time.  Has anybody took a look into the issue?
> >
> > Yeah, I looked at it today. Looks like an issue with not paying
> > attention to master->main default config, since I added a new test to
> > the 5411 suite (which means it wouldn't have made a conflict for someone
> > to say "ah yes, s/master/main/g"). I am tracking down couple of other CI
> > errors today and will send a reroll today or tomorrow.
> 
> Thanks.

I don't have a reroll today. I have been trying to get to the bottom of
a test which fails when built with clang but passes when built with gcc
(t6030-bisect-porcelain.sh after patch 12 of the part II series) and
have not made progress on that, let alone on the other tasks I wanted to
do before sending the next version.

Next week I will only work one day, so I'd anticipate a reroll sometime
the week following. Sorry for the wait - but I think even if I sent it
with the fix for this t5411 failure, it would still break 'seen' because
of whatever this clang vs. gcc problem is.

Hope you enjoy your holidays.

 - Emily

Junio C Hamano Dec. 18, 2020, 5:29 a.m. UTC | #6

Emily Shaffer <emilyshaffer@google.com> writes:

> I don't have a reroll today. I have been trying to get to the bottom of
> a test which fails when built with clang but passes when built with gcc
> (t6030-bisect-porcelain.sh after patch 12 of the part II series) and
> have not made progress on that, let alone on the other tasks I wanted to
> do before sending the next version.

Thanks for an interim report.  No need to rush.

> Hope you enjoy your holidays.

You too, and have fun.

[v6,00/17] propose config-based hooks (part I)

Message

Comments