From 9e62316df7f918eabc879f97da76543cc5f0b3fc Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 29 Apr 2015 13:14:50 -0700 Subject: merge: test the top-level merge driver 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 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 -- cgit v0.10.2-6-g49f6 From 00c7e7e7e8a2aa07b2cd3a09c8f9e11872727d86 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 13:01:44 -0700 Subject: merge: simplify code flow 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 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); -- cgit v0.10.2-6-g49f6 From 55691133291560eacaf03003528e007a3534213d Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 13:29:14 -0700 Subject: t5520: style fixes 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 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 && ( -- cgit v0.10.2-6-g49f6 From 7ad39a2784fd2d2ad6538e687404cf7f35a1e771 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 13:34:08 -0700 Subject: t5520: test pulling an octopus into an unborn branch 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 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 && -- cgit v0.10.2-6-g49f6 From 1faac1cedca475a364c72b063c79da08caee0fe9 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 13:46:44 -0700 Subject: merge: clarify "pulling into void" special case 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 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 HEAD ..." 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 HEAD ..." 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; -- cgit v0.10.2-6-g49f6 From eaa4e59c8545f61c6e61559df33dc4792e455d5a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 13:56:34 -0700 Subject: merge: do not check argc to determine number of remote heads To reject merging multiple commits into an unborn branch, we check argc, thinking that collect_parents() that reads the remaining command line arguments from 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 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); -- cgit v0.10.2-6-g49f6 From 1016658de3fe213eedebdb478cca0324ac8fbe87 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Apr 2015 14:37:13 -0700 Subject: merge: small leakfix and code simplification 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 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") && -- cgit v0.10.2-6-g49f6 From 0b10b8a3d53c7aaf42b8f14da1021ea59ea4e0ec Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 10:25:43 -0700 Subject: merge: clarify collect_parents() logic 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 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; } -- cgit v0.10.2-6-g49f6 From 34349dbff8632c353f083959881e38e1c853abf8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 12:00:14 -0700 Subject: merge: split reduce_parents() out of collect_parents() 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 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]; -- cgit v0.10.2-6-g49f6 From 018b3fbc7e66439786492662b88cb34d7d000621 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 12:31:57 -0700 Subject: merge: narrow scope of merge_names 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 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); } } -- cgit v0.10.2-6-g49f6 From 52fecab20cf28f4b5621e502a265ba551a26877e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 18:29:44 -0700 Subject: merge: extract prepare_merge_message() logic out Signed-off-by: Junio C Hamano 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); } } -- cgit v0.10.2-6-g49f6 From 1cf32f4d54d7db0da9b2fb35cdd277f9ab7ae0af Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 18:34:22 -0700 Subject: merge: make collect_parents() auto-generate the merge message Signed-off-by: Junio C Hamano 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) -- cgit v0.10.2-6-g49f6 From 770380156dd06cd03d83957d55484f4a98ad284f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 18:39:43 -0700 Subject: merge: decide if we auto-generate the message early in collect_parents() Signed-off-by: Junio C Hamano 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; -- cgit v0.10.2-6-g49f6 From 74e8bc59cb324d2d7a55c90195db004219770eec Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sat, 25 Apr 2015 18:47:21 -0700 Subject: merge: handle FETCH_HEAD internally 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 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 && -- cgit v0.10.2-6-g49f6 From d45366e8aa922037e7e84c3f35924d2b1399a453 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 25 Mar 2015 21:58:45 -0700 Subject: merge: deprecate 'git merge HEAD ' syntax We had this in "git merge" manual for eternity: 'git merge' HEAD ... [This] syntax ( `HEAD` ...) 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 ...`. 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 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 HEAD ' 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 @@ if ($orig_branch) { 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 <