{"thread":{"id":"56208","subject":"git revert --continue --no-verify","startedAt":"2021-08-03T19:17:20Z","lastAt":"2021-08-05T01:57:13Z","messageCount":10,"participants":["Cameron Steffen","Taylor Blau","Junio C Hamano","Phillip Wood"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"431841","messageId":"CAAVFnNkW6Bc4bBDeea2v-VFydvEC0dBw+QXVA0-6OnmF8km3ZA@mail.gmail.com","threadId":"56208","inReplyTo":null,"subject":"git revert --continue --no-verify","fromName":"Cameron Steffen","fromEmail":"cam.steffen94@gmail.com","sentAt":"2021-08-03T19:17:07Z","receivedAt":"2021-08-03T19:17:20Z","isPatch":false,"sender":{"key":"cam.steffen94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5565418?v=4"},"body":"Today I tried to run this command and I just got a help screen. I am\nusing 2.32.0.\n\ngit revert --continue --no-verify\n"},{"id":"431880","messageId":"YQmsJUe6hAMy/QGA@nand.local","threadId":"56208","inReplyTo":"CAAVFnNkW6Bc4bBDeea2v-VFydvEC0dBw+QXVA0-6OnmF8km3ZA@mail.gmail.com","subject":"Re: git revert --continue --no-verify","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-08-03T20:50:45Z","receivedAt":"2021-08-03T20:50:49Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi Cameron,\n\nOn Tue, Aug 03, 2021 at 02:17:07PM -0500, Cameron Steffen wrote:\n> Today I tried to run this command and I just got a help screen. I am\n> using 2.32.0.\n>\n> git revert --continue --no-verify\n\nI can't think of any sequencer commands that support a `--[no-]verify`\nargument. Did you mistake `--no-verify` for something else, like\n`--no-edit` or `--no-commit`?\n\nThanks,\nTaylor\n"},{"id":"431881","messageId":"CAAVFnN=W27rdE1EH-joscyJEooAsDrdtPropVVaBYwhte=cPJA@mail.gmail.com","threadId":"56208","inReplyTo":"YQmsJUe6hAMy/QGA@nand.local","subject":"Re: git revert --continue --no-verify","fromName":"Cameron Steffen","fromEmail":"cam.steffen94@gmail.com","sentAt":"2021-08-03T20:56:49Z","receivedAt":"2021-08-03T20:57:03Z","isPatch":false,"sender":{"key":"cam.steffen94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5565418?v=4"},"body":"`--no-verify` is an argument typically used with `git commit` in order\nto skip the pre-commit hook. Since the pre-commit hook also runs on\n`git revert --continue`, I expected to be able to use `--no-verify`.\n\nOn Tue, Aug 3, 2021 at 3:50 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> Hi Cameron,\n>\n> On Tue, Aug 03, 2021 at 02:17:07PM -0500, Cameron Steffen wrote:\n> > Today I tried to run this command and I just got a help screen. I am\n> > using 2.32.0.\n> >\n> > git revert --continue --no-verify\n>\n> I can't think of any sequencer commands that support a `--[no-]verify`\n> argument. Did you mistake `--no-verify` for something else, like\n> `--no-edit` or `--no-commit`?\n>\n> Thanks,\n> Taylor\n"},{"id":"431882","messageId":"YQmuT2ZYe1mzHBjI@nand.local","threadId":"56208","inReplyTo":"CAAVFnN=W27rdE1EH-joscyJEooAsDrdtPropVVaBYwhte=cPJA@mail.gmail.com","subject":"Re: git revert --continue --no-verify","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-08-03T20:59:59Z","receivedAt":"2021-08-03T21:00:03Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Aug 03, 2021 at 03:56:49PM -0500, Cameron Steffen wrote:\n> `--no-verify` is an argument typically used with `git commit` in order\n> to skip the pre-commit hook. Since the pre-commit hook also runs on\n> `git revert --continue`, I expected to be able to use `--no-verify`.\n\nNo, `git revert` doesn't pass unknown options down to `git commit`.\n\nThanks,\nTaylor\n"},{"id":"431885","messageId":"CAAVFnNmitHeimogaGUUwWGWhve+wbF6CXcSNUonGCWAgOSKzjA@mail.gmail.com","threadId":"56208","inReplyTo":"YQmuT2ZYe1mzHBjI@nand.local","subject":"Re: git revert --continue --no-verify","fromName":"Cameron Steffen","fromEmail":"cam.steffen94@gmail.com","sentAt":"2021-08-03T21:33:09Z","receivedAt":"2021-08-03T21:33:24Z","isPatch":false,"sender":{"key":"cam.steffen94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5565418?v=4"},"body":"Perhaps the issue then is that the pre-commit hook should not run for\n`git revert --continue`? It does not run for `git revert`.\n\nOn Tue, Aug 3, 2021 at 4:00 PM Taylor Blau <me@ttaylorr.com> wrote:\n>\n> On Tue, Aug 03, 2021 at 03:56:49PM -0500, Cameron Steffen wrote:\n> > `--no-verify` is an argument typically used with `git commit` in order\n> > to skip the pre-commit hook. Since the pre-commit hook also runs on\n> > `git revert --continue`, I expected to be able to use `--no-verify`.\n>\n> No, `git revert` doesn't pass unknown options down to `git commit`.\n>\n> Thanks,\n> Taylor\n"},{"id":"431888","messageId":"YQm+PWAtc3rixqsw@nand.local","threadId":"56208","inReplyTo":"CAAVFnNmitHeimogaGUUwWGWhve+wbF6CXcSNUonGCWAgOSKzjA@mail.gmail.com","subject":"Re: git revert --continue --no-verify","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-08-03T22:07:57Z","receivedAt":"2021-08-03T22:08:00Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Aug 03, 2021 at 04:33:09PM -0500, Cameron Steffen wrote:\n> Perhaps the issue then is that the pre-commit hook should not run for\n> `git revert --continue`? It does not run for `git revert`.\n\nThis does look like an oversight to me, but you'll have to bear with me\nsince I am relatively unfamiliar with the sequencer code.\n\nUltimately `git revert` calls do_pick_commit() which either calls\ndo_commit() or run_git_commit(). A couple of curiosities there:\n\n  - do_commit() does fall back to run_git_commit() if it has the\n    VERIFY_MSG bit set in `flags`.\n  - run_git_commit() passes `-n` only when VERIFY_MSG *isn't* set, so\n    the VERIFY_MSG bit does imply that the pre-commit hook would be run\n    there.\n  - when do_pick_commit() does have to fall back to run_git_commit(), it\n    sets the VERIFY_MSG bit in flags.\n\nBut we never end up calling run_git_commit() (except in the case of\nerrors) because do_pick_commit() special-cases `command == TODO_REVERT`\n(which is the case for `git revert`) and calls `do_commit()`.\n\nBut it gets weirder: do_commit() calls run_git_commit() itself, but\nbefore the caller in do_pick_commit() has had a chance to add VERIFY_MSG\nto the flags.\n\nSo I suspect that this is an oversight, but perhaps somebody more\nfamiliar with this code could confirm my thinking.\n\nThanks,\nTaylor\n"},{"id":"431913","messageId":"xmqq4kc6rqmi.fsf@gitster.g","threadId":"56208","inReplyTo":"YQm+PWAtc3rixqsw@nand.local","subject":"Re: git revert --continue --no-verify","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-08-04T00:38:13Z","receivedAt":"2021-08-04T00:38:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau <me@ttaylorr.com> writes:\n\n> On Tue, Aug 03, 2021 at 04:33:09PM -0500, Cameron Steffen wrote:\n>> Perhaps the issue then is that the pre-commit hook should not run for\n>> `git revert --continue`? It does not run for `git revert`.\n>\n> This does look like an oversight to me, but you'll have to bear with me\n> since I am relatively unfamiliar with the sequencer code.\n>\n> Ultimately `git revert` calls do_pick_commit() which either calls\n> do_commit() or run_git_commit(). A couple of curiosities there:\n>\n>   - do_commit() does fall back to run_git_commit() if it has the\n>     VERIFY_MSG bit set in `flags`.\n>   - run_git_commit() passes `-n` only when VERIFY_MSG *isn't* set, so\n>     the VERIFY_MSG bit does imply that the pre-commit hook would be run\n>     there.\n>   - when do_pick_commit() does have to fall back to run_git_commit(), it\n>     sets the VERIFY_MSG bit in flags.\n>\n> But we never end up calling run_git_commit() (except in the case of\n> errors) because do_pick_commit() special-cases `command == TODO_REVERT`\n> (which is the case for `git revert`) and calls `do_commit()`.\n>\n> But it gets weirder: do_commit() calls run_git_commit() itself, but\n> before the caller in do_pick_commit() has had a chance to add VERIFY_MSG\n> to the flags.\n>\n> So I suspect that this is an oversight, but perhaps somebody more\n> familiar with this code could confirm my thinking.\n\nIIRC, the \"--continue\" option that creates a commit came much later\nfor lazy folks; the norm was to conclude your conflict resolution\nwith \"git commit\" so that the HEAD and the index and the working\ntree matches before running \"--continue\".  Wouldn't that work in\nthis case?  That is\n\n    $ git <some sequencing command> A..B\n    ...\n    ... stops with conflict\n    $ edit\n    $ git add ...\n    $ git commit --no-verify\n    $ git <that sequencing command> --continue\n\n\n    \n"},{"id":"431973","messageId":"74443c64-efe9-ea47-e918-d62b8c976abc@gmail.com","threadId":"56208","inReplyTo":"YQm+PWAtc3rixqsw@nand.local","subject":"Re: git revert --continue --no-verify","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-08-04T18:14:34Z","receivedAt":"2021-08-04T18:14:40Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Cameron and Taylor\n\nOn 03/08/2021 23:07, Taylor Blau wrote:\n> On Tue, Aug 03, 2021 at 04:33:09PM -0500, Cameron Steffen wrote:\n>> Perhaps the issue then is that the pre-commit hook should not run for\n>> `git revert --continue`? It does not run for `git revert`.\n\nThe general rule for cherry-pick, revert and rebase is that if the \ncommit message is edited then the commit is made with --verify and if \nthe commit message is not being edited then the commit is made with \n--no-verify. I think the reasoning for this is that if the user has \naltered the commit message or contents and they have a pre-commit hook \nset then it is reasonable to check the new commit is acceptable to the \nhook. There are some exceptions to this which are oversights (for \nexample I think we always pass --no-verify when committing a conflict \nresolution with 'git rebase --continue' even though the editor opens \nwhen committing in that case - I should try and fix that)\n\nFor 'git revert --continue' the commit is made when sequencer_continue() \ncalls continue_single_pick() which looks like\n\nstatic int continue_single_pick(struct repository *r, struct replay_opts \n*opts)\n{\n\tstruct strvec argv = STRVEC_INIT;\n\tint ret;\n\n\tif (!refs_ref_exists(get_main_ref_store(r), \"CHERRY_PICK_HEAD\") &&\n\t    !refs_ref_exists(get_main_ref_store(r), \"REVERT_HEAD\"))\n\t\treturn error(_(\"no cherry-pick or revert in progress\"));\n\n\tstrvec_push(&argv, \"commit\");\n\n\t/*\n\t * continue_single_pick() handles the case of recovering from a\n\t * conflict.  should_edit() doesn't handle that case; for a conflict,\n\t * we want to edit if the user asked for it, or if they didn't specify\n\t * and stdin is a tty.\n\t */\n\tif (!opts->edit || (opts->edit < 0 && !isatty(0)))\n\t\t/*\n\t\t * Include --cleanup=strip as well because we don't want the\n\t\t * \"# Conflicts:\" messages.\n\t\t */\n\t\tstrvec_pushl(&argv, \"--no-edit\", \"--cleanup=strip\", NULL);\n\n\tret = run_command_v_opt(argv.v, RUN_GIT_CMD);\n\tstrvec_clear(&argv);\n\treturn ret;\n}\n\nIt runs git commit directly and never passes --no-verify. I wouldn't be \nopposed to someone adding support for --no-verify (and --no-edit) to \n\"cherry-pick/revert/rebase --continue\" on the understanding that it only \napplied when committing the conflict resolution. There is a possible \nconfusion for users though who might expect that the options passed with \n'--continue' applied to all the commits made by the command.\n\n> This does look like an oversight to me, but you'll have to bear with me\n> since I am relatively unfamiliar with the sequencer code.\n> \n> Ultimately `git revert` calls do_pick_commit()\n\nThis is what happens when we are picking commits, the commit made with \n'--continue' uses a different code path\n\n> which either calls\n> do_commit() or run_git_commit(). A couple of curiosities there:\n> \n>    - do_commit() does fall back to run_git_commit() if it has the\n>      VERIFY_MSG bit set in `flags`.\n>    - run_git_commit() passes `-n` only when VERIFY_MSG *isn't* set, so\n>      the VERIFY_MSG bit does imply that the pre-commit hook would be run\n>      there.\n>    - when do_pick_commit() does have to fall back to run_git_commit(), it\n>      sets the VERIFY_MSG bit in flags.\n\nCalling run_git_commit() there is not a fallback it, that call is used \nto reword commits once they have been picked (see 450efe2d53 (\"rebase \n-i: always update HEAD before rewording\", 2019-08-19) for the reasoning \nbehind this) . Because the message is being edited there is no point in \ngoing through do_commit().\n\n> But we never end up calling run_git_commit() (except in the case of\n> errors) because do_pick_commit() special-cases `command == TODO_REVERT`\n> (which is the case for `git revert`) and calls `do_commit()`.\n> \n> But it gets weirder: do_commit() calls run_git_commit() itself, but\n> before the caller in do_pick_commit() has had a chance to add VERIFY_MSG\n> to the flags.\n\ndo_commit() does not change the flags that it is called with - callers \nthat want VERIFY_MSG will set that before they call do_commit(). \ndo_commit() is there to commit simple picks without forking 'git commit'\n\n> So I suspect that this is an oversight, but perhaps somebody more\n> familiar with this code could confirm my thinking.\n\nI hope the above helps - basically the idea is \"if the commit has been \nedited use VERIFY_MSG\" and --continue unhelpfully uses a completely \ndifferent code-path to the main commit picking/reverting loop.\n\nBest Wishes\n\nPhillip\n\n> Thanks,\n> Taylor\n> \n\n"},{"id":"432042","messageId":"YQtBezRE3l09cILa@nand.local","threadId":"56208","inReplyTo":"74443c64-efe9-ea47-e918-d62b8c976abc@gmail.com","subject":"Re: git revert --continue --no-verify","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-08-05T01:40:11Z","receivedAt":"2021-08-05T01:40:17Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Aug 04, 2021 at 07:14:34PM +0100, Phillip Wood wrote:\n> [...] I wouldn't be opposed to someone adding support for --no-verify\n> (and --no-edit) to \"cherry-pick/revert/rebase --continue\" on the\n> understanding that it only applied when committing the conflict\n> resolution. There is a possible confusion for users though who might\n> expect that the options passed with '--continue' applied to all the\n> commits made by the command.\n\nYeah, that feels like we are just trying to confuse the user ;). So I\nagree that I'd rather not go any further along that direction.\n\n> do_commit() does not change the flags that it is called with - callers that\n> want VERIFY_MSG will set that before they call do_commit(). do_commit() is\n> there to commit simple picks without forking 'git commit'\n>\n> > So I suspect that this is an oversight, but perhaps somebody more\n> > familiar with this code could confirm my thinking.\n>\n> I hope the above helps - basically the idea is \"if the commit has been\n> edited use VERIFY_MSG\" and --continue unhelpfully uses a completely\n> different code-path to the main commit picking/reverting loop.\n\nMakes sense, thanks.\n\nTaylor\n"},{"id":"432044","messageId":"CAAVFnN=ynj8sa-5YPmU7=s2OcXzy8a_gsxRxVxFD_9LgWQOBtg@mail.gmail.com","threadId":"56208","inReplyTo":"YQtBezRE3l09cILa@nand.local","subject":"Re: git revert --continue --no-verify","fromName":"Cameron Steffen","fromEmail":"cam.steffen94@gmail.com","sentAt":"2021-08-05T01:56:58Z","receivedAt":"2021-08-05T01:57:13Z","isPatch":false,"sender":{"key":"cam.steffen94@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5565418?v=4"},"body":"I've updated my mental model to include \"--continue is a shortcut for\n`git commit && ..`\". That is a big help and resolves the issue for me\nsince I know I can always fall back to running `git commit` manually.\nThanks for all the replies!\n\nCameron\n"}]}