git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Dave Abrahams <dave@boostpro.com>
Cc: git@vger.kernel.org
Subject: Re: cherry-pick / pre-commit hook?
Date: Wed, 8 Dec 2010 11:53:24 -0600	[thread overview]
Message-ID: <20101208175324.GB5687@burratino> (raw)
In-Reply-To: <m2wrnktcl2.wl%dave@boostpro.com>

Dave Abrahams wrote:

> Is there a good reason that git cherry-pick (without --no-commit)
> doesn't run my pre-commit hook?

Interesting question.

 $ git grep -F -e '"cherry-pick"'
 [...]
 git.c:          { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
 $ git grep -F -e cmd_cherry_pick
 builtin.h:extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);
 builtin/revert.c:int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 git.c:          { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },

cherry-pick is implemented in builtin/revert.c.  How does it invoke
the "git commit" machinery?  Explicitly, as luck would have it.

 $ git grep --show-function -F -h -C5 -e '"commit"' -- builtin/revert.c
 static int run_git_commit(const char *defmsg)
 {
         /* 6 is max possible length of our args array including NULL */
         const char *args[6];
         int i = 0;
 
         args[i++] = "commit";
         args[i++] = "-n";
         if (signoff)
                 args[i++] = "-s";
 [...]

So cherry-pick deliberately uses -n (= --no-verify) when it calls "git commit".
Why, though?

 $ git log --oneline --follow -S'"-n"' -- builtin/revert.c
 cfd9c27 Allow cherry-pick (and revert) to add signoff line
 f810379 Make builtin-revert.c use parse_options.
 9509af6 Make git-revert & git-cherry-pick a builtin

The '-n' was copied from the old git-revert.sh script when cherry-pick
was made builtin.  Not to let the trail grow cold:

 $ git log --oneline --follow -S-n -- git-revert.sh
 9509af6 Make git-revert & git-cherry-pick a builtin
 abd6970 cherry-pick: make -r the default
 674b280 Add documentation for git-revert and git-cherry-pick.
 8bf14d6 Document the --(no-)edit switch of git-revert and git-cherry-pick
 b788498 git-revert: make --edit default.
 e2f5f6e Do not require clean tree when reverting and cherry-picking.
 9fa4db5 Do not verify reverted/cherry-picked/rebased patches.
 [...]
 $ git show -s 9fa4db5
 commit 9fa4db544e2e4d6c931f6adabc5270daec041536
 Author: Junio C Hamano <junkio@cox.net>
 Date:   Mon Aug 29 21:19:04 2005 -0700

     Do not verify reverted/cherry-picked/rebased patches.
    
     The original committer may have used validation criteria that is less
     stricter than yours.  You do not want to lose the changes even if they
     are done in substandard way from your 'commit -v' verifier's point of
     view.
    
     Signed-off-by: Junio C Hamano <junkio@cox.net>
 $

At last, an answer.  The main purpose of the pre-commit hook (and
builtin checks that preceded it) is to avoid introducing regressions
in whitespace style, encoding, and so forth; but it would make
cherry-picking unnecessarily difficult, without preventing
regressions, to apply the same standards to existing code.

> Is there a hook that cherry-pick
> /will/ run instead?

"git log --grep=pre-commit" seems to suggest that the commit-msg and
post-commit hooks will be run.  But first, what are you trying to
accomplish?  Maybe there is a simpler way, or maybe with that use
case in mind we can make changes to support it better.

Hope that helps,
Jonathan

  reply	other threads:[~2010-12-08 17:53 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-08 17:10 cherry-pick / pre-commit hook? Dave Abrahams
2010-12-08 17:53 ` Jonathan Nieder [this message]
2010-12-08 21:22   ` Dave Abrahams
2010-12-08 22:05     ` Jonathan Nieder
2010-12-27  2:18       ` Dave Abrahams
2010-12-27  9:37         ` Jonathan Nieder
2010-12-27 20:58           ` Junio C Hamano
2010-12-27 21:33             ` Dave Abrahams
2010-12-28 18:16           ` Junio C Hamano
2010-12-28 22:38             ` Jakub Narebski
2010-12-29  1:00               ` Dave Abrahams

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101208175324.GB5687@burratino \
    --to=jrnieder@gmail.com \
    --cc=dave@boostpro.com \
    --cc=git@vger.kernel.org \
    /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.
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 NNTP newsgroup(s).