[BUG] "git pull" will regress between 'master' and 'pu'

[BUG] "git pull" will regress between 'master' and 'pu'

[Junio C Hamano]

This is primarily note-to-self; even though I haven't got around
bisecting yet, I think I know I did some bad change myself.

"git pull $URL $tag" seems to:

 * fail to invoke the editor without "--edit".
 * show the summary merge log message twice.

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Jeff King]

On Sat, Apr 18, 2015 at 06:39:20PM -0700, Junio C Hamano wrote:

> This is primarily note-to-self; even though I haven't got around
> bisecting yet, I think I know I did some bad change myself.
> 
> "git pull $URL $tag" seems to:
> 
>  * fail to invoke the editor without "--edit".
>  * show the summary merge log message twice.

I think that "git merge -m $msg $commit" is not quite the same as "git
merge $msg HEAD $commit".

The former suppresses the editor. This makes sense, as it's consistent
with "commit -m", etc. You can override that with "--edit".

But the latter also respects merge.log. Which also makes sense, because
the contents of "-m" are generally user-created, not the output of
fmt-merge-msg. So it is merge's job to format the content appropriately.

It is tempting to "solve" both by dropping the call to git-fmt-merge-msg
right before calling git-merge. Then "git-merge" will call fmt-merge-msg
itself. However, we feed it only the sha1 of the merge-head, whereas our
fmt-merge-msg call gets to see FETCH_HEAD. So we would lose the nice
"Merge tag 'v1.0' of git://..." part of the message. Instead we get
"Merge tag '1234abcd'".

So this is _almost_ enough:

diff --git a/git-pull.sh b/git-pull.sh
index 252969e..63493ee 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,7 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
+merge_name=$(git fmt-merge-msg --no-log <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +334,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval -m \"\$merge_name\" $merge_head"
+	eval="$eval --edit -m \"\$merge_name\" $merge_head"
 	;;
 esac
 eval "exec $eval"

But that is starting to feel pretty hacky.  Moreover, both fmt-merge-msg
and "merge -m" will verify the tag signature and output the tag message.
I don't see a way to suppress that. So it really would be nice to be
able to just drop the extra fmt-merge-msg call and have git-merge do it
all internally, which would mean telling it to use FETCH_HEAD and not
$merge_name.

Which I guess is just:

diff --git a/git-pull.sh b/git-pull.sh
index 252969e..15d9431 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,6 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +333,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval -m \"\$merge_name\" $merge_head"
+	eval="$eval FETCH_HEAD"
 	;;
 esac
 eval "exec $eval"

as we seem to special-case the name FETCH_HEAD. It assumes that
git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
but that seems safe. Unfortunately we still have to compute $merge_head
ourselves here for the "git pull --rebase" case.

-Peff

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[brian m. carlson]

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

On Sun, Apr 19, 2015 at 09:07:46AM -0400, Jeff King wrote:
> Which I guess is just:
> 
> diff --git a/git-pull.sh b/git-pull.sh
> index 252969e..15d9431 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -323,7 +323,6 @@ then
>  	fi
>  fi
>  
> -merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
>  case "$rebase" in
>  true)
>  	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
> @@ -334,7 +333,7 @@ true)
>  	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
>  	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
>  	eval="$eval $gpg_sign_args"
> -	eval="$eval -m \"\$merge_name\" $merge_head"
> +	eval="$eval FETCH_HEAD"
>  	;;
>  esac
>  eval "exec $eval"
> 
> as we seem to special-case the name FETCH_HEAD. It assumes that
> git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
> but that seems safe. Unfortunately we still have to compute $merge_head
> ourselves here for the "git pull --rebase" case.

I agree that this is a better choice.  My concern with your other
suggestion is that it looks like it wouldn't honor the --no-edit flag or
GIT_MERGE_AUTOEDIT=no.  That might break some use cases, such as
non-interactive applications.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Jeff King]

On Sun, Apr 19, 2015 at 05:38:26PM +0000, brian m. carlson wrote:

> >  	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
> > @@ -334,7 +333,7 @@ true)
> >  	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
> >  	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
> >  	eval="$eval $gpg_sign_args"
> > -	eval="$eval -m \"\$merge_name\" $merge_head"
> > +	eval="$eval FETCH_HEAD"
> >  	;;
> >  esac
> >  eval "exec $eval"
> > 
> > as we seem to special-case the name FETCH_HEAD. It assumes that
> > git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
> > but that seems safe. Unfortunately we still have to compute $merge_head
> > ourselves here for the "git pull --rebase" case.
> 
> I agree that this is a better choice.  My concern with your other
> suggestion is that it looks like it wouldn't honor the --no-edit flag or
> GIT_MERGE_AUTOEDIT=no.  That might break some use cases, such as
> non-interactive applications.

Yeah, you're right. I think you could work around it by munging the
$edit variable as appropriate. But the whole thing is still gross. We
should be able to convince "git merge" to do the fmt-merge-msg bit for
us, and if not, then I think it needs to be extended.

-Peff

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> @@ -334,7 +333,7 @@ true)
>  	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
>  	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
>  	eval="$eval $gpg_sign_args"
> -	eval="$eval -m \"\$merge_name\" $merge_head"
> +	eval="$eval FETCH_HEAD"
>  	;;
>  esac
>  eval "exec $eval"
>
> as we seem to special-case the name FETCH_HEAD. It assumes that
> git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
> but that seems safe.

Unfortunately, "git merge"'s parsing of FETCH_HEAD forgets that we
may be creating an Octopus.  Otherwise the above should work well.

> Unfortunately we still have to compute $merge_head ourselves here
> for the "git pull --rebase" case.

That is not that unfortunate, I would say.

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Jeff King]

On Mon, Apr 20, 2015 at 11:59:04AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > @@ -334,7 +333,7 @@ true)
> >  	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
> >  	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
> >  	eval="$eval $gpg_sign_args"
> > -	eval="$eval -m \"\$merge_name\" $merge_head"
> > +	eval="$eval FETCH_HEAD"
> >  	;;
> >  esac
> >  eval "exec $eval"
> >
> > as we seem to special-case the name FETCH_HEAD. It assumes that
> > git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
> > but that seems safe.
> 
> Unfortunately, "git merge"'s parsing of FETCH_HEAD forgets that we
> may be creating an Octopus.  Otherwise the above should work well.

That sounds like a bug we should fix regardless.

> > Unfortunately we still have to compute $merge_head ourselves here
> > for the "git pull --rebase" case.
> 
> That is not that unfortunate, I would say.

I guess not. It is only a few lines of sed. And having the details there
does let us customize the error cases. My main worry would just be a
maintenance one: that somebody modifies git-pull to calculate merge_head
differently, but it turns out that we ignore it when calling git-merge.
But that's probably not that likely to matter in practice.

-Peff

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Apr 20, 2015 at 11:59:04AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > @@ -334,7 +333,7 @@ true)
>> >  	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
>> >  	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
>> >  	eval="$eval $gpg_sign_args"
>> > -	eval="$eval -m \"\$merge_name\" $merge_head"
>> > +	eval="$eval FETCH_HEAD"
>> >  	;;
>> >  esac
>> >  eval "exec $eval"
>> >
>> > as we seem to special-case the name FETCH_HEAD. It assumes that
>> > git-merge's parsing of FETCH_HEAD is the same as what we do in git-pull,
>> > but that seems safe.
>> 
>> Unfortunately, "git merge"'s parsing of FETCH_HEAD forgets that we
>> may be creating an Octopus.  Otherwise the above should work well.
>
> That sounds like a bug we should fix regardless.

But I am not sure how it should behave. "git fetch $there A B C"
followed by "git merge FETCH_HEAD" merges only A, and I do not know
if people have come to depend on this behaviour.

I suspect there may be larger fallout from such a change, namely,
what should "git log FETCH_HEAD" do?  Should it traverse the history
starting from all things that are not marked as not-for-merge, or
should it just say "git rev-parse FETCH_HEAD" and use only the first
one as the starting point?

I would argue that it would be more consistent with how we envision
the "git merge FETCH_HEAD" should work if "git log FETCH_HEAD"
traversed from all fetched HEAD for merging, but surely it is a huge
potential incompatibility.

For that matter, "git rev-parse FETCH_HEAD" and even get_sha1() should
yield all fetched HEAD for merging if we want to be consistent.  I
haven't thought this through yet but it does not look pretty.

[PATCH 00/14] Teach "git merge FETCH_HEAD" octopus merges

[Junio C Hamano]

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

> Jeff King <peff@peff.net> writes:
>> On Mon, Apr 20, 2015 at 11:59:04AM -0700, Junio C Hamano wrote:
>>
>>> Unfortunately, "git merge"'s parsing of FETCH_HEAD forgets that we
>>> may be creating an Octopus.  Otherwise the above should work well.
>>
>> That sounds like a bug we should fix regardless.
>
> But I am not sure how it should behave. "git fetch $there A B C"
> followed by "git merge FETCH_HEAD" merges only A, and I do not know
> if people have come to depend on this behaviour.
>
> I suspect there may be larger fallout from such a change, namely,
> what should "git log FETCH_HEAD" do?  Should it traverse the history
> starting from all things that are not marked as not-for-merge, or
> should it just say "git rev-parse FETCH_HEAD" and use only the first
> one as the starting point?

So, I thought we may want to try this and see how it goes.

Tentatively, I am saying that "FETCH_HEAD" is a magic token
understood by "git merge", like "git merge <msg> HEAD commits..."
syntax was a magic that made "git merge" work differently from "git
merge -m <msg> <commits>..."; no changes to get_sha1() or anything
heavy like that is intended.


Earlier, we thought that it would just be the matter of turning
existing invocation of "git merge <msg> HEAD $commits..." in "git
pull" into "git merge -m <msg> $commits..." to deprecate the ugly
original "merge" command line syntax.

This unfortunately failed for two reasons.

 * "-m <msg>" stops the editor from running; recent "git pull"
   encourage the users to justify the merge in the log message,
   and the auto-generated <msg> that comes from format-merge-msg
   should still be shown to the user in the editor to be edited.

 * "-m <msg>" also adds auto-generated summary when merge.log
   configuration is enabled, but "git pull" calls "git merge" with
   the message _with_ that log already in there.

Invoking "git merge FETCH_HEAD" (no messages fed by the caller) from
"git pull" almost works.  "git merge" knows how to extract the name
of the repository and the branch from FETCH_HEAD to use in the merge
log message it auto-generates.  However, this is done only for a
single branch; if you did "git pull $there topic-A topic-B", and
then invoked "git merge FETCH_HEAD" from there, we would end up
recording a merge with only one branch, which is not what we want.

This teaches "git merge FETCH_HEAD" that FETCH_HEAD may describe
multiple branches that were fetched for merging.  With that, the
last step eradicates the "merge <msg> HEAD <commit>..." syntax from
our codebase, finally.

This may be rough in a few places and some patches that are done as
preparatory clean-up steps may want to be squashed into the patch
that follows them that implements the real change.

These patches are designed to apply on top of v2.2.2; I'll push them
out on 'pu' later, on 'jc/merge' topic.

Junio C Hamano (14):
  merge: simplify code flow
  t5520: style fixes
  t5520: test pulling an octopus into an unborn branch
  merge: clarify "pulling into void" special case
  merge: do not check argc to determine number of remote heads
  merge: small leakfix and code simplification
  merge: clarify collect_parents() logic
  merge: split reduce_parents() out of collect_parents()
  merge: narrow scope of merge_names
  merge: extract prepare_merge_message() logic out
  merge: make collect_parents() auto-generate the merge message
  merge: decide if we auto-generate the message early in
    collect_parents()
  merge: handle FETCH_HEAD internally
  merge: deprecate 'git merge <message> HEAD <commit>' syntax

 Documentation/git-merge.txt   |   4 +
 builtin/merge.c               | 248 +++++++++++++++++++++++++++---------------
 git-cvsimport.perl            |   2 +-
 git-pull.sh                   |   3 +-
 t/t3402-rebase-merge.sh       |   2 +-
 t/t5520-pull.sh               |  31 +++---
 t/t6020-merge-df.sh           |   2 +-
 t/t6021-merge-criss-cross.sh  |   6 +-
 t/t9402-git-cvsserver-refs.sh |   2 +-
 9 files changed, 188 insertions(+), 112 deletions(-)

-- 
2.4.0-rc3-238-g36f5934

[PATCH 01/14] merge: simplify code flow

[Junio C Hamano]

One of the first things cmd_merge() does is to see if the "--abort"
option is given and run "reset --merge" and exit.  When the control
reaches this point, we know "--abort" was not given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index bebbe5b..8477878 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1165,15 +1165,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_commit = 0;
 	}
 
-	if (!abort_current_merge) {
-		if (!argc) {
-			if (default_to_upstream)
-				argc = setup_with_upstream(&argv);
-			else
-				die(_("No commit specified and merge.defaultToUpstream not set."));
-		} else if (argc == 1 && !strcmp(argv[0], "-"))
-			argv[0] = "@{-1}";
+	if (!argc) {
+		if (default_to_upstream)
+			argc = setup_with_upstream(&argv);
+		else
+			die(_("No commit specified and merge.defaultToUpstream not set."));
+	} else if (argc == 1 && !strcmp(argv[0], "-")) {
+		argv[0] = "@{-1}";
 	}
+
 	if (!argc)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
-- 
2.4.0-rc3-238-g36f5934

[PATCH 02/14] t5520: style fixes

[Junio C Hamano]

Fix style funnies in early part of this test script that checks "git
pull" into an unborn branch.  The primary change is that 'chdir' to
a newly created empty test repository is now protected by being done
in a subshell to make it more robust without having to chdir back to
the original place.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5520-pull.sh | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 227d293..5195a21 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -9,36 +9,27 @@ modify () {
 	mv "$2.x" "$2"
 }
 
-D=`pwd`
-
 test_expect_success setup '
-
 	echo file >file &&
 	git add file &&
 	git commit -a -m original
-
 '
 
 test_expect_success 'pulling into void' '
-	mkdir cloned &&
-	cd cloned &&
-	git init &&
-	git pull ..
-'
-
-cd "$D"
-
-test_expect_success 'checking the results' '
+	git init cloned &&
+	(
+		cd cloned &&
+		git pull ..
+	) &&
 	test -f file &&
 	test -f cloned/file &&
 	test_cmp file cloned/file
 '
 
 test_expect_success 'pulling into void using master:master' '
-	mkdir cloned-uho &&
+	git init cloned-uho &&
 	(
 		cd cloned-uho &&
-		git init &&
 		git pull .. master:master
 	) &&
 	test -f file &&
@@ -71,7 +62,6 @@ test_expect_success 'pulling into void does not overwrite staged files' '
 	)
 '
 
-
 test_expect_success 'pulling into void does not remove new staged files' '
 	git init cloned-staged-new &&
 	(
-- 
2.4.0-rc3-238-g36f5934

[PATCH 03/14] t5520: test pulling an octopus into an unborn branch

[Junio C Hamano]

The code comment for "git merge" in builtin/merge.c, we say

    If the merged head is a valid one there is no reason
    to forbid "git merge" into a branch yet to be born.
    We do the same for "git pull".

and t5520 does have an existing test for that behaviour.  However,
there was no test to make sure that 'git pull' to pull multiple
branches into an unborn branch must fail.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5520-pull.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 5195a21..7efd45b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -76,6 +76,15 @@ test_expect_success 'pulling into void does not remove new staged files' '
 	)
 '
 
+test_expect_success 'pulling into void must not create an octopus' '
+	git init cloned-octopus &&
+	(
+		cd cloned-octopus &&
+		test_must_fail git pull .. master master &&
+		! test -f file
+	)
+'
+
 test_expect_success 'test . as a remote' '
 
 	git branch copy master &&
-- 
2.4.0-rc3-238-g36f5934

[PATCH 04/14] merge: clarify "pulling into void" special case

[Junio C Hamano]

Instead of having it as one of the three if/elseif/.. case arms,
test the condition and handle this special case upfront.  This makes
it easier to follow the flow of logic.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 8477878..878f82a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1178,23 +1178,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
 
-	/*
-	 * This could be traditional "merge <msg> HEAD <commit>..."  and
-	 * the way we can tell it is to see if the second token is HEAD,
-	 * but some people might have misused the interface and used a
-	 * commit-ish that is the same as HEAD there instead.
-	 * Traditional format never would have "-m" so it is an
-	 * additional safety measure to check for it.
-	 */
-
-	if (!have_message && head_commit &&
-	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
-		strbuf_addstr(&merge_msg, argv[0]);
-		head_arg = argv[1];
-		argv += 2;
-		argc -= 2;
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-	} else if (!head_commit) {
+	if (!head_commit) {
 		struct commit *remote_head;
 		/*
 		 * If the merged head is a valid one there is no reason
@@ -1217,6 +1201,23 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		update_ref("initial pull", "HEAD", remote_head->object.sha1,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 		goto done;
+	}
+
+	/*
+	 * This could be traditional "merge <msg> HEAD <commit>..."  and
+	 * the way we can tell it is to see if the second token is HEAD,
+	 * but some people might have misused the interface and used a
+	 * commit-ish that is the same as HEAD there instead.
+	 * Traditional format never would have "-m" so it is an
+	 * additional safety measure to check for it.
+	 */
+	if (!have_message &&
+	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
+		strbuf_addstr(&merge_msg, argv[0]);
+		head_arg = argv[1];
+		argv += 2;
+		argc -= 2;
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else {
 		struct strbuf merge_names = STRBUF_INIT;
 
-- 
2.4.0-rc3-238-g36f5934

[PATCH 05/14] merge: do not check argc to determine number of remote heads

[Junio C Hamano]

To reject merging multiple commits into an unborn branch, we check
argc, thinking that collect_parents() that reads the remaining
command line arguments from <argc, argv> will give us the same
number of commits as its input, i.e. argc.

Because what we really care about is the number of commits, let the
function run and then make sure it returns only one commit instead.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 878f82a..1d4fbd3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1185,9 +1185,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to forbid "git merge" into a branch yet to be born.
 		 * We do the same for "git pull".
 		 */
-		if (argc != 1)
-			die(_("Can merge only exactly one commit into "
-				"empty head"));
 		if (squash)
 			die(_("Squash commit into empty head not supported yet"));
 		if (fast_forward == FF_NO)
@@ -1197,6 +1194,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
+		if (remoteheads->next)
+			die(_("Can merge only exactly one commit into empty head"));
 		read_empty(remote_head->object.sha1, 0);
 		update_ref("initial pull", "HEAD", remote_head->object.sha1,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
-- 
2.4.0-rc3-238-g36f5934

[PATCH 06/14] merge: small leakfix and code simplification

[Junio C Hamano]

When parsing a merged object name like "foo~20" to formulate a merge
summary "Merge branch foo (early part)", a temporary strbuf is used,
but we forgot to deallocate it when we failed to find the named
branch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 1d4fbd3..b2d0332 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -491,8 +491,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	}
 	if (len) {
 		struct strbuf truname = STRBUF_INIT;
-		strbuf_addstr(&truname, "refs/heads/");
-		strbuf_addstr(&truname, remote);
+		strbuf_addf(&truname, "refs/heads/%s", remote);
 		strbuf_setlen(&truname, truname.len - len);
 		if (ref_exists(truname.buf)) {
 			strbuf_addf(msg,
@@ -503,6 +502,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 			strbuf_release(&truname);
 			goto cleanup;
 		}
+		strbuf_release(&truname);
 	}
 
 	if (!strcmp(remote, "FETCH_HEAD") &&
-- 
2.4.0-rc3-238-g36f5934

[PATCH 07/14] merge: clarify collect_parents() logic

[Junio C Hamano]

Clarify this small function in three ways.

 - The function initially collects all commits to be merged into a
   commit_list "remoteheads"; the "remotes" pointer always points at
   the tail of this list (either the remoteheads variable itself, or
   the ->next slot of the element at the end of the list) to help
   elongate the list by repeated calls to commit_list_insert().
   Because the new element appended by commit_list_insert() will
   always have its ->next slot NULLed out, there is no need for us
   to assign NULL to *remotes to terminate the list at the end.

 - The variable "head_subsumed" always confused me every time I read
   this code.  What is happening here is that we inspect what the
   caller told us to merge (including the current HEAD) and come up
   with the list of parents to be recorded for the resulting merge
   commit, omitting commits that are ancestor of other commits.
   This filtering may remove the current HEAD from the resulting
   parent list---and we signal that fact with this variable, so that
   we can later record it as the first parent when "--no-ff" is in
   effect.

 - The "parents" list is created for this function by reduce_heads()
   and was not deallocated after its use, even though the loop
   control was written in such a way to allow us to do so by taking
   the "next" element in a separate variable so that it can be used
   in the next-step part of the loop control.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index b2d0332..d2e36e2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1061,11 +1061,19 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 					 "not something we can merge");
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
-	*remotes = NULL;
 
+	/*
+	 * Is the current HEAD reachable from another commit being
+	 * merged?  If so we do not want to record it as a parent of
+	 * the resulting merge, unless --no-ff is given.  We will flip
+	 * this variable to 0 when we find HEAD among the independent
+	 * tips being merged.
+	 */
+	*head_subsumed = 1;
+
+	/* Find what parents to record by checking independent ones. */
 	parents = reduce_heads(remoteheads);
 
-	*head_subsumed = 1; /* we will flip this to 0 when we find it */
 	for (remoteheads = NULL, remotes = &remoteheads;
 	     parents;
 	     parents = next) {
@@ -1075,6 +1083,7 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 			*head_subsumed = 0;
 		else
 			remotes = &commit_list_insert(commit, remotes)->next;
+		free(parents);
 	}
 	return remoteheads;
 }
-- 
2.4.0-rc3-238-g36f5934

[PATCH 08/14] merge: split reduce_parents() out of collect_parents()

[Junio C Hamano]

The latter does two separate things:

 - Parse the list of commits on the command line, and formulate the
   list of commits to be merged (including the current HEAD);

 - Compute the list of parents to be recorded in the resulting merge
   commit.

Split the latter into a separate helper function, so that we can
later supply the list commits to be merged from a different source
(namely, FETCH_HEAD).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index d2e36e2..054f052 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1044,23 +1044,11 @@ static int default_edit_option(void)
 		st_stdin.st_mode == st_stdout.st_mode);
 }
 
-static struct commit_list *collect_parents(struct commit *head_commit,
-					   int *head_subsumed,
-					   int argc, const char **argv)
+static struct commit_list *reduce_parents(struct commit *head_commit,
+					  int *head_subsumed,
+					  struct commit_list *remoteheads)
 {
-	int i;
-	struct commit_list *remoteheads = NULL, *parents, *next;
-	struct commit_list **remotes = &remoteheads;
-
-	if (head_commit)
-		remotes = &commit_list_insert(head_commit, remotes)->next;
-	for (i = 0; i < argc; i++) {
-		struct commit *commit = get_merge_parent(argv[i]);
-		if (!commit)
-			help_unknown_ref(argv[i], "merge",
-					 "not something we can merge");
-		remotes = &commit_list_insert(commit, remotes)->next;
-	}
+	struct commit_list *parents, *next, **remotes = &remoteheads;
 
 	/*
 	 * Is the current HEAD reachable from another commit being
@@ -1088,6 +1076,27 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 	return remoteheads;
 }
 
+static struct commit_list *collect_parents(struct commit *head_commit,
+					   int *head_subsumed,
+					   int argc, const char **argv)
+{
+	int i;
+	struct commit_list *remoteheads = NULL;
+	struct commit_list **remotes = &remoteheads;
+
+	if (head_commit)
+		remotes = &commit_list_insert(head_commit, remotes)->next;
+	for (i = 0; i < argc; i++) {
+		struct commit *commit = get_merge_parent(argv[i]);
+		if (!commit)
+			help_unknown_ref(argv[i], "merge",
+					 "not something we can merge");
+		remotes = &commit_list_insert(commit, remotes)->next;
+	}
+
+	return reduce_parents(head_commit, head_subsumed, remoteheads);
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	unsigned char result_tree[20];
-- 
2.4.0-rc3-238-g36f5934

[PATCH 09/14] merge: narrow scope of merge_names

[Junio C Hamano]

In order to pass the list of parents to fmt_merge_msg(), cmd_merge()
uses this strbuf to create something that look like FETCH_HEAD that
describes commits that are being merged.  This is necessary only
when we are creating the merge commit message ourselves, but was
done unconditionally.

Move the variable and the logic to populate it to confine them in a
block that needs them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 054f052..d853c9d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1236,8 +1236,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		argc -= 2;
 		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else {
-		struct strbuf merge_names = STRBUF_INIT;
-
 		/* We are invoked directly as the first-class UI. */
 		head_arg = "HEAD";
 
@@ -1247,11 +1245,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to the given message.
 		 */
 		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, &merge_names);
 
 		if (!have_message || shortlog_len) {
+			struct strbuf merge_names = STRBUF_INIT;
 			struct fmt_merge_msg_opts opts;
+
+			for (p = remoteheads; p; p = p->next)
+				merge_name(merge_remote_util(p->item)->name, &merge_names);
+
 			memset(&opts, 0, sizeof(opts));
 			opts.add_title = !have_message;
 			opts.shortlog_len = shortlog_len;
@@ -1260,6 +1261,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			fmt_merge_msg(&merge_names, &merge_msg, &opts);
 			if (merge_msg.len)
 				strbuf_setlen(&merge_msg, merge_msg.len - 1);
+
+			strbuf_release(&merge_names);
 		}
 	}
 
-- 
2.4.0-rc3-238-g36f5934

[PATCH 10/14] merge: extract prepare_merge_message() logic out

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index d853c9d..a972ed6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1076,6 +1076,20 @@ static struct commit_list *reduce_parents(struct commit *head_commit,
 	return remoteheads;
 }
 
+static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *merge_msg)
+{
+	struct fmt_merge_msg_opts opts;
+
+	memset(&opts, 0, sizeof(opts));
+	opts.add_title = !have_message;
+	opts.shortlog_len = shortlog_len;
+	opts.credit_people = (0 < option_edit);
+
+	fmt_merge_msg(merge_names, merge_msg, &opts);
+	if (merge_msg->len)
+		strbuf_setlen(merge_msg, merge_msg->len - 1);
+}
+
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
 					   int argc, const char **argv)
@@ -1248,20 +1262,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		if (!have_message || shortlog_len) {
 			struct strbuf merge_names = STRBUF_INIT;
-			struct fmt_merge_msg_opts opts;
 
 			for (p = remoteheads; p; p = p->next)
 				merge_name(merge_remote_util(p->item)->name, &merge_names);
-
-			memset(&opts, 0, sizeof(opts));
-			opts.add_title = !have_message;
-			opts.shortlog_len = shortlog_len;
-			opts.credit_people = (0 < option_edit);
-
-			fmt_merge_msg(&merge_names, &merge_msg, &opts);
-			if (merge_msg.len)
-				strbuf_setlen(&merge_msg, merge_msg.len - 1);
-
+			prepare_merge_message(&merge_names, &merge_msg);
 			strbuf_release(&merge_names);
 		}
 	}
-- 
2.4.0-rc3-238-g36f5934

[PATCH 11/14] merge: make collect_parents() auto-generate the merge message

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a972ed6..84ebb22 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1092,7 +1092,8 @@ static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *mer
 
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
-					   int argc, const char **argv)
+					   int argc, const char **argv,
+					   struct strbuf *merge_msg)
 {
 	int i;
 	struct commit_list *remoteheads = NULL;
@@ -1108,7 +1109,19 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
 
-	return reduce_parents(head_commit, head_subsumed, remoteheads);
+	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+
+	if (merge_msg &&
+	    (!have_message || shortlog_len)) {
+		struct strbuf merge_names = STRBUF_INIT;
+
+		for (p = remoteheads; p; p = p->next)
+			merge_name(merge_remote_util(p->item)->name, &merge_names);
+		prepare_merge_message(&merge_names, merge_msg);
+		strbuf_release(&merge_names);
+	}
+
+	return remoteheads;
 }
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
@@ -1222,7 +1235,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (fast_forward == FF_NO)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, NULL);
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
@@ -1248,7 +1262,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_arg = argv[1];
 		argv += 2;
 		argc -= 2;
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, NULL);
 	} else {
 		/* We are invoked directly as the first-class UI. */
 		head_arg = "HEAD";
@@ -1258,16 +1273,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * the standard merge summary message to be appended
 		 * to the given message.
 		 */
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-
-		if (!have_message || shortlog_len) {
-			struct strbuf merge_names = STRBUF_INIT;
-
-			for (p = remoteheads; p; p = p->next)
-				merge_name(merge_remote_util(p->item)->name, &merge_names);
-			prepare_merge_message(&merge_names, &merge_msg);
-			strbuf_release(&merge_names);
-		}
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, &merge_msg);
 	}
 
 	if (!head_commit || !argc)
-- 
2.4.0-rc3-238-g36f5934

[PATCH 12/14] merge: decide if we auto-generate the message early in collect_parents()

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 84ebb22..c7d9d6e 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1098,6 +1098,10 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 	int i;
 	struct commit_list *remoteheads = NULL;
 	struct commit_list **remotes = &remoteheads;
+	struct strbuf merge_names = STRBUF_INIT, *autogen = NULL;
+
+	if (merge_msg && (!have_message || shortlog_len))
+		autogen = &merge_names;
 
 	if (head_commit)
 		remotes = &commit_list_insert(head_commit, remotes)->next;
@@ -1111,14 +1115,12 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
 
-	if (merge_msg &&
-	    (!have_message || shortlog_len)) {
-		struct strbuf merge_names = STRBUF_INIT;
-
+	if (autogen) {
 		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, &merge_names);
-		prepare_merge_message(&merge_names, merge_msg);
-		strbuf_release(&merge_names);
+			merge_name(merge_remote_util(p->item)->name, autogen);
+
+		prepare_merge_message(autogen, merge_msg);
+		strbuf_release(autogen);
 	}
 
 	return remoteheads;
-- 
2.4.0-rc3-238-g36f5934

[PATCH 13/14] merge: handle FETCH_HEAD internally

[Junio C Hamano]

The collect_parents() function now is responsible for

 1. parsing the commits given on the command line into a list of
    commits to be merged;

 2. filtering these parents into independent ones; and

 3. optionally calling fmt_merge_msg() via prepare_merge_message()
    to prepare an auto-generated merge log message, using fake
    contents that FETCH_HEAD would have had if these commits were
    fetched from the current repository with "git pull . $args..."

Make "git merge FETCH_HEAD" to be the same as the traditional

    git merge "$(git fmt-merge-msg <.git/FETCH_HEAD)" $commits

invocation of the command in "git pull", where $commits are the ones
that appear in FETCH_HEAD that are not marked as not-for-merge, by
making it do a bit more, specifically:

 - noticing "FETCH_HEAD" is the only "commit" on the command line
   and picking the commits that are not marked as not-for-merge as
   the list of commits to be merged (substitute for step #1 above);

 - letting the resulting list fed to step #2 above;

 - doing the step #3 above, using the contents of the FETCH_HEAD
   instead of fake contents crafted from the list of commits parsed
   in the step #1 above.

Note that this changes the semantics.  "git merge FETCH_HEAD" has
always behaved as if the first commit in the FETCH_HEAD file were
directly specified on the command line, creating a two-way merge
whose auto-generated merge log said "merge commit xyz".  With this
change, if the previous fetch was to grab multiple branches (e.g.
"git fetch $there topic-a topic-b"), the new world order is to
create an octopus, behaving as if "git pull $there topic-a topic-b"
were run.  This is a deliberate change to make that happen.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-merge.txt |   4 ++
 builtin/merge.c             | 105 ++++++++++++++++++++++++++++++--------------
 2 files changed, 76 insertions(+), 33 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index cf2c374..d9aa6b6 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -104,6 +104,10 @@ commit or stash your changes before running 'git merge'.
 If no commit is given from the command line, merge the remote-tracking
 branches that the current branch is configured to use as its upstream.
 See also the configuration section of this manual page.
++
+When `FETCH_HEAD` (and no other commit) is specified, the branches
+recorded in the `.git/FETCH_HEAD` file by the previous invocation
+of `git fetch` for merging are merged to the current branch.
 
 
 PRE-MERGE CHECKS
diff --git a/builtin/merge.c b/builtin/merge.c
index c7d9d6e..42f9fcc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -505,28 +505,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		strbuf_release(&truname);
 	}
 
-	if (!strcmp(remote, "FETCH_HEAD") &&
-			!access(git_path("FETCH_HEAD"), R_OK)) {
-		const char *filename;
-		FILE *fp;
-		struct strbuf line = STRBUF_INIT;
-		char *ptr;
-
-		filename = git_path("FETCH_HEAD");
-		fp = fopen(filename, "r");
-		if (!fp)
-			die_errno(_("could not open '%s' for reading"),
-				  filename);
-		strbuf_getline(&line, fp, '\n');
-		fclose(fp);
-		ptr = strstr(line.buf, "\tnot-for-merge\t");
-		if (ptr)
-			strbuf_remove(&line, ptr-line.buf+1, 13);
-		strbuf_addbuf(msg, &line);
-		strbuf_release(&line);
-		goto cleanup;
-	}
-
 	if (remote_head->util) {
 		struct merge_remote_desc *desc;
 		desc = merge_remote_util(remote_head);
@@ -1090,6 +1068,60 @@ static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *mer
 		strbuf_setlen(merge_msg, merge_msg->len - 1);
 }
 
+static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge_names)
+{
+	const char *filename;
+	int fd, pos, npos;
+	struct strbuf fetch_head_file = STRBUF_INIT;
+
+	if (!merge_names)
+		merge_names = &fetch_head_file;
+
+	filename = git_path("FETCH_HEAD");
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		die_errno(_("could not open '%s' for reading"), filename);
+
+	if (strbuf_read(merge_names, fd, 0) < 0)
+		die_errno(_("could not read '%s'"), filename);
+	if (close(fd) < 0)
+		die_errno(_("could not close '%s'"), filename);
+
+	for (pos = 0; pos < merge_names->len; pos = npos) {
+		unsigned char sha1[20];
+		char *ptr;
+		struct commit *commit;
+
+		ptr = strchr(merge_names->buf + pos, '\n');
+		if (ptr)
+			npos = ptr - merge_names->buf + 1;
+		else
+			npos = merge_names->len;
+
+		if (npos - pos < 40 + 2 ||
+		    get_sha1_hex(merge_names->buf + pos, sha1))
+			commit = NULL; /* bad */
+		else if (memcmp(merge_names->buf + pos + 40, "\t\t", 2))
+			continue; /* not-for-merge */
+		else {
+			char saved = merge_names->buf[pos + 40];
+			merge_names->buf[pos + 40] = '\0';
+			commit = get_merge_parent(merge_names->buf + pos);
+			merge_names->buf[pos + 40] = saved;
+		}
+		if (!commit) {
+			if (ptr)
+				*ptr = '\0';
+			die("not something we can merge in %s: %s",
+			    filename, merge_names->buf + pos);
+		}
+		remotes = &commit_list_insert(commit, remotes)->next;
+	}
+
+	if (merge_names == &fetch_head_file)
+		strbuf_release(&fetch_head_file);
+}
+
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
 					   int argc, const char **argv,
@@ -1105,20 +1137,27 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 	if (head_commit)
 		remotes = &commit_list_insert(head_commit, remotes)->next;
-	for (i = 0; i < argc; i++) {
-		struct commit *commit = get_merge_parent(argv[i]);
-		if (!commit)
-			help_unknown_ref(argv[i], "merge",
-					 "not something we can merge");
-		remotes = &commit_list_insert(commit, remotes)->next;
-	}
 
-	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+	if (argc == 1 && !strcmp(argv[0], "FETCH_HEAD")) {
+		handle_fetch_head(remotes, autogen);
+		remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+	} else {
+		for (i = 0; i < argc; i++) {
+			struct commit *commit = get_merge_parent(argv[i]);
+			if (!commit)
+				help_unknown_ref(argv[i], "merge",
+						 "not something we can merge");
+			remotes = &commit_list_insert(commit, remotes)->next;
+		}
+		remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+		if (autogen) {
+			struct commit_list *p;
+			for (p = remoteheads; p; p = p->next)
+				merge_name(merge_remote_util(p->item)->name, autogen);
+		}
+	}
 
 	if (autogen) {
-		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, autogen);
-
 		prepare_merge_message(autogen, merge_msg);
 		strbuf_release(autogen);
 	}
-- 
2.4.0-rc3-238-g36f5934

[PATCH 14/14] merge: deprecate 'git merge <message> HEAD <commit>' syntax

[Junio C Hamano]

We had this in "git merge" manual for eternity:

    'git merge' <msg> HEAD <commit>...

    [This] syntax (<msg> `HEAD` <commit>...) is supported for
    historical reasons.  Do not use it from the command line or in
    new scripts.  It is the same as `git merge -m <msg> <commit>...`.

With the update to "git merge" to make it understand what is
recorded in FETCH_HEAD directly, including Octopus merge cases, we
now can rewrite the use of this syntax in "git pull" with a simple
"git merge FETCH_HEAD".

Also there are quite a few fallouts in the test scripts, and it
turns out that "git cvsimport" also uses this old syntax to record
a merge.

Judging from this result, I would not be surprised if dropping the
support of the old syntax broke scripts people have written and been
relying on for the past ten years.  But at least we can start the
deprecation process by throwing a warning message when the syntax is
used.

With luck, we might be able to drop the support in a few years.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c               | 1 +
 git-cvsimport.perl            | 2 +-
 git-pull.sh                   | 3 +--
 t/t3402-rebase-merge.sh       | 2 +-
 t/t6020-merge-df.sh           | 2 +-
 t/t6021-merge-criss-cross.sh  | 6 +++---
 t/t9402-git-cvsserver-refs.sh | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 42f9fcc..67fbfaf 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1299,6 +1299,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 */
 	if (!have_message &&
 	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
+		warning("old-style 'git merge <msg> HEAD <commit>' is deprecated.");
 		strbuf_addstr(&merge_msg, argv[0]);
 		head_arg = argv[1];
 		argv += 2;
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 73d367c..82ecb03 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -1162,7 +1162,7 @@ sub commit {
 		die "Fast-forward update failed: $?\n" if $?;
 	}
 	else {
-		system(qw(git merge cvsimport HEAD), "$remote/$opt_o");
+		system(qw(git merge -m cvsimport), "$remote/$opt_o");
 		die "Could not merge $opt_o into the current branch.\n" if $?;
 	}
 } else {
diff --git a/git-pull.sh b/git-pull.sh
index 4d4fc77..15d9431 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,6 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +333,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval \"\$merge_name\" HEAD $merge_head"
+	eval="$eval FETCH_HEAD"
 	;;
 esac
 eval "exec $eval"
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 5a27ec9..8f64505 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -47,7 +47,7 @@ test_expect_success setup '
 '
 
 test_expect_success 'reference merge' '
-	git merge -s recursive "reference merge" HEAD master
+	git merge -s recursive -m "reference merge" master
 '
 
 PRE_REBASE=$(git rev-parse test-rebase)
diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh
index 27c3d73..2af1bee 100755
--- a/t/t6020-merge-df.sh
+++ b/t/t6020-merge-df.sh
@@ -24,7 +24,7 @@ test_expect_success 'prepare repository' '
 '
 
 test_expect_success 'Merge with d/f conflicts' '
-	test_expect_code 1 git merge "merge msg" B master
+	test_expect_code 1 git merge -m "merge msg" master
 '
 
 test_expect_success 'F/D conflict' '
diff --git a/t/t6021-merge-criss-cross.sh b/t/t6021-merge-criss-cross.sh
index d15b313..213deec 100755
--- a/t/t6021-merge-criss-cross.sh
+++ b/t/t6021-merge-criss-cross.sh
@@ -48,7 +48,7 @@ echo "1
 " > file &&
 git commit -m "C3" file &&
 git branch C3 &&
-git merge "pre E3 merge" B A &&
+git merge -m "pre E3 merge" A &&
 echo "1
 2
 3 changed in E3, branch B. New file size
@@ -61,7 +61,7 @@ echo "1
 " > file &&
 git commit -m "E3" file &&
 git checkout A &&
-git merge "pre D8 merge" A C3 &&
+git merge -m "pre D8 merge" C3 &&
 echo "1
 2
 3 changed in C3, branch B
@@ -73,7 +73,7 @@ echo "1
 9" > file &&
 git commit -m D8 file'
 
-test_expect_success 'Criss-cross merge' 'git merge "final merge" A B'
+test_expect_success 'Criss-cross merge' 'git merge -m "final merge" B'
 
 cat > file-expect <<EOF
 1
diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
index 1e266ef..d00df08 100755
--- a/t/t9402-git-cvsserver-refs.sh
+++ b/t/t9402-git-cvsserver-refs.sh
@@ -496,7 +496,7 @@ test_expect_success 'check [cvswork3] diff' '
 '
 
 test_expect_success 'merge early [cvswork3] b3 with b1' '
-	( cd gitwork3 && git merge "message" HEAD b1 ) &&
+	( cd gitwork3 && git merge -m "message" b1 ) &&
 	git fetch gitwork3 b3:b3 &&
 	git tag v3merged b3 &&
 	git push --tags gitcvs.git b3:b3
-- 
2.4.0-rc3-238-g36f5934

[PATCH v2 00/15] Teach "git merge FETCH_HEAD" octopus merges

[Junio C Hamano]

This series is an attempt to make these two operations truly equivalent:

    $ git pull . topic-a topic-b...

    $ git fetch . topic-a topic-b...
    $ git merge FETCH_HEAD

Compared to the previous one ($gmane/267809), there are only a few
minor changes:

 - The first patch is new; it adds tests to illustrate what the
   current code does.  Accordingly, the penultimate patch is the
   same as the previous, but updates the tests that expect failure
   in this step to expect success.

 - One step failed to compile (the offending code was removed in a
   later patch and the endgame did not break the build), which has
   been corrected.

Junio C Hamano (15):
  merge: test the top-level merge driver
  merge: simplify code flow
  t5520: style fixes
  t5520: test pulling an octopus into an unborn branch
  merge: clarify "pulling into void" special case
  merge: do not check argc to determine number of remote heads
  merge: small leakfix and code simplification
  merge: clarify collect_parents() logic
  merge: split reduce_parents() out of collect_parents()
  merge: narrow scope of merge_names
  merge: extract prepare_merge_message() logic out
  merge: make collect_parents() auto-generate the merge message
  merge: decide if we auto-generate the message early in collect_parents()
  merge: handle FETCH_HEAD internally
  merge: deprecate 'git merge <message> HEAD <commit>' syntax

 Documentation/git-merge.txt   |   4 +
 builtin/merge.c               | 248 +++++++++++++++++++++++++++---------------
 git-cvsimport.perl            |   2 +-
 git-pull.sh                   |   3 +-
 t/t3033-merge-toplevel.sh     | 136 +++++++++++++++++++++++
 t/t3402-rebase-merge.sh       |   2 +-
 t/t5520-pull.sh               |  31 +++---
 t/t6020-merge-df.sh           |   2 +-
 t/t6021-merge-criss-cross.sh  |   6 +-
 t/t9402-git-cvsserver-refs.sh |   2 +-
 10 files changed, 324 insertions(+), 112 deletions(-)
 create mode 100755 t/t3033-merge-toplevel.sh

-- 
2.4.0-rc3-300-g052d062

[PATCH v2 01/15] merge: test the top-level merge driver

[Junio C Hamano]

We seem to have tests for specific merge strategy backends
(e.g. recursive), but not much test coverage for the "git merge"
itself.  As I am planning to update the semantics of merging
"FETCH_HEAD" in such a way that these two

    git pull . topic_a topic_b...

vs.

    git fetch . topic_a topic_b...
    git merge FETCH_HEAD

are truly equivalent, let me add a few test cases to cover the
tricky ones.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t3033-merge-toplevel.sh | 136 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 136 insertions(+)
 create mode 100755 t/t3033-merge-toplevel.sh

diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
new file mode 100755
index 0000000..9d92d3c
--- /dev/null
+++ b/t/t3033-merge-toplevel.sh
@@ -0,0 +1,136 @@
+#!/bin/sh
+
+test_description='"git merge" top-level frontend'
+
+. ./test-lib.sh
+
+t3033_reset () {
+	git checkout -B master two &&
+	git branch -f left three &&
+	git branch -f right four
+}
+
+test_expect_success setup '
+	test_commit one &&
+	git branch left &&
+	git branch right &&
+	test_commit two &&
+	git checkout left &&
+	test_commit three &&
+	git checkout right &&
+	test_commit four &&
+	git checkout master
+'
+
+# Local branches
+
+test_expect_success 'merge an octopus into void' '
+	t3033_reset &&
+	git checkout --orphan test &&
+	git rm -fr . &&
+	test_must_fail git merge left right &&
+	test_must_fail git rev-parse --verify HEAD &&
+	git diff --quiet &&
+	test_must_fail git rev-parse HEAD
+'
+
+test_expect_success 'merge an octopus, fast-forward (ff)' '
+	t3033_reset &&
+	git reset --hard one &&
+	git merge left right &&
+	# one is ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^3 &&
+	git rev-parse HEAD^1 HEAD^2 | sort >actual &&
+	git rev-parse three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'merge octopus, non-fast-forward (ff)' '
+	t3033_reset &&
+	git reset --hard one &&
+	git merge --no-ff left right &&
+	# one is ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse one three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'merge octopus, fast-forward (does not ff)' '
+	t3033_reset &&
+	git merge left right &&
+	# two (master) is not an ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse two three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'merge octopus, non-fast-forward' '
+	t3033_reset &&
+	git merge --no-ff left right &&
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse two three four | sort >expect &&
+	test_cmp expect actual
+'
+
+# The same set with FETCH_HEAD
+
+test_expect_failure 'merge FETCH_HEAD octopus into void' '
+	t3033_reset &&
+	git checkout --orphan test &&
+	git rm -fr . &&
+	git fetch . left right &&
+	test_must_fail git merge FETCH_HEAD &&
+	test_must_fail git rev-parse --verify HEAD &&
+	git diff --quiet &&
+	test_must_fail git rev-parse HEAD
+'
+
+test_expect_failure 'merge FETCH_HEAD octopus fast-forward (ff)' '
+	t3033_reset &&
+	git reset --hard one &&
+	git fetch . left right &&
+	git merge FETCH_HEAD &&
+	# one is ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^3 &&
+	git rev-parse HEAD^1 HEAD^2 | sort >actual &&
+	git rev-parse three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'merge FETCH_HEAD octopus non-fast-forward (ff)' '
+	t3033_reset &&
+	git reset --hard one &&
+	git fetch . left right &&
+	git merge --no-ff FETCH_HEAD &&
+	# one is ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse one three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'merge FETCH_HEAD octopus fast-forward (does not ff)' '
+	t3033_reset &&
+	git fetch . left right &&
+	git merge FETCH_HEAD &&
+	# two (master) is not an ancestor of three (left) and four (right)
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse two three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'merge FETCH_HEAD octopus non-fast-forward' '
+	t3033_reset &&
+	git fetch . left right &&
+	git merge --no-ff FETCH_HEAD &&
+	test_must_fail git rev-parse --verify HEAD^4 &&
+	git rev-parse HEAD^1 HEAD^2 HEAD^3 | sort >actual &&
+	git rev-parse two three four | sort >expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 02/15] merge: simplify code flow

[Junio C Hamano]

One of the first things cmd_merge() does is to see if the "--abort"
option is given and run "reset --merge" and exit.  When the control
reaches this point, we know "--abort" was not given.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index bebbe5b..8477878 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1165,15 +1165,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		option_commit = 0;
 	}
 
-	if (!abort_current_merge) {
-		if (!argc) {
-			if (default_to_upstream)
-				argc = setup_with_upstream(&argv);
-			else
-				die(_("No commit specified and merge.defaultToUpstream not set."));
-		} else if (argc == 1 && !strcmp(argv[0], "-"))
-			argv[0] = "@{-1}";
+	if (!argc) {
+		if (default_to_upstream)
+			argc = setup_with_upstream(&argv);
+		else
+			die(_("No commit specified and merge.defaultToUpstream not set."));
+	} else if (argc == 1 && !strcmp(argv[0], "-")) {
+		argv[0] = "@{-1}";
 	}
+
 	if (!argc)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 03/15] t5520: style fixes

[Junio C Hamano]

Fix style funnies in early part of this test script that checks "git
pull" into an unborn branch.  The primary change is that 'chdir' to
a newly created empty test repository is now protected by being done
in a subshell to make it more robust without having to chdir back to
the original place.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5520-pull.sh | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 227d293..5195a21 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -9,36 +9,27 @@ modify () {
 	mv "$2.x" "$2"
 }
 
-D=`pwd`
-
 test_expect_success setup '
-
 	echo file >file &&
 	git add file &&
 	git commit -a -m original
-
 '
 
 test_expect_success 'pulling into void' '
-	mkdir cloned &&
-	cd cloned &&
-	git init &&
-	git pull ..
-'
-
-cd "$D"
-
-test_expect_success 'checking the results' '
+	git init cloned &&
+	(
+		cd cloned &&
+		git pull ..
+	) &&
 	test -f file &&
 	test -f cloned/file &&
 	test_cmp file cloned/file
 '
 
 test_expect_success 'pulling into void using master:master' '
-	mkdir cloned-uho &&
+	git init cloned-uho &&
 	(
 		cd cloned-uho &&
-		git init &&
 		git pull .. master:master
 	) &&
 	test -f file &&
@@ -71,7 +62,6 @@ test_expect_success 'pulling into void does not overwrite staged files' '
 	)
 '
 
-
 test_expect_success 'pulling into void does not remove new staged files' '
 	git init cloned-staged-new &&
 	(
-- 
2.4.0-rc3-300-g052d062

Re: [PATCH v2 03/15] t5520: style fixes

[Paul Tan]

Hi Junio,

On Thu, Apr 30, 2015 at 5:29 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Fix style funnies in early part of this test script that checks "git
> pull" into an unborn branch.  The primary change is that 'chdir' to
> a newly created empty test repository is now protected by being done
> in a subshell to make it more robust without having to chdir back to
> the original place.
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 227d293..5195a21 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
>  test_expect_success 'pulling into void' '
> -       mkdir cloned &&
> -       cd cloned &&
> -       git init &&
> -       git pull ..
> -'
> -
> -cd "$D"
> -
> -test_expect_success 'checking the results' '
> +       git init cloned &&
> +       (
> +               cd cloned &&
> +               git pull ..
> +       ) &&
>         test -f file &&
>         test -f cloned/file &&
>         test_cmp file cloned/file
>  '
>
>  test_expect_success 'pulling into void using master:master' '
> -       mkdir cloned-uho &&
> +       git init cloned-uho &&
>         (
>                 cd cloned-uho &&
> -               git init &&
>                 git pull .. master:master
>         ) &&
>         test -f file &&
> @@ -71,7 +62,6 @@ test_expect_success 'pulling into void does not overwrite staged files' '
>         )

I'm currently studying the t5520 tests in order to improve test
coverage of git-pull.sh, and I find it hard to understand whenever
tests depend on the tests before them in subtle ways.

Just wondering, would it be good to clean up the created repos in the
above tests to make it clear that they won't be used anymore?
Something like:

    git init cloned &&
    test_when_finished "rm -rf cloned" &&
    ...

Thanks,
Paul

Re: [PATCH v2 03/15] t5520: style fixes

[Junio C Hamano]

Paul Tan <pyokagan@gmail.com> writes:

> Just wondering, would it be good to clean up the created repos in the
> above tests to make it clear that they won't be used anymore?
> Something like:
>
>     git init cloned &&
>     test_when_finished "rm -rf cloned" &&
>     ...

It might be a good idea when you are doing the wholesale style fix
of the entire script. That was not a part of the scope of my series,
so I kept my changes to the minimum and made my additions in line
with the existing tests (i.e. new one-time-use repository that is
just left behind without being used by others).

Having said that, when it is very clear that each of the new
directories are for one-time-use (like the use of them in 5520), I
tend to prefer leaving them around, if only to make it easier to
insert "exit", go there and manually inspect the situation, which
will become necessary when the tests start failing.

[PATCH v2 04/15] t5520: test pulling an octopus into an unborn branch

[Junio C Hamano]

The code comment for "git merge" in builtin/merge.c, we say

    If the merged head is a valid one there is no reason
    to forbid "git merge" into a branch yet to be born.
    We do the same for "git pull".

and t5520 does have an existing test for that behaviour.  However,
there was no test to make sure that 'git pull' to pull multiple
branches into an unborn branch must fail.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5520-pull.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 5195a21..7efd45b 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -76,6 +76,15 @@ test_expect_success 'pulling into void does not remove new staged files' '
 	)
 '
 
+test_expect_success 'pulling into void must not create an octopus' '
+	git init cloned-octopus &&
+	(
+		cd cloned-octopus &&
+		test_must_fail git pull .. master master &&
+		! test -f file
+	)
+'
+
 test_expect_success 'test . as a remote' '
 
 	git branch copy master &&
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 05/15] merge: clarify "pulling into void" special case

[Junio C Hamano]

Instead of having it as one of the three if/elseif/.. case arms,
test the condition and handle this special case upfront.  This makes
it easier to follow the flow of logic.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 8477878..878f82a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1178,23 +1178,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		usage_with_options(builtin_merge_usage,
 			builtin_merge_options);
 
-	/*
-	 * This could be traditional "merge <msg> HEAD <commit>..."  and
-	 * the way we can tell it is to see if the second token is HEAD,
-	 * but some people might have misused the interface and used a
-	 * commit-ish that is the same as HEAD there instead.
-	 * Traditional format never would have "-m" so it is an
-	 * additional safety measure to check for it.
-	 */
-
-	if (!have_message && head_commit &&
-	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
-		strbuf_addstr(&merge_msg, argv[0]);
-		head_arg = argv[1];
-		argv += 2;
-		argc -= 2;
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-	} else if (!head_commit) {
+	if (!head_commit) {
 		struct commit *remote_head;
 		/*
 		 * If the merged head is a valid one there is no reason
@@ -1217,6 +1201,23 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		update_ref("initial pull", "HEAD", remote_head->object.sha1,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 		goto done;
+	}
+
+	/*
+	 * This could be traditional "merge <msg> HEAD <commit>..."  and
+	 * the way we can tell it is to see if the second token is HEAD,
+	 * but some people might have misused the interface and used a
+	 * commit-ish that is the same as HEAD there instead.
+	 * Traditional format never would have "-m" so it is an
+	 * additional safety measure to check for it.
+	 */
+	if (!have_message &&
+	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
+		strbuf_addstr(&merge_msg, argv[0]);
+		head_arg = argv[1];
+		argv += 2;
+		argc -= 2;
+		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else {
 		struct strbuf merge_names = STRBUF_INIT;
 
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 06/15] merge: do not check argc to determine number of remote heads

[Junio C Hamano]

To reject merging multiple commits into an unborn branch, we check
argc, thinking that collect_parents() that reads the remaining
command line arguments from <argc, argv> will give us the same
number of commits as its input, i.e. argc.

Because what we really care about is the number of commits, let the
function run and then make sure it returns only one commit instead.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 878f82a..1d4fbd3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1185,9 +1185,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to forbid "git merge" into a branch yet to be born.
 		 * We do the same for "git pull".
 		 */
-		if (argc != 1)
-			die(_("Can merge only exactly one commit into "
-				"empty head"));
 		if (squash)
 			die(_("Squash commit into empty head not supported yet"));
 		if (fast_forward == FF_NO)
@@ -1197,6 +1194,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
+		if (remoteheads->next)
+			die(_("Can merge only exactly one commit into empty head"));
 		read_empty(remote_head->object.sha1, 0);
 		update_ref("initial pull", "HEAD", remote_head->object.sha1,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 07/15] merge: small leakfix and code simplification

[Junio C Hamano]

When parsing a merged object name like "foo~20" to formulate a merge
summary "Merge branch foo (early part)", a temporary strbuf is used,
but we forgot to deallocate it when we failed to find the named
branch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 1d4fbd3..b2d0332 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -491,8 +491,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	}
 	if (len) {
 		struct strbuf truname = STRBUF_INIT;
-		strbuf_addstr(&truname, "refs/heads/");
-		strbuf_addstr(&truname, remote);
+		strbuf_addf(&truname, "refs/heads/%s", remote);
 		strbuf_setlen(&truname, truname.len - len);
 		if (ref_exists(truname.buf)) {
 			strbuf_addf(msg,
@@ -503,6 +502,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 			strbuf_release(&truname);
 			goto cleanup;
 		}
+		strbuf_release(&truname);
 	}
 
 	if (!strcmp(remote, "FETCH_HEAD") &&
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 08/15] merge: clarify collect_parents() logic

[Junio C Hamano]

Clarify this small function in three ways.

 - The function initially collects all commits to be merged into a
   commit_list "remoteheads"; the "remotes" pointer always points at
   the tail of this list (either the remoteheads variable itself, or
   the ->next slot of the element at the end of the list) to help
   elongate the list by repeated calls to commit_list_insert().
   Because the new element appended by commit_list_insert() will
   always have its ->next slot NULLed out, there is no need for us
   to assign NULL to *remotes to terminate the list at the end.

 - The variable "head_subsumed" always confused me every time I read
   this code.  What is happening here is that we inspect what the
   caller told us to merge (including the current HEAD) and come up
   with the list of parents to be recorded for the resulting merge
   commit, omitting commits that are ancestor of other commits.
   This filtering may remove the current HEAD from the resulting
   parent list---and we signal that fact with this variable, so that
   we can later record it as the first parent when "--no-ff" is in
   effect.

 - The "parents" list is created for this function by reduce_heads()
   and was not deallocated after its use, even though the loop
   control was written in such a way to allow us to do so by taking
   the "next" element in a separate variable so that it can be used
   in the next-step part of the loop control.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index b2d0332..d2e36e2 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1061,11 +1061,19 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 					 "not something we can merge");
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
-	*remotes = NULL;
 
+	/*
+	 * Is the current HEAD reachable from another commit being
+	 * merged?  If so we do not want to record it as a parent of
+	 * the resulting merge, unless --no-ff is given.  We will flip
+	 * this variable to 0 when we find HEAD among the independent
+	 * tips being merged.
+	 */
+	*head_subsumed = 1;
+
+	/* Find what parents to record by checking independent ones. */
 	parents = reduce_heads(remoteheads);
 
-	*head_subsumed = 1; /* we will flip this to 0 when we find it */
 	for (remoteheads = NULL, remotes = &remoteheads;
 	     parents;
 	     parents = next) {
@@ -1075,6 +1083,7 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 			*head_subsumed = 0;
 		else
 			remotes = &commit_list_insert(commit, remotes)->next;
+		free(parents);
 	}
 	return remoteheads;
 }
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 09/15] merge: split reduce_parents() out of collect_parents()

[Junio C Hamano]

The latter does two separate things:

 - Parse the list of commits on the command line, and formulate the
   list of commits to be merged (including the current HEAD);

 - Compute the list of parents to be recorded in the resulting merge
   commit.

Split the latter into a separate helper function, so that we can
later supply the list commits to be merged from a different source
(namely, FETCH_HEAD).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index d2e36e2..054f052 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1044,23 +1044,11 @@ static int default_edit_option(void)
 		st_stdin.st_mode == st_stdout.st_mode);
 }
 
-static struct commit_list *collect_parents(struct commit *head_commit,
-					   int *head_subsumed,
-					   int argc, const char **argv)
+static struct commit_list *reduce_parents(struct commit *head_commit,
+					  int *head_subsumed,
+					  struct commit_list *remoteheads)
 {
-	int i;
-	struct commit_list *remoteheads = NULL, *parents, *next;
-	struct commit_list **remotes = &remoteheads;
-
-	if (head_commit)
-		remotes = &commit_list_insert(head_commit, remotes)->next;
-	for (i = 0; i < argc; i++) {
-		struct commit *commit = get_merge_parent(argv[i]);
-		if (!commit)
-			help_unknown_ref(argv[i], "merge",
-					 "not something we can merge");
-		remotes = &commit_list_insert(commit, remotes)->next;
-	}
+	struct commit_list *parents, *next, **remotes = &remoteheads;
 
 	/*
 	 * Is the current HEAD reachable from another commit being
@@ -1088,6 +1076,27 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 	return remoteheads;
 }
 
+static struct commit_list *collect_parents(struct commit *head_commit,
+					   int *head_subsumed,
+					   int argc, const char **argv)
+{
+	int i;
+	struct commit_list *remoteheads = NULL;
+	struct commit_list **remotes = &remoteheads;
+
+	if (head_commit)
+		remotes = &commit_list_insert(head_commit, remotes)->next;
+	for (i = 0; i < argc; i++) {
+		struct commit *commit = get_merge_parent(argv[i]);
+		if (!commit)
+			help_unknown_ref(argv[i], "merge",
+					 "not something we can merge");
+		remotes = &commit_list_insert(commit, remotes)->next;
+	}
+
+	return reduce_parents(head_commit, head_subsumed, remoteheads);
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	unsigned char result_tree[20];
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 10/15] merge: narrow scope of merge_names

[Junio C Hamano]

In order to pass the list of parents to fmt_merge_msg(), cmd_merge()
uses this strbuf to create something that look like FETCH_HEAD that
describes commits that are being merged.  This is necessary only
when we are creating the merge commit message ourselves, but was
done unconditionally.

Move the variable and the logic to populate it to confine them in a
block that needs them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 054f052..d853c9d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1236,8 +1236,6 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		argc -= 2;
 		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
 	} else {
-		struct strbuf merge_names = STRBUF_INIT;
-
 		/* We are invoked directly as the first-class UI. */
 		head_arg = "HEAD";
 
@@ -1247,11 +1245,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to the given message.
 		 */
 		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, &merge_names);
 
 		if (!have_message || shortlog_len) {
+			struct strbuf merge_names = STRBUF_INIT;
 			struct fmt_merge_msg_opts opts;
+
+			for (p = remoteheads; p; p = p->next)
+				merge_name(merge_remote_util(p->item)->name, &merge_names);
+
 			memset(&opts, 0, sizeof(opts));
 			opts.add_title = !have_message;
 			opts.shortlog_len = shortlog_len;
@@ -1260,6 +1261,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			fmt_merge_msg(&merge_names, &merge_msg, &opts);
 			if (merge_msg.len)
 				strbuf_setlen(&merge_msg, merge_msg.len - 1);
+
+			strbuf_release(&merge_names);
 		}
 	}
 
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 11/15] merge: extract prepare_merge_message() logic out

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index d853c9d..a972ed6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1076,6 +1076,20 @@ static struct commit_list *reduce_parents(struct commit *head_commit,
 	return remoteheads;
 }
 
+static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *merge_msg)
+{
+	struct fmt_merge_msg_opts opts;
+
+	memset(&opts, 0, sizeof(opts));
+	opts.add_title = !have_message;
+	opts.shortlog_len = shortlog_len;
+	opts.credit_people = (0 < option_edit);
+
+	fmt_merge_msg(merge_names, merge_msg, &opts);
+	if (merge_msg->len)
+		strbuf_setlen(merge_msg, merge_msg->len - 1);
+}
+
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
 					   int argc, const char **argv)
@@ -1248,20 +1262,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 
 		if (!have_message || shortlog_len) {
 			struct strbuf merge_names = STRBUF_INIT;
-			struct fmt_merge_msg_opts opts;
 
 			for (p = remoteheads; p; p = p->next)
 				merge_name(merge_remote_util(p->item)->name, &merge_names);
-
-			memset(&opts, 0, sizeof(opts));
-			opts.add_title = !have_message;
-			opts.shortlog_len = shortlog_len;
-			opts.credit_people = (0 < option_edit);
-
-			fmt_merge_msg(&merge_names, &merge_msg, &opts);
-			if (merge_msg.len)
-				strbuf_setlen(&merge_msg, merge_msg.len - 1);
-
+			prepare_merge_message(&merge_names, &merge_msg);
 			strbuf_release(&merge_names);
 		}
 	}
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 12/15] merge: make collect_parents() auto-generate the merge message

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index a972ed6..9f98538 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1092,7 +1092,8 @@ static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *mer
 
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
-					   int argc, const char **argv)
+					   int argc, const char **argv,
+					   struct strbuf *merge_msg)
 {
 	int i;
 	struct commit_list *remoteheads = NULL;
@@ -1108,7 +1109,20 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 		remotes = &commit_list_insert(commit, remotes)->next;
 	}
 
-	return reduce_parents(head_commit, head_subsumed, remoteheads);
+	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+
+	if (merge_msg &&
+	    (!have_message || shortlog_len)) {
+		struct strbuf merge_names = STRBUF_INIT;
+		struct commit_list *p;
+
+		for (p = remoteheads; p; p = p->next)
+			merge_name(merge_remote_util(p->item)->name, &merge_names);
+		prepare_merge_message(&merge_names, merge_msg);
+		strbuf_release(&merge_names);
+	}
+
+	return remoteheads;
 }
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
@@ -1222,7 +1236,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		if (fast_forward == FF_NO)
 			die(_("Non-fast-forward commit does not make sense into "
 			    "an empty head"));
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, NULL);
 		remote_head = remoteheads->item;
 		if (!remote_head)
 			die(_("%s - not something we can merge"), argv[0]);
@@ -1248,7 +1263,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		head_arg = argv[1];
 		argv += 2;
 		argc -= 2;
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, NULL);
 	} else {
 		/* We are invoked directly as the first-class UI. */
 		head_arg = "HEAD";
@@ -1258,16 +1274,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * the standard merge summary message to be appended
 		 * to the given message.
 		 */
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-
-		if (!have_message || shortlog_len) {
-			struct strbuf merge_names = STRBUF_INIT;
-
-			for (p = remoteheads; p; p = p->next)
-				merge_name(merge_remote_util(p->item)->name, &merge_names);
-			prepare_merge_message(&merge_names, &merge_msg);
-			strbuf_release(&merge_names);
-		}
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, &merge_msg);
 	}
 
 	if (!head_commit || !argc)
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 13/15] merge: decide if we auto-generate the message early in collect_parents()

[Junio C Hamano]

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 9f98538..eb3be68 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1098,6 +1098,10 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 	int i;
 	struct commit_list *remoteheads = NULL;
 	struct commit_list **remotes = &remoteheads;
+	struct strbuf merge_names = STRBUF_INIT, *autogen = NULL;
+
+	if (merge_msg && (!have_message || shortlog_len))
+		autogen = &merge_names;
 
 	if (head_commit)
 		remotes = &commit_list_insert(head_commit, remotes)->next;
@@ -1111,15 +1115,13 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
 
-	if (merge_msg &&
-	    (!have_message || shortlog_len)) {
-		struct strbuf merge_names = STRBUF_INIT;
+	if (autogen) {
 		struct commit_list *p;
-
 		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, &merge_names);
-		prepare_merge_message(&merge_names, merge_msg);
-		strbuf_release(&merge_names);
+			merge_name(merge_remote_util(p->item)->name, autogen);
+
+		prepare_merge_message(autogen, merge_msg);
+		strbuf_release(autogen);
 	}
 
 	return remoteheads;
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 14/15] merge: handle FETCH_HEAD internally

[Junio C Hamano]

The collect_parents() function now is responsible for

 1. parsing the commits given on the command line into a list of
    commits to be merged;

 2. filtering these parents into independent ones; and

 3. optionally calling fmt_merge_msg() via prepare_merge_message()
    to prepare an auto-generated merge log message, using fake
    contents that FETCH_HEAD would have had if these commits were
    fetched from the current repository with "git pull . $args..."

Make "git merge FETCH_HEAD" to be the same as the traditional

    git merge "$(git fmt-merge-msg <.git/FETCH_HEAD)" $commits

invocation of the command in "git pull", where $commits are the ones
that appear in FETCH_HEAD that are not marked as not-for-merge, by
making it do a bit more, specifically:

 - noticing "FETCH_HEAD" is the only "commit" on the command line
   and picking the commits that are not marked as not-for-merge as
   the list of commits to be merged (substitute for step #1 above);

 - letting the resulting list fed to step #2 above;

 - doing the step #3 above, using the contents of the FETCH_HEAD
   instead of fake contents crafted from the list of commits parsed
   in the step #1 above.

Note that this changes the semantics.  "git merge FETCH_HEAD" has
always behaved as if the first commit in the FETCH_HEAD file were
directly specified on the command line, creating a two-way merge
whose auto-generated merge log said "merge commit xyz".  With this
change, if the previous fetch was to grab multiple branches (e.g.
"git fetch $there topic-a topic-b"), the new world order is to
create an octopus, behaving as if "git pull $there topic-a topic-b"
were run.  This is a deliberate change to make that happen, and
can be seen in the changes to t3033 tests.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-merge.txt |   4 ++
 builtin/merge.c             | 106 ++++++++++++++++++++++++++++++--------------
 t/t3033-merge-toplevel.sh   |  10 ++---
 3 files changed, 81 insertions(+), 39 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index cf2c374..d9aa6b6 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -104,6 +104,10 @@ commit or stash your changes before running 'git merge'.
 If no commit is given from the command line, merge the remote-tracking
 branches that the current branch is configured to use as its upstream.
 See also the configuration section of this manual page.
++
+When `FETCH_HEAD` (and no other commit) is specified, the branches
+recorded in the `.git/FETCH_HEAD` file by the previous invocation
+of `git fetch` for merging are merged to the current branch.
 
 
 PRE-MERGE CHECKS
diff --git a/builtin/merge.c b/builtin/merge.c
index eb3be68..42f9fcc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -505,28 +505,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		strbuf_release(&truname);
 	}
 
-	if (!strcmp(remote, "FETCH_HEAD") &&
-			!access(git_path("FETCH_HEAD"), R_OK)) {
-		const char *filename;
-		FILE *fp;
-		struct strbuf line = STRBUF_INIT;
-		char *ptr;
-
-		filename = git_path("FETCH_HEAD");
-		fp = fopen(filename, "r");
-		if (!fp)
-			die_errno(_("could not open '%s' for reading"),
-				  filename);
-		strbuf_getline(&line, fp, '\n');
-		fclose(fp);
-		ptr = strstr(line.buf, "\tnot-for-merge\t");
-		if (ptr)
-			strbuf_remove(&line, ptr-line.buf+1, 13);
-		strbuf_addbuf(msg, &line);
-		strbuf_release(&line);
-		goto cleanup;
-	}
-
 	if (remote_head->util) {
 		struct merge_remote_desc *desc;
 		desc = merge_remote_util(remote_head);
@@ -1090,6 +1068,60 @@ static void prepare_merge_message(struct strbuf *merge_names, struct strbuf *mer
 		strbuf_setlen(merge_msg, merge_msg->len - 1);
 }
 
+static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge_names)
+{
+	const char *filename;
+	int fd, pos, npos;
+	struct strbuf fetch_head_file = STRBUF_INIT;
+
+	if (!merge_names)
+		merge_names = &fetch_head_file;
+
+	filename = git_path("FETCH_HEAD");
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		die_errno(_("could not open '%s' for reading"), filename);
+
+	if (strbuf_read(merge_names, fd, 0) < 0)
+		die_errno(_("could not read '%s'"), filename);
+	if (close(fd) < 0)
+		die_errno(_("could not close '%s'"), filename);
+
+	for (pos = 0; pos < merge_names->len; pos = npos) {
+		unsigned char sha1[20];
+		char *ptr;
+		struct commit *commit;
+
+		ptr = strchr(merge_names->buf + pos, '\n');
+		if (ptr)
+			npos = ptr - merge_names->buf + 1;
+		else
+			npos = merge_names->len;
+
+		if (npos - pos < 40 + 2 ||
+		    get_sha1_hex(merge_names->buf + pos, sha1))
+			commit = NULL; /* bad */
+		else if (memcmp(merge_names->buf + pos + 40, "\t\t", 2))
+			continue; /* not-for-merge */
+		else {
+			char saved = merge_names->buf[pos + 40];
+			merge_names->buf[pos + 40] = '\0';
+			commit = get_merge_parent(merge_names->buf + pos);
+			merge_names->buf[pos + 40] = saved;
+		}
+		if (!commit) {
+			if (ptr)
+				*ptr = '\0';
+			die("not something we can merge in %s: %s",
+			    filename, merge_names->buf + pos);
+		}
+		remotes = &commit_list_insert(commit, remotes)->next;
+	}
+
+	if (merge_names == &fetch_head_file)
+		strbuf_release(&fetch_head_file);
+}
+
 static struct commit_list *collect_parents(struct commit *head_commit,
 					   int *head_subsumed,
 					   int argc, const char **argv,
@@ -1105,21 +1137,27 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 	if (head_commit)
 		remotes = &commit_list_insert(head_commit, remotes)->next;
-	for (i = 0; i < argc; i++) {
-		struct commit *commit = get_merge_parent(argv[i]);
-		if (!commit)
-			help_unknown_ref(argv[i], "merge",
-					 "not something we can merge");
-		remotes = &commit_list_insert(commit, remotes)->next;
-	}
 
-	remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+	if (argc == 1 && !strcmp(argv[0], "FETCH_HEAD")) {
+		handle_fetch_head(remotes, autogen);
+		remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+	} else {
+		for (i = 0; i < argc; i++) {
+			struct commit *commit = get_merge_parent(argv[i]);
+			if (!commit)
+				help_unknown_ref(argv[i], "merge",
+						 "not something we can merge");
+			remotes = &commit_list_insert(commit, remotes)->next;
+		}
+		remoteheads = reduce_parents(head_commit, head_subsumed, remoteheads);
+		if (autogen) {
+			struct commit_list *p;
+			for (p = remoteheads; p; p = p->next)
+				merge_name(merge_remote_util(p->item)->name, autogen);
+		}
+	}
 
 	if (autogen) {
-		struct commit_list *p;
-		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, autogen);
-
 		prepare_merge_message(autogen, merge_msg);
 		strbuf_release(autogen);
 	}
diff --git a/t/t3033-merge-toplevel.sh b/t/t3033-merge-toplevel.sh
index 9d92d3c..46aadc4 100755
--- a/t/t3033-merge-toplevel.sh
+++ b/t/t3033-merge-toplevel.sh
@@ -77,7 +77,7 @@ test_expect_success 'merge octopus, non-fast-forward' '
 
 # The same set with FETCH_HEAD
 
-test_expect_failure 'merge FETCH_HEAD octopus into void' '
+test_expect_success 'merge FETCH_HEAD octopus into void' '
 	t3033_reset &&
 	git checkout --orphan test &&
 	git rm -fr . &&
@@ -88,7 +88,7 @@ test_expect_failure 'merge FETCH_HEAD octopus into void' '
 	test_must_fail git rev-parse HEAD
 '
 
-test_expect_failure 'merge FETCH_HEAD octopus fast-forward (ff)' '
+test_expect_success 'merge FETCH_HEAD octopus fast-forward (ff)' '
 	t3033_reset &&
 	git reset --hard one &&
 	git fetch . left right &&
@@ -100,7 +100,7 @@ test_expect_failure 'merge FETCH_HEAD octopus fast-forward (ff)' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'merge FETCH_HEAD octopus non-fast-forward (ff)' '
+test_expect_success 'merge FETCH_HEAD octopus non-fast-forward (ff)' '
 	t3033_reset &&
 	git reset --hard one &&
 	git fetch . left right &&
@@ -112,7 +112,7 @@ test_expect_failure 'merge FETCH_HEAD octopus non-fast-forward (ff)' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'merge FETCH_HEAD octopus fast-forward (does not ff)' '
+test_expect_success 'merge FETCH_HEAD octopus fast-forward (does not ff)' '
 	t3033_reset &&
 	git fetch . left right &&
 	git merge FETCH_HEAD &&
@@ -123,7 +123,7 @@ test_expect_failure 'merge FETCH_HEAD octopus fast-forward (does not ff)' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'merge FETCH_HEAD octopus non-fast-forward' '
+test_expect_success 'merge FETCH_HEAD octopus non-fast-forward' '
 	t3033_reset &&
 	git fetch . left right &&
 	git merge --no-ff FETCH_HEAD &&
-- 
2.4.0-rc3-300-g052d062

[PATCH v2 15/15] merge: deprecate 'git merge <message> HEAD <commit>' syntax

[Junio C Hamano]

We had this in "git merge" manual for eternity:

    'git merge' <msg> HEAD <commit>...

    [This] syntax (<msg> `HEAD` <commit>...) is supported for
    historical reasons.  Do not use it from the command line or in
    new scripts.  It is the same as `git merge -m <msg> <commit>...`.

With the update to "git merge" to make it understand what is
recorded in FETCH_HEAD directly, including Octopus merge cases, we
now can rewrite the use of this syntax in "git pull" with a simple
"git merge FETCH_HEAD".

Also there are quite a few fallouts in the test scripts, and it
turns out that "git cvsimport" also uses this old syntax to record
a merge.

Judging from this result, I would not be surprised if dropping the
support of the old syntax broke scripts people have written and been
relying on for the past ten years.  But at least we can start the
deprecation process by throwing a warning message when the syntax is
used.

With luck, we might be able to drop the support in a few years.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/merge.c               | 1 +
 git-cvsimport.perl            | 2 +-
 git-pull.sh                   | 3 +--
 t/t3402-rebase-merge.sh       | 2 +-
 t/t6020-merge-df.sh           | 2 +-
 t/t6021-merge-criss-cross.sh  | 6 +++---
 t/t9402-git-cvsserver-refs.sh | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 42f9fcc..67fbfaf 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1299,6 +1299,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 */
 	if (!have_message &&
 	    is_old_style_invocation(argc, argv, head_commit->object.sha1)) {
+		warning("old-style 'git merge <msg> HEAD <commit>' is deprecated.");
 		strbuf_addstr(&merge_msg, argv[0]);
 		head_arg = argv[1];
 		argv += 2;
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 73d367c..82ecb03 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -1162,7 +1162,7 @@ sub commit {
 		die "Fast-forward update failed: $?\n" if $?;
 	}
 	else {
-		system(qw(git merge cvsimport HEAD), "$remote/$opt_o");
+		system(qw(git merge -m cvsimport), "$remote/$opt_o");
 		die "Could not merge $opt_o into the current branch.\n" if $?;
 	}
 } else {
diff --git a/git-pull.sh b/git-pull.sh
index 4d4fc77..15d9431 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -323,7 +323,6 @@ then
 	fi
 fi
 
-merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
 case "$rebase" in
 true)
 	eval="git-rebase $diffstat $strategy_args $merge_args $rebase_args $verbosity"
@@ -334,7 +333,7 @@ true)
 	eval="git-merge $diffstat $no_commit $verify_signatures $edit $squash $no_ff $ff_only"
 	eval="$eval $log_arg $strategy_args $merge_args $verbosity $progress"
 	eval="$eval $gpg_sign_args"
-	eval="$eval \"\$merge_name\" HEAD $merge_head"
+	eval="$eval FETCH_HEAD"
 	;;
 esac
 eval "exec $eval"
diff --git a/t/t3402-rebase-merge.sh b/t/t3402-rebase-merge.sh
index 5a27ec9..8f64505 100755
--- a/t/t3402-rebase-merge.sh
+++ b/t/t3402-rebase-merge.sh
@@ -47,7 +47,7 @@ test_expect_success setup '
 '
 
 test_expect_success 'reference merge' '
-	git merge -s recursive "reference merge" HEAD master
+	git merge -s recursive -m "reference merge" master
 '
 
 PRE_REBASE=$(git rev-parse test-rebase)
diff --git a/t/t6020-merge-df.sh b/t/t6020-merge-df.sh
index 27c3d73..2af1bee 100755
--- a/t/t6020-merge-df.sh
+++ b/t/t6020-merge-df.sh
@@ -24,7 +24,7 @@ test_expect_success 'prepare repository' '
 '
 
 test_expect_success 'Merge with d/f conflicts' '
-	test_expect_code 1 git merge "merge msg" B master
+	test_expect_code 1 git merge -m "merge msg" master
 '
 
 test_expect_success 'F/D conflict' '
diff --git a/t/t6021-merge-criss-cross.sh b/t/t6021-merge-criss-cross.sh
index d15b313..213deec 100755
--- a/t/t6021-merge-criss-cross.sh
+++ b/t/t6021-merge-criss-cross.sh
@@ -48,7 +48,7 @@ echo "1
 " > file &&
 git commit -m "C3" file &&
 git branch C3 &&
-git merge "pre E3 merge" B A &&
+git merge -m "pre E3 merge" A &&
 echo "1
 2
 3 changed in E3, branch B. New file size
@@ -61,7 +61,7 @@ echo "1
 " > file &&
 git commit -m "E3" file &&
 git checkout A &&
-git merge "pre D8 merge" A C3 &&
+git merge -m "pre D8 merge" C3 &&
 echo "1
 2
 3 changed in C3, branch B
@@ -73,7 +73,7 @@ echo "1
 9" > file &&
 git commit -m D8 file'
 
-test_expect_success 'Criss-cross merge' 'git merge "final merge" A B'
+test_expect_success 'Criss-cross merge' 'git merge -m "final merge" B'
 
 cat > file-expect <<EOF
 1
diff --git a/t/t9402-git-cvsserver-refs.sh b/t/t9402-git-cvsserver-refs.sh
index 1e266ef..d00df08 100755
--- a/t/t9402-git-cvsserver-refs.sh
+++ b/t/t9402-git-cvsserver-refs.sh
@@ -496,7 +496,7 @@ test_expect_success 'check [cvswork3] diff' '
 '
 
 test_expect_success 'merge early [cvswork3] b3 with b1' '
-	( cd gitwork3 && git merge "message" HEAD b1 ) &&
+	( cd gitwork3 && git merge -m "message" b1 ) &&
 	git fetch gitwork3 b3:b3 &&
 	git tag v3merged b3 &&
 	git push --tags gitcvs.git b3:b3
-- 
2.4.0-rc3-300-g052d062

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Junio C Hamano]

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

> This is primarily note-to-self; even though I haven't got around
> bisecting yet, I think I know I did some bad change myself.
>
> "git pull $URL $tag" seems to:
>
>  * fail to invoke the editor without "--edit".
>  * show the summary merge log message twice.

We seem to be making a good progress on the discussion on this
specific issue, but a larger tangent of this is that we do not seem
to have test coverage to catch this regression.  As we are planning
to rewrite "git pull", we need to make sure we have enough test
coverage to ensure that nothing will regress before doing so.

Thanks.

Re: [BUG] "git pull" will regress between 'master' and 'pu'

[Johannes Schindelin]

Hi Junio,

On 2015-04-20 21:28, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> This is primarily note-to-self; even though I haven't got around
>> bisecting yet, I think I know I did some bad change myself.
>>
>> "git pull $URL $tag" seems to:
>>
>>  * fail to invoke the editor without "--edit".
>>  * show the summary merge log message twice.
> 
> We seem to be making a good progress on the discussion on this
> specific issue, but a larger tangent of this is that we do not seem
> to have test coverage to catch this regression.  As we are planning
> to rewrite "git pull", we need to make sure we have enough test
> coverage to ensure that nothing will regress before doing so.

Yes, the plan is to use code coverage tools to ensure that a decent amount of code paths/parameters is covered.

Ciao,
Dscho