{"thread":{"id":"45702","subject":"[PATCH 0/3] rebase --signoff","startedAt":"2017-04-15T14:41:19Z","lastAt":"2017-04-18T08:43:33Z","messageCount":10,"participants":["Giuseppe Bilotta","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"316893","messageId":"20170415144103.11986-1-giuseppe.bilotta@gmail.com","threadId":"45702","inReplyTo":null,"subject":"[PATCH 0/3] rebase --signoff","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-15T14:41:00Z","receivedAt":"2017-04-15T14:41:19Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Allow signing off a whole patchset by rebasing it with the --signoff\noption, which is simply passed through to git am.\n\nCompared to previous incarnations, I've split the am massaging to\nseparate commits (for cleanliness and easier reverts if needed),\nand introduced a test case for both --signoff and its negation.\n\nGiuseppe Bilotta (3):\n  builtin/am: obey --signoff also when --rebasing\n  builtin/am: fold am_signoff() into am_append_signoff()\n  rebase: pass --[no-]signoff option to git am\n\n Documentation/git-rebase.txt |  5 +++++\n builtin/am.c                 | 39 +++++++++++++++++--------------------\n git-rebase.sh                |  3 ++-\n t/t3428-rebase-signoff.sh    | 46 ++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 71 insertions(+), 22 deletions(-)\n create mode 100755 t/t3428-rebase-signoff.sh\n\n-- \n2.12.2.765.g2bf946761b\n\n"},{"id":"316894","messageId":"20170415144103.11986-2-giuseppe.bilotta@gmail.com","threadId":"45702","inReplyTo":"20170415144103.11986-1-giuseppe.bilotta@gmail.com","subject":"[PATCH 1/3] builtin/am: obey --signoff also when --rebasing","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-15T14:41:01Z","receivedAt":"2017-04-15T14:41:28Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Signoff is handled in parse_mail(), but not in parse_mail_rebasing(),\nsince the latter is only used when git-rebase calls git-am with the\n--rebasing option, and --signoff is never passed in this case.\n\nIn order to introduce (in the upcoming commits) support for `git-rebase\n--signoff`, we must make gi-am obey it also in the rebase case. This is\ntrivially fixed by moving the conditional addition of the signoff from\nparse_mail() to the caller am_run(), after either of the parse_mail*()\nfunctions were called.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n builtin/am.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex f7a7a971fb..d072027b5a 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1321,9 +1321,6 @@ static int parse_mail(struct am_state *state, const char *mail)\n \tstrbuf_addbuf(&msg, &mi.log_message);\n \tstrbuf_stripspace(&msg, 0);\n \n-\tif (state->signoff)\n-\t\tam_signoff(&msg);\n-\n \tassert(!state->author_name);\n \tstate->author_name = strbuf_detach(&author_name, NULL);\n \n@@ -1848,6 +1845,9 @@ static void am_run(struct am_state *state, int resume)\n \t\t\tif (skip)\n \t\t\t\tgoto next; /* mail should be skipped */\n \n+\t\t\tif (state->signoff)\n+\t\t\t\tam_append_signoff(state);\n+\n \t\t\twrite_author_script(state);\n \t\t\twrite_commit_msg(state);\n \t\t}\n-- \n2.12.2.765.g2bf946761b\n\n"},{"id":"316895","messageId":"20170415144103.11986-3-giuseppe.bilotta@gmail.com","threadId":"45702","inReplyTo":"20170415144103.11986-1-giuseppe.bilotta@gmail.com","subject":"[PATCH 2/3] builtin/am: fold am_signoff() into am_append_signoff()","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-15T14:41:02Z","receivedAt":"2017-04-15T14:41:29Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"There are no more direct calls to am_signoff(), so we can fold its\nlogic  in am_append_signoff().\n\n(This is done in a separate commit rather than in the previous one, to\nmake it easier to revert this specific change if additional calls are\never introduced.)\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n builtin/am.c | 33 +++++++++++++++------------------\n 1 file changed, 15 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex d072027b5a..b29f885e41 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1181,42 +1181,39 @@ static void NORETURN die_user_resolve(const struct am_state *state)\n \texit(128);\n }\n \n-static void am_signoff(struct strbuf *sb)\n+/**\n+ * Appends signoff to the \"msg\" field of the am_state.\n+ */\n+static void am_append_signoff(struct am_state *state)\n {\n \tchar *cp;\n \tstruct strbuf mine = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n-\t/* Does it end with our own sign-off? */\n+\tstrbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);\n+\n+\t/* our sign-off */\n \tstrbuf_addf(&mine, \"\\n%s%s\\n\",\n \t\t    sign_off_header,\n \t\t    fmt_name(getenv(\"GIT_COMMITTER_NAME\"),\n \t\t\t     getenv(\"GIT_COMMITTER_EMAIL\")));\n-\tif (mine.len < sb->len &&\n-\t    !strcmp(mine.buf, sb->buf + sb->len - mine.len))\n+\n+\t/* Does sb end with it already? */\n+\tif (mine.len < sb.len &&\n+\t    !strcmp(mine.buf, sb.buf + sb.len - mine.len))\n \t\tgoto exit; /* no need to duplicate */\n \n \t/* Does it have any Signed-off-by: in the text */\n-\tfor (cp = sb->buf;\n+\tfor (cp = sb.buf;\n \t     cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;\n \t     cp = strchr(cp, '\\n')) {\n-\t\tif (sb->buf == cp || cp[-1] == '\\n')\n+\t\tif (sb.buf == cp || cp[-1] == '\\n')\n \t\t\tbreak;\n \t}\n \n-\tstrbuf_addstr(sb, mine.buf + !!cp);\n+\tstrbuf_addstr(&sb, mine.buf + !!cp);\n exit:\n \tstrbuf_release(&mine);\n-}\n-\n-/**\n- * Appends signoff to the \"msg\" field of the am_state.\n- */\n-static void am_append_signoff(struct am_state *state)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\n-\tstrbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);\n-\tam_signoff(&sb);\n \tstate->msg = strbuf_detach(&sb, &state->msg_len);\n }\n \n-- \n2.12.2.765.g2bf946761b\n\n"},{"id":"316896","messageId":"20170415144103.11986-4-giuseppe.bilotta@gmail.com","threadId":"45702","inReplyTo":"20170415144103.11986-1-giuseppe.bilotta@gmail.com","subject":"[PATCH 3/3] rebase: pass --[no-]signoff option to git am","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-15T14:41:03Z","receivedAt":"2017-04-15T14:41:31Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"This makes it easy to sign off a whole patchset before submission.\n\nSigned-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>\n---\n Documentation/git-rebase.txt |  5 +++++\n git-rebase.sh                |  3 ++-\n t/t3428-rebase-signoff.sh    | 46 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 53 insertions(+), 1 deletion(-)\n create mode 100755 t/t3428-rebase-signoff.sh\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 67d48e6883..e6f0b93337 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -385,6 +385,11 @@ have the long commit hash prepended to the format.\n \tRecreate merge commits instead of flattening the history by replaying\n \tcommits a merge commit introduces. Merge conflict resolutions or manual\n \tamendments to merge commits are not preserved.\n+\n+--signoff::\n+\tThis flag is passed to 'git am' to sign off all the rebased\n+\tcommits (see linkgit:git-am[1]).\n+\n +\n This uses the `--interactive` machinery internally, but combining it\n with the `--interactive` option explicitly is generally not a good\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 48d7c5ded4..6889fd19f3 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -34,6 +34,7 @@ root!              rebase all reachable commits up to the root(s)\n autosquash         move commits that begin with squash!/fixup! under -i\n committer-date-is-author-date! passed to 'git am'\n ignore-date!       passed to 'git am'\n+signoff!           passed to 'git am'\n whitespace=!       passed to 'git apply'\n ignore-whitespace! passed to 'git apply'\n C=!                passed to 'git apply'\n@@ -321,7 +322,7 @@ 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)\n+\t--committer-date-is-author-date|--ignore-date|--signoff|--no-signoff)\n \t\tgit_am_opt=\"$git_am_opt $1\"\n \t\tforce_rebase=t\n \t\t;;\ndiff --git a/t/t3428-rebase-signoff.sh b/t/t3428-rebase-signoff.sh\nnew file mode 100755\nindex 0000000000..2afb564701\n--- /dev/null\n+++ b/t/t3428-rebase-signoff.sh\n@@ -0,0 +1,46 @@\n+#!/bin/sh\n+\n+test_description='git rebase --signoff\n+\n+This test runs git rebase --signoff and make sure that it works.\n+'\n+\n+. ./test-lib.sh\n+\n+# A simple file to commit\n+cat >file <<EOF\n+a\n+EOF\n+\n+# Expected commit message after rebase --signoff\n+cat >expected-signed <<EOF\n+first\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n+# Expected commit message after rebase without --signoff (or with --no-signoff)\n+cat >expected-unsigned <<EOF\n+first\n+EOF\n+\n+\n+# We configure an alias to do the rebase --signoff so that\n+# on the next subtest we can show that --no-signoff overrides the alias\n+test_expect_success 'rebase --signoff adds a sign-off line' '\n+\tgit commit --allow-empty -m \"Initial empty commit\" &&\n+\tgit add file && git commit -m first &&\n+\tgit config alias.rbs \"rebase --signoff\" &&\n+\tgit rbs 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 --no-signoff does not add a sign-off line' '\n+\tgit commit --amend -m \"first\" &&\n+\tgit rbs --no-signoff HEAD^ &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+test_done\n-- \n2.12.2.765.g2bf946761b\n\n"},{"id":"316897","messageId":"CAOxFTczZS6aDGzDVLskQGObtDOeLL_i3-8jJdh8OrQFXVGbOHQ@mail.gmail.com","threadId":"45702","inReplyTo":"20170415144103.11986-4-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 3/3] rebase: pass --[no-]signoff option to git am","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-15T17:45:54Z","receivedAt":"2017-04-15T17:46:20Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"Damnit! I just realized that I forgot to amend before the format-patch:\n\nOn Sat, Apr 15, 2017 at 4:41 PM, Giuseppe Bilotta\n<giuseppe.bilotta@gmail.com> wrote:\n\n> +signoff!           passed to 'git am'\n\nThis should be without the ! or --no-signoff is not accepted. Do I\nneed to resend or ... ?\n"},{"id":"316985","messageId":"xmqqa87fk6r4.fsf@gitster.mtv.corp.google.com","threadId":"45702","inReplyTo":"CAOxFTczZS6aDGzDVLskQGObtDOeLL_i3-8jJdh8OrQFXVGbOHQ@mail.gmail.com","subject":"Re: [PATCH 3/3] rebase: pass --[no-]signoff option to git am","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-17T04:17:19Z","receivedAt":"2017-04-17T04:17:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:\n\n> Damnit! I just realized that I forgot to amend before the format-patch:\n>\n> On Sat, Apr 15, 2017 at 4:41 PM, Giuseppe Bilotta\n> <giuseppe.bilotta@gmail.com> wrote:\n>\n>> +signoff!           passed to 'git am'\n>\n> This should be without the ! or --no-signoff is not accepted. Do I\n> need to resend or ... ?\n\nResend or send something that can be \"git apply\"ed, which would\nreduce the chance of mistakes.  Let the maintainer _TYPE_ as little\nas possible, or typoes will sneak in.\n\nThanks.\n"},{"id":"316992","messageId":"xmqqshl7ik21.fsf@gitster.mtv.corp.google.com","threadId":"45702","inReplyTo":"20170415144103.11986-1-giuseppe.bilotta@gmail.com","subject":"Re: [PATCH 0/3] rebase --signoff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-17T07:12:54Z","receivedAt":"2017-04-17T07:13:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:\n\n> Allow signing off a whole patchset by rebasing it with the --signoff\n> option, which is simply passed through to git am.\n\n>  Documentation/git-rebase.txt |  5 +++++\n>  builtin/am.c                 | 39 +++++++++++++++++--------------------\n>  git-rebase.sh                |  3 ++-\n>  t/t3428-rebase-signoff.sh    | 46 ++++++++++++++++++++++++++++++++++++++++++++\n\nTwo questions.\n\n - Is it better to add a brand new test script than adding new tests\n   to existing scripts that test \"git rebase\"?\n\n - How does this interact with \"git rebase -i\" and other modes of\n   operation?\n\n"},{"id":"317009","messageId":"CAOxFTczhfvzhrSiCj7SgLXbO3hrBW_QaDVZMpOqrij_hCJyCzg@mail.gmail.com","threadId":"45702","inReplyTo":"xmqqshl7ik21.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/3] rebase --signoff","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-17T14:12:19Z","receivedAt":"2017-04-17T14:12:46Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Mon, Apr 17, 2017 at 9:12 AM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Two questions.\n>\n>  - Is it better to add a brand new test script than adding new tests\n>    to existing scripts that test \"git rebase\"?\n\nSince this is a completely (in some sense) new feature, I felt it was\nappropriate to put all --signoff-related tests in their own file. So,\nif the need arises to put more tests concerning the interaction of\nsignoff with other stuff, this new test file can be extended.\n\n>  - How does this interact with \"git rebase -i\" and other modes of\n>    operation?\n\nA better question would maybe be how do we want this to interact? For\nexample, with -i: do we want -i --signoff to just sign off everything?\nOr do we want a new -i command (o, signoff) to signoff only individual\ncommits on request? When preserving merges, do we want to sign-off\nmerge-commits too? I'm not entirely sure what the best policy would\nbe.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"},{"id":"317050","messageId":"xmqqinm2inc2.fsf@gitster.mtv.corp.google.com","threadId":"45702","inReplyTo":"CAOxFTczhfvzhrSiCj7SgLXbO3hrBW_QaDVZMpOqrij_hCJyCzg@mail.gmail.com","subject":"Re: [PATCH 0/3] rebase --signoff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-18T00:14:21Z","receivedAt":"2017-04-18T00:14:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:\n\n>>  - How does this interact with \"git rebase -i\" and other modes of\n>>    operation?\n>\n> A better question would maybe be how do we want this to interact?\n\nIf \"git rebase -i/-m --signoff\" will not do anything (which I\nsuspect is what we have here), we at least would want it to be\ndocumented, or the combination be made to error out, I would think.\n\nA better question can wait until that happens ;-)\n"},{"id":"317086","messageId":"CAOxFTcyv_o8Gbjj-R0g6eK-i1QDZUA9okwq=7Yb4iW3n=kF-mQ@mail.gmail.com","threadId":"45702","inReplyTo":"xmqqinm2inc2.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/3] rebase --signoff","fromName":"Giuseppe Bilotta","fromEmail":"giuseppe.bilotta@gmail.com","sentAt":"2017-04-18T08:40:52Z","receivedAt":"2017-04-18T08:43:33Z","isPatch":true,"sender":{"key":"giuseppe.bilotta@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1464?v=4"},"body":"On Tue, Apr 18, 2017 at 2:14 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:\n>\n>>>  - How does this interact with \"git rebase -i\" and other modes of\n>>>    operation?\n>>\n>> A better question would maybe be how do we want this to interact?\n>\n> If \"git rebase -i/-m --signoff\" will not do anything (which I\n> suspect is what we have here), we at least would want it to be\n> documented, or the combination be made to error out, I would think.\n>\n> A better question can wait until that happens ;-)\n\nI've been looking into adding signoff support to the rest of\ngit-rebase, but the thing is far less trivial to do than I initially\nimagined, since the interactive part is split across a number of\nsections and files, including C helpers and the sequencer itself. It\ncan _probably_ be done, but building tests for all corner cases is\nquite the daunting task. I think that for the moment I'll resubmit\nwith the Documentation fix to declare it non-interactive only, and\nthen leave the extended for later.\n\n-- \nGiuseppe \"Oblomov\" Bilotta\n"}]}