[PATCH] area: git-merge: add --signoff flag to git-merge

[PATCH] area: git-merge: add --signoff flag to git-merge

[Łukasz Gryglicki]

Added --signoff flag to `git-merge` command, added test coverage,
updated documentation.

Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
---
 Documentation/git-merge.txt  |  8 ++++++
 builtin/merge.c              |  4 +++
 t/t9904-git-merge-signoff.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)
 create mode 100755 t/t9904-git-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..6b308ab6d0b52 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 -------
 include::merge-options.txt[]
 
+--signoff::
+	Add Signed-off-by line by the committer at the end of the commit
+	log message.  The meaning of a signoff depends on the project,
+	but it typically certifies that committer has
+	the rights to submit this work under the same license and
+	agrees to a Developer Certificate of Origin
+	(see http://developercertificate.org/ for more information).
+
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
 	GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..cb09f4138136b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
 	OPT_END()
 };
 
@@ -775,6 +777,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	strbuf_stripspace(&msg, 0 < option_edit);
 	if (!msg.len)
 		abort_commit(remoteheads, _("Empty commit message."));
+	if (signoff)
+		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	strbuf_release(&merge_msg);
 	strbuf_addbuf(&merge_msg, &msg);
 	strbuf_release(&msg);
diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
new file mode 100755
index 0000000000000..eed15b9a85371
--- /dev/null
+++ b/t/t9904-git-merge-signoff.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and make sure that it works.
+'
+
+. ./test-lib.sh
+
+# A simple files to commit
+cat >file1 <<EOF
+1
+EOF
+
+cat >file2 <<EOF
+2
+EOF
+
+cat >file3 <<EOF
+3
+EOF
+
+# Expected commit message after merge --signoff
+cat >expected-signed <<EOF
+Merge branch 'master' into other-branch
+
+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
+EOF
+
+# Expected commit message after merge without --signoff (or with --no-signoff)
+cat >expected-unsigned <<EOF
+Merge branch 'master' into other-branch
+EOF
+
+
+# We configure an alias to do the merge --signoff so that
+# on the next subtest we can show that --no-signoff overrides the alias
+test_expect_success 'merge --signoff adds a sign-off line' '
+	git commit --allow-empty -m "Initial empty commit" &&
+  git checkout -b other-branch &&
+	git add file1 && git commit -m other-branch &&
+  git checkout master &&
+	git add file2 && git commit -m master-branch &&
+  git checkout other-branch &&
+  git config alias.msob "merge --signoff --no-edit" &&
+	git msob master &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+	test_cmp expected-signed actual
+'
+
+test_expect_success 'master --no-signoff does not add a sign-off line' '
+	git checkout master &&
+  git add file3 && git commit -m master-branch-2 &&
+  git checkout other-branch &&
+	git msob --no-signoff master &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+	test_cmp expected-unsigned actual
+'
+
+test_done

--
https://github.com/git/git/pull/381

Re: [PATCH] area: git-merge: add --signoff flag to git-merge

[Junio C Hamano]

Łukasz Gryglicki <lukaszgryglicki@o2.pl> writes:

> Subject: Re: [PATCH] area: git-merge: add --signoff flag to git-merge

s/area: //; 

> Added --signoff flag to `git-merge` command, added test coverage,
> updated documentation.

That can be seen from the title and the patch text.  While it is not
wrong to list what you did, that shouldn't be the only thing the log
records.  Explain the problem the patch attempts to fix, why it is a
problem, and then give orders to the code to "be like so".

	[PATCH] git-merge: honor --signoff flag

	The "Signed-off-by:" line is a social convention to certify
	that you are legally allowed to do so when you are giving
	changes to or recording changes for the project.  

	The "git commit" command has a handy option "--signoff" to
	add it under your name, because committing is the primary
	way for you to record your changes (which may later be sent
	to the project in a patch form).

	On the other hand, the "git merge" command does not.  [You
	make an argument to justify why merge commits also should
	have signoffs here].

	Teach "git merge" to honor "--signoff" just like "git commit"
	does.

or something like that.  It is a norm for a new feature to have
documentation and test, so it is of lessor importance to say "Add
tests and document the new feature" (on the other hand, those who do
not test and document need to justify their omission).

Alternatively, if the problem is so obvious that it does not need to
be explained, the solution often does not need more explanation than
the patch title.

I this case, I think the "problem" is not that obvious; the need for
signing off a merge commit deserves explanation in the log message.


> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
> ---

"Signed-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>", like you wrote
on your "From:" e-mail header.

>  Documentation/git-merge.txt  |  8 ++++++
>  builtin/merge.c              |  4 +++
>  t/t9904-git-merge-signoff.sh | 60 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 72 insertions(+)
>  create mode 100755 t/t9904-git-merge-signoff.sh
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 04fdd8cf086db..6b308ab6d0b52 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -64,6 +64,14 @@ OPTIONS
>  -------
>  include::merge-options.txt[]
>  
> +--signoff::
> +	Add Signed-off-by line by the committer at the end of the commit
> +	log message.  The meaning of a signoff depends on the project,
> +	but it typically certifies that committer has
> +	the rights to submit this work under the same license and
> +	agrees to a Developer Certificate of Origin
> +	(see http://developercertificate.org/ for more information).
> +

Good description.

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 900bafdb45d0b..cb09f4138136b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -70,6 +70,7 @@ static int continue_current_merge;
>  static int allow_unrelated_histories;
>  static int show_progress = -1;
>  static int default_to_upstream = 1;
> +static int signoff;
>  static const char *sign_commit;
>  
>  static struct strategy all_strategy[] = {
> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
>  	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
>  	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
>  	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
> +	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
>  	OPT_END()
>  };
>  
> @@ -775,6 +777,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>  	strbuf_stripspace(&msg, 0 < option_edit);
>  	if (!msg.len)
>  		abort_commit(remoteheads, _("Empty commit message."));
> +	if (signoff)
> +		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);

I think this is a wrong place to do this, because 

  (1) it is too late to allow "prepare-commit-msg" to futz with it.
  (2) it is too late to allow the end user to further edit it.

A better place probably is before merge_editor_comment is added to
the msg strbuf in the same function, but I didn't think it through.

> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
> new file mode 100755
> index 0000000000000..eed15b9a85371
> --- /dev/null
> +++ b/t/t9904-git-merge-signoff.sh

Do we need a new script?    I think t7608 is about the messages in
the merge commit.

> +# A simple files to commit
> +cat >file1 <<EOF
> +1
> +EOF
> +
> +cat >file2 <<EOF
> +2
> +EOF
> +
> +cat >file3 <<EOF
> +3
> +EOF
>
> +
> +# Expected commit message after merge --signoff
> +cat >expected-signed <<EOF
> +Merge branch 'master' into other-branch
> +
> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +EOF
> +
> +# Expected commit message after merge without --signoff (or with --no-signoff)
> +cat >expected-unsigned <<EOF
> +Merge branch 'master' into other-branch
> +EOF
> +

All of the above should be done inside the first set-up test so that
they can be protected from errors.

> +
> +# We configure an alias to do the merge --signoff so that
> +# on the next subtest we can show that --no-signoff overrides the alias

Do we even need to risk these tests broken by an unrelated breakage
to the alias mechanism?  Wouldn't testing

	git merge --signoff --no-signoff ...

directly sufficient?  The alias test may be a good thing to have _in
addition to_ such a basic test, though.

> +test_expect_success 'merge --signoff adds a sign-off line' '
> +	git commit --allow-empty -m "Initial empty commit" &&
> +  git checkout -b other-branch &&
> +	git add file1 && git commit -m other-branch &&
> +  git checkout master &&
> +	git add file2 && git commit -m master-branch &&
> +  git checkout other-branch &&
> +  git config alias.msob "merge --signoff --no-edit" &&

Strange indentation.

> +	git msob master &&
> +	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&

Style: no space between redirection and target.

> +	test_cmp expected-signed actual
> +'
> +
> +test_expect_success 'master --no-signoff does not add a sign-off line' '
> +	git checkout master &&
> +  git add file3 && git commit -m master-branch-2 &&
> +  git checkout other-branch &&
> +	git msob --no-signoff master &&
> +	git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
> +	test_cmp expected-unsigned actual
> +'
> +
> +test_done

Other than the above issues, looks like a very well done patch for
somebody who is posting a patch for the first time here.  Welcome to
Git development community.

[PATCH v2] add --signoff flag to `git-merge` command.

[Łukasz Gryglicki]

Some projects require every commit to be signed off.
Our workflow is to create feature branches and require every commit to
be signed off. When feature is finally approved we need to merge it into
master. Merge itself is usually trivial and is done by
`git merge origin/master`. Unfortunatelly this command have no --signoff
flag, so we need to either add signoff line manually or use
`git commit --amend -s` after the merge. First solution is not ideal
because not all developers are familiar with exact sign-off syntax.
The second solution works, but is obviously tedious.
This patch adds --signoff support to git-merge command. It works just
like --signoff in `git-commit` command.
More details here:
https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u

Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
---
 Documentation/git-merge.txt  |  8 +++++
 builtin/merge.c              |  4 +++
 t/t9904-git-merge-signoff.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+)
 create mode 100755 t/t9904-git-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..6b308ab6d0b52 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 -------
 include::merge-options.txt[]
 
+--signoff::
+	Add Signed-off-by line by the committer at the end of the commit
+	log message.  The meaning of a signoff depends on the project,
+	but it typically certifies that committer has
+	the rights to submit this work under the same license and
+	agrees to a Developer Certificate of Origin
+	(see http://developercertificate.org/ for more information).
+
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
 	GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..78c36e9bf353b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
 	OPT_END()
 };
 
@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	strbuf_addch(&msg, '\n');
 	if (0 < option_edit)
 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
+	if (signoff)
+		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
 	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
 			    git_path_merge_msg(), "merge", NULL))
diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
new file mode 100755
index 0000000000000..f542f136f5dda
--- /dev/null
+++ b/t/t9904-git-merge-signoff.sh
@@ -0,0 +1,75 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and makes sure that it works.
+'
+
+. ./test-lib.sh
+
+# Setup test files
+test_setup() {
+	# A simples files to commit
+	echo "1" >file1
+	echo "2" >file2
+	echo "3" >file3
+	echo "4" >file4
+
+	# Expected commit message after merge --signoff
+	cat >expected-signed <<EOF
+Merge branch 'master' into other-branch
+
+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
+EOF
+
+	# Expected commit message after merge without --signoff (or with --no-signoff)
+	cat >expected-unsigned <<EOF
+Merge branch 'master' into other-branch
+EOF
+
+	# Initial commit and feature branch to merge master into it.
+	git commit --allow-empty -m "Initial empty commit"
+	git checkout -b other-branch
+	git add file1
+	git commit -m other-branch
+}
+
+# Setup repository, files & feature branch
+test_expect_success 'setup' '
+	test_setup
+'
+
+# Test with --signoff flag
+test_expect_success 'git merge --signoff adds a sign-off line' '
+	git checkout master &&
+	git add file2 &&
+	git commit -m master-branch &&
+	git checkout other-branch &&
+	git merge master --signoff --no-edit &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-signed actual
+'
+
+# Test without --signoff flag
+test_expect_success 'git merge does not add a sign-off line' '
+	git checkout master &&
+	git add file3 &&
+	git commit -m master-branch-2 &&
+	git checkout other-branch &&
+	git merge master --no-edit &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-unsigned actual
+'
+
+# Test for --no-signoff flag
+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
+	git checkout master &&
+	git add file4 &&
+	git commit -m master-branch-3 &&
+	git checkout other-branch &&
+	git merge master --no-edit --signoff --no-signoff &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-unsigned actual
+'
+
+test_done

--
https://github.com/git/git/pull/382

Re: [PATCH v2] add --signoff flag to `git-merge` command.

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 04 2017, Łukasz Gryglicki jotted:

> add --signoff flag to `git-merge` command.

We'd usually say this as:

merge: add a --signoff flag

Or something like that.

> Some projects require every commit to be signed off.
> Our workflow is to create feature branches and require every commit to
> be signed off. When feature is finally approved we need to merge it into
> master. Merge itself is usually trivial and is done by
> `git merge origin/master`. Unfortunatelly this command have no --signoff
> flag, so we need to either add signoff line manually or use
> `git commit --amend -s` after the merge. First solution is not ideal
> because not all developers are familiar with exact sign-off syntax.
> The second solution works, but is obviously tedious.
> This patch adds --signoff support to git-merge command. It works just
> like --signoff in `git-commit` command.

It would be nice to split this into a at least a couple of paragraphs,
and more closely follow the format suggested by
Documentation/SubmittingPatches.

> More details here:
> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u

These more details include my outstanding question in
87fueferd4.fsf@gmail.com which hasn't been answered yet.

> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
> ---
>  Documentation/git-merge.txt  |  8 +++++
>  builtin/merge.c              |  4 +++
>  t/t9904-git-merge-signoff.sh | 75 ++++++++++++++++++++++++++++++++++++++++++++
>
>  3 files changed, 87 insertions(+)
>  create mode 100755 t/t9904-git-merge-signoff.sh
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 04fdd8cf086db..6b308ab6d0b52 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -64,6 +64,14 @@ OPTIONS
>  -------
>  include::merge-options.txt[]
>
> +--signoff::
> +	Add Signed-off-by line by the committer at the end of the commit
> +	log message.  The meaning of a signoff depends on the project,
> +	but it typically certifies that committer has
> +	the rights to submit this work under the same license and
> +	agrees to a Developer Certificate of Origin
> +	(see http://developercertificate.org/ for more information).
> +
>  -S[<keyid>]::
>  --gpg-sign[=<keyid>]::
>  	GPG-sign the resulting merge commit. The `keyid` argument is
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 900bafdb45d0b..78c36e9bf353b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -70,6 +70,7 @@ static int continue_current_merge;
>  static int allow_unrelated_histories;
>  static int show_progress = -1;
>  static int default_to_upstream = 1;
> +static int signoff;
>  static const char *sign_commit;
>
>  static struct strategy all_strategy[] = {
> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
>  	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
>  	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
>  	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
> +	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
>  	OPT_END()
>  };
>
> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>  	strbuf_addch(&msg, '\n');
>  	if (0 < option_edit)
>  		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
> +	if (signoff)
> +		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
>  	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
>  	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
>  			    git_path_merge_msg(), "merge", NULL))
> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
> new file mode 100755
> index 0000000000000..f542f136f5dda
> --- /dev/null
> +++ b/t/t9904-git-merge-signoff.sh

The convention for adding new tests is not to add a new one after
whatever name sorts the highest, see "Naming Tests" in t/README.

I.e. this should be somewhere in t[6-7]* with the other merge tests.

> @@ -0,0 +1,75 @@
> +#!/bin/sh
> +
> +test_description='git merge --signoff
> +
> +This test runs git merge --signoff and makes sure that it works.
> +'
> +
> +. ./test-lib.sh
> +
> +# Setup test files
> +test_setup() {
> +	# A simples files to commit
> +	echo "1" >file1
> +	echo "2" >file2
> +	echo "3" >file3
> +	echo "4" >file4
> +
> +	# Expected commit message after merge --signoff
> +	cat >expected-signed <<EOF
> +Merge branch 'master' into other-branch
> +
> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +EOF
> +
> +	# Expected commit message after merge without --signoff (or with --no-signoff)
> +	cat >expected-unsigned <<EOF
> +Merge branch 'master' into other-branch
> +EOF
> +
> +	# Initial commit and feature branch to merge master into it.
> +	git commit --allow-empty -m "Initial empty commit"
> +	git checkout -b other-branch
> +	git add file1
> +	git commit -m other-branch

This setup function doesn't && its commands together, so a fail in any
one of these won't be detecten. Any reason you don't just add this to
the test_expect_success below, where we would detect this?

Also most of your commit/add etc. could probably be replaced with the
test_commit helper, see t/README.

[PATCH v3] merge: add a --signoff flag.

[Łukasz Gryglicki]

Some projects require every commit to be signed off.
Our workflow is to create feature branches and require every commit to
be signed off. When feature is finally approved we need to merge it into
master. Merge itself is usually trivial and is done by
`git merge origin/master`.

Unfortunatelly `merge` command have no --signoff
flag, so we need to either add signoff line manually or use
`git commit --amend -s` after the merge.

First solution is not ideal because not all developers are familiar with
exact sign-off syntax. The second solution works, but is obviously tedious.

This patch adds --signoff support to `git-merge` command.
It works just like --signoff in `git-commit` command.

More details can be found here:
https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u

Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
---
 Documentation/git-merge.txt |  8 ++++++
 builtin/merge.c             |  4 +++
 t/t7614-merge-signoff.sh    | 69 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+)
 create mode 100755 t/t7614-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..6b308ab6d0b52 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 -------
 include::merge-options.txt[]
 
+--signoff::
+	Add Signed-off-by line by the committer at the end of the commit
+	log message.  The meaning of a signoff depends on the project,
+	but it typically certifies that committer has
+	the rights to submit this work under the same license and
+	agrees to a Developer Certificate of Origin
+	(see http://developercertificate.org/ for more information).
+
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
 	GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..78c36e9bf353b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
 	OPT_END()
 };
 
@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	strbuf_addch(&msg, '\n');
 	if (0 < option_edit)
 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
+	if (signoff)
+		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
 	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
 			    git_path_merge_msg(), "merge", NULL))
diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
new file mode 100755
index 0000000000000..c1b8446f491dc
--- /dev/null
+++ b/t/t7614-merge-signoff.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and makes sure that it works.
+'
+
+. ./test-lib.sh
+
+# Setup test files
+test_setup() {
+	# Expected commit message after merge --signoff
+	cat >expected-signed <<EOF &&
+Merge branch 'master' into other-branch
+
+Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
+EOF
+
+	# Expected commit message after merge without --signoff (or with --no-signoff)
+	cat >expected-unsigned <<EOF &&
+Merge branch 'master' into other-branch
+EOF
+
+	# Initial commit and feature branch to merge master into it.
+	git commit --allow-empty -m "Initial empty commit" &&
+	git checkout -b other-branch &&
+	test_commit other-branch file1 1
+}
+
+# Setup repository, files & feature branch
+# This step must be run if You want to test 2,3 or 4
+# Order of 2,3,4 is not important, but 1 must be run before
+# For example `-r 1,4` or `-r 1,4,2 -v` etc
+# But not `-r 2` or `-r 4,3,2,1`
+test_expect_success 'setup' '
+	test_setup
+'
+
+# Test with --signoff flag
+test_expect_success 'git merge --signoff adds a sign-off line' '
+	git checkout master &&
+	test_commit master-branch-2 file2 2 &&
+	git checkout other-branch &&
+	git merge master --signoff --no-edit &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-signed actual
+'
+
+# Test without --signoff flag
+test_expect_success 'git merge does not add a sign-off line' '
+	git checkout master &&
+	test_commit master-branch-3 file3 3 &&
+	git checkout other-branch &&
+	git merge master --no-edit &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-unsigned actual
+'
+
+# Test for --no-signoff flag
+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
+	git checkout master &&
+	test_commit master-branch-4 file4 4 &&
+	git checkout other-branch &&
+	git merge master --no-edit --signoff --no-signoff &&
+	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+	test_cmp expected-unsigned actual
+'
+
+test_done

--
https://github.com/git/git/pull/383

Re: [PATCH v2] add --signoff flag to `git-merge` command.

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Tue, Jul 04 2017, Łukasz Gryglicki jotted:
>
>> add --signoff flag to `git-merge` command.
>
> We'd usually say this as:
>
> merge: add a --signoff flag
>
> Or something like that.

I thought I gave a fairly complete example that can be imitated, but
apparently it didn't go through X-<.

>> Some projects require every commit to be signed off.
>> Our workflow is to create feature branches and require every commit to
>> be signed off. When feature is finally approved we need to merge it into
>> master. Merge itself is usually trivial and is done by
>> `git merge origin/master`. Unfortunatelly this command have no --signoff
>> flag, so we need to either add signoff line manually or use
>> `git commit --amend -s` after the merge. First solution is not ideal
>> because not all developers are familiar with exact sign-off syntax.
>> The second solution works, but is obviously tedious.
>> This patch adds --signoff support to git-merge command. It works just
>> like --signoff in `git-commit` command.
>
> It would be nice to split this into a at least a couple of paragraphs,
> and more closely follow the format suggested by
> Documentation/SubmittingPatches.

Good suggestion.

>> More details here:
>> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u
>
> These more details include my outstanding question in
> 87fueferd4.fsf@gmail.com which hasn't been answered yet.

I have an opinion on that topic, but I'd prefer to hear from others
first before I speak out.

>> diff --git a/t/t9904-git-merge-signoff.sh b/t/t9904-git-merge-signoff.sh
>> new file mode 100755
>> index 0000000000000..f542f136f5dda
>> --- /dev/null
>> +++ b/t/t9904-git-merge-signoff.sh
>
> The convention for adding new tests is not to add a new one after
> whatever name sorts the highest, see "Naming Tests" in t/README.

Correct.

> I.e. this should be somewhere in t[6-7]* with the other merge tests.

Yeah.  While most of t[67]??? series are about the contents of the
merge, i.e. resulting trees and what happens in the working tree,
there are some tests about the merge messages in there.  t7608 is
exactly about how the command prepares the messages before giving
them to human to edit, and I think "merge can be told to optionally
add sign-off" fits there just fine.  All existing tests there are
only interested about the title, but that does not mean there must
not be tests that care more than the title in the script.

Also, as you suggest, these will become a lot shorter when the
standard test helper shell functions are used.  I do not think we
necessarily want a brand new test script to test only three or so
combinations (i.e. last one wins when --option and --no-option is
given, --option has an effect, --no-option does not have an effect).

Re: [PATCH v3] merge: add a --signoff flag.

[lukaszgryglicki]

Hi, what is the state of this patch?
What else should I do to get it merged into git?
Thanks.

> On 4 Jul 2017, at 11:33, Łukasz Gryglicki <lukaszgryglicki@o2.pl> wrote:
> 
> Some projects require every commit to be signed off.
> Our workflow is to create feature branches and require every commit to
> be signed off. When feature is finally approved we need to merge it into
> master. Merge itself is usually trivial and is done by
> `git merge origin/master`.
> 
> Unfortunatelly `merge` command have no --signoff
> flag, so we need to either add signoff line manually or use
> `git commit --amend -s` after the merge.
> 
> First solution is not ideal because not all developers are familiar with
> exact sign-off syntax. The second solution works, but is obviously tedious.
> 
> This patch adds --signoff support to `git-merge` command.
> It works just like --signoff in `git-commit` command.
> 
> More details can be found here:
> https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u
> 
> Signed-off-by: lukaszgryglicki <lukaszgryglicki@o2.pl>
> ---
> Documentation/git-merge.txt |  8 ++++++
> builtin/merge.c             |  4 +++
> t/t7614-merge-signoff.sh    | 69 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 81 insertions(+)
> create mode 100755 t/t7614-merge-signoff.sh
> 
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 04fdd8cf086db..6b308ab6d0b52 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -64,6 +64,14 @@ OPTIONS
> -------
> include::merge-options.txt[]
> 
> +--signoff::
> +	Add Signed-off-by line by the committer at the end of the commit
> +	log message.  The meaning of a signoff depends on the project,
> +	but it typically certifies that committer has
> +	the rights to submit this work under the same license and
> +	agrees to a Developer Certificate of Origin
> +	(see http://developercertificate.org/ for more information).
> +
> -S[<keyid>]::
> --gpg-sign[=<keyid>]::
> 	GPG-sign the resulting merge commit. The `keyid` argument is
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 900bafdb45d0b..78c36e9bf353b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -70,6 +70,7 @@ static int continue_current_merge;
> static int allow_unrelated_histories;
> static int show_progress = -1;
> static int default_to_upstream = 1;
> +static int signoff;
> static const char *sign_commit;
> 
> static struct strategy all_strategy[] = {
> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
> 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
> 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
> +	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
> 	OPT_END()
> };
> 
> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
> 	strbuf_addch(&msg, '\n');
> 	if (0 < option_edit)
> 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
> +	if (signoff)
> +		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
> 	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
> 	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
> 			    git_path_merge_msg(), "merge", NULL))
> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
> new file mode 100755
> index 0000000000000..c1b8446f491dc
> --- /dev/null
> +++ b/t/t7614-merge-signoff.sh
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +
> +test_description='git merge --signoff
> +
> +This test runs git merge --signoff and makes sure that it works.
> +'
> +
> +. ./test-lib.sh
> +
> +# Setup test files
> +test_setup() {
> +	# Expected commit message after merge --signoff
> +	cat >expected-signed <<EOF &&
> +Merge branch 'master' into other-branch
> +
> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +EOF
> +
> +	# Expected commit message after merge without --signoff (or with --no-signoff)
> +	cat >expected-unsigned <<EOF &&
> +Merge branch 'master' into other-branch
> +EOF
> +
> +	# Initial commit and feature branch to merge master into it.
> +	git commit --allow-empty -m "Initial empty commit" &&
> +	git checkout -b other-branch &&
> +	test_commit other-branch file1 1
> +}
> +
> +# Setup repository, files & feature branch
> +# This step must be run if You want to test 2,3 or 4
> +# Order of 2,3,4 is not important, but 1 must be run before
> +# For example `-r 1,4` or `-r 1,4,2 -v` etc
> +# But not `-r 2` or `-r 4,3,2,1`
> +test_expect_success 'setup' '
> +	test_setup
> +'
> +
> +# Test with --signoff flag
> +test_expect_success 'git merge --signoff adds a sign-off line' '
> +	git checkout master &&
> +	test_commit master-branch-2 file2 2 &&
> +	git checkout other-branch &&
> +	git merge master --signoff --no-edit &&
> +	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +	test_cmp expected-signed actual
> +'
> +
> +# Test without --signoff flag
> +test_expect_success 'git merge does not add a sign-off line' '
> +	git checkout master &&
> +	test_commit master-branch-3 file3 3 &&
> +	git checkout other-branch &&
> +	git merge master --no-edit &&
> +	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +	test_cmp expected-unsigned actual
> +'
> +
> +# Test for --no-signoff flag
> +test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
> +	git checkout master &&
> +	test_commit master-branch-4 file4 4 &&
> +	git checkout other-branch &&
> +	git merge master --no-edit --signoff --no-signoff &&
> +	git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
> +	test_cmp expected-unsigned actual
> +'
> +
> +test_done
> 
> --
> https://github.com/git/git/pull/383
> 

Re: [PATCH v3] merge: add a --signoff flag.

[Junio C Hamano]

lukaszgryglicki <lukaszgryglicki@o2.pl> writes:

> Hi, what is the state of this patch?

I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>
to explain how your new version addresses his concerns, or him to
respond to your new patch to say that it addresses his concerns
adequately.  I think neither has happened, so the topic is still in
limbo, I guess...

Re: [PATCH v3] merge: add a --signoff flag.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:
>
>> Hi, what is the state of this patch?
>
> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>
> to explain how your new version addresses his concerns, or him to
> respond to your new patch to say that it addresses his concerns
> adequately.  I think neither has happened, so the topic is still in
> limbo, I guess...

Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.

Re: [PATCH v3] merge: add a --signoff flag.

[lukaszgryglicki]

I can't reach Him, every time I send an email it is returned "no such address".
Can You please ask Him to take a look?
> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:
>> 
>>> Hi, what is the state of this patch?
>> 
>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>
>> to explain how your new version addresses his concerns, or him to
>> respond to your new patch to say that it addresses his concerns
>> adequately.  I think neither has happened, so the topic is still in
>> limbo, I guess...
> 
> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.

Re: [PATCH v3] merge: add a --signoff flag.

[Stefan Beller]

On Mon, Jul 24, 2017 at 10:00 PM, lukaszgryglicki <lukaszgryglicki@o2.pl> wrote:
> I can't reach Him, every time I send an email it is returned "no such address".
> Can You please ask Him to take a look?

Junio referred to
https://public-inbox.org/git/87fueferd4.fsf@gmail.com/
His actual email address is in the cc list.



>> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:
>>>
>>>> Hi, what is the state of this patch?
>>>
>>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>
>>> to explain how your new version addresses his concerns, or him to
>>> respond to your new patch to say that it addresses his concerns
>>> adequately.  I think neither has happened, so the topic is still in
>>> limbo, I guess...
>>
>> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.
>

Re: [PATCH v3] merge: add a --signoff flag.

[lukaszgryglicki]

Hi, can You take a look at my newest patch version (v3)?
> On 24 Jul 2017, at 22:42, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> lukaszgryglicki <lukaszgryglicki@o2.pl> writes:
>> 
>>> Hi, what is the state of this patch?
>> 
>> I was expecting you to respond to Ævar's <87tw2sbnyl.fsf@gmail.com>
>> to explain how your new version addresses his concerns, or him to
>> respond to your new patch to say that it addresses his concerns
>> adequately.  I think neither has happened, so the topic is still in
>> limbo, I guess...
> 
> Sorry, Ævar's message I meant was <87fueferd4.fsf@gmail.com>.

Re: [PATCH v3] merge: add a --signoff flag.

[Junio C Hamano]

Łukasz Gryglicki <lukaszgryglicki@o2.pl> writes:

> Some projects require every commit to be signed off.
> Our workflow is to create feature branches and require every commit to
> be signed off. When feature is finally approved we need to merge it into
> master. Merge itself is usually trivial and is done by
> `git merge origin/master`.
>
> Unfortunatelly `merge` command have no --signoff
> flag, so we need to either add signoff line manually or use
> `git commit --amend -s` after the merge.

Who are "we" in the above?  Certainly not the Git development
project who stands behind the log message.  I also find the first
paragraph overly verbose.  

Perhaps something like this instead?

    Some projects require every commit, even merges, to be signed
    off [*1*].  Because "git merge" does not have a "--signoff"
    option like "git commit" does, the user needs to add one
    manually when the command presents an editor to describe the
    merge, or later use "git commit --amend --signoff".

    Help developers of these projects by teaching "--signoff" option
    to "git merge".

    *1* https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u

    Requested-by: Dan Kohn <dan@linuxfoundation.org>
    Signed-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>

Notice that I updated your s-o-b line in the above illustration
because we prefer to see the same name as patch author (which is
usually taken from your e-mail From: header) there.

> +--signoff::
> +	Add Signed-off-by line by the committer at the end of the commit
> +	log message.  The meaning of a signoff depends on the project,
> +	but it typically certifies that committer has
> +	the rights to submit this work under the same license and
> +	agrees to a Developer Certificate of Origin
> +	(see http://developercertificate.org/ for more information).

This is taken verbatim from Documentation/git-commit.txt and "this
work" in that context is entirely sensible, but it is not quite
clear what it means in the context of "git merge".

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 900bafdb45d0b..78c36e9bf353b 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -70,6 +70,7 @@ static int continue_current_merge;
>  static int allow_unrelated_histories;
>  static int show_progress = -1;
>  static int default_to_upstream = 1;
> +static int signoff;
>  static const char *sign_commit;
>  
>  static struct strategy all_strategy[] = {
> @@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
>  	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
>  	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
>  	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
> +	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
>  	OPT_END()
>  };
>  
> @@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
>  	strbuf_addch(&msg, '\n');
>  	if (0 < option_edit)
>  		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
> +	if (signoff)
> +		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
>  	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
>  	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
>  			    git_path_merge_msg(), "merge", NULL))

The invocation of the editor comes after this post-context, and the
new code seems to sit at the right place.  Good.

> diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
> new file mode 100755
> index 0000000000000..c1b8446f491dc
> --- /dev/null
> +++ b/t/t7614-merge-signoff.sh
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +
> +test_description='git merge --signoff
> +
> +This test runs git merge --signoff and makes sure that it works.
> +'
> +
> +. ./test-lib.sh
> +
> +# Setup test files
> +test_setup() {

Style: "test_setup () {" but see below.

> +	# Expected commit message after merge --signoff
> +	cat >expected-signed <<EOF &&
> +Merge branch 'master' into other-branch
> +
> +Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +EOF

Indenting the here-text with "<<-EOF" makes it easier to read, e.g.

	cat >expected-signed <<-EOF &&
	Merge branch 'master' into other-branch

	Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
	EOF

Likewise for the other one.

> +...
> +}

I do not see much point in making this a shell function.  I'd just
do all of the above in the first "test_expect_success 'setup'"
thing, if I were doing this patch.

> +
> +# Setup repository, files & feature branch
> +# This step must be run if You want to test 2,3 or 4
> +# Order of 2,3,4 is not important, but 1 must be run before
> +# For example `-r 1,4` or `-r 1,4,2 -v` etc
> +# But not `-r 2` or `-r 4,3,2,1`

That is pretty much the standard practice to require 'setup' to
always run; no need to waste 5 lines to document it.

> +test_expect_success 'setup' '
> +	test_setup
> +'
> +
> +# Test with --signoff flag

That can already be seen on the test title below.  Remove it?

> +test_expect_success 'git merge --signoff adds a sign-off line' '
> +...
> +test_expect_success 'git merge does not add a sign-off line' '
> +...
> +test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
> +...
> +'

They all look sensible thing to check.  We would also need to make
sure that the end user sees the S-o-b: prepopulated when the editor
is spawned, I would think, perhaps like (completely untested)

	write_script save-editor <<-\EOF &&
	cp "$1" "$1".saved
	EOF
	GIT_EDITOR=./save-editor git merge --signoff --no-signoff ... &&
 	... check the contents of the MERGE_MSG.saved file to 
	... ensure string Signed-off-by: appears (or does not
	... appear) here, e.g.
	test_i18ngrep ! "^Signed-off-by: " .git/MERGE_MSG.saved

Thanks.

Re: [PATCH v3] merge: add a --signoff flag.

[Junio C Hamano]

lukaszgryglicki <lukaszgryglicki@o2.pl> writes:

>>> +--signoff::
>>> +	Add Signed-off-by line by the committer at the end of the commit
>>> +	log message.  The meaning of a signoff depends on the project,
>>> +	but it typically certifies that committer has
>>> +	the rights to submit this work under the same license and
>>> +	agrees to a Developer Certificate of Origin
>>> +	(see http://developercertificate.org/ for more information).
>> 
>> This is taken verbatim from Documentation/git-commit.txt and "this
>> work" in that context is entirely sensible, but it is not quite
>> clear what it means in the context of "git merge”.
>
> Changed slightly, but I’m not a native English speaker, and
> don’t really see what is wrong there.

Oh, the language is not the issue.  

I was trying to remind you that a "merge" may be an automated and
mechanical one, in which there is no original work that requires
signing it off in the first place, which was the point Ævar raised
when Dan's original post came up.

[PATCH v4] merge: add a --signoff flag.

[Łukasz Gryglicki]

Some projects require every commit, even merges, to be signed off [*1*].
Because "git merge" does not have a "--signoff"
option like "git commit" does, the user needs to add one
manually when the command presents an editor to describe the
merge, or later use "git commit --amend --signoff".

Help developers of these projects by teaching "--signoff" option to "git merge".

*1*
https://public-inbox.org/git/CAHv71zK5SqbwrBFX=a8-DY9H3KT4FEyMgv__p2gZzNr0WUAPUw@mail.gmail.com/T/#u

Requested-by: Dan Kohn <dan@linuxfoundation.org>
Signed-off-by: Łukasz Gryglicki <lukaszgryglicki@o2.pl>
---
 Documentation/git-merge.txt |  8 +++++
 builtin/merge.c             |  4 +++
 t/t7614-merge-signoff.sh    | 88 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100755 t/t7614-merge-signoff.sh

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 04fdd8cf086db..630cb4b7f90d7 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,6 +64,14 @@ OPTIONS
 -------
 include::merge-options.txt[]
 
+--signoff::
+	Add Signed-off-by line by the committer at the end of the commit
+	log message.  The meaning of a signoff depends on the project,
+	but it typically certifies that committer has
+	the rights to merge work under the same license and
+	agrees to a Developer Certificate of Origin
+	(see http://developercertificate.org/ for more information).
+
 -S[<keyid>]::
 --gpg-sign[=<keyid>]::
 	GPG-sign the resulting merge commit. The `keyid` argument is
diff --git a/builtin/merge.c b/builtin/merge.c
index 900bafdb45d0b..78c36e9bf353b 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -70,6 +70,7 @@ static int continue_current_merge;
 static int allow_unrelated_histories;
 static int show_progress = -1;
 static int default_to_upstream = 1;
+static int signoff;
 static const char *sign_commit;
 
 static struct strategy all_strategy[] = {
@@ -233,6 +234,7 @@ static struct option builtin_merge_options[] = {
 	{ OPTION_STRING, 'S', "gpg-sign", &sign_commit, N_("key-id"),
 	  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 	OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
+	OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
 	OPT_END()
 };
 
@@ -763,6 +765,8 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 	strbuf_addch(&msg, '\n');
 	if (0 < option_edit)
 		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
+	if (signoff)
+		append_signoff(&msg, ignore_non_trailer(msg.buf, msg.len), 0);
 	write_file_buf(git_path_merge_msg(), msg.buf, msg.len);
 	if (run_commit_hook(0 < option_edit, get_index_file(), "prepare-commit-msg",
 			    git_path_merge_msg(), "merge", NULL))
diff --git a/t/t7614-merge-signoff.sh b/t/t7614-merge-signoff.sh
new file mode 100755
index 0000000000000..823d840d423df
--- /dev/null
+++ b/t/t7614-merge-signoff.sh
@@ -0,0 +1,88 @@
+#!/bin/sh
+
+test_description='git merge --signoff
+
+This test runs git merge --signoff and makes sure that it works.
+'
+
+. ./test-lib.sh
+
+# Setup test files
+test_setup () {
+  # Expected commit message after merge --signoff
+  printf "Merge branch 'master' into other-branch\n\n" >expected-signed &&
+  printf "Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")\n" >>expected-signed &&
+
+  # Expected commit message after merge without --signoff (or with --no-signoff)
+  echo "Merge branch 'master' into other-branch" >expected-unsigned &&
+
+  # Initial commit and feature branch to merge master into it.
+  git commit --allow-empty -m "Initial empty commit" &&
+  git checkout -b other-branch &&
+  test_commit other-branch file1 1
+}
+
+# Create fake editor that just copies file
+create_fake_editor () {
+  echo 'cp "$1" "$1.saved"' | write_script save-editor
+}
+
+test_expect_success 'setup' '
+  test_setup && create_fake_editor
+'
+
+test_expect_success 'git merge --signoff adds a sign-off line' '
+  git checkout master &&
+  test_commit master-branch-2 file2 2 &&
+  git checkout other-branch &&
+  git merge master --signoff --no-edit &&
+  git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+  test_cmp expected-signed actual
+'
+
+test_expect_success 'git merge does not add a sign-off line' '
+  git checkout master &&
+  test_commit master-branch-3 file3 3 &&
+  git checkout other-branch &&
+  git merge master --no-edit &&
+  git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+  test_cmp expected-unsigned actual
+'
+
+test_expect_success 'git merge --no-signoff flag cancels --signoff flag' '
+  git checkout master &&
+  test_commit master-branch-4 file4 4 &&
+  git checkout other-branch &&
+  git merge master --no-edit --signoff --no-signoff &&
+  git cat-file commit HEAD | sed -e "1,/^\$/d" >actual &&
+  test_cmp expected-unsigned actual
+'
+
+test_expect_success 'git merge --signoff adds S-o-b line in commit message editor' '
+  git checkout master &&
+  test_commit master-branch-5 file5 5 &&
+  git checkout other-branch &&
+  GIT_EDITOR=./save-editor git merge master -m "My Message" --edit --signoff &&
+  test_i18ngrep "^My Message" .git/MERGE_MSG.saved &&
+  test_i18ngrep "^Signed-off-by: " .git/MERGE_MSG.saved
+'
+
+test_expect_success 'git merge --no-signoff does not add S-o-b line in commit message editor' '
+  git checkout master &&
+  test_commit master-branch-6 file6 6 &&
+  git checkout other-branch &&
+  GIT_EDITOR=./save-editor git merge master -m "My Message" --edit --no-signoff &&
+  test_i18ngrep "^My Message" .git/MERGE_MSG.saved &&
+  test_i18ngrep ! "^Signed-off-by: " .git/MERGE_MSG.saved
+'
+
+test_expect_success 'git merge --no-signoff cancels --signoff flag in commit message editor' '
+  git checkout master &&
+  test_commit master-branch-7 file7 7 &&
+  git checkout other-branch &&
+  GIT_EDITOR=./save-editor git merge master -m "My Message" --edit --signoff --no-signoff &&
+  test_i18ngrep "^My Message" .git/MERGE_MSG.saved &&
+  test_i18ngrep ! "^Signed-off-by: " .git/MERGE_MSG.saved
+'
+
+test_done

--
https://github.com/git/git/pull/390