git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option

From
Fabian Ruch <bafain@gmail.com>
Date
Sep 5, 2014, 21:05 UTC
Message-ID
<540A2598.8000101@gmail.com>
In-Reply-To
<1409753034-9459-1-git-send-email-johan@herland.net>
Hi Johan,
Johan Herland writes:
> A colleague of mine noticed that cherry-pick does not accept the
> --no-verify option to skip running the pre-commit/commit-msg hooks.

neither git-cherry-pick nor git-revert execute the pre-commit or commit-msg hooks at the moment. The underlying rationale can be found in the log message of commit 9fa4db5 ("Do not verify reverted/cherry-picked/rebased patches."). Indeed, the sequencer uses git-commit internally which executes the two verify hooks by default. However, the particular command line being used implicitly specifies the --no-verify option. This behaviour is implemented in sequencer.c#run_git_commit as well, right before the configurable git-commit options are handled. I guess that's easily overlooked since the documentation doesn't mention it and the implementation uses the short version -n of --no-verify.

The reasons why the new test cases succeed nonetheless are manifold. I hope they're still understandable even though I don't put the comments next to the code.

The "revert with failing hook" test case fails if run in isolation, which can be achieved by using the very cool --run option of test-lib. More specifically, git-revert does not fail because it executes the failing hook but because the preceding test case leaves behind an uncommitted index.

In the "cherry-pick with failing hook" test case, git-cherry-pick really fails because it doesn't know the --no-verify option yet, which presumably ended up there only by accident. This test case is meaningless if run in isolation because it assumes that "revert with failing hook" creates a commit (else HEAD^ points nowhere).

I like your patchset for that it makes it explicit in both the documentation and the tests whether the commits resulting from cherry-picks are being verified or not.

Kind regards,
   Fabian
Show 20 quoted lines
> Here's a first attempt at adding --no-verify to the revert/cherry-pick.
> 
> Have fun! :)
> 
> ...Johan
> 
> Johan Herland (3):
>   t7503/4: Add failing testcases for revert/cherry-pick --no-verify
>   revert/cherry-pick: Add --no-verify option, and pass it on to commit
>   revert/cherry-pick --no-verify: Update documentation
> 
>  Documentation/git-cherry-pick.txt |  4 ++++
>  Documentation/git-revert.txt      |  4 ++++
>  Documentation/githooks.txt        | 20 ++++++++++----------
>  builtin/revert.c                  |  1 +
>  sequencer.c                       |  7 +++++++
>  sequencer.h                       |  1 +
>  t/t7503-pre-commit-hook.sh        | 24 ++++++++++++++++++++++++
>  t/t7504-commit-msg-hook.sh        | 24 ++++++++++++++++++++++++
>  8 files changed, 75 insertions(+), 10 deletions(-)
Previous: Junio C HamanoNext: Johan Herland
Message 19 of 20 in “Teach revert/cherry-pick the --no-verify option”
  1. 0/3 Teach revert/cherry-pick the --no-verify optionJohan Herland, Sep 3, 2014
  2. 1/3 t7503/4: Add failing testcases for revert/cherry-pick --no-verifyJohan Herland, Sep 3, 2014
  3. Junio C HamanoSep 3, 2014
  4. 2/3 revert/cherry-pick: Add --no-verify option, and pass it on to commitJohan Herland, Sep 3, 2014
  5. Junio C HamanoSep 3, 2014
  6. parse-options: detect attempt to add a duplicate short option nameJunio C Hamano, Sep 3, 2014
  7. René ScharfeSep 3, 2014
  8. Junio C HamanoSep 3, 2014
  9. René ScharfeSep 3, 2014
  10. Junio C HamanoSep 3, 2014
  11. René ScharfeSep 4, 2014
  12. Junio C HamanoSep 4, 2014
  13. Junio C HamanoSep 4, 2014
  14. Jonathan NiederSep 3, 2014
  15. Jonathan NiederSep 3, 2014
  16. Johan HerlandSep 4, 2014
  17. 3/3 revert/cherry-pick --no-verify: Update documentationJohan Herland, Sep 3, 2014
  18. Junio C HamanoSep 3, 2014
  19. Fabian RuchSep 5, 2014
  20. Johan HerlandSep 8, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.