[PATCH] pull: pass --signoff/--no-signoff to "git merge"

[PATCH] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
add a --signoff flag, 2017-07-04).

The order of options in merge-options.txt isn't clear to me, but I've
put --signoff between --log and --stat as somewhat alphabetized and
having an "add to the commit message" function like --log.

The tests aren't as extensive as t7614-merge-signoff.sh, but they
exercises both the --signoff and --no-signoff options.  There may be a
more efficient way to set them up (like t7614-merge-signoff.sh's
test_setup), but with all the pull options packed into a single test
script it seemed easiest to just copy/paste the duplicate setup code.

09c2cb87 didn't motivate the addition of --allow-unrelated-histories
to pull; only citing the reason from e379fdf3 (merge: refuse to create
too cool a merge by default, 2016-03-18) gave for *not* including it.
I like having both exposed in pull because while the fetch-and-merge
approach might be a more popular way to judge "how well they fit
together", you can also do that after an optimistic pull.  And in
cases where an optimistic pull is likely to succeed, suggesting it is
easier to explain to Git newbies than a FETCH_HEAD merge.

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 Documentation/git-merge.txt     |  8 --------
 Documentation/merge-options.txt | 10 ++++++++++
 builtin/pull.c                  |  8 ++++++++
 t/t5521-pull-options.sh         | 43 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ 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/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
 With --no-log do not list one-line descriptions from the
 actual commits being merged.
 
+--signoff::
+--no-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).
++
+With --no-signoff do not add a Signed-off-by line.
 
 --stat::
 -n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..4469342f51 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -97,6 +97,7 @@ static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
 static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
 static char *opt_gpg_sign;
 static int opt_allow_unrelated_histories;
+static int opt_signoff;
 
 /* Options passed to git-fetch */
 static char *opt_all;
@@ -175,6 +176,9 @@ static struct option pull_options[] = {
 	OPT_SET_INT(0, "allow-unrelated-histories",
 		    &opt_allow_unrelated_histories,
 		    N_("allow merging unrelated histories"), 1),
+	OPT_BOOL(0, "signoff",
+		    &opt_signoff,
+		    N_("add Signed-off-by:")),
 
 	/* Options passed to git-fetch */
 	OPT_GROUP(N_("Options related to fetching")),
@@ -610,6 +614,10 @@ static int run_merge(void)
 		argv_array_push(&args, opt_gpg_sign);
 	if (opt_allow_unrelated_histories > 0)
 		argv_array_push(&args, "--allow-unrelated-histories");
+	if (opt_signoff > 0)
+		argv_array_push(&args, "--signoff");
+	else
+		argv_array_push(&args, "--no-signoff");
 
 	argv_array_push(&args, "FETCH_HEAD");
 	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..d95789ab8c 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,47 @@ test_expect_success 'git pull --allow-unrelated-histories' '
 	)
 '
 
+test_expect_success 'git pull --signoff add a sign-off line' '
+	test_when_finished "rm -fr src dst actual expected" &&
+	cat >expected <<-EOF &&
+		Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
+	EOF
+	git init src &&
+	(
+		cd src &&
+		test_commit one
+	) &&
+	git clone src dst &&
+	(
+		cd src &&
+		test_commit two
+	) &&
+	(
+		cd dst &&
+		git pull --signoff --no-ff &&
+		git cat-file commit HEAD | tail -n1 >../actual
+	) &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	(
+		cd src &&
+		test_commit one
+	) &&
+	git clone src dst &&
+	(
+		cd src &&
+		test_commit two
+	) &&
+	(
+		cd dst &&
+		git pull --signoff --no-signoff --no-ff &&
+		git cat-file commit HEAD | sed -n /Signed-off-by/p >../actual
+	) &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.13.6

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
> merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
> add a --signoff flag, 2017-07-04).

I cannot find a verb in the above.

> The order of options in merge-options.txt isn't clear to me, but I've
> put --signoff between --log and --stat as somewhat alphabetized and
> having an "add to the commit message" function like --log.
>
> The tests aren't as extensive as t7614-merge-signoff.sh, but they
> exercises both the --signoff and --no-signoff options.  There may be a
> more efficient way to set them up (like t7614-merge-signoff.sh's
> test_setup), but with all the pull options packed into a single test
> script it seemed easiest to just copy/paste the duplicate setup code.

The above two paragraphs read more like "requesting help for hints
to improve this patch" than commit log message.  Perhaps move them
below the three-dash line and instead describe what you actually did
here (if they were worth explaining, that is)?

> 09c2cb87 didn't motivate the addition of --allow-unrelated-histories
> to pull; only citing the reason from e379fdf3 (merge: refuse to create
> too cool a merge by default, 2016-03-18) gave for *not* including it.
> I like having both exposed in pull because while the fetch-and-merge
> approach might be a more popular way to judge "how well they fit
> together", you can also do that after an optimistic pull.  And in
> cases where an optimistic pull is likely to succeed, suggesting it is
> easier to explain to Git newbies than a FETCH_HEAD merge.

I find this paragraph totally unrelated to what the patch does.
Save it for the patch you add to pass --allow-unrelated-histories
given to pull down to underlying merge, perhaps?

>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
>  Documentation/git-merge.txt     |  8 --------
>  Documentation/merge-options.txt | 10 ++++++++++
>  builtin/pull.c                  |  8 ++++++++
>  t/t5521-pull-options.sh         | 43 +++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 61 insertions(+), 8 deletions(-)
> ...
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index ded8f98dbe..d95789ab8c 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -165,4 +165,47 @@ test_expect_success 'git pull --allow-unrelated-histories' '
>  	)
>  '
>  
> +test_expect_success 'git pull --signoff add a sign-off line' '
> +	test_when_finished "rm -fr src dst actual expected" &&
> +	cat >expected <<-EOF &&
> +		Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> +	EOF

	echo "Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>" >expect

or

	git var GIT_COMMITTER_IDENT |
	sed -e 's/^\([^>]*>\).*/Signed-off-by: \1/' >expect

> +	git init src &&
> +	(
> +		cd src &&
> +		test_commit one
> +	) &&

I suspect somebody will suggest "test_commit -C" ;-)

> +	git clone src dst &&
> +	(
> +		cd src &&
> +		test_commit two
> +	) &&
> +	(
> +		cd dst &&
> +		git pull --signoff --no-ff &&
> +		git cat-file commit HEAD | tail -n1 >../actual

I think it makes it more robust to replace "tail" with "collect all
the signed-off-by lines" like the other test (below) does.  Perhaps
have a helper function and use it in both?

	get_signoff () {
		git cat-file commit "$1" | sed -n -e '/^Signed-off-by: /p'
	}

Some may say "cat-file can fail, and having it on the LHS of a pipe
hides its failure", advocating for something like:

	get_signoff () {
		git cat-file commit "$1" >sign-off-temp &&
		sed -n -e '/^Signed-off-by: /p' sign-off-temp
	}

> +	) &&
> +	test_cmp expected actual
> +'

> +test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
> +	test_when_finished "rm -fr src dst actual" &&
> +	git init src &&
> +	(
> +		cd src &&
> +		test_commit one
> +	) &&
> +	git clone src dst &&
> +	(
> +		cd src &&
> +		test_commit two
> +	) &&
> +	(
> +		cd dst &&
> +		git pull --signoff --no-signoff --no-ff &&
> +		git cat-file commit HEAD | sed -n /Signed-off-by/p >../actual
> +	) &&
> +	test_must_be_empty actual
> +'
> +
>  test_done

I think "--signoff" and "--signoff --no-signoff" are reasonable
minimum things to test.  Two more cases, i.e. running it without
either and with "--no-signoff" alone, to ensure that the sign-off
mechanism does not kick in would make it even better.

Thanks.

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 8217 bytes --]

On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> 
> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
> > add a --signoff flag, 2017-07-04).
> 
> I cannot find a verb in the above.

I'd meant it as either a continuation of the subject line, or with an
implicit leading “I did this…” :p.  I can reword if you like, maybe
just “Following” → “Follow”?  Something more drastic?

> > The order of options in merge-options.txt isn't clear to me, but
> > I've put --signoff between --log and --stat as somewhat
> > alphabetized and having an "add to the commit message" function
> > like --log.
> >
> > The tests aren't as extensive as t7614-merge-signoff.sh, but they
> > exercises both the --signoff and --no-signoff options.  There may
> > be a more efficient way to set them up (like
> > t7614-merge-signoff.sh's test_setup), but with all the pull
> > options packed into a single test script it seemed easiest to just
> > copy/paste the duplicate setup code.
>
> The above two paragraphs read more like "requesting help for hints
> to improve this patch" than commit log message.  Perhaps move them
> below the three-dash line and instead describe what you actually did
> here (if they were worth explaining, that is)?

I think something about merge-options.txt ordering should end up in
the history of that content.  Reading through:

  $ git log Documentation/merge-options.txt

only turned up 690b2975 (Documentation/merge-options.txt: group "ff"
related options together, 2012-02-22) discussing option order (it
suggested grouping similar options together, although --ff and
--ff-only would also be close alphabetically).

I agree that the first paragraph you quote above doesn't have me
coming down firmly in favor of a particular ordering strategy, but I
think having something like it in the Git history will help whoever
ends up giving merge-options.txt a well-defined strategy by showing I
didn't have any strong opinions to account for ;).  Silence can mean
“doesn't have a strong opinion”, but sometimes it means “feels the
choice is so obvious that it doesn't need explicit motivation”.

I'm fine moving the second paragraph you quote below the fold in a v2,
although you're calling for more tests below, and it won't apply
anymore once I've added those :).

> > 09c2cb87 didn't motivate the addition of --allow-unrelated-histories
> > to pull; only citing the reason from e379fdf3 (merge: refuse to create
> > too cool a merge by default, 2016-03-18) gave for *not* including it.
> > I like having both exposed in pull because while the fetch-and-merge
> > approach might be a more popular way to judge "how well they fit
> > together", you can also do that after an optimistic pull.  And in
> > cases where an optimistic pull is likely to succeed, suggesting it is
> > easier to explain to Git newbies than a FETCH_HEAD merge.
> 
> I find this paragraph totally unrelated to what the patch does.
> Save it for the patch you add to pass --allow-unrelated-histories
> given to pull down to underlying merge, perhaps?

09c2cb87 is your commit in master (v2.9.0-rc0~88^2) that is doing just
that.  I haven't gone through the list history to figure out why it
ended up getting landed with its current commit message; “Prepare a
patch to make it a reality, just in case it is needed” sounds more
like it was “here's the code in case folks want it, I'll reroll the
motivation if they do”.  This paragraph was aiming to motivate both
the --signoff pass-through I'm adding here and (retroactively) the
--allow-unrelated-histories pass-through you added there.  I'll add
more context in v2 to try to make that more clear.

> > +	cat >expected <<-EOF &&
> > +		Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")
> > +	EOF
> 
> 	echo "Signed-off-by: $GIT_COMMITER_NAME <$GIT_COMMITTER_EMAIL>" >expect

Much nicer, thanks.  I'll add a patch to v2 to make the same change to
t7614.

> > +	git init src &&
> > +	(
> > +		cd src &&
> > +		test_commit one
> > +	) &&
> 
> I suspect somebody will suggest "test_commit -C" ;-)

Sounds good.  I'll add a patch to v2 to make the same change to the
existing t5521 --allow-unrelated-histories test.

> > +	git clone src dst &&
> > +	(
> > +		cd src &&
> > +		test_commit two
> > +	) &&
> > +	(
> > +		cd dst &&
> > +		git pull --signoff --no-ff &&
> > +		git cat-file commit HEAD | tail -n1 >../actual
> 
> I think it makes it more robust to replace "tail" with "collect all
> the signed-off-by lines" like the other test (below) does.  Perhaps
> have a helper function and use it in both?
> 
> 	get_signoff () {
> 		git cat-file commit "$1" | sed -n -e '/^Signed-off-by: /p'
> 	}
> 
> Some may say "cat-file can fail, and having it on the LHS of a pipe
> hides its failure", advocating for something like:
> 
> 	get_signoff () {
> 		git cat-file commit "$1" >sign-off-temp &&
> 		sed -n -e '/^Signed-off-by: /p' sign-off-temp
> 	}

There are several existing consumers using grep and sed for this:

  wking@ullr ~/src/git/git $ git grep Signed-off-by v2.15.0-rc1 -- 't/*.sh' | grep 'grep\|sed'
  …
  v2.15.0-rc1:t/t3501-revert-cherry-pick.sh:      git cat-file commit HEAD | grep ^Signed-off-by: >signoff &&
  v2.15.0-rc1:t/t3507-cherry-pick-conflict.sh:    test_i18ngrep -e "Signed-off-by" .git/MERGE_MSG
  v2.15.0-rc1:t/t3507-cherry-pick-conflict.sh:    test $(git show -s |grep -c "Signed-off-by") = 1
  v2.15.0-rc1:t/t3507-cherry-pick-conflict.sh:    grep -e "^# Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual &&
  v2.15.0-rc1:t/t3507-cherry-pick-conflict.sh:    grep -e "^Conflicts:" -e '^Signed-off-by' <.git/COMMIT_EDITMSG >actual &&
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    ! grep "Signed-off-by:" initial_msg &&
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    grep "Signed-off-by:" unrelatedpick_msg &&
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    ! grep "Signed-off-by:" picked_msg &&
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    grep "Signed-off-by:" anotherpick_msg
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    ! grep Signed-off-by: msg
  v2.15.0-rc1:t/t3510-cherry-pick-sequence.sh:    ! grep Signed-off-by: msg
  v2.15.0-rc1:t/t4014-format-patch.sh:    grep "^Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" out
  v2.15.0-rc1:t/t4014-format-patch.sh:    ! sed "/^Signed-off-by: /q" out | grep "test message" &&
  v2.15.0-rc1:t/t4014-format-patch.sh:    sed "1,/^Signed-off-by: /d" out | grep "test message" &&
  v2.15.0-rc1:t/t4153-am-resume-override-opts.sh: git cat-file commit HEAD^ | grep "Signed-off-by:" >actual &&
  v2.15.0-rc1:t/t4153-am-resume-override-opts.sh: test $(git cat-file commit HEAD | grep -c "Signed-off-by:") -eq 0
  v2.15.0-rc1:t/t7501-commit.sh:          sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
  v2.15.0-rc1:t/t7501-commit.sh:          sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
  v2.15.0-rc1:t/t7501-commit.sh:          sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
  v2.15.0-rc1:t/t7501-commit.sh:          sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
  v2.15.0-rc1:t/t7502-commit.sh:  actual=$(git cat-file commit HEAD | sed -ne "s/Signed-off-by: //p") &&
  v2.15.0-rc1:t/t7614-merge-signoff.sh:Signed-off-by: $(git var GIT_COMMITTER_IDENT | sed -e "s/>.*/>/")

Perhaps we want something like:

  test_has_trailer $OBJECT $TOKEN $VALUE

and:

  test_has_no_trailer $OBJECT $TOKEN

in test-lib-functions.sh that we can use in all of these cases?

> I think "--signoff" and "--signoff --no-signoff" are reasonable
> minimum things to test.  Two more cases, i.e. running it without
> either and with "--no-signoff" alone, to ensure that the sign-off
> mechanism does not kick in would make it even better.

Sounds good, I'll add those in v2.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> 
>> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
>> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
>> > add a --signoff flag, 2017-07-04).
>> 
>> I cannot find a verb in the above.
>
> I'd meant it as either a continuation of the subject line, or with an

Never do that.  The title should be able to stand on its own, and
must not be an early part of incomplete sentence.

> Much nicer, thanks.  I'll add a patch to v2 to make the same change to
> t7614.
> ...
> Sounds good.  I'll add a patch to v2 to make the same change to the
> existing t5521 --allow-unrelated-histories test.

Please don't, unless you are actively working on the features that
they test.  We do not have infinite amount of bandwidth to deal with
changes for the sake of perceived consistency and no other real
gain.

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1803 bytes --]

On Thu, Oct 12, 2017 at 02:42:30PM +0900, Junio C Hamano wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > On Thu, Oct 12, 2017 at 10:17:51AM +0900, Junio C Hamano wrote:
> >> "W. Trevor King" <wking@tremily.us> writes:
> >> 
> >> > Following 09c2cb87 (pull: pass --allow-unrelated-histories to "git
> >> > merge", 2016-03-18) with the tests also drawing on 14d01b4f (merge:
> >> > add a --signoff flag, 2017-07-04).
> >> 
> >> I cannot find a verb in the above.
> >
> > I'd meant it as either a continuation of the subject line, or with an
> 
> Never do that.  The title should be able to stand on its own, and
> must not be an early part of incomplete sentence.

“Following” to an imperative “Follow” it is then, unless you want a
more drastic rewording.

> > Sounds good.  I'll add a patch to v2 to make the same change to
> > the existing t5521 --allow-unrelated-histories test.
> 
> Please don't, unless you are actively working on the features that
> they test.  We do not have infinite amount of bandwidth to deal with
> changes for the sake of perceived consistency and no other real
> gain.

By extention, I'm guessing that means that while the:

  test_has_trailer $OBJECT $TOKEN $VALUE

and:

  test_has_no_trailer $OBJECT $TOKEN

test-lib-functions.sh helpers I floated may be acceptable (or not, no
need to commit before you've seen a patch), you don't want me updating
existing tests to use them.  I'll just use them in my new tests, and
folks can gradually transition existing tests to them as they touch
those tests (if they remember the helpers exist ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v2] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

e379fdf3 (merge: refuse to create too cool a merge by default,
2016-03-18) gave a reason for *not* passing options from pull to
merge:

  ...because such a "two project merge" would be done after fetching
  the other project into some location in the working tree of an
  existing project and making sure how well they fit together...

That's certainly an acceptable workflow, but I'd also like to support
merge options in pull for folks who want to optimistically pull and
then sort out "how well they fit together" after pull exits (possibly
with a merge failure).  And in cases where an optimistic pull is
likely to succeed, suggesting it is easier to explain to Git newbies
than a FETCH_HEAD merge or remote-addition/merge/remote-removal.

09c2cb87 (pull: pass --allow-unrelated-histories to "git merge",
2016-03-18) added a pull-to-merge pass for a different option but
didn't motivate its change, only citing the reason from e379fdf3 for
not adding the pull-to-merge pass for that option.  I'm personally in
favor of pull-to-merge passing for any unambiguous options, but if the
decision for pull-to-merge passes depends on the specific option, then
--allow-unrelated-histories is probably the weakest candidate because
unrelated-history merged are more likely to have "fit together" issues
than the other merge-only options.

The test_has_trailer helper gives folks a convenient way check these
sorts of things.  I haven't gone through and updated existing trailer
checks (e.g. the ones in t7614-merge-signoff.sh) to avoid the "only to
correct the inconconsistency" issue discussed in SubmittingPatches.
Other test may gradually migrate to the new helper if they find it
useful.  The helper may be useful enough to eventually become a
plumbing command (a read version of interpret-trailers with an API
similar to 'git config ...'?), but I'm not going that far in this
commit ;).

The order of options in merge-options.txt isn't clear to me, but I've
put --signoff between --log and --stat as somewhat alphabetized and
having an "add to the commit message" function like --log.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: W. Trevor King <wking@tremily.us>
---
Changes since v1 [1]:

* Dropped "Following" paragraph.  Junio took issue with the phrasing
  [2], and the implementation in v2 has diverged sufficiently from
  09c2cb87 and 14d01b4f that I don't think citing them as
  implementation references is useful anymore.

* Lead the commit message with reworked motivation paragraphs, since
  Junio read the v1 motivation paragraph as off-topic [2].

* Add a test_has_trailer helper, which I'd floated in [3] after
  Junio's get_signoff suggestion in [2].

* Drop subshells in favor of '-C <directory>' in the tests, as
  suggested in [2].

* Add tests for the bare pull and lonely --no-signoff cases, as
  suggested in [2].  With these additions in place, I've dropped v1's
  "The tests aren't as extensive..." paragraph from the commit
  message.

* Use OPT_PASSTHRU in pull.c.  I'm not sure why
  --allow-unrelated-histories didn't go this route, but there are lots
  of other pull-to-merge options using OPT_PASSTHRU, so using it for
  --signoff isn't breaking consistency.

Not changed since v1:

* The merge-options.txt order paragraph.  Junio had suggested it be
  moved after the break [2], but I think having some commit-message
  discussion of merge-options.txt ordering is useful, even though I
  don't have strong opinions on what the ordering should be [3].

This patch (and v1) are based on v2.15.0-rc1, although I expect
they'll apply cleanly to anything in that vicinity.

Cheers,
Trevor

[1]: https://public-inbox.org/git/18953f46ffb5e3dbc4da8fbda7fe3ab4298d7cbd.1507752482.git.wking@tremily.us/
[2]: https://public-inbox.org/git/xmqqefq92mgw.fsf@gitster.mtv.corp.google.com/
[3]: https://public-inbox.org/git/20171012053002.GZ11004@valgrind.tremily.us/

 Documentation/git-merge.txt     |  8 --------
 Documentation/merge-options.txt | 10 ++++++++++
 builtin/pull.c                  |  6 ++++++
 t/t5521-pull-options.sh         | 40 ++++++++++++++++++++++++++++++++++++++
 t/test-lib-functions.sh         | 43 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ 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/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
 With --no-log do not list one-line descriptions from the
 actual commits being merged.
 
+--signoff::
+--no-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).
++
+With --no-signoff do not add a Signed-off-by line.
 
 --stat::
 -n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..0413c78a3a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum rebase_type opt_rebase = -1;
 static char *opt_diffstat;
 static char *opt_log;
+static char *opt_signoff;
 static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
@@ -142,6 +143,9 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU(0, "log", &opt_log, N_("n"),
 		N_("add (at most <n>) entries from shortlog to merge commit message"),
 		PARSE_OPT_OPTARG),
+	OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
+		N_("add Signed-off-by:"),
+		PARSE_OPT_OPTARG),
 	OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
 		N_("create a single commit instead of doing a merge"),
 		PARSE_OPT_NOARG),
@@ -594,6 +598,8 @@ static int run_merge(void)
 		argv_array_push(&args, opt_diffstat);
 	if (opt_log)
 		argv_array_push(&args, opt_log);
+	if (opt_signoff)
+		argv_array_push(&args, opt_signoff);
 	if (opt_squash)
 		argv_array_push(&args, opt_squash);
 	if (opt_commit)
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..82680a30f8 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,44 @@ test_expect_success 'git pull --allow-unrelated-histories' '
 	)
 '
 
+test_expect_success 'git pull does not add a sign-off line' '
+	test_when_finished "rm -fr src dst" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --no-ff &&
+	! test_has_trailer -C dst HEAD Signed-off-by
+'
+
+test_expect_success 'git pull --no-signoff does not add sign-off line' '
+	test_when_finished "rm -fr src dst" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --no-signoff --no-ff &&
+	! test_has_trailer -C dst HEAD Signed-off-by
+'
+
+test_expect_success 'git pull --signoff add a sign-off line' '
+	test_when_finished "rm -fr src dst" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --signoff --no-ff &&
+	test_has_trailer -C dst HEAD Signed-off-by "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
+'
+
+test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --signoff --no-signoff --no-ff &&
+	! test_has_trailer -C dst HEAD Signed-off-by
+'
+
 test_done
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 1701fe2a06..08409b1c25 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -726,6 +726,49 @@ test_must_be_empty () {
 	fi
 }
 
+# Call test_has_trailer with the arguments:
+# [-C <directory>] <object> <token> [<value>]
+# where <object> is an object name as described in gitrevisions(7),
+# <token> is a trailer token (e.g. 'Signed-off-by'), and
+# <value> is an optional value (e.g. 'A U Thor <author@example.com>').
+# test_has_trailer returns success if the specified trailer is found
+# in the object content.  If <value> is unset, any value will match.
+#
+# Both <token> and <value> are basic regular expressions.
+#
+# If the first argument is "-C", the second argument is used as a path for
+# the git invocations.
+test_has_trailer () {
+	INDIR=
+	case "$1" in
+	-C)
+		INDIR="$2"
+		shift 2 || error "<directory> not specified"
+		;;
+	esac
+	INDIR="${INDIR:+${INDIR}/}"
+	OBJECT="$1"
+	shift || error "<object> not specified"
+	TOKEN="$1"
+	shift || error "<token> not specified"
+	SEP=':'  # FIXME: read from trailer.separators?
+	CONTENT="$(git ${INDIR:+ -C "${INDIR}"} cat-file -p "${OBJECT}")" || error "object ${OBJECT} not found${INDIR:+ in ${INDIR}}"
+	PATTERN="^${TOKEN}${SEP}"
+	if test 0 -lt "$#"
+	then
+		VALUE="$1"
+		PATTERN="${PATTERN} *${VALUE}$"
+	fi
+	if (echo "${CONTENT}" | grep -q "${PATTERN}")
+	then
+		printf "%s found in:\n%s\n" "${PATTERN}" "${CONTENT}"
+		return 0
+	else
+		printf "%s not found in:\n%s\n" "${PATTERN}" "${CONTENT}"
+		return 1
+	fi
+}
+
 # Tests that its two parameters refer to the same revision
 test_cmp_rev () {
 	git rev-parse --verify "$1" >expect.rev &&
-- 
2.13.6

Re: [PATCH v2] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1468 bytes --]

On Thu, Oct 12, 2017 at 01:46:39AM -0700, W. Trevor King wrote:
> The order of options in merge-options.txt isn't clear to me, but
> I've put --signoff between --log and --stat as somewhat alphabetized
> and having an "add to the commit message" function like --log.

The order of options in merge-options.txt was intended to be by
"alphabetical groups", at least back in 7c85d274
(Documentation/merge-options.txt: order options in alphabetical
groups, 2009-10-22).  I'm not quite clear on what that means.  After
7c85d274 landed there were already long-option irregularities:

  $ git grep -h ^-- 7c85d27 -- Documentation/merge-options.txt
  --commit::
  --no-commit::
  --ff::
  --no-ff::
  --log::
  --no-log::
  --stat::
  --no-stat::
  --squash::
  --no-squash::
  --strategy=<strategy>::
  --summary::
  --no-summary::
  --quiet::
  --verbose::

If the order was purely alphabetical, --stat/--no-stat should have
after --squash/--no-squash, and --quiet should have been much earlier.
And putting --signoff after --log is still alphabetical in v2.15.0-rc1
(ignoring a few outliers).  So I don't think it's a reason to change
where I'd put the option, but in v3 of this patch I'll update the
commit message to cite 7c85d274 when motivating the location.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> e379fdf3 (merge: refuse to create too cool a merge by default,
> 2016-03-18) gave a reason for *not* passing options from pull to
> merge:
>
>   ...because such a "two project merge" would be done after fetching
>   the other project into some location in the working tree of an
>   existing project and making sure how well they fit together...

Read the above again and notice the phrase "two project merge".  The
reasoning applies only to the --allow-unrelated-histories option.
It gave a reason for not passing *THAT* option and nothing else, and
does not mean to say anything about passing or not passing any other
options.  

That is why I said the reference to that commit was irrelevant in
the context of this patch.

If you find somebody saying "we should not pass --signoff from pull
to merge" when we taught "--signoff" to "merge", then that may be
worth referring to, as this commit _will_ be changing that earlier
decision.  I however do not think that is a case.  Just saying
"merge can take --signoff, but without pull passing --signoff down,
it is inconvenient, so let's pass it through" is sufficient to
justify this change.

> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index ded8f98dbe..82680a30f8 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -165,4 +165,44 @@ test_expect_success 'git pull --allow-unrelated-histories' '
>  	)
>  '
>  
> +test_expect_success 'git pull does not add a sign-off line' '
> +	test_when_finished "rm -fr src dst" &&
> +	git init src &&
> +	test_commit -C src one &&
> +	git clone src dst &&
> +	test_commit -C src two &&
> +	git -C dst pull --no-ff &&
> +	! test_has_trailer -C dst HEAD Signed-off-by
> +'
> +
> +test_expect_success 'git pull --no-signoff does not add sign-off line' '
> +	test_when_finished "rm -fr src dst" &&
> +	git init src &&
> +	test_commit -C src one &&
> +	git clone src dst &&
> +	test_commit -C src two &&
> +	git -C dst pull --no-signoff --no-ff &&
> +	! test_has_trailer -C dst HEAD Signed-off-by
> +'
> +
> +test_expect_success 'git pull --signoff add a sign-off line' '
> +	test_when_finished "rm -fr src dst" &&
> +	git init src &&
> +	test_commit -C src one &&
> +	git clone src dst &&
> +	test_commit -C src two &&
> +	git -C dst pull --signoff --no-ff &&
> +	test_has_trailer -C dst HEAD Signed-off-by "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"
> +'
> +
> +test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
> +	test_when_finished "rm -fr src dst actual" &&
> +	git init src &&
> +	test_commit -C src one &&
> +	git clone src dst &&
> +	test_commit -C src two &&
> +	git -C dst pull --signoff --no-signoff --no-ff &&
> +	! test_has_trailer -C dst HEAD Signed-off-by
> +'
> +
>  test_done
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 1701fe2a06..08409b1c25 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -726,6 +726,49 @@ test_must_be_empty () {
>  	fi
>  }
>  
> +# Call test_has_trailer with the arguments:
> +# [-C <directory>] <object> <token> [<value>]
> +# where <object> is an object name as described in gitrevisions(7),
> +# <token> is a trailer token (e.g. 'Signed-off-by'), and
> +# <value> is an optional value (e.g. 'A U Thor <author@example.com>').
> +# test_has_trailer returns success if the specified trailer is found
> +# in the object content.  If <value> is unset, any value will match.
> +#
> +# Both <token> and <value> are basic regular expressions.
> +#
> +# If the first argument is "-C", the second argument is used as a path for
> +# the git invocations.
> +test_has_trailer () {
> +	INDIR=
> +	case "$1" in
> +	-C)
> +		INDIR="$2"
> +		shift 2 || error "<directory> not specified"
> +		;;
> +	esac
> +	INDIR="${INDIR:+${INDIR}/}"
> +	OBJECT="$1"
> +	shift || error "<object> not specified"
> +	TOKEN="$1"
> +	shift || error "<token> not specified"
> +	SEP=':'  # FIXME: read from trailer.separators?
> +	CONTENT="$(git ${INDIR:+ -C "${INDIR}"} cat-file -p "${OBJECT}")" || error "object ${OBJECT} not found${INDIR:+ in ${INDIR}}"
> +	PATTERN="^${TOKEN}${SEP}"
> +	if test 0 -lt "$#"
> +	then
> +		VALUE="$1"
> +		PATTERN="${PATTERN} *${VALUE}$"
> +	fi
> +	if (echo "${CONTENT}" | grep -q "${PATTERN}")
> +	then
> +		printf "%s found in:\n%s\n" "${PATTERN}" "${CONTENT}"
> +		return 0
> +	else
> +		printf "%s not found in:\n%s\n" "${PATTERN}" "${CONTENT}"
> +		return 1
> +	fi
> +}

The reason why I suggested a simple "sed -n -e ...p" you used in
your original was because it could be used to extract not just one
Signed-off-by: lines to store in >actual, to be compared with an
expect that has multiple S-o-b lines and the output is in correct
order, etc.  An elaborate filter that can onlyl give "found/not
found" boolean looks a bit over-engineered for no real gain.

>  # Tests that its two parameters refer to the same revision
>  test_cmp_rev () {
>  	git rev-parse --verify "$1" >expect.rev &&

Re: [PATCH] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

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

> 	get_signoff () {
> 		git cat-file commit "$1" | sed -n -e '/^Signed-off-by: /p'
> 	}
>
> Some may say "cat-file can fail, and having it on the LHS of a pipe
> hides its failure", advocating for something like:
>
> 	get_signoff () {
> 		git cat-file commit "$1" >sign-off-temp &&
> 		sed -n -e '/^Signed-off-by: /p' sign-off-temp
> 	}

Actually we should use git itself for things like this, e.g.

	git -C dst show -s --pretty='format:%(trailers)' HEAD >actual &&
	test_cmp expect actual

[PATCH v3] pull: pass --signoff/--no-signoff to "git merge"

[W. Trevor King]

merge can take --signoff, but without pull passing --signoff down, it
is inconvenient, so let's pass it through.

The order of options in merge-options.txt is mostly alphabetical by
long option since 7c85d274 (Documentation/merge-options.txt: order
options in alphabetical groups, 2009-10-22).  The long-option bit
didn't make it into the commit message, but it's under the fold in
[1].  I've put --signoff between --log and --stat to preserve the
alphabetical order.

[1]: https://public-inbox.org/git/87iqe7zspn.fsf@jondo.cante.net/

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: W. Trevor King <wking@tremily.us>
---
Changes since v2 [1]:

* Replace the two motivational paragraphs with Junio's suggested
  sentence [2].
* Drop test_hash_trailer in favor of --pretty='format:%(trailers)'
  [3].  It turns out that the builtin tooling I was looking for while
  working on test_hash_trailer already exists :).

This patch (like v1 and v2) is based on v2.15.0-rc1, although I expect
it will apply cleanly to anything in that vicinity.

Cheers,
Trevor

[1]: https://public-inbox.org/git/51d67d6d707182d4973d9961ab29358f26c4988a.1507796638.git.wking@tremily.us/
[2]: https://public-inbox.org/git/xmqqk200znel.fsf@gitster.mtv.corp.google.com/
[3]: https://public-inbox.org/git/xmqq7ew0zkqv.fsf@gitster.mtv.corp.google.com/

 Documentation/git-merge.txt     |  8 --------
 Documentation/merge-options.txt | 10 +++++++++
 builtin/pull.c                  |  6 ++++++
 t/t5521-pull-options.sh         | 45 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 4df6431c34..0ada8c856b 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -64,14 +64,6 @@ 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/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 4e32304301..f394622d65 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -51,6 +51,16 @@ set to `no` at the beginning of them.
 With --no-log do not list one-line descriptions from the
 actual commits being merged.
 
+--signoff::
+--no-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).
++
+With --no-signoff do not add a Signed-off-by line.
 
 --stat::
 -n::
diff --git a/builtin/pull.c b/builtin/pull.c
index 6f772e8a22..0413c78a3a 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -86,6 +86,7 @@ static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static enum rebase_type opt_rebase = -1;
 static char *opt_diffstat;
 static char *opt_log;
+static char *opt_signoff;
 static char *opt_squash;
 static char *opt_commit;
 static char *opt_edit;
@@ -142,6 +143,9 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU(0, "log", &opt_log, N_("n"),
 		N_("add (at most <n>) entries from shortlog to merge commit message"),
 		PARSE_OPT_OPTARG),
+	OPT_PASSTHRU(0, "signoff", &opt_signoff, NULL,
+		N_("add Signed-off-by:"),
+		PARSE_OPT_OPTARG),
 	OPT_PASSTHRU(0, "squash", &opt_squash, NULL,
 		N_("create a single commit instead of doing a merge"),
 		PARSE_OPT_NOARG),
@@ -594,6 +598,8 @@ static int run_merge(void)
 		argv_array_push(&args, opt_diffstat);
 	if (opt_log)
 		argv_array_push(&args, opt_log);
+	if (opt_signoff)
+		argv_array_push(&args, opt_signoff);
 	if (opt_squash)
 		argv_array_push(&args, opt_squash);
 	if (opt_commit)
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index ded8f98dbe..c19d8dbc9d 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -165,4 +165,49 @@ test_expect_success 'git pull --allow-unrelated-histories' '
 	)
 '
 
+test_expect_success 'git pull does not add a sign-off line' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --no-ff &&
+	git -C dst show -s --pretty="format:%(trailers)" HEAD >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'git pull --no-signoff does not add sign-off line' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --no-signoff --no-ff &&
+	git -C dst show -s --pretty="format:%(trailers)" HEAD >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'git pull --signoff add a sign-off line' '
+	test_when_finished "rm -fr src dst expected actual" &&
+	echo "Signed-off-by: $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>" >expected &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --signoff --no-ff &&
+	git -C dst show -s --pretty="format:%(trailers)" HEAD >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'git pull --no-signoff flag cancels --signoff flag' '
+	test_when_finished "rm -fr src dst actual" &&
+	git init src &&
+	test_commit -C src one &&
+	git clone src dst &&
+	test_commit -C src two &&
+	git -C dst pull --signoff --no-signoff --no-ff &&
+	git -C dst show -s --pretty="format:%(trailers)" HEAD >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.13.6

Re: [PATCH v3] pull: pass --signoff/--no-signoff to "git merge"

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 4df6431c34..0ada8c856b 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -64,14 +64,6 @@ 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/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 4e32304301..f394622d65 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -51,6 +51,16 @@ set to `no` at the beginning of them.
>  With --no-log do not list one-line descriptions from the
>  actual commits being merged.
>  
> +--signoff::
> +--no-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).
> ++
> +With --no-signoff do not add a Signed-off-by line.

Makes sense.  Thanks, will queue.