{"thread":{"id":"42992","subject":"[RFC/PATCH] rebase--interactive: Add \"sign\" command","startedAt":"2016-08-03T08:49:24Z","lastAt":"2016-08-05T16:05:11Z","messageCount":11,"participants":["Chris Packham","Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"292887","messageId":"20160803084743.3299-1-judge.packham@gmail.com","threadId":"42992","inReplyTo":null,"subject":"[RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2016-08-03T08:47:43Z","receivedAt":"2016-08-03T08:49:24Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"This is similar to the existing \"reword\" command in that it can be used\nto update the commit message the difference is that the editor presented\nto the user for the commit. It provides a useful shorthand for \"exec git\ncommit --amend --no-edit -s\"\n\nSigned-off-by: Chris Packham <judge.packham@gmail.com>\n---\nHi,\n\nAt $dayjob we have a patch based work-flow where committers sign patches on the\nway through. Occasionally when a larger patch series is involved it's easier\npull from the submitter's repo and use git rebase -i to add the required sign\noff either by using 'reword' or 'exec git commit --amend -s'.\n\nThis is my attempt at making this a little less cumbersome by adding a\n'sign' command. I decided not to add a short version of the command,\npartly because 's' was taken and partly because it is a bit of a niche\nuse-case.\n\nThanks,\nChris\n\n git-rebase--interactive.sh    | 10 +++++++++-\n t/lib-rebase.sh               |  2 +-\n t/t3404-rebase-interactive.sh |  9 +++++++++\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex ded4595..1cd8bc6 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -148,6 +148,7 @@ append_todo_help () {\n Commands:\n  p, pick = use commit\n  r, reword = use commit, but edit the commit message\n+ sign = use commit, add sign-off to the commit message\n  e, edit = use commit, but stop for amending\n  s, squash = use commit, but meld into previous commit\n  f, fixup = like \\\"squash\\\", but discard this commit's log message\n@@ -594,6 +595,13 @@ you are able to reword the commit.\")\"\n \t\t}\n \t\trecord_in_rewritten $sha1\n \t\t;;\n+\tsign)\n+\t\tcomment_for_reflog sign\n+\t\tmark_action_done\n+\t\tdo_pick $sha1 \"$rest\"\n+\t\tgit commit --amend -s --no-post-rewrite --no-edit ${gpg_sign_opt:+\"$gpg_sign_opt\"}\n+\t\trecord_in_rewritten $sha1\n+\t\t;;\n \tedit|e)\n \t\tcomment_for_reflog edit\n \n@@ -959,7 +967,7 @@ check_bad_cmd_and_sha () {\n \t\t\t# Work around CR left by \"read\" (e.g. with Git for\n \t\t\t# Windows' Bash).\n \t\t\t;;\n-\t\tpick|p|drop|d|reword|r|edit|e|squash|s|fixup|f)\n+\t\tpick|p|drop|d|reword|r|sign|edit|e|squash|s|fixup|f)\n \t\t\tif ! check_commit_sha \"${rest%%[ \t]*}\" \"$lineno\" \"$1\"\n \t\t\tthen\n \t\t\t\tretval=1\ndiff --git a/t/lib-rebase.sh b/t/lib-rebase.sh\nindex 25a77ee..5a54228 100644\n--- a/t/lib-rebase.sh\n+++ b/t/lib-rebase.sh\n@@ -47,7 +47,7 @@ set_fake_editor () {\n \taction=pick\n \tfor line in $FAKE_LINES; do\n \t\tcase $line in\n-\t\tsquash|fixup|edit|reword|drop)\n+\t\tsquash|fixup|edit|reword|sign|drop)\n \t\t\taction=\"$line\";;\n \t\texec*)\n \t\t\techo \"$line\" | sed 's/_/ /g' >> \"$1\";;\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 197914b..e473ffb 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -690,6 +690,15 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n+test_expect_success 'sign-off' '\n+\tgit checkout -b sign-off-branch master &&\n+\tset_fake_editor &&\n+\tFAKE_LINES=\"1 2 3 sign 4\" git rebase -i A &&\n+\tgit show HEAD | grep \"Signed-off-by:\" &&\n+\ttest $(git rev-parse master) != $(git rev-parse HEAD) &&\n+\ttest $(git rev-parse master^) = $(git rev-parse HEAD^)\n+'\n+\n test_expect_success 'rebase -i can copy notes' '\n \tgit config notes.rewrite.rebase true &&\n \tgit config notes.rewriteRef \"refs/notes/*\" &&\n-- \n2.9.2.518.ged577c6.dirty\n\n"},{"id":"292902","messageId":"alpine.DEB.2.20.1608031813410.107993@virtualbox","threadId":"42992","inReplyTo":"xmqqr3a5al7z.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-03T16:15:50Z","receivedAt":"2016-08-03T17:04:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 3 Aug 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > ... my Git garden shears [*1*] (essentially, what\n> > git rebase --interactive --preserve-merges *should* have been).\n> \n> Any plan to fold it into \"git rebase -i\" as a new (improved) mode of\n> operation, by the way?\n\nIt was my plan to integrate it into the sequencer, once the rebase--helper\nlanded in `master`. Little did I know how long it would take to get this\ndone...\n\nMy loose idea was to deprecate --preserve-merges after introducing a\n--recreate-branches, or some such. But enough of that. The rebase--helper\nneeds to get here first.\n\n> > However, I could imagine that we actually want this to be more extensible.\n> > After all, all you are doing is to introduce a new rebase -i command that\n> > does nothing else than shelling out to a command.\n> \n> Yup, I tend to agree.\n\nAwesome!\n\nCiao,\nDscho\n"},{"id":"292908","messageId":"alpine.DEB.2.20.1608031714410.107993@virtualbox","threadId":"42992","inReplyTo":"alpine.DEB.2.20.1608031621590.107993@virtualbox","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-03T15:19:46Z","receivedAt":"2016-08-03T17:04:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Chris,\n\nOn Wed, 3 Aug 2016, Johannes Schindelin wrote:\n\n> I can understand how this \"sign\" command helps you. I myself wished for\n> new commands when working on my Git garden shears [*1*] (essentially, what\n> git rebase --interactive --preserve-merges *should* have been).\n\nAnd of course I forgot to provide the footnote. Sigh. So here they are,\nthe Git garden shears, intended to rebase a thicket of branches:\n\nhttps://github.com/git-for-windows/build-extra/blob/master/shears.sh\n\nThe edit script will look somewhat like this:\n\n\tmark onto\n\n\t# branch \"something\"\n\tbud\n\tpick abcdef first commit\n\tpick abcde0 second commit\n\tmark something\n\n\t# branch \"another-one\"\n\tbud\n\tpick cafebabe yep, that's a different branch\n\tmark another-one\n\n\tbud\n\tmerge -C 0123456 something\n\tmerge -C 6543210 another-one\n\n\tcleanup something another-one\n\nSo you see, I needed to introduce new commands: bud, mark, merge and\ncleanup (I actually also added a \"reset\" one).\n\nThe fake editor I talked about is really the script itself, which detects\nthat it was run as the fake editor, and which converts all those commands\ninto the form \"exec git .r <command> <parameters>...\". The alias..r\nsetting also points to the same script, so that I really only need one\nscript. It's one big hack, but it works.\n\nNeedless to say, my idea to support new rebase -i commands via config\nsettings would be something I would use myself in the Git garden shears.\n\nCiao,\nJohannes\n"},{"id":"292920","messageId":"xmqqr3a5al7z.fsf@gitster.mtv.corp.google.com","threadId":"42992","inReplyTo":"alpine.DEB.2.20.1608031621590.107993@virtualbox","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-03T16:08:48Z","receivedAt":"2016-08-03T17:05:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> ... my Git garden shears [*1*] (essentially, what\n> git rebase --interactive --preserve-merges *should* have been).\n\nAny plan to fold it into \"git rebase -i\" as a new (improved) mode of\noperation, by the way?\n\n> However, I could imagine that we actually want this to be more extensible.\n> After all, all you are doing is to introduce a new rebase -i command that\n> does nothing else than shelling out to a command.\n\nYup, I tend to agree.\n\nAdding \"sign\" feature (i.e. make it pass -S to \"commit [--amend]\")\nmay be a good thing, but adding \"sign\" command to do so is not a\ngreat design.\n\nThere is no inherent reason why \"sign\" feature implies \"--no-edit\",\nand adding a \"sign\" command like this patch means that the next\ncommand somebody else proposes will be \"sign-and-reword\".\n\nWe should be able to treat Signing and Rewording as two orthogonal\nfeatures, one that passes -S, and the other that refrains from\npassing --no-edit.  Otherwise as the number of features grow, the\nnumber of commands will see combinatorial growth.\n\nThanks.\n\n\n"},{"id":"292924","messageId":"alpine.DEB.2.20.1608031621590.107993@virtualbox","threadId":"42992","inReplyTo":"20160803084743.3299-1-judge.packham@gmail.com","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-03T14:31:41Z","receivedAt":"2016-08-03T17:05:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Chris,\n\nOn Wed, 3 Aug 2016, Chris Packham wrote:\n\n> This is similar to the existing \"reword\" command in that it can be used\n> to update the commit message the difference is that the editor presented\n> to the user for the commit. It provides a useful shorthand for \"exec git\n> commit --amend --no-edit -s\"\n\nI can understand how this \"sign\" command helps you. I myself wished for\nnew commands when working on my Git garden shears [*1*] (essentially, what\ngit rebase --interactive --preserve-merges *should* have been).\n\nMy solution was to introduce a new fake editor that calls the real editor\nand afterwards converts the \"new\" commands into exec lines.\n\nHaving said that, this patch clashes seriously with my current effort to\nmove a lot of the interactive rebase from shell into plain C. It is\nactually ready, but getting this into the code base is really slow-going,\nunfortunately.\n\nNow, after looking at your patch it looks to me as if this would be easily\nported, so there is not a big deal here.\n\nHowever, I could imagine that we actually want this to be more extensible.\nAfter all, all you are doing is to introduce a new rebase -i command that\ndoes nothing else than shelling out to a command. Why not introduce a much\nmore flexible feature, where you add something like \"rebase -i aliases\"?\n\nMaybe something like this:\n\n[rebase \"command\"]\n\tsign = git commit --amend -s --no-post-rewrite --no-edit -S\n\nI have not completely thought this through, but maybe this direction would\nmake the interactive rebase even more powerful?\n\nCiao,\nJohannes\n"},{"id":"292927","messageId":"xmqqvazhalof.fsf@gitster.mtv.corp.google.com","threadId":"42992","inReplyTo":"20160803084743.3299-1-judge.packham@gmail.com","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-03T15:58:56Z","receivedAt":"2016-08-03T17:05:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris Packham <judge.packham@gmail.com> writes:\n\n> This is similar to the existing \"reword\" command in that it can be used\n> to update the commit message the difference is that the editor presented\n> to the user for the commit. It provides a useful shorthand for \"exec git\n> commit --amend --no-edit -s\"\n\nHmm, what should those who want to amend and sign do, though?\n"},{"id":"292959","messageId":"20160803180820.2raazmsfjavoaogo@sigill.intra.peff.net","threadId":"42992","inReplyTo":"xmqqr3a5al7z.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-08-03T18:08:20Z","receivedAt":"2016-08-03T18:15:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 03, 2016 at 09:08:48AM -0700, Junio C Hamano wrote:\n\n> > However, I could imagine that we actually want this to be more extensible.\n> > After all, all you are doing is to introduce a new rebase -i command that\n> > does nothing else than shelling out to a command.\n> \n> Yup, I tend to agree.\n> \n> Adding \"sign\" feature (i.e. make it pass -S to \"commit [--amend]\")\n> may be a good thing, but adding \"sign\" command to do so is not a\n> great design.\n\nI'm not sure what you mean by \"feature\" here, but it reminded me of\nMichael's proposal to allow options to todo lines:\n\n  http://public-inbox.org/git/530DA00E.4090402@alum.mit.edu/\n\nwhich would allow:\n\n  pick -S 1234abcd\n\nIf that's what you meant, I think it is a good idea. :)\n\n-Peff\n"},{"id":"293055","messageId":"CAFOYHZDHGn2HsV5U4z3Or7=7ypSkuKwbtQCmNaNuK+n06c0YXA@mail.gmail.com","threadId":"42992","inReplyTo":"alpine.DEB.2.20.1608031621590.107993@virtualbox","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2016-08-04T00:41:47Z","receivedAt":"2016-08-04T00:51:07Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Thu, Aug 4, 2016 at 2:31 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi Chris,\n>\n> On Wed, 3 Aug 2016, Chris Packham wrote:\n>\n>> This is similar to the existing \"reword\" command in that it can be used\n>> to update the commit message the difference is that the editor presented\n>> to the user for the commit. It provides a useful shorthand for \"exec git\n>> commit --amend --no-edit -s\"\n\n<snip>\n\n> Having said that, this patch clashes seriously with my current effort to\n> move a lot of the interactive rebase from shell into plain C. It is\n> actually ready, but getting this into the code base is really slow-going,\n> unfortunately.\n>\n> Now, after looking at your patch it looks to me as if this would be easily\n> ported, so there is not a big deal here.\n\nYeah sorry. I knew there was something in flight but ended up doing a\nquick hack on top of master.\n\n> However, I could imagine that we actually want this to be more extensible.\n> After all, all you are doing is to introduce a new rebase -i command that\n> does nothing else than shelling out to a command. Why not introduce a much\n> more flexible feature, where you add something like \"rebase -i aliases\"?\n>\n> Maybe something like this:\n>\n> [rebase \"command\"]\n>         sign = git commit --amend -s --no-post-rewrite --no-edit -S\n\nI did briefly consider that. I ended up taking the shortcut because I\nhad a patch series I needed to sign.\n\nElsewhere in this thread the idea of pick -S or reword -S was raised.\nI'd actually prefer that because there seems little between 'git\nconfig rebase.command.sign blah' and 'exec ~/sign.sh'\n\n> I have not completely thought this through, but maybe this direction would\n> make the interactive rebase even more powerful?\n\nThe uses I can think of are adding sign-off and running \"make check\".\nFor me rebase -i doesn't need to be much more powerful than that.\n"},{"id":"293056","messageId":"CAFOYHZB080T51QYLaaJasoaJyBycjTRBrNQ-Gh2fitz-fcS5Sw@mail.gmail.com","threadId":"42992","inReplyTo":"20160803180820.2raazmsfjavoaogo@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2016-08-04T00:53:56Z","receivedAt":"2016-08-04T00:54:41Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On Thu, Aug 4, 2016 at 6:08 AM, Jeff King <peff@peff.net> wrote:\n> On Wed, Aug 03, 2016 at 09:08:48AM -0700, Junio C Hamano wrote:\n>\n>> > However, I could imagine that we actually want this to be more extensible.\n>> > After all, all you are doing is to introduce a new rebase -i command that\n>> > does nothing else than shelling out to a command.\n>>\n>> Yup, I tend to agree.\n>>\n>> Adding \"sign\" feature (i.e. make it pass -S to \"commit [--amend]\")\n>> may be a good thing, but adding \"sign\" command to do so is not a\n>> great design.\n>\n> I'm not sure what you mean by \"feature\" here, but it reminded me of\n> Michael's proposal to allow options to todo lines:\n>\n>   http://public-inbox.org/git/530DA00E.4090402@alum.mit.edu/\n>\n> which would allow:\n>\n>   pick -S 1234abcd\n>\n> If that's what you meant, I think it is a good idea. :)\n\nThat would definitely suit me. I see there was some discussion of\nwhich options were sensible to support. --signoff and --reset-author\nwould be a good start from my point of view. Maybe that's something\nthat could be build on top of Johannes work.\n\n>\n> -Peff\n"},{"id":"293093","messageId":"xmqqinvg5xjv.fsf@gitster.mtv.corp.google.com","threadId":"42992","inReplyTo":"20160803180820.2raazmsfjavoaogo@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-08-04T16:05:56Z","receivedAt":"2016-08-04T16:06:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Wed, Aug 03, 2016 at 09:08:48AM -0700, Junio C Hamano wrote:\n>\n>> > However, I could imagine that we actually want this to be more extensible.\n>> > After all, all you are doing is to introduce a new rebase -i command that\n>> > does nothing else than shelling out to a command.\n>> \n>> Yup, I tend to agree.\n>> \n>> Adding \"sign\" feature (i.e. make it pass -S to \"commit [--amend]\")\n>> may be a good thing, but adding \"sign\" command to do so is not a\n>> great design.\n>\n> I'm not sure what you mean by \"feature\" here, but it reminded me of\n> Michael's proposal to allow options to todo lines:\n>\n>   http://public-inbox.org/git/530DA00E.4090402@alum.mit.edu/\n>\n> which would allow:\n>\n>   pick -S 1234abcd\n>\n> If that's what you meant, I think it is a good idea. :)\n\nYes, by \"feature\" I meant \"giving the ability to decide if the\nresulting commit gets signature\", which can and should be orthogonal\nto the choice of using editor to reword the message when the commit\nis created or \"--no-edit\" is passed and the original message is used\nverbatim.\n"},{"id":"293204","messageId":"alpine.DEB.2.20.1608051800190.5786@virtualbox","threadId":"42992","inReplyTo":"20160803180820.2raazmsfjavoaogo@sigill.intra.peff.net","subject":"Re: [RFC/PATCH] rebase--interactive: Add \"sign\" command","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-08-05T16:04:11Z","receivedAt":"2016-08-05T16:05:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Wed, 3 Aug 2016, Jeff King wrote:\n\n> On Wed, Aug 03, 2016 at 09:08:48AM -0700, Junio C Hamano wrote:\n> \n> > > However, I could imagine that we actually want this to be more\n> > > extensible.  After all, all you are doing is to introduce a new\n> > > rebase -i command that does nothing else than shelling out to a\n> > > command.\n> > \n> > Yup, I tend to agree.\n> > \n> > Adding \"sign\" feature (i.e. make it pass -S to \"commit [--amend]\") may\n> > be a good thing, but adding \"sign\" command to do so is not a great\n> > design.\n> \n> I'm not sure what you mean by \"feature\" here, but it reminded me of\n> Michael's proposal to allow options to todo lines:\n> \n>   http://public-inbox.org/git/530DA00E.4090402@alum.mit.edu/\n> \n> which would allow:\n> \n>   pick -S 1234abcd\n> \n> If that's what you meant, I think it is a good idea. :)\n\nI looked at the code in git-rebase--interactive.sh again and stumbled over\nsomething important: if you \"pick\" a commit, it *already* uses the\ninformation provided to the rebase command via the -S option, *unless* the\npick fast-forwards.\n\nThat is, I came to believe that the \"sign\" command is unnecessary, and\nthat the --force-rebase option in conjunction with the -S option is what\nshould be used.\n\nCiao,\nDscho\n"}]}