Merge branch 'jc/merge'

"git merge FETCH_HEAD" learned that the previous "git fetch" could
be to create an Octopus merge, i.e. recording multiple branches
that are not marked as "not-for-merge"; this allows us to lose an
old style invocation "git merge <msg> HEAD $commits..." in the
implementation of "git pull" script; the old style syntax can now
be deprecated.

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

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

builtin/merge.c

@@ -492,8 +492,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,
@@ -504,28 +503,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 			strbuf_release(&truname);
 			goto cleanup;
 		}
-	}
-
-	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;
+		strbuf_release(&truname);
 	}
 
 	if (remote_head->util) {
@@ -1037,28 +1015,24 @@ 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;
+	struct commit_list *parents, *next, **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;
-	}
-	*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) {
@@ -1068,10 +1042,122 @@ 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;
 }
 
+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 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,
+					   struct strbuf *merge_msg)
+{
+	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;
+
+	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) {
+		prepare_merge_message(autogen, merge_msg);
+		strbuf_release(autogen);
+	}
+
+	return remoteheads;
+}
+
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
 	unsigned char result_tree[20];
@@ -1158,19 +1244,44 @@ 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);
 
+	if (!head_commit) {
+		struct commit *remote_head;
+		/*
+		 * 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".
+		 */
+		if (squash)
+			die(_("Squash commit into empty head not supported yet"));
+		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, NULL);
+		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);
+		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,
@@ -1179,40 +1290,16 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * Traditional format never would have "-m" so it is an
 	 * additional safety measure to check for it.
 	 */
-
-	if (!have_message && head_commit &&
+	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;
 		argc -= 2;
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-	} else if (!head_commit) {
-		struct commit *remote_head;
-		/*
-		 * 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".
-		 */
-		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)
-			die(_("Non-fast-forward commit does not make sense into "
-			    "an empty head"));
-		remoteheads = collect_parents(head_commit, &head_subsumed, argc, argv);
-		remote_head = remoteheads->item;
-		if (!remote_head)
-			die(_("%s - not something we can merge"), argv[0]);
-		read_empty(remote_head->object.sha1, 0);
-		update_ref("initial pull", "HEAD", remote_head->object.sha1,
-			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
-		goto done;
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, NULL);
 	} else {
-		struct strbuf merge_names = STRBUF_INIT;
-
 		/* We are invoked directly as the first-class UI. */
 		head_arg = "HEAD";
 
@@ -1221,21 +1308,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);
-		for (p = remoteheads; p; p = p->next)
-			merge_name(merge_remote_util(p->item)->name, &merge_names);
-
-		if (!have_message || shortlog_len) {
-			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);
-		}
+		remoteheads = collect_parents(head_commit, &head_subsumed,
+					      argc, argv, &merge_msg);
 	}
 
 	if (!head_commit || !argc)

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 {

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"

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_success '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_success '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_success '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_success '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_success '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

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)

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 &&
 	(
@@ -86,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 &&

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' '

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

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