{"thread":{"id":"48039","subject":"[PATCH 1/2] rebase: support --signoff with implicit rebase","startedAt":"2018-03-14T11:11:53Z","lastAt":"2018-03-20T18:45:06Z","messageCount":15,"participants":["Phillip Wood","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"341628","messageId":"20180314111127.14217-1-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":null,"subject":"[PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-14T11:11:26Z","receivedAt":"2018-03-14T11:11:53Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nThis allows one to run 'git rebase --exec \"make check\" --signoff'\nwhich is useful when preparing a patch series for publication and is\nmore convenient than doing the signoff with another --exec command.\nThis change also allows --root without --onto to work with --signoff\nas well (--root with --onto was already supported). Note that the\nfailing test is due to a bug in 'rebase --root' when the root commit\nis empty which will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-rebase--interactive.sh |  6 +++---\n git-rebase.sh              | 26 +++++++++++++++++++++++++-\n sequencer.c                |  6 ++++++\n t/t3428-rebase-signoff.sh  | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 66 insertions(+), 4 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 81c5b42875..4ea54fc1c4 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -283,7 +283,7 @@ pick_one () {\n \t\tpick_one_preserving_merges \"$@\" && return\n \toutput eval git cherry-pick $allow_rerere_autoupdate $allow_empty_message \\\n \t\t\t${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")} \\\n-\t\t\t\"$strategy_args\" $empty_args $ff \"$@\"\n+\t\t\t$signoff \"$strategy_args\" $empty_args $ff \"$@\"\n \n \t# If cherry-pick dies it leaves the to-be-picked commit unrecorded. Reschedule\n \t# previous task so this commit is not lost.\n@@ -525,10 +525,10 @@ do_pick () {\n \t\t# resolve before manually running git commit --amend then git\n \t\t# rebase --continue.\n \t\tgit commit --allow-empty --allow-empty-message --amend \\\n-\t\t\t   --no-post-rewrite -n -q -C $sha1 &&\n+\t\t\t   --no-post-rewrite -n -q -C $sha1 $signoff &&\n \t\t\tpick_one -n $sha1 &&\n \t\t\tgit commit --allow-empty --allow-empty-message \\\n-\t\t\t\t   --amend --no-post-rewrite -n -q -C $sha1 \\\n+\t\t\t\t   --amend --no-post-rewrite -n -q -C $sha1 $signoff \\\n \t\t\t\t   ${gpg_sign_opt:+\"$gpg_sign_opt\"} ||\n \t\t\t\t   die_with_patch $sha1 \"$(eval_gettext \"Could not apply \\$sha1... \\$rest\")\"\n \telse\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex b353c33d41..40301756be 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -91,6 +91,7 @@ preserve_merges=\n autosquash=\n keep_empty=\n allow_empty_message=\n+signoff=\n test \"$(git config --bool rebase.autosquash)\" = \"true\" && autosquash=t\n case \"$(git config --bool commit.gpgsign)\" in\n true)\tgpg_sign_opt=-S ;;\n@@ -120,6 +121,10 @@ read_basic_state () {\n \t\tallow_rerere_autoupdate=\"$(cat \"$state_dir\"/allow_rerere_autoupdate)\"\n \ttest -f \"$state_dir\"/gpg_sign_opt &&\n \t\tgpg_sign_opt=\"$(cat \"$state_dir\"/gpg_sign_opt)\"\n+\ttest -f \"$state_dir\"/signoff && {\n+\t\tsignoff=\"$(cat \"$state_dir\"/signoff)\"\n+\t\tforce_rebase=t\n+\t}\n }\n \n write_basic_state () {\n@@ -134,6 +139,7 @@ write_basic_state () {\n \ttest -n \"$allow_rerere_autoupdate\" && echo \"$allow_rerere_autoupdate\" > \\\n \t\t\"$state_dir\"/allow_rerere_autoupdate\n \ttest -n \"$gpg_sign_opt\" && echo \"$gpg_sign_opt\" > \"$state_dir\"/gpg_sign_opt\n+\ttest -n \"$signoff\" && echo \"$signoff\" >\"$state_dir\"/signoff\n }\n \n output () {\n@@ -329,7 +335,13 @@ do\n \t--ignore-whitespace)\n \t\tgit_am_opt=\"$git_am_opt $1\"\n \t\t;;\n-\t--committer-date-is-author-date|--ignore-date|--signoff|--no-signoff)\n+\t--signoff)\n+\t\tsignoff=--signoff\n+\t\t;;\n+\t--no-signoff)\n+\t\tsignoff=\n+\t\t;;\n+\t--committer-date-is-author-date|--ignore-date)\n \t\tgit_am_opt=\"$git_am_opt $1\"\n \t\tforce_rebase=t\n \t\t;;\n@@ -459,6 +471,18 @@ then\n \tgit_format_patch_opt=\"$git_format_patch_opt --progress\"\n fi\n \n+if test -n \"$signoff\"\n+then\n+\ttest \"$interactive_rebase\" = explicit &&\n+\t\tdie \"$(gettext \"error: interactive rebase does not support --signoff\")\"\n+\ttest \"$type\" = merge &&\n+\t\tdie \"$(gettext \"error: merge rebase does not support --signoff\")\"\n+\ttest -n \"$preserve_merges\" &&\n+\t\tdie \"$(gettext \"error: cannot combine '--signoff' with '--preserve-merges'\")\"\n+\tgit_am_opt=\"$git_am_opt $signoff\"\n+\tforce_rebase=t\n+fi\n+\n if test -z \"$rebase_root\"\n then\n \tcase \"$#\" in\ndiff --git a/sequencer.c b/sequencer.c\nindex e9baaf59bd..311569e37d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -127,6 +127,7 @@ static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, \"rebase-merge/gpg_sign_opt\")\n static GIT_PATH_FUNC(rebase_path_orig_head, \"rebase-merge/orig-head\")\n static GIT_PATH_FUNC(rebase_path_verbose, \"rebase-merge/verbose\")\n+static GIT_PATH_FUNC(rebase_path_signoff, \"rebase-merge/signoff\")\n static GIT_PATH_FUNC(rebase_path_head_name, \"rebase-merge/head-name\")\n static GIT_PATH_FUNC(rebase_path_onto, \"rebase-merge/onto\")\n static GIT_PATH_FUNC(rebase_path_autostash, \"rebase-merge/autostash\")\n@@ -2035,6 +2036,11 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\tif (file_exists(rebase_path_verbose()))\n \t\t\topts->verbose = 1;\n \n+\t\tif (file_exists(rebase_path_signoff())) {\n+\t\t\topts->allow_ff = 0;\n+\t\t\topts->signoff = 1;\n+\t\t}\n+\n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_release(&buf);\n \ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nindex 2afb564701..2ff7f534e3 100755\n--- a/t/t3428-rebase-signoff.sh\n+++ b/t/t3428-rebase-signoff.sh\n@@ -12,6 +12,13 @@ cat >file <<EOF\n a\n EOF\n \n+# Expected commit message for initial commit after rebase --signoff\n+cat >expected-initial-signed <<EOF\n+Initial empty commit\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n # Expected commit message after rebase --signoff\n cat >expected-signed <<EOF\n first\n@@ -43,4 +50,29 @@ test_expect_success 'rebase --no-signoff does not add a sign-off line' '\n \ttest_cmp expected-unsigned actual\n '\n \n+test_expect_success 'rebase --exec --signoff adds a sign-off line' '\n+\ttest_when_finished \"rm exec\" &&\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase --exec \"touch exec\" --signoff HEAD^ &&\n+\ttest_path_is_file exec &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_failure 'rebase --root --signoff adds a sign-off line' '\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase --root --keep-empty --signoff &&\n+\tgit cat-file commit HEAD^ | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-initial-signed actual &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_success 'rebase -i --signoff fails' '\n+\ttest_must_fail git rebase -i --signoff HEAD^\n+'\n+\n+test_expect_success 'rebase -m --signoff fails' '\n+\ttest_must_fail git rebase -m --signoff HEAD^\n+'\n test_done\n-- \n2.16.2\n\n"},{"id":"341629","messageId":"20180314111127.14217-2-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":"20180314111127.14217-1-phillip.wood@talktalk.net","subject":"[PATCH 2/2] rebase --root -k: fix when root commit is empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-14T11:11:27Z","receivedAt":"2018-03-14T11:12:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nWhen the root commit was empty it was being pruned by the\n--cherry-pick option passed to rev-parse. This is because when --onto\nis omitted rebase creates an empty commit (which it later amends) for\nthe new root commit.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-rebase--interactive.sh | 7 ++++++-\n git-rebase.sh              | 1 +\n t/t3428-rebase-signoff.sh  | 2 +-\n 3 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 4ea54fc1c4..3ad74fc57c 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -894,7 +894,12 @@ then\n \trevisions=$upstream...$orig_head\n \tshortrevisions=$shortupstream..$shorthead\n else\n-\trevisions=$onto...$orig_head\n+\tif test -n \"$squash_onto\"\n+\tthen\n+\t\trevisions=$orig_head\n+\telse\n+\t\trevisions=$onto...$orig_head\n+\tfi\n \tshortrevisions=$shorthead\n fi\n if test t != \"$preserve_merges\"\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 40301756be..30b8eaf489 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -61,6 +61,7 @@ $(gettext 'Resolve all conflicts manually, mark them as resolved with\n You can instead skip this commit: run \"git rebase --skip\".\n To abort and get back to the state before \"git rebase\", run \"git rebase --abort\".')\n \"\n+squash_onto=\n unset onto\n unset restrict_revision\n cmd=\ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nindex 2ff7f534e3..90ca6636d5 100755\n--- a/t/t3428-rebase-signoff.sh\n+++ b/t/t3428-rebase-signoff.sh\n@@ -59,7 +59,7 @@ test_expect_success 'rebase --exec --signoff adds a sign-off line' '\n \ttest_cmp expected-signed actual\n '\n \n-test_expect_failure 'rebase --root --signoff adds a sign-off line' '\n+test_expect_success 'rebase --root --signoff adds a sign-off line' '\n \tgit commit --amend -m \"first\" &&\n \tgit rebase --root --keep-empty --signoff &&\n \tgit cat-file commit HEAD^ | sed -e \"1,/^\\$/d\" >actual &&\n-- \n2.16.2\n\n"},{"id":"341646","messageId":"xmqqh8pizgxg.fsf@gitster-ct.c.googlers.com","threadId":"48039","inReplyTo":"20180314111127.14217-1-phillip.wood@talktalk.net","subject":"Re: [PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-14T17:48:59Z","receivedAt":"2018-03-14T17:49:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood@talktalk.net> writes:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n>\n> This allows one to run 'git rebase --exec \"make check\" --signoff'\n> which is useful when preparing a patch series for publication and is\n> more convenient than doing the signoff with another --exec command.\n> This change also allows --root without --onto to work with --signoff\n> as well (--root with --onto was already supported). Note that the\n> failing test is due to a bug in 'rebase --root' when the root commit\n> is empty which will be fixed in the next commit.\n>\n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n\nHow important is the word \"implicit\" in the title?  Is it your\nintention to actively ignore --signoff when we fall into the\nrebase--interactive codepath explicitly?\n\nI offhand do not think of a strong reason why it is a bad idea to\nrun \"git rebase -i --signoff\", turn a few \"pick\" to either \"reword\"\nor \"edit\", and then expect that the editor to edit log messages for\nthese commits to add your sign-off when you start editing them.\nThe \"pick\"s that are left as-is would also turn into doing an\notherwise no-op \"commit --amend -s\", I guess.\n\nIf you are teaching --signoff to the whole of \"rebase--interactive\",\nthen \"git rebase --help\" needs a bit of update.\n\n    --signoff::\n            This flag is passed to 'git am' to sign off all the rebased\n            commits (see linkgit:git-am[1]). Incompatible with the\n            --interactive option.\n\n"},{"id":"341740","messageId":"nycvar.QRO.7.76.6.1803151110340.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48039","inReplyTo":"xmqqh8pizgxg.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-15T10:11:51Z","receivedAt":"2018-03-15T10:12:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 14 Mar 2018, Junio C Hamano wrote:\n\n> Phillip Wood <phillip.wood@talktalk.net> writes:\n> \n> > From: Phillip Wood <phillip.wood@dunelm.org.uk>\n> >\n> > This allows one to run 'git rebase --exec \"make check\" --signoff'\n> > which is useful when preparing a patch series for publication and is\n> > more convenient than doing the signoff with another --exec command.\n> > This change also allows --root without --onto to work with --signoff\n> > as well (--root with --onto was already supported). Note that the\n> > failing test is due to a bug in 'rebase --root' when the root commit\n> > is empty which will be fixed in the next commit.\n> >\n> > Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> > ---\n> \n> How important is the word \"implicit\" in the title?  Is it your\n> intention to actively ignore --signoff when we fall into the\n> rebase--interactive codepath explicitly?\n\nThe patch makes the explicit case fail ;-)\n\n> I offhand do not think of a strong reason why it is a bad idea to\n> run \"git rebase -i --signoff\", turn a few \"pick\" to either \"reword\"\n> or \"edit\", and then expect that the editor to edit log messages for\n> these commits to add your sign-off when you start editing them.\n> The \"pick\"s that are left as-is would also turn into doing an\n> otherwise no-op \"commit --amend -s\", I guess.\n> \n> If you are teaching --signoff to the whole of \"rebase--interactive\",\n> then \"git rebase --help\" needs a bit of update.\n> \n>     --signoff::\n>             This flag is passed to 'git am' to sign off all the rebased\n>             commits (see linkgit:git-am[1]). Incompatible with the\n>             --interactive option.\n\nGood point. I'd actually be in favor of having all rebase modes support\n--signoff, but I do not want to pile onto Phillip's workload.\n\nCiao,\nDscho\n"},{"id":"341741","messageId":"nycvar.QRO.7.76.6.1803151112390.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48039","inReplyTo":"20180314111127.14217-1-phillip.wood@talktalk.net","subject":"Re: [PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-15T10:18:34Z","receivedAt":"2018-03-15T10:18:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Wed, 14 Mar 2018, Phillip Wood wrote:\n\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index b353c33d41..40301756be 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -459,6 +471,18 @@ then\n>  \tgit_format_patch_opt=\"$git_format_patch_opt --progress\"\n>  fi\n>  \n> +if test -n \"$signoff\"\n> +then\n> +\ttest \"$interactive_rebase\" = explicit &&\n> +\t\tdie \"$(gettext \"error: interactive rebase does not support --signoff\")\"\n> +\ttest \"$type\" = merge &&\n> +\t\tdie \"$(gettext \"error: merge rebase does not support --signoff\")\"\n> +\ttest -n \"$preserve_merges\" &&\n> +\t\tdie \"$(gettext \"error: cannot combine '--signoff' with '--preserve-merges'\")\"\n> +\tgit_am_opt=\"$git_am_opt $signoff\"\n> +\tforce_rebase=t\n> +fi\n\nI wonder whether we can have this change as a separate commit? Otherwise\nwe would lump that change (--interactive --signoff was previously allowed\nbut the --signoff was simply ignored) with the other changes...\n\nAs I mentioned in my reply to Junio's comment, it'd be awesome if\n--interactive --signoff was supported (and likewise --merge --signoff),\nbut it feels like an undue feature request to be dumped on you, so I'm\nfine with the patch series simply erroring out on those combinations.\n\n(I don't care about --preserve-merges anymore, as everybody knows by now.)\n\nCiao,\nDscho\n"},{"id":"341742","messageId":"nycvar.QRO.7.76.6.1803151118490.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48039","inReplyTo":"20180314111127.14217-2-phillip.wood@talktalk.net","subject":"Re: [PATCH 2/2] rebase --root -k: fix when root commit is empty","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-15T10:28:10Z","receivedAt":"2018-03-15T10:28:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Wed, 14 Mar 2018, Phillip Wood wrote:\n\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n> \n> When the root commit was empty it was being pruned by the\n> --cherry-pick option passed to rev-parse. This is because when --onto\n> is omitted rebase creates an empty commit (which it later amends) for\n> the new root commit.\n\nThis will have ramifications on the upcoming patches on top of my\n--recreate-merges patches that introduce support for running -i --root\ndirectly through the sequencer. But from your patch, it looks as if things\nshould Just Work.\n\nFWIW the patch in question is currently here (will be rebased frequently):\nhttps://github.com/dscho/git/commit/147e8f3c2bf5f07708d76efd9da51cbd0eb5958c\n\n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 4ea54fc1c4..3ad74fc57c 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -894,7 +894,12 @@ then\n>  \trevisions=$upstream...$orig_head\n>  \tshortrevisions=$shortupstream..$shorthead\n>  else\n> -\trevisions=$onto...$orig_head\n> +\tif test -n \"$squash_onto\"\n> +\tthen\n> +\t\trevisions=$orig_head\n> +\telse\n> +\t\trevisions=$onto...$orig_head\n> +\tfi\n\nBefore anybody else can post the suggestion: this could be written more\ncompactly as\n\n\trevisions=${squash_onto:+$onto...}$orig_head\n\nAs it would not only be more compact, but also a lot less readable, I\nwould *also* like to point out that I prefer your version.\n\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 40301756be..30b8eaf489 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -61,6 +61,7 @@ $(gettext 'Resolve all conflicts manually, mark them as resolved with\n>  You can instead skip this commit: run \"git rebase --skip\".\n>  To abort and get back to the state before \"git rebase\", run \"git rebase --abort\".')\n>  \"\n> +squash_onto=\n>  unset onto\n>  unset restrict_revision\n>  cmd=\n\nThis is a bug fix. Maybe we can pull it out into its own commit? Commit\nmessage would be something like this:\n\n\trebase --root: stop assuming that squash_onto is unset\n\n\tWhen the user set the environment variable `squash_onto`, the\n\t`rebase` command would erroneously assume that the user passed the\n\toption `--root`.\n\n> diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\n> index 2ff7f534e3..90ca6636d5 100755\n> --- a/t/t3428-rebase-signoff.sh\n> +++ b/t/t3428-rebase-signoff.sh\n> @@ -59,7 +59,7 @@ test_expect_success 'rebase --exec --signoff adds a sign-off line' '\n>  \ttest_cmp expected-signed actual\n>  '\n>  \n> -test_expect_failure 'rebase --root --signoff adds a sign-off line' '\n> +test_expect_success 'rebase --root --signoff adds a sign-off line' '\n>  \tgit commit --amend -m \"first\" &&\n>  \tgit rebase --root --keep-empty --signoff &&\n>  \tgit cat-file commit HEAD^ | sed -e \"1,/^\\$/d\" >actual &&\n\nHehe... I guess this fixes that test case for a surprising reason: it is\nnot that all of a sudden, rebase --root respects --signoff. The reason it\nworks now is that an \"empty\" root commit is now rebased correctly.\n\nThe test case is simple enough to pass my \"can a regression be debugged\neasily, even by somebody else than the original author\" test, so I do not\nmind at all lumping the two issues (--root respects --signoff, and --root\nrespects empty root commits) into one test case, as that will make the\ntest suite run faster.\n\nPlease note that I do not feel all that strongly about my suggested\nmodifications to pull out changes from 1/2 and 2/2 into their own\nrespective commits. If you have the time, and if you feel that there is\nmerit to my suggestions, please do it. Otherwise, please feel free not to\ndo it ;-)\n\nSo: ACK from me.\n\nCiao,\nDscho\n"},{"id":"341743","messageId":"c9978ac6-ded3-592e-aa49-53172aad75bf@talktalk.net","threadId":"48039","inReplyTo":"nycvar.QRO.7.76.6.1803151112390.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-15T10:55:33Z","receivedAt":"2018-03-15T10:56:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 15/03/18 10:18, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Wed, 14 Mar 2018, Phillip Wood wrote:\n> \n>> diff --git a/git-rebase.sh b/git-rebase.sh\n>> index b353c33d41..40301756be 100755\n>> --- a/git-rebase.sh\n>> +++ b/git-rebase.sh\n>> @@ -459,6 +471,18 @@ then\n>>  \tgit_format_patch_opt=\"$git_format_patch_opt --progress\"\n>>  fi\n>>  \n>> +if test -n \"$signoff\"\n>> +then\n>> +\ttest \"$interactive_rebase\" = explicit &&\n>> +\t\tdie \"$(gettext \"error: interactive rebase does not support --signoff\")\"\n>> +\ttest \"$type\" = merge &&\n>> +\t\tdie \"$(gettext \"error: merge rebase does not support --signoff\")\"\n>> +\ttest -n \"$preserve_merges\" &&\n>> +\t\tdie \"$(gettext \"error: cannot combine '--signoff' with '--preserve-merges'\")\"\n>> +\tgit_am_opt=\"$git_am_opt $signoff\"\n>> +\tforce_rebase=t\n>> +fi\n> \n> I wonder whether we can have this change as a separate commit? Otherwise\n> we would lump that change (--interactive --signoff was previously allowed\n> but the --signoff was simply ignored) with the other changes...\n> \n> As I mentioned in my reply to Junio's comment, it'd be awesome if\n> --interactive --signoff was supported (and likewise --merge --signoff),\n> but it feels like an undue feature request to be dumped on you, so I'm\n> fine with the patch series simply erroring out on those combinations.\n\nI'll have a look, if it's not too much work I'll do that, it would be\nnice to have --signoff better supported.\n\n> (I don't care about --preserve-merges anymore, as everybody knows by now.)\n> \n> Ciao,\n> Dscho\n> \n\n"},{"id":"341744","messageId":"e17efcc9-63c2-5c88-e74b-c9205e42d6ad@talktalk.net","threadId":"48039","inReplyTo":"20180314111127.14217-2-phillip.wood@talktalk.net","subject":"Re: [PATCH 2/2] rebase --root -k: fix when root commit is empty","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-15T10:59:40Z","receivedAt":"2018-03-15T10:59:49Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 14/03/18 11:11, Phillip Wood wrote:\n> From: Phillip Wood <phillip.wood@dunelm.org.uk>\n> \n> When the root commit was empty it was being pruned by the\n> --cherry-pick option passed to rev-parse. This is because when --onto\n> is omitted rebase creates an empty commit (which it later amends) for\n> the new root commit.\n> \n> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n> ---\n>  git-rebase--interactive.sh | 7 ++++++-\n>  git-rebase.sh              | 1 +\n>  t/t3428-rebase-signoff.sh  | 2 +-\n>  3 files changed, 8 insertions(+), 2 deletions(-)\n> \n> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\n> index 4ea54fc1c4..3ad74fc57c 100644\n> --- a/git-rebase--interactive.sh\n> +++ b/git-rebase--interactive.sh\n> @@ -894,7 +894,12 @@ then\n>  \trevisions=$upstream...$orig_head\n>  \tshortrevisions=$shortupstream..$shorthead\n>  else\n> -\trevisions=$onto...$orig_head\n> +\tif test -n \"$squash_onto\"\n> +\tthen\n> +\t\trevisions=$orig_head\n> +\telse\n> +\t\trevisions=$onto...$orig_head\n> +\tfi\n>  \tshortrevisions=$shorthead\n>  fi\n>  if test t != \"$preserve_merges\"\n\nOn reflection I'm not sure this is the best way to fix the problem. This\nis a specific instance of a wider problem where rebase -k does not\npreserve empty commits if there is an empty commit upstream (see\nt3421:rebase -i --keep-empty keeps empty even if already in upstream). I\nthink the way to solve it is to get two lists of revs, the first with\n--cherry-pick and the second without and use all the commits from the\nfirst list and the empty ones from the second. I'll have a look and send\na reroll.\n\n> diff --git a/git-rebase.sh b/git-rebase.sh\n> index 40301756be..30b8eaf489 100755\n> --- a/git-rebase.sh\n> +++ b/git-rebase.sh\n> @@ -61,6 +61,7 @@ $(gettext 'Resolve all conflicts manually, mark them as resolved with\n>  You can instead skip this commit: run \"git rebase --skip\".\n>  To abort and get back to the state before \"git rebase\", run \"git rebase --abort\".')\n>  \"\n> +squash_onto=\n>  unset onto\n>  unset restrict_revision\n>  cmd=\n> diff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\n> index 2ff7f534e3..90ca6636d5 100755\n> --- a/t/t3428-rebase-signoff.sh\n> +++ b/t/t3428-rebase-signoff.sh\n> @@ -59,7 +59,7 @@ test_expect_success 'rebase --exec --signoff adds a sign-off line' '\n>  \ttest_cmp expected-signed actual\n>  '\n>  \n> -test_expect_failure 'rebase --root --signoff adds a sign-off line' '\n> +test_expect_success 'rebase --root --signoff adds a sign-off line' '\n>  \tgit commit --amend -m \"first\" &&\n>  \tgit rebase --root --keep-empty --signoff &&\n>  \tgit cat-file commit HEAD^ | sed -e \"1,/^\\$/d\" >actual &&\n> \n\n"},{"id":"341871","messageId":"nycvar.QRO.7.76.6.1803161335400.56@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48039","inReplyTo":"c9978ac6-ded3-592e-aa49-53172aad75bf@talktalk.net","subject":"Re: [PATCH 1/2] rebase: support --signoff with implicit rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-16T12:36:04Z","receivedAt":"2018-03-16T12:36:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Thu, 15 Mar 2018, Phillip Wood wrote:\n\n> On 15/03/18 10:18, Johannes Schindelin wrote:\n>\n> > As I mentioned in my reply to Junio's comment, it'd be awesome if\n> > --interactive --signoff was supported (and likewise --merge\n> > --signoff), but it feels like an undue feature request to be dumped on\n> > you, so I'm fine with the patch series simply erroring out on those\n> > combinations.\n> \n> I'll have a look, if it's not too much work I'll do that, it would be\n> nice to have --signoff better supported.\n\nThank you so much!\nDscho\n"},{"id":"342313","messageId":"20180320111057.23862-1-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":"20180314111127.14217-1-phillip.wood@talktalk.net","subject":"[PATCH v2 0/3] rebase: extend --signoff support","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T11:10:54Z","receivedAt":"2018-03-20T11:11:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nThanks for the feedback on v1, based on that I've added support for\n--interacive --signoff and --merge --signoff. I've split this series\nso the fixes from patch two in the last verion are now in a separate\nseries [1]. The tests in this version depends on those fixes. The\nthird patch is new.\n\n[1] https://public-inbox.org/git/20180320100315.15261-1-phillip.wood@talktalk.net/T/#t\n\nPhillip Wood (3):\n  rebase: extend --signoff support\n  rebase -p: error out if --signoff is given\n  rebase --keep-empty: always use interactive rebase\n\n Documentation/git-rebase.txt      |  7 ++--\n git-rebase--am.sh                 | 79 ++++++++++++++++-----------------------\n git-rebase--interactive.sh        |  6 +--\n git-rebase--merge.sh              |  2 +-\n git-rebase.sh                     | 27 ++++++++++++-\n sequencer.c                       |  8 +++-\n t/t3421-rebase-topology-linear.sh |  4 +-\n t/t3428-rebase-signoff.sh         | 38 +++++++++++++++++++\n 8 files changed, 114 insertions(+), 57 deletions(-)\n\n-- \n2.16.2\n\n"},{"id":"342314","messageId":"20180320111057.23862-3-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":"20180320111057.23862-1-phillip.wood@talktalk.net","subject":"[PATCH v2 2/3] rebase -p: error out if --signoff is given","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T11:10:56Z","receivedAt":"2018-03-20T11:11:22Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nrebase --preserve-merges does not support --signoff so error out\nrather than just silently ignoring it so that the user knows the\ncommits will not be signed off.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-rebase.sh | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 5d854657a7..7d3bb9d443 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -479,6 +479,8 @@ fi\n \n if test -n \"$signoff\"\n then\n+\ttest -n \"$preserve_merges\" &&\n+\t\tdie \"$(gettext \"error: cannot combine '--signoff' with '--preserve-merges'\")\"\n \tgit_am_opt=\"$git_am_opt $signoff\"\n \tforce_rebase=t\n fi\n-- \n2.16.2\n\n"},{"id":"342315","messageId":"20180320111057.23862-2-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":"20180320111057.23862-1-phillip.wood@talktalk.net","subject":"[PATCH v2 1/3] rebase: extend --signoff support","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T11:10:55Z","receivedAt":"2018-03-20T11:11:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nAllow --signoff to be used with --interactive and --merge. In\ninteractive mode only commits marked to be picked, edited or reworded\nwill be signed off.\n\nThe main motivation for this patch was to allow one to run 'git rebase\n--exec \"make check\" --signoff' which is useful when preparing a patch\nseries for publication and is more convenient than doing the signoff\nwith another --exec command.\n\nThis change also allows --root without --onto to work with --signoff\nas well (--root with --onto was already supported).\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n\nNotes:\n    changes since v1:\n     - added support for --signoff with --interactive and --merge\n     - split out the --preserve-merges test into the next commit\n     - updated the documentation for --signoff\n     - reworded commit message to reflect above changes\n\n Documentation/git-rebase.txt |  7 ++++---\n git-rebase--interactive.sh   |  6 +++---\n git-rebase--merge.sh         |  2 +-\n git-rebase.sh                | 20 +++++++++++++++++++-\n sequencer.c                  |  8 +++++++-\n t/t3428-rebase-signoff.sh    | 38 ++++++++++++++++++++++++++++++++++++++\n 6 files changed, 72 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 3277ca1432..dd852068b1 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -364,9 +364,10 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`.\n \tIncompatible with the --interactive option.\n \n --signoff::\n-\tThis flag is passed to 'git am' to sign off all the rebased\n-\tcommits (see linkgit:git-am[1]). Incompatible with the\n-\t--interactive option.\n+\tAdd a Signed-off-by: trailer to all the rebased commits. Note\n+\tthat if `--interactive` is given then only commits marked to be\n+\tpicked, edited or reworded will have the trailer added. Incompatible\n+\twith the `--preserve-merges` option.\n \n -i::\n --interactive::\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 331c8dfeac..104de328ee 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -285,7 +285,7 @@ pick_one () {\n \t\tpick_one_preserving_merges \"$@\" && return\n \toutput eval git cherry-pick $allow_rerere_autoupdate $allow_empty_message \\\n \t\t\t${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")} \\\n-\t\t\t\"$strategy_args\" $empty_args $ff \"$@\"\n+\t\t\t$signoff \"$strategy_args\" $empty_args $ff \"$@\"\n \n \t# If cherry-pick dies it leaves the to-be-picked commit unrecorded. Reschedule\n \t# previous task so this commit is not lost.\n@@ -527,10 +527,10 @@ do_pick () {\n \t\t# resolve before manually running git commit --amend then git\n \t\t# rebase --continue.\n \t\tgit commit --allow-empty --allow-empty-message --amend \\\n-\t\t\t   --no-post-rewrite -n -q -C $sha1 &&\n+\t\t\t   --no-post-rewrite -n -q -C $sha1 $signoff &&\n \t\t\tpick_one -n $sha1 &&\n \t\t\tgit commit --allow-empty --allow-empty-message \\\n-\t\t\t\t   --amend --no-post-rewrite -n -q -C $sha1 \\\n+\t\t\t\t   --amend --no-post-rewrite -n -q -C $sha1 $signoff \\\n \t\t\t\t   ${gpg_sign_opt:+\"$gpg_sign_opt\"} ||\n \t\t\t\t   die_with_patch $sha1 \"$(eval_gettext \"Could not apply \\$sha1... \\$rest\")\"\n \telse\ndiff --git a/git-rebase--merge.sh b/git-rebase--merge.sh\nindex ceb715453c..368b63671d 100644\n--- a/git-rebase--merge.sh\n+++ b/git-rebase--merge.sh\n@@ -28,7 +28,7 @@ continue_merge () {\n \tif ! git diff-index --quiet --ignore-submodules HEAD --\n \tthen\n \t\tif ! git commit ${gpg_sign_opt:+\"$gpg_sign_opt\"} $allow_empty_message \\\n-\t\t\t--no-verify -C \"$cmt\"\n+\t\t\t$signoff --no-verify -C \"$cmt\"\n \t\tthen\n \t\t\techo \"Commit failed, please do not call \\\"git commit\\\"\"\n \t\t\techo \"directly, but instead do one of the following: \"\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex a1f6e5de6a..5d854657a7 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -92,6 +92,7 @@ preserve_merges=\n autosquash=\n keep_empty=\n allow_empty_message=\n+signoff=\n test \"$(git config --bool rebase.autosquash)\" = \"true\" && autosquash=t\n case \"$(git config --bool commit.gpgsign)\" in\n true)\tgpg_sign_opt=-S ;;\n@@ -121,6 +122,10 @@ read_basic_state () {\n \t\tallow_rerere_autoupdate=\"$(cat \"$state_dir\"/allow_rerere_autoupdate)\"\n \ttest -f \"$state_dir\"/gpg_sign_opt &&\n \t\tgpg_sign_opt=\"$(cat \"$state_dir\"/gpg_sign_opt)\"\n+\ttest -f \"$state_dir\"/signoff && {\n+\t\tsignoff=\"$(cat \"$state_dir\"/signoff)\"\n+\t\tforce_rebase=t\n+\t}\n }\n \n write_basic_state () {\n@@ -135,6 +140,7 @@ write_basic_state () {\n \ttest -n \"$allow_rerere_autoupdate\" && echo \"$allow_rerere_autoupdate\" > \\\n \t\t\"$state_dir\"/allow_rerere_autoupdate\n \ttest -n \"$gpg_sign_opt\" && echo \"$gpg_sign_opt\" > \"$state_dir\"/gpg_sign_opt\n+\ttest -n \"$signoff\" && echo \"$signoff\" >\"$state_dir\"/signoff\n }\n \n output () {\n@@ -331,7 +337,13 @@ do\n \t--ignore-whitespace)\n \t\tgit_am_opt=\"$git_am_opt $1\"\n \t\t;;\n-\t--committer-date-is-author-date|--ignore-date|--signoff|--no-signoff)\n+\t--signoff)\n+\t\tsignoff=--signoff\n+\t\t;;\n+\t--no-signoff)\n+\t\tsignoff=\n+\t\t;;\n+\t--committer-date-is-author-date|--ignore-date)\n \t\tgit_am_opt=\"$git_am_opt $1\"\n \t\tforce_rebase=t\n \t\t;;\n@@ -465,6 +477,12 @@ then\n \tgit_format_patch_opt=\"$git_format_patch_opt --progress\"\n fi\n \n+if test -n \"$signoff\"\n+then\n+\tgit_am_opt=\"$git_am_opt $signoff\"\n+\tforce_rebase=t\n+fi\n+\n if test -z \"$rebase_root\"\n then\n \tcase \"$#\" in\ndiff --git a/sequencer.c b/sequencer.c\nindex 091bd6bda5..68683edfc1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -127,6 +127,7 @@ static GIT_PATH_FUNC(rebase_path_rewritten_pending,\n static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, \"rebase-merge/gpg_sign_opt\")\n static GIT_PATH_FUNC(rebase_path_orig_head, \"rebase-merge/orig-head\")\n static GIT_PATH_FUNC(rebase_path_verbose, \"rebase-merge/verbose\")\n+static GIT_PATH_FUNC(rebase_path_signoff, \"rebase-merge/signoff\")\n static GIT_PATH_FUNC(rebase_path_head_name, \"rebase-merge/head-name\")\n static GIT_PATH_FUNC(rebase_path_onto, \"rebase-merge/onto\")\n static GIT_PATH_FUNC(rebase_path_autostash, \"rebase-merge/autostash\")\n@@ -1605,7 +1606,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit,\n \t\t}\n \t}\n \n-\tif (opts->signoff)\n+\tif (opts->signoff && !is_fixup(command))\n \t\tappend_signoff(&msgbuf, 0, 0);\n \n \tif (is_rebase_i(opts) && write_author_script(msg.message) < 0)\n@@ -2044,6 +2045,11 @@ static int read_populate_opts(struct replay_opts *opts)\n \t\tif (file_exists(rebase_path_verbose()))\n \t\t\topts->verbose = 1;\n \n+\t\tif (file_exists(rebase_path_signoff())) {\n+\t\t\topts->allow_ff = 0;\n+\t\t\topts->signoff = 1;\n+\t\t}\n+\n \t\tread_strategy_opts(opts, &buf);\n \t\tstrbuf_release(&buf);\n \ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nindex 2afb564701..f6993b7e14 100755\n--- a/t/t3428-rebase-signoff.sh\n+++ b/t/t3428-rebase-signoff.sh\n@@ -12,6 +12,13 @@ cat >file <<EOF\n a\n EOF\n \n+# Expected commit message for initial commit after rebase --signoff\n+cat >expected-initial-signed <<EOF\n+Initial empty commit\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n # Expected commit message after rebase --signoff\n cat >expected-signed <<EOF\n first\n@@ -43,4 +50,35 @@ test_expect_success 'rebase --no-signoff does not add a sign-off line' '\n \ttest_cmp expected-unsigned actual\n '\n \n+test_expect_success 'rebase --exec --signoff adds a sign-off line' '\n+\ttest_when_finished \"rm exec\" &&\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase --exec \"touch exec\" --signoff HEAD^ &&\n+\ttest_path_is_file exec &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_success 'rebase --root --signoff adds a sign-off line' '\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase --root --keep-empty --signoff &&\n+\tgit cat-file commit HEAD^ | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-initial-signed actual &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_success 'rebase -i --signoff fails' '\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase -i --signoff HEAD^ &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_success 'rebase -m --signoff fails' '\n+\tgit commit --amend -m \"first\" &&\n+\tgit rebase -m --signoff HEAD^ &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n test_done\n-- \n2.16.2\n\n"},{"id":"342316","messageId":"20180320111057.23862-4-phillip.wood@talktalk.net","threadId":"48039","inReplyTo":"20180320111057.23862-1-phillip.wood@talktalk.net","subject":"[PATCH v2 3/3] rebase --keep-empty: always use interactive rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T11:10:57Z","receivedAt":"2018-03-20T11:11:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood <phillip.wood@dunelm.org.uk>\n\nrebase --merge accepts --keep-empty but just ignores it, by using an\nimplicit interactive rebase the user still gets the rename detection\nof a merge based rebase but with with --keep-empty support.\n\nIf rebase --keep-empty without --interactive or --merge stops for the\nuser to resolve merge conflicts then 'git rebase --continue' will\nfail. This is because it uses a different code path that does not\ncreate $git_dir/rebase-apply. As rebase --keep-empty was implemented\nusing cherry-pick it has never supported the am options and now that\ninteractive rebases support --signoff there is no loss of\nfunctionality by using an implicit interactive rebase.\n\nSigned-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>\n---\n git-rebase--am.sh                 | 79 ++++++++++++++++-----------------------\n git-rebase.sh                     |  5 +++\n t/t3421-rebase-topology-linear.sh |  4 +-\n 3 files changed, 40 insertions(+), 48 deletions(-)\n\ndiff --git a/git-rebase--am.sh b/git-rebase--am.sh\nindex be3f068922..ae596d3731 100644\n--- a/git-rebase--am.sh\n+++ b/git-rebase--am.sh\n@@ -41,60 +41,47 @@ else\n fi\n \n ret=0\n-if test -n \"$keep_empty\"\n-then\n-\t# we have to do this the hard way.  git format-patch completely squashes\n-\t# empty commits and even if it didn't the format doesn't really lend\n-\t# itself well to recording empty patches.  fortunately, cherry-pick\n-\t# makes this easy\n-\tgit cherry-pick ${gpg_sign_opt:+\"$gpg_sign_opt\"} --allow-empty \\\n-\t\t$allow_rerere_autoupdate --right-only \"$revisions\" \\\n-\t\t$allow_empty_message \\\n-\t\t${restrict_revision+^$restrict_revision}\n-\tret=$?\n-else\n-\trm -f \"$GIT_DIR/rebased-patches\"\n+rm -f \"$GIT_DIR/rebased-patches\"\n \n-\tgit format-patch -k --stdout --full-index --cherry-pick --right-only \\\n-\t\t--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \\\n-\t\t--pretty=mboxrd \\\n-\t\t$git_format_patch_opt \\\n-\t\t\"$revisions\" ${restrict_revision+^$restrict_revision} \\\n-\t\t>\"$GIT_DIR/rebased-patches\"\n-\tret=$?\n+git format-patch -k --stdout --full-index --cherry-pick --right-only \\\n+\t--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \\\n+\t--pretty=mboxrd \\\n+\t$git_format_patch_opt \\\n+\t\"$revisions\" ${restrict_revision+^$restrict_revision} \\\n+\t>\"$GIT_DIR/rebased-patches\"\n+ret=$?\n \n-\tif test 0 != $ret\n-\tthen\n-\t\trm -f \"$GIT_DIR/rebased-patches\"\n-\t\tcase \"$head_name\" in\n-\t\trefs/heads/*)\n-\t\t\tgit checkout -q \"$head_name\"\n-\t\t\t;;\n-\t\t*)\n-\t\t\tgit checkout -q \"$orig_head\"\n-\t\t\t;;\n-\t\tesac\n+if test 0 != $ret\n+then\n+\trm -f \"$GIT_DIR/rebased-patches\"\n+\tcase \"$head_name\" in\n+\trefs/heads/*)\n+\t\tgit checkout -q \"$head_name\"\n+\t\t;;\n+\t*)\n+\t\tgit checkout -q \"$orig_head\"\n+\t\t;;\n+\tesac\n \n-\t\tcat >&2 <<-EOF\n+\tcat >&2 <<-EOF\n \n-\t\tgit encountered an error while preparing the patches to replay\n-\t\tthese revisions:\n+\tgit encountered an error while preparing the patches to replay\n+\tthese revisions:\n \n-\t\t    $revisions\n+\t    $revisions\n \n-\t\tAs a result, git cannot rebase them.\n-\t\tEOF\n-\t\treturn $ret\n-\tfi\n+\tAs a result, git cannot rebase them.\n+\tEOF\n+\treturn $ret\n+fi\n \n-\tgit am $git_am_opt --rebasing --resolvemsg=\"$resolvemsg\" \\\n-\t\t--patch-format=mboxrd \\\n-\t\t$allow_rerere_autoupdate \\\n-\t\t${gpg_sign_opt:+\"$gpg_sign_opt\"} <\"$GIT_DIR/rebased-patches\"\n-\tret=$?\n+git am $git_am_opt --rebasing --resolvemsg=\"$resolvemsg\" \\\n+\t--patch-format=mboxrd \\\n+\t$allow_rerere_autoupdate \\\n+\t${gpg_sign_opt:+\"$gpg_sign_opt\"} <\"$GIT_DIR/rebased-patches\"\n+ret=$?\n \n-\trm -f \"$GIT_DIR/rebased-patches\"\n-fi\n+rm -f \"$GIT_DIR/rebased-patches\"\n \n if test 0 != $ret\n then\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 7d3bb9d443..ad1a3bd3ef 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -459,6 +459,11 @@ then\n \ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n fi\n \n+if test -n \"$keep_empty\"\n+then\n+\ttest -z \"$interactive_rebase\" && interactive_rebase=implied\n+fi\n+\n if test -n \"$interactive_rebase\"\n then\n \ttype=interactive\ndiff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh\nindex 68fe2003ef..13e5c1a2f1 100755\n--- a/t/t3421-rebase-topology-linear.sh\n+++ b/t/t3421-rebase-topology-linear.sh\n@@ -199,7 +199,7 @@ test_run_rebase () {\n \t\"\n }\n test_run_rebase success ''\n-test_run_rebase failure -m\n+test_run_rebase success -m\n test_run_rebase success -i\n test_run_rebase failure -p\n \n@@ -214,7 +214,7 @@ test_run_rebase () {\n \t\"\n }\n test_run_rebase success ''\n-test_run_rebase failure -m\n+test_run_rebase success -m\n test_run_rebase failure -i\n test_run_rebase failure -p\n \n-- \n2.16.2\n\n"},{"id":"342336","messageId":"nycvar.QRO.7.76.6.1803201707560.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48039","inReplyTo":"20180320111057.23862-4-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 3/3] rebase --keep-empty: always use interactive rebase","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-20T16:08:35Z","receivedAt":"2018-03-20T16:09:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Tue, 20 Mar 2018, Phillip Wood wrote:\n\n>  git-rebase--am.sh                 | 79 ++++++++++++++++-----------------------\n\nThose are a lot of changes, but pretty much all of it is a change in\nindentation.\n\nAll three patches look good to me.\n\nThanks,\nDscho\n"},{"id":"342354","messageId":"d1e9e836-5048-5d69-2f87-b1a4b341965f@talktalk.net","threadId":"48039","inReplyTo":"nycvar.QRO.7.76.6.1803201707560.55@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","subject":"Re: [PATCH v2 3/3] rebase --keep-empty: always use interactive rebase","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-20T18:44:59Z","receivedAt":"2018-03-20T18:45:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 20/03/18 16:08, Johannes Schindelin wrote:\n> Hi Phillip,\n> \n> On Tue, 20 Mar 2018, Phillip Wood wrote:\n> \n>>  git-rebase--am.sh                 | 79 ++++++++++++++++-----------------------\n> \n> Those are a lot of changes, but pretty much all of it is a change in\n> indentation.\n\nYes it's a big diff for what is really just deleting a few lines.\n\n> All three patches look good to me.\n\nThat's good, thanks for looking at these and the other ones\n\nBest Wishes\n\nPhillip\n\n"}]}