Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges
From: Eric Sunshine <hidden>
Date: 2018-07-11 18:49:17
On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget [off-list ref] wrote:
Previously, we introduced the `merge` command for use in todo lists, to allow to recreate and modify branch topology. For ease of implementation, and to make review easier, the initial implementation only supported merge commits with exactly two parents. This patch adds support for octopus merges, making use of the just-introduced `-F <file>` option for the `git merge` command: to keep things simple, we spawn a new Git command instead of trying to call a library function, also opening an easier door to enhance `rebase --rebase-merges` to optionally use a merge strategy different from `recursive` for regular merges: this feature would use the same code path as octopus merges and simply spawn a `git merge`. Signed-off-by: Johannes Schindelin <redacted>
A few minor notes below (not a proper review)...
quoted hunk ↗ jump to hunk
diff --git a/sequencer.c b/sequencer.c@@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, - strbuf_addf(&buf, "Merge branch '%.*s'", + strbuf_addf(&buf, "Merge %s '%.*s'", + to_merge->next ? "branches" : "branch",
Error messages in this file are already localizable. If this text ever becomes localizable, then this "sentence lego" could be problematic for translators.
quoted hunk ↗ jump to hunk
@@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len, + cmd.git_cmd = 1; + argv_array_push(&cmd.args, "merge"); + argv_array_push(&cmd.args, "-s"); + argv_array_push(&cmd.args, "octopus");
argv_array_pushl(&cmd.args, "-s", "octopus", NULL); which would make it clear that these two arguments must remain together, and prevent someone from coming along and inserting a new argument between them.
+ argv_array_push(&cmd.args, "--no-edit"); + argv_array_push(&cmd.args, "--no-ff"); + argv_array_push(&cmd.args, "--no-log"); + argv_array_push(&cmd.args, "--no-stat"); + argv_array_push(&cmd.args, "-F"); + argv_array_push(&cmd.args, git_path_merge_msg());
Ditto: argv_array_pushl(&cmd.args, "-L", git_path_merge_msg(), NULL);
quoted hunk ↗ jump to hunk
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh@@ -329,4 +329,38 @@ test_expect_success 'labels that are object IDs are rewritten' ' +test_expect_success 'octopus merges' ' + git checkout -b three && + test_commit before-octopus && + test_commit three && + git checkout -b two HEAD^ && + test_commit two && + git checkout -b one HEAD^ && + test_commit one && + test_tick && + (GIT_AUTHOR_NAME="Hank" GIT_AUTHOR_EMAIL="hank@sea.world" \ + git merge -m "Tüntenfüsch" two three) &&
Unnecessary subshell?
+ : fast forward if possible && + before="$(git rev-parse --verify HEAD)" && + test_tick && + git rebase -i -r HEAD^^ && + test_cmp_rev HEAD $before && + + test_tick && + git rebase -i --force -r HEAD^^ && + test "Hank" = "$(git show -s --format=%an HEAD)" && + test "$before" != $(git rev-parse HEAD) && + test_cmp_graph HEAD^^.. <<-\EOF + *-. Tüntenfüsch + |\ \ + | | * three + | * | two + | |/ + * | one + |/ + o before-octopus + EOF +'