{"thread":{"id":"46305","subject":"[PATCH] area: git-merge: add --signoff flag to git-merge","startedAt":"2017-07-03T09:57:31Z","lastAt":"2017-07-26T07:39:52Z","messageCount":15,"participants":["Łukasz Gryglicki","Junio C Hamano","Ævar Arnfjörð Bjarmason","lukaszgryglicki","Stefan Beller"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"323747","messageId":"0102015d07e215ae-a711670e-8315-40b9-90cf-f95075525622-000000@eu-west-1.amazonses.com","threadId":"46305","inReplyTo":null,"subject":"[PATCH] area: git-merge: add --signoff flag to git-merge","fromName":"Łukasz Gryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-03T09:57:23Z","receivedAt":"2017-07-03T09:57:31Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Added --signoff flag to `git-merge` command, added test coverage,\nupdated documentation.\n\nSigned-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n---\n Documentation/git-merge.txt  |  8 ++++++\n builtin/merge.c              |  4 +++\n t/t9904-git-merge-signoff.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 72 insertions(+)\n create mode 100755 t/t9904-git-merge-signoff.sh\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 04fdd8cf086db..6b308ab6d0b52 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -64,6 +64,14 @@ OPTIONS\n -------\n include::merge-options.txt[]\n \n+--signoff::\n+\tAdd Signed-off-by line by the committer at the end of the commit\n+\tlog message.  The meaning of a signoff depends on the project,\n+\tbut it typically certifies that committer has\n+\tthe rights to submit this work under the same license and\n+\tagrees to a Developer Certificate of Origin\n+\t(see http://developercertificate.org/ for more information).\n+\n -S[<keyid>]::\n --gpg-sign[=<keyid>]::\n \tGPG-sign the resulting merge commit. The `keyid` argument is\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 900bafdb45d0b..cb09f4138136b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -70,6 +70,7 @@ static int continue_current_merge;\n static int allow_unrelated_histories;\n static int show_progress = -1;\n static int default_to_upstream = 1;\n+static int signoff;\n static const char *sign_commit;\n \n static struct strategy all_strategy[] = {\n@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n+\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n \tOPT_END()\n };\n \n@@ -775,6 +777,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstrbuf_stripspace(&msg, 0 < option_edit);\n \tif (!msg.len)\n \t\tabort_commit(remoteheads, _(\"Empty commit message.\"));\n+\tif (signoff)\n+\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n \tstrbuf_release(&merge_msg);\n \tstrbuf_addbuf(&merge_msg, &msg);\n \tstrbuf_release(&msg);\ndiff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh\nnew file mode 100755\nindex 0000000000000..eed15b9a85371\n--- /dev/null\n+++ b/t/t9904-git-merge-signoff.sh\n@@ -0,0 +1,60 @@\n+#!/bin/sh\n+\n+test_description='git merge --signoff\n+\n+This test runs git merge --signoff and make sure that it works.\n+'\n+\n+. ./test-lib.sh\n+\n+# A simple files to commit\n+cat >file1 <<EOF\n+1\n+EOF\n+\n+cat >file2 <<EOF\n+2\n+EOF\n+\n+cat >file3 <<EOF\n+3\n+EOF\n+\n+# Expected commit message after merge --signoff\n+cat >expected-signed <<EOF\n+Merge branch 'master' into other-branch\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n+# Expected commit message after merge without --signoff (or with --no-signoff)\n+cat >expected-unsigned <<EOF\n+Merge branch 'master' into other-branch\n+EOF\n+\n+\n+# We configure an alias to do the merge --signoff so that\n+# on the next subtest we can show that --no-signoff overrides the alias\n+test_expect_success 'merge --signoff adds a sign-off line' '\n+\tgit commit --allow-empty -m \"Initial empty commit\" &&\n+  git checkout -b other-branch &&\n+\tgit add file1 && git commit -m other-branch &&\n+  git checkout master &&\n+\tgit add file2 && git commit -m master-branch &&\n+  git checkout other-branch &&\n+  git config alias.msob \"merge --signoff --no-edit\" &&\n+\tgit msob master &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+test_expect_success 'master --no-signoff does not add a sign-off line' '\n+\tgit checkout master &&\n+  git add file3 && git commit -m master-branch-2 &&\n+  git checkout other-branch &&\n+\tgit msob --no-signoff master &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+test_done\n\n--\nhttps://github.com/git/git/pull/381\n"},{"id":"323756","messageId":"xmqqwp7pl7yh.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"0102015d07e215ae-a711670e-8315-40b9-90cf-f95075525622-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH] area: git-merge: add --signoff flag to git-merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-03T17:57:10Z","receivedAt":"2017-07-03T17:57:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Łukasz Gryglicki <lukaszgryglicki@o2.pl> writes:\n\n> Subject: Re: [PATCH] area: git-merge: add --signoff flag to git-merge\n\ns/area: //; \n\n> Added --signoff flag to `git-merge` command, added test coverage,\n> updated documentation.\n\nThat can be seen from the title and the patch text.  While it is not\nwrong to list what you did, that shouldn't be the only thing the log\nrecords.  Explain the problem the patch attempts to fix, why it is a\nproblem, and then give orders to the code to \"be like so\".\n\n\t[PATCH] git-merge: honor --signoff flag\n\n\tThe \"Signed-off-by:\" line is a social convention to certify\n\tthat you are legally allowed to do so when you are giving\n\tchanges to or recording changes for the project.  \n\n\tThe \"git commit\" command has a handy option \"--signoff\" to\n\tadd it under your name, because committing is the primary\n\tway for you to record your changes (which may later be sent\n\tto the project in a patch form).\n\n\tOn the other hand, the \"git merge\" command does not.  [You\n\tmake an argument to justify why merge commits also should\n\thave signoffs here].\n\n\tTeach \"git merge\" to honor \"--signoff\" just like \"git commit\"\n\tdoes.\n\nor something like that.  It is a norm for a new feature to have\ndocumentation and test, so it is of lessor importance to say \"Add\ntests and document the new feature\" (on the other hand, those who do\nnot test and document need to justify their omission).\n\nAlternatively, if the problem is so obvious that it does not need to\nbe explained, the solution often does not need more explanation than\nthe patch title.\n\nI this case, I think the \"problem\" is not that obvious; the need for\nsigning off a merge commit deserves explanation in the log message.\n\n\n> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n> ---\n\n\"Signed-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>\", like you wrote\non your \"From:\" e-mail header.\n\n>  Documentation/git-merge.txt  |  8 ++++++\n>  builtin/merge.c              |  4 +++\n>  t/t9904-git-merge-signoff.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++\n>  3 files changed, 72 insertions(+)\n>  create mode 100755 t/t9904-git-merge-signoff.sh\n>\n> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\n> index 04fdd8cf086db..6b308ab6d0b52 100644\n> --- a/Documentation/git-merge.txt\n> +++ b/Documentation/git-merge.txt\n> @@ -64,6 +64,14 @@ OPTIONS\n>  -------\n>  include::merge-options.txt[]\n>  \n> +--signoff::\n> +\tAdd Signed-off-by line by the committer at the end of the commit\n> +\tlog message.  The meaning of a signoff depends on the project,\n> +\tbut it typically certifies that committer has\n> +\tthe rights to submit this work under the same license and\n> +\tagrees to a Developer Certificate of Origin\n> +\t(see http://developercertificate.org/ for more information).\n> +\n\nGood description.\n\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 900bafdb45d0b..cb09f4138136b 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -70,6 +70,7 @@ static int continue_current_merge;\n>  static int allow_unrelated_histories;\n>  static int show_progress = -1;\n>  static int default_to_upstream = 1;\n> +static int signoff;\n>  static const char *sign_commit;\n>  \n>  static struct strategy all_strategy[] = {\n> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n>  \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n>  \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n>  \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n> +\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n>  \tOPT_END()\n>  };\n>  \n> @@ -775,6 +777,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>  \tstrbuf_stripspace(&msg, 0 < option_edit);\n>  \tif (!msg.len)\n>  \t\tabort_commit(remoteheads, _(\"Empty commit message.\"));\n> +\tif (signoff)\n> +\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n\nI think this is a wrong place to do this, because \n\n  (1) it is too late to allow \"prepare-commit-msg\" to futz with it.\n  (2) it is too late to allow the end user to further edit it.\n\nA better place probably is before merge_editor_comment is added to\nthe msg strbuf in the same function, but I didn't think it through.\n\n> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh\n> new file mode 100755\n> index 0000000000000..eed15b9a85371\n> --- /dev/null\n> +++ b/t/t9904-git-merge-signoff.sh\n\nDo we need a new script?    I think t7608 is about the messages in\nthe merge commit.\n\n> +# A simple files to commit\n> +cat >file1 <<EOF\n> +1\n> +EOF\n> +\n> +cat >file2 <<EOF\n> +2\n> +EOF\n> +\n> +cat >file3 <<EOF\n> +3\n> +EOF\n>\n> +\n> +# Expected commit message after merge --signoff\n> +cat >expected-signed <<EOF\n> +Merge branch 'master' into other-branch\n> +\n> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n> +EOF\n> +\n> +# Expected commit message after merge without --signoff (or with --no-signoff)\n> +cat >expected-unsigned <<EOF\n> +Merge branch 'master' into other-branch\n> +EOF\n> +\n\nAll of the above should be done inside the first set-up test so that\nthey can be protected from errors.\n\n> +\n> +# We configure an alias to do the merge --signoff so that\n> +# on the next subtest we can show that --no-signoff overrides the alias\n\nDo we even need to risk these tests broken by an unrelated breakage\nto the alias mechanism?  Wouldn't testing\n\n\tgit merge --signoff --no-signoff ...\n\ndirectly sufficient?  The alias test may be a good thing to have _in\naddition to_ such a basic test, though.\n\n> +test_expect_success 'merge --signoff adds a sign-off line' '\n> +\tgit commit --allow-empty -m \"Initial empty commit\" &&\n> +  git checkout -b other-branch &&\n> +\tgit add file1 && git commit -m other-branch &&\n> +  git checkout master &&\n> +\tgit add file2 && git commit -m master-branch &&\n> +  git checkout other-branch &&\n> +  git config alias.msob \"merge --signoff --no-edit\" &&\n\nStrange indentation.\n\n> +\tgit msob master &&\n> +\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n\nStyle: no space between redirection and target.\n\n> +\ttest_cmp expected-signed actual\n> +'\n> +\n> +test_expect_success 'master --no-signoff does not add a sign-off line' '\n> +\tgit checkout master &&\n> +  git add file3 && git commit -m master-branch-2 &&\n> +  git checkout other-branch &&\n> +\tgit msob --no-signoff master &&\n> +\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" > actual &&\n> +\ttest_cmp expected-unsigned actual\n> +'\n> +\n> +test_done\n\nOther than the above issues, looks like a very well done patch for\nsomebody who is posting a patch for the first time here.  Welcome to\nGit development community.\n"},{"id":"323794","messageId":"0102015d0c41db74-34ff8613-05e2-44bb-a05b-2624108c36c8-000000@eu-west-1.amazonses.com","threadId":"46305","inReplyTo":"0102015d07e215ae-a711670e-8315-40b9-90cf-f95075525622-000000@eu-west-1.amazonses.com","subject":"[PATCH v2] add --signoff flag to `git-merge` command.","fromName":"Łukasz Gryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-04T06:20:28Z","receivedAt":"2017-07-04T06:20:37Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Some projects require every commit to be signed off.\nOur workflow is to create feature branches and require every commit to\nbe signed off. When feature is finally approved we need to merge it into\nmaster. Merge itself is usually trivial and is done by\n`git merge origin/master`. Unfortunatelly this command have no --signoff\nflag, so we need to either add signoff line manually or use\n`git commit --amend -s` after the merge. First solution is not ideal\nbecause not all developers are familiar with exact sign-off syntax.\nThe second solution works, but is obviously tedious.\nThis patch adds --signoff support to git-merge command. It works just\nlike --signoff in `git-commit` command.\nMore details here:\nhttps://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n\nSigned-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n---\n Documentation/git-merge.txt  |  8 +++++\n builtin/merge.c              |  4 +++\n t/t9904-git-merge-signoff.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 87 insertions(+)\n create mode 100755 t/t9904-git-merge-signoff.sh\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 04fdd8cf086db..6b308ab6d0b52 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -64,6 +64,14 @@ OPTIONS\n -------\n include::merge-options.txt[]\n \n+--signoff::\n+\tAdd Signed-off-by line by the committer at the end of the commit\n+\tlog message.  The meaning of a signoff depends on the project,\n+\tbut it typically certifies that committer has\n+\tthe rights to submit this work under the same license and\n+\tagrees to a Developer Certificate of Origin\n+\t(see http://developercertificate.org/ for more information).\n+\n -S[<keyid>]::\n --gpg-sign[=<keyid>]::\n \tGPG-sign the resulting merge commit. The `keyid` argument is\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 900bafdb45d0b..78c36e9bf353b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -70,6 +70,7 @@ static int continue_current_merge;\n static int allow_unrelated_histories;\n static int show_progress = -1;\n static int default_to_upstream = 1;\n+static int signoff;\n static const char *sign_commit;\n \n static struct strategy all_strategy[] = {\n@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n+\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n \tOPT_END()\n };\n \n@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstrbuf_addch(&msg, '\\n');\n \tif (0 < option_edit)\n \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n+\tif (signoff)\n+\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n \t\t\t    git_path_merge_msg(), \"merge\", NULL))\ndiff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh\nnew file mode 100755\nindex 0000000000000..f542f136f5dda\n--- /dev/null\n+++ b/t/t9904-git-merge-signoff.sh\n@@ -0,0 +1,75 @@\n+#!/bin/sh\n+\n+test_description='git merge --signoff\n+\n+This test runs git merge --signoff and makes sure that it works.\n+'\n+\n+. ./test-lib.sh\n+\n+# Setup test files\n+test_setup() {\n+\t# A simples files to commit\n+\techo \"1\" >file1\n+\techo \"2\" >file2\n+\techo \"3\" >file3\n+\techo \"4\" >file4\n+\n+\t# Expected commit message after merge --signoff\n+\tcat >expected-signed <<EOF\n+Merge branch 'master' into other-branch\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n+\t# Expected commit message after merge without --signoff (or with --no-signoff)\n+\tcat >expected-unsigned <<EOF\n+Merge branch 'master' into other-branch\n+EOF\n+\n+\t# Initial commit and feature branch to merge master into it.\n+\tgit commit --allow-empty -m \"Initial empty commit\"\n+\tgit checkout -b other-branch\n+\tgit add file1\n+\tgit commit -m other-branch\n+}\n+\n+# Setup repository, files & feature branch\n+test_expect_success 'setup' '\n+\ttest_setup\n+'\n+\n+# Test with --signoff flag\n+test_expect_success 'git merge --signoff adds a sign-off line' '\n+\tgit checkout master &&\n+\tgit add file2 &&\n+\tgit commit -m master-branch &&\n+\tgit checkout other-branch &&\n+\tgit merge master --signoff --no-edit &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+# Test without --signoff flag\n+test_expect_success 'git merge does not add a sign-off line' '\n+\tgit checkout master &&\n+\tgit add file3 &&\n+\tgit commit -m master-branch-2 &&\n+\tgit checkout other-branch &&\n+\tgit merge master --no-edit &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+# Test for --no-signoff flag\n+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n+\tgit checkout master &&\n+\tgit add file4 &&\n+\tgit commit -m master-branch-3 &&\n+\tgit checkout other-branch &&\n+\tgit merge master --no-edit --signoff --no-signoff &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+test_done\n\n--\nhttps://github.com/git/git/pull/382\n"},{"id":"323798","messageId":"87tw2sbnyl.fsf@gmail.com","threadId":"46305","inReplyTo":"0102015d0c41db74-34ff8613-05e2-44bb-a05b-2624108c36c8-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v2] add --signoff flag to `git-merge` command.","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-07-04T08:33:54Z","receivedAt":"2017-07-04T08:34:03Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Jul 04 2017, Łukasz Gryglicki jotted:\n\n> add --signoff flag to `git-merge` command.\n\nWe'd usually say this as:\n\nmerge: add a --signoff flag\n\nOr something like that.\n\n> Some projects require every commit to be signed off.\n> Our workflow is to create feature branches and require every commit to\n> be signed off. When feature is finally approved we need to merge it into\n> master. Merge itself is usually trivial and is done by\n> `git merge origin/master`. Unfortunatelly this command have no --signoff\n> flag, so we need to either add signoff line manually or use\n> `git commit --amend -s` after the merge. First solution is not ideal\n> because not all developers are familiar with exact sign-off syntax.\n> The second solution works, but is obviously tedious.\n> This patch adds --signoff support to git-merge command. It works just\n> like --signoff in `git-commit` command.\n\nIt would be nice to split this into a at least a couple of paragraphs,\nand more closely follow the format suggested by\nDocumentation/SubmittingPatches.\n\n> More details here:\n> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n\nThese more details include my outstanding question in\n87fueferd4.fsf@gmail.com which hasn't been answered yet.\n\n> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n> ---\n>  Documentation/git-merge.txt  |  8 +++++\n>  builtin/merge.c              |  4 +++\n>  t/t9904-git-merge-signoff.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++\n>\n>  3 files changed, 87 insertions(+)\n>  create mode 100755 t/t9904-git-merge-signoff.sh\n>\n> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\n> index 04fdd8cf086db..6b308ab6d0b52 100644\n> --- a/Documentation/git-merge.txt\n> +++ b/Documentation/git-merge.txt\n> @@ -64,6 +64,14 @@ OPTIONS\n>  -------\n>  include::merge-options.txt[]\n>\n> +--signoff::\n> +\tAdd Signed-off-by line by the committer at the end of the commit\n> +\tlog message.  The meaning of a signoff depends on the project,\n> +\tbut it typically certifies that committer has\n> +\tthe rights to submit this work under the same license and\n> +\tagrees to a Developer Certificate of Origin\n> +\t(see http://developercertificate.org/ for more information).\n> +\n>  -S[<keyid>]::\n>  --gpg-sign[=<keyid>]::\n>  \tGPG-sign the resulting merge commit. The `keyid` argument is\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 900bafdb45d0b..78c36e9bf353b 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -70,6 +70,7 @@ static int continue_current_merge;\n>  static int allow_unrelated_histories;\n>  static int show_progress = -1;\n>  static int default_to_upstream = 1;\n> +static int signoff;\n>  static const char *sign_commit;\n>\n>  static struct strategy all_strategy[] = {\n> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n>  \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n>  \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n>  \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n> +\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n>  \tOPT_END()\n>  };\n>\n> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>  \tstrbuf_addch(&msg, '\\n');\n>  \tif (0 < option_edit)\n>  \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n> +\tif (signoff)\n> +\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n>  \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n>  \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n>  \t\t\t    git_path_merge_msg(), \"merge\", NULL))\n> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh\n> new file mode 100755\n> index 0000000000000..f542f136f5dda\n> --- /dev/null\n> +++ b/t/t9904-git-merge-signoff.sh\n\nThe convention for adding new tests is not to add a new one after\nwhatever name sorts the highest, see \"Naming Tests\" in t/README.\n\nI.e. this should be somewhere in t[6-7]* with the other merge tests.\n\n> @@ -0,0 +1,75 @@\n> +#!/bin/sh\n> +\n> +test_description='git merge --signoff\n> +\n> +This test runs git merge --signoff and makes sure that it works.\n> +'\n> +\n> +. ./test-lib.sh\n> +\n> +# Setup test files\n> +test_setup() {\n> +\t# A simples files to commit\n> +\techo \"1\" >file1\n> +\techo \"2\" >file2\n> +\techo \"3\" >file3\n> +\techo \"4\" >file4\n> +\n> +\t# Expected commit message after merge --signoff\n> +\tcat >expected-signed <<EOF\n> +Merge branch 'master' into other-branch\n> +\n> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n> +EOF\n> +\n> +\t# Expected commit message after merge without --signoff (or with --no-signoff)\n> +\tcat >expected-unsigned <<EOF\n> +Merge branch 'master' into other-branch\n> +EOF\n> +\n> +\t# Initial commit and feature branch to merge master into it.\n> +\tgit commit --allow-empty -m \"Initial empty commit\"\n> +\tgit checkout -b other-branch\n> +\tgit add file1\n> +\tgit commit -m other-branch\n\nThis setup function doesn't && its commands together, so a fail in any\none of these won't be detecten. Any reason you don't just add this to\nthe test_expect_success below, where we would detect this?\n\nAlso most of your commit/add etc. could probably be replaced with the\ntest_commit helper, see t/README.\n"},{"id":"323799","messageId":"0102015d0cf235f7-9be8e1fc-a926-4e6f-8180-c131da1c4161-000000@eu-west-1.amazonses.com","threadId":"46305","inReplyTo":"0102015d0c41db74-34ff8613-05e2-44bb-a05b-2624108c36c8-000000@eu-west-1.amazonses.com","subject":"[PATCH v3] merge: add a --signoff flag.","fromName":"Łukasz Gryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-04T09:33:06Z","receivedAt":"2017-07-04T09:33:13Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Some projects require every commit to be signed off.\nOur workflow is to create feature branches and require every commit to\nbe signed off. When feature is finally approved we need to merge it into\nmaster. Merge itself is usually trivial and is done by\n`git merge origin/master`.\n\nUnfortunatelly `merge` command have no --signoff\nflag, so we need to either add signoff line manually or use\n`git commit --amend -s` after the merge.\n\nFirst solution is not ideal because not all developers are familiar with\nexact sign-off syntax. The second solution works, but is obviously tedious.\n\nThis patch adds --signoff support to `git-merge` command.\nIt works just like --signoff in `git-commit` command.\n\nMore details can be found here:\nhttps://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n\nSigned-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n---\n Documentation/git-merge.txt |  8 ++++++\n builtin/merge.c             |  4 +++\n t/t7614-merge-signoff.sh    | 69 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 81 insertions(+)\n create mode 100755 t/t7614-merge-signoff.sh\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 04fdd8cf086db..6b308ab6d0b52 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -64,6 +64,14 @@ OPTIONS\n -------\n include::merge-options.txt[]\n \n+--signoff::\n+\tAdd Signed-off-by line by the committer at the end of the commit\n+\tlog message.  The meaning of a signoff depends on the project,\n+\tbut it typically certifies that committer has\n+\tthe rights to submit this work under the same license and\n+\tagrees to a Developer Certificate of Origin\n+\t(see http://developercertificate.org/ for more information).\n+\n -S[<keyid>]::\n --gpg-sign[=<keyid>]::\n \tGPG-sign the resulting merge commit. The `keyid` argument is\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 900bafdb45d0b..78c36e9bf353b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -70,6 +70,7 @@ static int continue_current_merge;\n static int allow_unrelated_histories;\n static int show_progress = -1;\n static int default_to_upstream = 1;\n+static int signoff;\n static const char *sign_commit;\n \n static struct strategy all_strategy[] = {\n@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n+\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n \tOPT_END()\n };\n \n@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstrbuf_addch(&msg, '\\n');\n \tif (0 < option_edit)\n \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n+\tif (signoff)\n+\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n \t\t\t    git_path_merge_msg(), \"merge\", NULL))\ndiff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\nnew file mode 100755\nindex 0000000000000..c1b8446f491dc\n--- /dev/null\n+++ b/t/t7614-merge-signoff.sh\n@@ -0,0 +1,69 @@\n+#!/bin/sh\n+\n+test_description='git merge --signoff\n+\n+This test runs git merge --signoff and makes sure that it works.\n+'\n+\n+. ./test-lib.sh\n+\n+# Setup test files\n+test_setup() {\n+\t# Expected commit message after merge --signoff\n+\tcat >expected-signed <<EOF &&\n+Merge branch 'master' into other-branch\n+\n+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n+EOF\n+\n+\t# Expected commit message after merge without --signoff (or with --no-signoff)\n+\tcat >expected-unsigned <<EOF &&\n+Merge branch 'master' into other-branch\n+EOF\n+\n+\t# Initial commit and feature branch to merge master into it.\n+\tgit commit --allow-empty -m \"Initial empty commit\" &&\n+\tgit checkout -b other-branch &&\n+\ttest_commit other-branch file1 1\n+}\n+\n+# Setup repository, files & feature branch\n+# This step must be run if You want to test 2,3 or 4\n+# Order of 2,3,4 is not important, but 1 must be run before\n+# For example `-r 1,4` or `-r 1,4,2 -v` etc\n+# But not `-r 2` or `-r 4,3,2,1`\n+test_expect_success 'setup' '\n+\ttest_setup\n+'\n+\n+# Test with --signoff flag\n+test_expect_success 'git merge --signoff adds a sign-off line' '\n+\tgit checkout master &&\n+\ttest_commit master-branch-2 file2 2 &&\n+\tgit checkout other-branch &&\n+\tgit merge master --signoff --no-edit &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-signed actual\n+'\n+\n+# Test without --signoff flag\n+test_expect_success 'git merge does not add a sign-off line' '\n+\tgit checkout master &&\n+\ttest_commit master-branch-3 file3 3 &&\n+\tgit checkout other-branch &&\n+\tgit merge master --no-edit &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+# Test for --no-signoff flag\n+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n+\tgit checkout master &&\n+\ttest_commit master-branch-4 file4 4 &&\n+\tgit checkout other-branch &&\n+\tgit merge master --no-edit --signoff --no-signoff &&\n+\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+\ttest_cmp expected-unsigned actual\n+'\n+\n+test_done\n\n--\nhttps://github.com/git/git/pull/383\n"},{"id":"323803","messageId":"xmqqzickjdyn.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"87tw2sbnyl.fsf@gmail.com","subject":"Re: [PATCH v2] add --signoff flag to `git-merge` command.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-04T17:42:40Z","receivedAt":"2017-07-04T17:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> On Tue, Jul 04 2017, Łukasz Gryglicki jotted:\n>\n>> add --signoff flag to `git-merge` command.\n>\n> We'd usually say this as:\n>\n> merge: add a --signoff flag\n>\n> Or something like that.\n\nI thought I gave a fairly complete example that can be imitated, but\napparently it didn't go through X-<.\n\n>> Some projects require every commit to be signed off.\n>> Our workflow is to create feature branches and require every commit to\n>> be signed off. When feature is finally approved we need to merge it into\n>> master. Merge itself is usually trivial and is done by\n>> `git merge origin/master`. Unfortunatelly this command have no --signoff\n>> flag, so we need to either add signoff line manually or use\n>> `git commit --amend -s` after the merge. First solution is not ideal\n>> because not all developers are familiar with exact sign-off syntax.\n>> The second solution works, but is obviously tedious.\n>> This patch adds --signoff support to git-merge command. It works just\n>> like --signoff in `git-commit` command.\n>\n> It would be nice to split this into a at least a couple of paragraphs,\n> and more closely follow the format suggested by\n> Documentation/SubmittingPatches.\n\nGood suggestion.\n\n>> More details here:\n>> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n>\n> These more details include my outstanding question in\n> 87fueferd4.fsf@gmail.com which hasn't been answered yet.\n\nI have an opinion on that topic, but I'd prefer to hear from others\nfirst before I speak out.\n\n>> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh\n>> new file mode 100755\n>> index 0000000000000..f542f136f5dda\n>> --- /dev/null\n>> +++ b/t/t9904-git-merge-signoff.sh\n>\n> The convention for adding new tests is not to add a new one after\n> whatever name sorts the highest, see \"Naming Tests\" in t/README.\n\nCorrect.\n\n> I.e. this should be somewhere in t[6-7]* with the other merge tests.\n\nYeah.  While most of t[67]??? series are about the contents of the\nmerge, i.e. resulting trees and what happens in the working tree,\nthere are some tests about the merge messages in there.  t7608 is\nexactly about how the command prepares the messages before giving\nthem to human to edit, and I think \"merge can be told to optionally\nadd sign-off\" fits there just fine.  All existing tests there are\nonly interested about the title, but that does not mean there must\nnot be tests that care more than the title in the script.\n\nAlso, as you suggest, these will become a lot shorter when the\nstandard test helper shell functions are used.  I do not think we\nnecessarily want a brand new test script to test only three or so\ncombinations (i.e. last one wins when --option and --no-option is\ngiven, --option has an effect, --no-option does not have an effect).\n"},{"id":"324940","messageId":"FE86ED74-D646-4CA8-B931-26BC12B030CB@o2.pl","threadId":"46305","inReplyTo":"0102015d0cf235f7-9be8e1fc-a926-4e6f-8180-c131da1c4161-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"lukaszgryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-24T06:14:01Z","receivedAt":"2017-07-24T06:14:14Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Hi, what is the state of this patch?\nWhat else should I do to get it merged into git?\nThanks.\n\n> On 4 Jul 2017, at 11:33, Łukasz Gryglicki <lukaszgryglicki@o2.pl> wrote:\n> \n> Some projects require every commit to be signed off.\n> Our workflow is to create feature branches and require every commit to\n> be signed off. When feature is finally approved we need to merge it into\n> master. Merge itself is usually trivial and is done by\n> `git merge origin/master`.\n> \n> Unfortunatelly `merge` command have no --signoff\n> flag, so we need to either add signoff line manually or use\n> `git commit --amend -s` after the merge.\n> \n> First solution is not ideal because not all developers are familiar with\n> exact sign-off syntax. The second solution works, but is obviously tedious.\n> \n> This patch adds --signoff support to `git-merge` command.\n> It works just like --signoff in `git-commit` command.\n> \n> More details can be found here:\n> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n> \n> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>\n> ---\n> Documentation/git-merge.txt |  8 ++++++\n> builtin/merge.c             |  4 +++\n> t/t7614-merge-signoff.sh    | 69 +++++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 81 insertions(+)\n> create mode 100755 t/t7614-merge-signoff.sh\n> \n> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\n> index 04fdd8cf086db..6b308ab6d0b52 100644\n> --- a/Documentation/git-merge.txt\n> +++ b/Documentation/git-merge.txt\n> @@ -64,6 +64,14 @@ OPTIONS\n> -------\n> include::merge-options.txt[]\n> \n> +--signoff::\n> +\tAdd Signed-off-by line by the committer at the end of the commit\n> +\tlog message.  The meaning of a signoff depends on the project,\n> +\tbut it typically certifies that committer has\n> +\tthe rights to submit this work under the same license and\n> +\tagrees to a Developer Certificate of Origin\n> +\t(see http://developercertificate.org/ for more information).\n> +\n> -S[<keyid>]::\n> --gpg-sign[=<keyid>]::\n> \tGPG-sign the resulting merge commit. The `keyid` argument is\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 900bafdb45d0b..78c36e9bf353b 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -70,6 +70,7 @@ static int continue_current_merge;\n> static int allow_unrelated_histories;\n> static int show_progress = -1;\n> static int default_to_upstream = 1;\n> +static int signoff;\n> static const char *sign_commit;\n> \n> static struct strategy all_strategy[] = {\n> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n> \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n> \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n> \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n> +\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n> \tOPT_END()\n> };\n> \n> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n> \tstrbuf_addch(&msg, '\\n');\n> \tif (0 < option_edit)\n> \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n> +\tif (signoff)\n> +\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n> \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n> \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n> \t\t\t    git_path_merge_msg(), \"merge\", NULL))\n> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\n> new file mode 100755\n> index 0000000000000..c1b8446f491dc\n> --- /dev/null\n> +++ b/t/t7614-merge-signoff.sh\n> @@ -0,0 +1,69 @@\n> +#!/bin/sh\n> +\n> +test_description='git merge --signoff\n> +\n> +This test runs git merge --signoff and makes sure that it works.\n> +'\n> +\n> +. ./test-lib.sh\n> +\n> +# Setup test files\n> +test_setup() {\n> +\t# Expected commit message after merge --signoff\n> +\tcat >expected-signed <<EOF &&\n> +Merge branch 'master' into other-branch\n> +\n> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n> +EOF\n> +\n> +\t# Expected commit message after merge without --signoff (or with --no-signoff)\n> +\tcat >expected-unsigned <<EOF &&\n> +Merge branch 'master' into other-branch\n> +EOF\n> +\n> +\t# Initial commit and feature branch to merge master into it.\n> +\tgit commit --allow-empty -m \"Initial empty commit\" &&\n> +\tgit checkout -b other-branch &&\n> +\ttest_commit other-branch file1 1\n> +}\n> +\n> +# Setup repository, files & feature branch\n> +# This step must be run if You want to test 2,3 or 4\n> +# Order of 2,3,4 is not important, but 1 must be run before\n> +# For example `-r 1,4` or `-r 1,4,2 -v` etc\n> +# But not `-r 2` or `-r 4,3,2,1`\n> +test_expect_success 'setup' '\n> +\ttest_setup\n> +'\n> +\n> +# Test with --signoff flag\n> +test_expect_success 'git merge --signoff adds a sign-off line' '\n> +\tgit checkout master &&\n> +\ttest_commit master-branch-2 file2 2 &&\n> +\tgit checkout other-branch &&\n> +\tgit merge master --signoff --no-edit &&\n> +\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\ttest_cmp expected-signed actual\n> +'\n> +\n> +# Test without --signoff flag\n> +test_expect_success 'git merge does not add a sign-off line' '\n> +\tgit checkout master &&\n> +\ttest_commit master-branch-3 file3 3 &&\n> +\tgit checkout other-branch &&\n> +\tgit merge master --no-edit &&\n> +\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\ttest_cmp expected-unsigned actual\n> +'\n> +\n> +# Test for --no-signoff flag\n> +test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n> +\tgit checkout master &&\n> +\ttest_commit master-branch-4 file4 4 &&\n> +\tgit checkout other-branch &&\n> +\tgit merge master --no-edit --signoff --no-signoff &&\n> +\tgit cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n> +\ttest_cmp expected-unsigned actual\n> +'\n> +\n> +test_done\n> \n> --\n> https://github.com/git/git/pull/383\n> \n\n"},{"id":"324941","messageId":"xmqqk22ye2rh.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"FE86ED74-D646-4CA8-B931-26BC12B030CB@o2.pl","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-24T07:02:26Z","receivedAt":"2017-07-24T07:02:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n\n> Hi, what is the state of this patch?\n\nI was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>\nto explain how your new version addresses his concerns, or him to\nrespond to your new patch to say that it addresses his concerns\nadequately.  I think neither has happened, so the topic is still in\nlimbo, I guess...\n\n"},{"id":"324996","messageId":"xmqqr2x5d0s9.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"xmqqk22ye2rh.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-24T20:42:46Z","receivedAt":"2017-07-24T20:42:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n>\n>> Hi, what is the state of this patch?\n>\n> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>\n> to explain how your new version addresses his concerns, or him to\n> respond to your new patch to say that it addresses his concerns\n> adequately.  I think neither has happened, so the topic is still in\n> limbo, I guess...\n\nSorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.\n"},{"id":"325024","messageId":"C48DBB77-7289-4F8B-B46D-32757B5903CF@o2.pl","threadId":"46305","inReplyTo":"xmqqr2x5d0s9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"lukaszgryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-25T05:00:55Z","receivedAt":"2017-07-25T05:01:02Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"I can't reach Him, every time I send an email it is returned \"no such address\".\nCan You please ask Him to take a look?\n> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n>> \n>>> Hi, what is the state of this patch?\n>> \n>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>\n>> to explain how your new version addresses his concerns, or him to\n>> respond to your new patch to say that it addresses his concerns\n>> adequately.  I think neither has happened, so the topic is still in\n>> limbo, I guess...\n> \n> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.\n\n"},{"id":"325025","messageId":"CAGZ79kbgzHZ85ODquMsVPQX2zbMBi0fOq2nB7FX56Zope8irDQ@mail.gmail.com","threadId":"46305","inReplyTo":"C48DBB77-7289-4F8B-B46D-32757B5903CF@o2.pl","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-07-25T05:10:18Z","receivedAt":"2017-07-25T05:15:30Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Jul 24, 2017 at 10:00 PM, lukaszgryglicki <lukaszgryglicki@o2.pl> wrote:\n> I can't reach Him, every time I send an email it is returned \"no such address\".\n> Can You please ask Him to take a look?\n\nJunio referred to\nhttps://public-inbox.org/git/87fueferd4.fsf@gmail.com/\nHis actual email address is in the cc list.\n\n\n\n>> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:\n>>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n>>>\n>>>> Hi, what is the state of this patch?\n>>>\n>>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>\n>>> to explain how your new version addresses his concerns, or him to\n>>> respond to your new patch to say that it addresses his concerns\n>>> adequately.  I think neither has happened, so the topic is still in\n>>> limbo, I guess...\n>>\n>> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.\n>\n"},{"id":"325035","messageId":"33E39FC8-6C5A-4323-AA0F-B65255464649@o2.pl","threadId":"46305","inReplyTo":"xmqqr2x5d0s9.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"lukaszgryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-25T11:28:36Z","receivedAt":"2017-07-25T11:28:44Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Hi, can You take a look at my newest patch version (v3)?\n> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n>> \n>>> Hi, what is the state of this patch?\n>> \n>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>\n>> to explain how your new version addresses his concerns, or him to\n>> respond to your new patch to say that it addresses his concerns\n>> adequately.  I think neither has happened, so the topic is still in\n>> limbo, I guess...\n> \n> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.\n\n"},{"id":"325054","messageId":"xmqqeft4bbqa.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"0102015d0cf235f7-9be8e1fc-a926-4e6f-8180-c131da1c4161-000000@eu-west-1.amazonses.com","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-25T18:41:33Z","receivedAt":"2017-07-25T18:41:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Łukasz Gryglicki <lukaszgryglicki@o2.pl> writes:\n\n> Some projects require every commit to be signed off.\n> Our workflow is to create feature branches and require every commit to\n> be signed off. When feature is finally approved we need to merge it into\n> master. Merge itself is usually trivial and is done by\n> `git merge origin/master`.\n>\n> Unfortunatelly `merge` command have no --signoff\n> flag, so we need to either add signoff line manually or use\n> `git commit --amend -s` after the merge.\n\nWho are \"we\" in the above?  Certainly not the Git development\nproject who stands behind the log message.  I also find the first\nparagraph overly verbose.  \n\nPerhaps something like this instead?\n\n    Some projects require every commit, even merges, to be signed\n    off [*1*].  Because \"git merge\" does not have a \"--signoff\"\n    option like \"git commit\" does, the user needs to add one\n    manually when the command presents an editor to describe the\n    merge, or later use \"git commit --amend --signoff\".\n\n    Help developers of these projects by teaching \"--signoff\" option\n    to \"git merge\".\n\n    *1* https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n\n    Requested-by: Dan Kohn <dan@linuxfoundation.org>\n    Signed-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>\n\nNotice that I updated your s-o-b line in the above illustration\nbecause we prefer to see the same name as patch author (which is\nusually taken from your e-mail From: header) there.\n\n> +--signoff::\n> +\tAdd Signed-off-by line by the committer at the end of the commit\n> +\tlog message.  The meaning of a signoff depends on the project,\n> +\tbut it typically certifies that committer has\n> +\tthe rights to submit this work under the same license and\n> +\tagrees to a Developer Certificate of Origin\n> +\t(see http://developercertificate.org/ for more information).\n\nThis is taken verbatim from Documentation/git-commit.txt and \"this\nwork\" in that context is entirely sensible, but it is not quite\nclear what it means in the context of \"git merge\".\n\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 900bafdb45d0b..78c36e9bf353b 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -70,6 +70,7 @@ static int continue_current_merge;\n>  static int allow_unrelated_histories;\n>  static int show_progress = -1;\n>  static int default_to_upstream = 1;\n> +static int signoff;\n>  static const char *sign_commit;\n>  \n>  static struct strategy all_strategy[] = {\n> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n>  \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n>  \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n>  \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n> +\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n>  \tOPT_END()\n>  };\n>  \n> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n>  \tstrbuf_addch(&msg, '\\n');\n>  \tif (0 < option_edit)\n>  \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n> +\tif (signoff)\n> +\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n>  \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n>  \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n>  \t\t\t    git_path_merge_msg(), \"merge\", NULL))\n\nThe invocation of the editor comes after this post-context, and the\nnew code seems to sit at the right place.  Good.\n\n> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\n> new file mode 100755\n> index 0000000000000..c1b8446f491dc\n> --- /dev/null\n> +++ b/t/t7614-merge-signoff.sh\n> @@ -0,0 +1,69 @@\n> +#!/bin/sh\n> +\n> +test_description='git merge --signoff\n> +\n> +This test runs git merge --signoff and makes sure that it works.\n> +'\n> +\n> +. ./test-lib.sh\n> +\n> +# Setup test files\n> +test_setup() {\n\nStyle: \"test_setup () {\" but see below.\n\n> +\t# Expected commit message after merge --signoff\n> +\tcat >expected-signed <<EOF &&\n> +Merge branch 'master' into other-branch\n> +\n> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n> +EOF\n\nIndenting the here-text with \"<<-EOF\" makes it easier to read, e.g.\n\n\tcat >expected-signed <<-EOF &&\n\tMerge branch 'master' into other-branch\n\n\tSigned-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\n\tEOF\n\nLikewise for the other one.\n\n> +...\n> +}\n\nI do not see much point in making this a shell function.  I'd just\ndo all of the above in the first \"test_expect_success 'setup'\"\nthing, if I were doing this patch.\n\n> +\n> +# Setup repository, files & feature branch\n> +# This step must be run if You want to test 2,3 or 4\n> +# Order of 2,3,4 is not important, but 1 must be run before\n> +# For example `-r 1,4` or `-r 1,4,2 -v` etc\n> +# But not `-r 2` or `-r 4,3,2,1`\n\nThat is pretty much the standard practice to require 'setup' to\nalways run; no need to waste 5 lines to document it.\n\n> +test_expect_success 'setup' '\n> +\ttest_setup\n> +'\n> +\n> +# Test with --signoff flag\n\nThat can already be seen on the test title below.  Remove it?\n\n> +test_expect_success 'git merge --signoff adds a sign-off line' '\n> +...\n> +test_expect_success 'git merge does not add a sign-off line' '\n> +...\n> +test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n> +...\n> +'\n\nThey all look sensible thing to check.  We would also need to make\nsure that the end user sees the S-o-b: prepopulated when the editor\nis spawned, I would think, perhaps like (completely untested)\n\n\twrite_script save-editor <<-\\EOF &&\n\tcp \"$1\" \"$1\".saved\n\tEOF\n\tGIT_EDITOR=./save-editor git merge --signoff --no-signoff ... &&\n \t... check the contents of the MERGE_MSG.saved file to \n\t... ensure string Signed-off-by: appears (or does not\n\t... appear) here, e.g.\n\ttest_i18ngrep ! \"^Signed-off-by: \" .git/MERGE_MSG.saved\n\nThanks.\n"},{"id":"325112","messageId":"xmqqa83rtzvm.fsf@gitster.mtv.corp.google.com","threadId":"46305","inReplyTo":"7A9ED766-4A25-4F34-8E02-3DFCE1D63ADF@o2.pl","subject":"Re: [PATCH v3] merge: add a --signoff flag.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-07-26T07:34:53Z","receivedAt":"2017-07-26T07:35:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"lukaszgryglicki <lukaszgryglicki@o2.pl> writes:\n\n>>> +--signoff::\n>>> +\tAdd Signed-off-by line by the committer at the end of the commit\n>>> +\tlog message.  The meaning of a signoff depends on the project,\n>>> +\tbut it typically certifies that committer has\n>>> +\tthe rights to submit this work under the same license and\n>>> +\tagrees to a Developer Certificate of Origin\n>>> +\t(see http://developercertificate.org/ for more information).\n>> \n>> This is taken verbatim from Documentation/git-commit.txt and \"this\n>> work\" in that context is entirely sensible, but it is not quite\n>> clear what it means in the context of \"git merge”.\n>\n> Changed slightly, but I’m not a native English speaker, and\n> don’t really see what is wrong there.\n\nOh, the language is not the issue.  \n\nI was trying to remind you that a \"merge\" may be an automated and\nmechanical one, in which there is no original work that requires\nsigning it off in the first place, which was the point Ævar raised\nwhen Dan's original post came up.\n\n"},{"id":"325113","messageId":"0102015d7dd656e3-4b41453e-ad40-4e56-98cf-67597b81c281-000000@eu-west-1.amazonses.com","threadId":"46305","inReplyTo":"0102015d0cf235f7-9be8e1fc-a926-4e6f-8180-c131da1c4161-000000@eu-west-1.amazonses.com","subject":"[PATCH v4] merge: add a --signoff flag.","fromName":"Łukasz Gryglicki","fromEmail":"lukaszgryglicki@o2.pl","sentAt":"2017-07-26T07:39:45Z","receivedAt":"2017-07-26T07:39:52Z","isPatch":true,"sender":{"key":"lukaszgryglicki@o2.pl","avatar":"https://avatars.githubusercontent.com/u/2469783?v=4"},"body":"Some projects require every commit, even merges, to be signed off [*1*].\nBecause \"git merge\" does not have a \"--signoff\"\noption like \"git commit\" does, the user needs to add one\nmanually when the command presents an editor to describe the\nmerge, or later use \"git commit --amend --signoff\".\n\nHelp developers of these projects by teaching \"--signoff\" option to \"git merge\".\n\n*1*\nhttps://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u\n\nRequested-by: Dan Kohn <dan@linuxfoundation.org>\nSigned-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>\n---\n Documentation/git-merge.txt |  8 +++++\n builtin/merge.c             |  4 +++\n t/t7614-merge-signoff.sh    | 88 +++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 100 insertions(+)\n create mode 100755 t/t7614-merge-signoff.sh\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 04fdd8cf086db..630cb4b7f90d7 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -64,6 +64,14 @@ OPTIONS\n -------\n include::merge-options.txt[]\n \n+--signoff::\n+\tAdd Signed-off-by line by the committer at the end of the commit\n+\tlog message.  The meaning of a signoff depends on the project,\n+\tbut it typically certifies that committer has\n+\tthe rights to merge work under the same license and\n+\tagrees to a Developer Certificate of Origin\n+\t(see http://developercertificate.org/ for more information).\n+\n -S[<keyid>]::\n --gpg-sign[=<keyid>]::\n \tGPG-sign the resulting merge commit. The `keyid` argument is\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 900bafdb45d0b..78c36e9bf353b 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -70,6 +70,7 @@ static int continue_current_merge;\n static int allow_unrelated_histories;\n static int show_progress = -1;\n static int default_to_upstream = 1;\n+static int signoff;\n static const char *sign_commit;\n \n static struct strategy all_strategy[] = {\n@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {\n \t{ OPTION_STRING, 'S', \"gpg-sign\", &sign_commit, N_(\"key-id\"),\n \t  N_(\"GPG sign commit\"), PARSE_OPT_OPTARG, NULL, (intptr_t) \"\" },\n \tOPT_BOOL(0, \"overwrite-ignore\", &overwrite_ignore, N_(\"update ignored files (default)\")),\n+\tOPT_BOOL(0, \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n \tOPT_END()\n };\n \n@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \tstrbuf_addch(&msg, '\\n');\n \tif (0 < option_edit)\n \t\tstrbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);\n+\tif (signoff)\n+\t\tappend_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);\n \twrite_file_buf(git_path_merge_msg(), msg.buf, msg.len);\n \tif (run_commit_hook(0 < option_edit, get_index_file(), \"prepare-commit-msg\",\n \t\t\t    git_path_merge_msg(), \"merge\", NULL))\ndiff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh\nnew file mode 100755\nindex 0000000000000..823d840d423df\n--- /dev/null\n+++ b/t/t7614-merge-signoff.sh\n@@ -0,0 +1,88 @@\n+#!/bin/sh\n+\n+test_description='git merge --signoff\n+\n+This test runs git merge --signoff and makes sure that it works.\n+'\n+\n+. ./test-lib.sh\n+\n+# Setup test files\n+test_setup () {\n+  # Expected commit message after merge --signoff\n+  printf \"Merge branch 'master' into other-branch\\n\\n\" >expected-signed &&\n+  printf \"Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e \"s/>.*/>/\")\\n\" >>expected-signed &&\n+\n+  # Expected commit message after merge without --signoff (or with --no-signoff)\n+  echo \"Merge branch 'master' into other-branch\" >expected-unsigned &&\n+\n+  # Initial commit and feature branch to merge master into it.\n+  git commit --allow-empty -m \"Initial empty commit\" &&\n+  git checkout -b other-branch &&\n+  test_commit other-branch file1 1\n+}\n+\n+# Create fake editor that just copies file\n+create_fake_editor () {\n+  echo 'cp \"$1\" \"$1.saved\"' | write_script save-editor\n+}\n+\n+test_expect_success 'setup' '\n+  test_setup && create_fake_editor\n+'\n+\n+test_expect_success 'git merge --signoff adds a sign-off line' '\n+  git checkout master &&\n+  test_commit master-branch-2 file2 2 &&\n+  git checkout other-branch &&\n+  git merge master --signoff --no-edit &&\n+  git cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+  test_cmp expected-signed actual\n+'\n+\n+test_expect_success 'git merge does not add a sign-off line' '\n+  git checkout master &&\n+  test_commit master-branch-3 file3 3 &&\n+  git checkout other-branch &&\n+  git merge master --no-edit &&\n+  git cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+  test_cmp expected-unsigned actual\n+'\n+\n+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '\n+  git checkout master &&\n+  test_commit master-branch-4 file4 4 &&\n+  git checkout other-branch &&\n+  git merge master --no-edit --signoff --no-signoff &&\n+  git cat-file commit HEAD | sed -e \"1,/^\\$/d\" >actual &&\n+  test_cmp expected-unsigned actual\n+'\n+\n+test_expect_success 'git merge --signoff adds S-o-b line in commit message editor' '\n+  git checkout master &&\n+  test_commit master-branch-5 file5 5 &&\n+  git checkout other-branch &&\n+  GIT_EDITOR=./save-editor git merge master -m \"My Message\" --edit --signoff &&\n+  test_i18ngrep \"^My Message\" .git/MERGE_MSG.saved &&\n+  test_i18ngrep \"^Signed-off-by: \" .git/MERGE_MSG.saved\n+'\n+\n+test_expect_success 'git merge --no-signoff does not add S-o-b line in commit message editor' '\n+  git checkout master &&\n+  test_commit master-branch-6 file6 6 &&\n+  git checkout other-branch &&\n+  GIT_EDITOR=./save-editor git merge master -m \"My Message\" --edit --no-signoff &&\n+  test_i18ngrep \"^My Message\" .git/MERGE_MSG.saved &&\n+  test_i18ngrep ! \"^Signed-off-by: \" .git/MERGE_MSG.saved\n+'\n+\n+test_expect_success 'git merge --no-signoff cancels --signoff flag in commit message editor' '\n+  git checkout master &&\n+  test_commit master-branch-7 file7 7 &&\n+  git checkout other-branch &&\n+  GIT_EDITOR=./save-editor git merge master -m \"My Message\" --edit --signoff --no-signoff &&\n+  test_i18ngrep \"^My Message\" .git/MERGE_MSG.saved &&\n+  test_i18ngrep ! \"^Signed-off-by: \" .git/MERGE_MSG.saved\n+'\n+\n+test_done\n\n--\nhttps://github.com/git/git/pull/390\n"}]}