From: Fabian Ruch <bafain@gmail.com>
To: Johan Herland <johan@herland.net>, git@vger.kernel.org
Cc: "Lars Gullik Bjønnes" <larsbj@gullik.org>,
"Junio C Hamano" <gitster@pobox.com>,
"Neil Horman" <nhorman@tuxdriver.com>,
"Ramkumar Ramachandra" <artagnon@gmail.com>
Subject: Re: [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option
Date: Fri, 05 Sep 2014 23:05:28 +0200 [thread overview]
Message-ID: <540A2598.8000101@gmail.com> (raw)
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
> 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(-)
next prev parent reply other threads:[~2014-09-05 21:05 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-03 14:03 [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option Johan Herland
2014-09-03 14:03 ` [RFC/PATCH 1/3] t7503/4: Add failing testcases for revert/cherry-pick --no-verify Johan Herland
2014-09-03 19:28 ` Junio C Hamano
2014-09-03 14:03 ` [RFC/PATCH 2/3] revert/cherry-pick: Add --no-verify option, and pass it on to commit Johan Herland
2014-09-03 19:32 ` Junio C Hamano
2014-09-03 19:42 ` [PATCH] parse-options: detect attempt to add a duplicate short option name Junio C Hamano
2014-09-03 20:29 ` René Scharfe
2014-09-03 21:05 ` Junio C Hamano
2014-09-03 21:46 ` René Scharfe
2014-09-03 22:16 ` Junio C Hamano
2014-09-04 6:13 ` René Scharfe
2014-09-04 17:24 ` Junio C Hamano
2014-09-04 18:07 ` Junio C Hamano
2014-09-03 21:46 ` Jonathan Nieder
2014-09-03 21:58 ` Jonathan Nieder
2014-09-04 8:34 ` [RFC/PATCH 2/3] revert/cherry-pick: Add --no-verify option, and pass it on to commit Johan Herland
2014-09-03 14:03 ` [RFC/PATCH 3/3] revert/cherry-pick --no-verify: Update documentation Johan Herland
2014-09-03 19:21 ` [RFC/PATCH 0/3] Teach revert/cherry-pick the --no-verify option Junio C Hamano
2014-09-05 21:05 ` Fabian Ruch [this message]
2014-09-08 15:13 ` Johan Herland
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=540A2598.8000101@gmail.com \
--to=bafain@gmail.com \
--cc=artagnon@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johan@herland.net \
--cc=larsbj@gullik.org \
--cc=nhorman@tuxdriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).