[RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
A colleague of mine noticed that cherry-pick does not accept the --no-verify option to skip running the pre-commit/commit-msg hooks. 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(-) -- 2.0.0.rc4.501.gdaf83ca -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
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. > > Here's a first attempt at adding --no-verify to the revert/cherry-pick. Back when cherry-pick was a single commit operation, lack of the option did not matter very much, but it probably makes sense to allow telling the command that the entire series of commits is expected to be full of ones that do not verify. In the same vain, we already support --allow-empty and --allow-empty-message, so in that sense this change probably is an improvement. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
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
> 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(-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
On Fri, Sep 5, 2014 at 11:05 PM, Fabian Ruch wrote:
> 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.
Damn. You're obviously correct, and my patch series is seriously
misguided. Please drop it, Junio, and I'm very sorry for the noise.
Hopefully, I will learn not to blindly follow my assumptions.
...Johan
--
Johan Herland,
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
