{"thread":{"id":"48862","subject":"[PATCH 0/3] rebase -r: support octopus merges","startedAt":"2017-12-21T14:52:45Z","lastAt":"2018-07-16T17:56:38Z","messageCount":20,"participants":["Johannes Schindelin via GitGitGadget","Elijah Newren","Junio C Hamano","Stefan Beller","Eric Sunshine","Johannes Schindelin","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"352233","messageId":"03d0907ec61f0ea53b59659c84b8a6662e9e7607.1531312689.git.gitgitgadget@gmail.com","threadId":"48862","inReplyTo":"pull.8.git.gitgitgadget@gmail.com","subject":"[PATCH 2/3] rebase --rebase-merges: add support for octopus merges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2017-12-21T14:52:45Z","receivedAt":"2017-12-21T14:52:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nPreviously, we introduced the `merge` command for use in todo lists,\nto allow to recreate and modify branch topology.\n\nFor ease of implementation, and to make review easier, the initial\nimplementation only supported merge commits with exactly two parents.\n\nThis patch adds support for octopus merges, making use of the\njust-introduced `-F ` option for the `git merge` command: to keep\nthings simple, we spawn a new Git command instead of trying to call a\nlibrary function, also opening an easier door to enhance `rebase\n--rebase-merges` to optionally use a merge strategy different from\n`recursive` for regular merges: this feature would use the same code\npath as octopus merges and simply spawn a `git merge`.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 168 +++++++++++++++++++++++++++++----------\n t/t3430-rebase-merges.sh | 34 ++++++++\n 2 files changed, 159 insertions(+), 43 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 5354d4d51..bcc43699c 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -2841,6 +2841,26 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)\n \treturn ret;\n }\n \n+static struct commit *lookup_label(const char *label, int len,\n+\t\t\t\t struct strbuf *buf)\n+{\n+\tstruct commit *commit;\n+\n+\tstrbuf_reset(buf);\n+\tstrbuf_addf(buf, \"refs/rewritten/%.*s\", len, label);\n+\tcommit = lookup_commit_reference_by_name(buf->buf);\n+\tif (!commit) {\n+\t\t/* fall back to non-rewritten ref or commit */\n+\t\tstrbuf_splice(buf, 0, strlen(\"refs/rewritten/\"), \"\", 0);\n+\t\tcommit = lookup_commit_reference_by_name(buf->buf);\n+\t}\n+\n+\tif (!commit)\n+\t\terror(_(\"could not resolve '%s'\"), buf->buf);\n+\n+\treturn commit;\n+}\n+\n static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\t int flags, struct replay_opts *opts)\n {\n@@ -2849,8 +2869,9 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \tstruct strbuf ref_name = STRBUF_INIT;\n \tstruct commit *head_commit, *merge_commit, *i;\n \tstruct commit_list *bases, *j, *reversed = NULL;\n+\tstruct commit_list *to_merge = NULL, **tail = &to_merge;\n \tstruct merge_options o;\n-\tint merge_arg_len, oneline_offset, can_fast_forward, ret;\n+\tint merge_arg_len, oneline_offset, can_fast_forward, ret, k;\n \tstatic struct lock_file lock;\n \tconst char *p;\n \n@@ -2865,26 +2886,34 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\tgoto leave_merge;\n \t}\n \n-\toneline_offset = arg_len;\n-\tmerge_arg_len = strcspn(arg, \" \\t\\n\");\n-\tp = arg + merge_arg_len;\n-\tp += strspn(p, \" \\t\\n\");\n-\tif (*p == '#' && (!p[1] || isspace(p[1]))) {\n-\t\tp += 1 + strspn(p + 1, \" \\t\\n\");\n-\t\toneline_offset = p - arg;\n-\t} else if (p - arg < arg_len)\n-\t\tBUG(\"octopus merges are not supported yet: '%s'\", p);\n-\n-\tstrbuf_addf(&ref_name, \"refs/rewritten/%.*s\", merge_arg_len, arg);\n-\tmerge_commit = lookup_commit_reference_by_name(ref_name.buf);\n-\tif (!merge_commit) {\n-\t\t/* fall back to non-rewritten ref or commit */\n-\t\tstrbuf_splice(&ref_name, 0, strlen(\"refs/rewritten/\"), \"\", 0);\n-\t\tmerge_commit = lookup_commit_reference_by_name(ref_name.buf);\n+\t/*\n+\t * For octopus merges, the arg starts with the list of revisions to be\n+\t * merged. The list is optionally followed by '#' and the oneline.\n+\t */\n+\tmerge_arg_len = oneline_offset = arg_len;\n+\tfor (p = arg; p - arg < arg_len; p += strspn(p, \" \\t\\n\")) {\n+\t\tif (!*p)\n+\t\t\tbreak;\n+\t\tif (*p == '#' && (!p[1] || isspace(p[1]))) {\n+\t\t\tp += 1 + strspn(p + 1, \" \\t\\n\");\n+\t\t\toneline_offset = p - arg;\n+\t\t\tbreak;\n+\t\t}\n+\t\tk = strcspn(p, \" \\t\\n\");\n+\t\tif (!k)\n+\t\t\tcontinue;\n+\t\tmerge_commit = lookup_label(p, k, &ref_name);\n+\t\tif (!merge_commit) {\n+\t\t\tret = error(_(\"unable to parse '%.*s'\"), k, p);\n+\t\t\tgoto leave_merge;\n+\t\t}\n+\t\ttail = &commit_list_insert(merge_commit, tail)->next;\n+\t\tp += k;\n+\t\tmerge_arg_len = p - arg;\n \t}\n \n-\tif (!merge_commit) {\n-\t\tret = error(_(\"could not resolve '%s'\"), ref_name.buf);\n+\tif (!to_merge) {\n+\t\tret = error(_(\"nothing to merge: '%.*s'\"), arg_len, arg);\n \t\tgoto leave_merge;\n \t}\n \n@@ -2895,8 +2924,13 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\t * \"[new root]\", let's simply fast-forward to the merge head.\n \t\t */\n \t\trollback_lock_file(&lock);\n-\t\tret = fast_forward_to(&merge_commit->object.oid,\n-\t\t\t\t &head_commit->object.oid, 0, opts);\n+\t\tif (to_merge->next)\n+\t\t\tret = error(_(\"octopus merge cannot be executed on \"\n+\t\t\t\t \"top of a [new root]\"));\n+\t\telse\n+\t\t\tret = fast_forward_to(&to_merge->item->object.oid,\n+\t\t\t\t\t &head_commit->object.oid, 0,\n+\t\t\t\t\t opts);\n \t\tgoto leave_merge;\n \t}\n \n@@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\t\tp = arg + oneline_offset;\n \t\t\tlen = arg_len - oneline_offset;\n \t\t} else {\n-\t\t\tstrbuf_addf(&buf, \"Merge branch '%.*s'\",\n+\t\t\tstrbuf_addf(&buf, \"Merge %s '%.*s'\",\n+\t\t\t\t to_merge->next ? \"branches\" : \"branch\",\n \t\t\t\t merge_arg_len, arg);\n \t\t\tp = buf.buf;\n \t\t\tlen = buf.len;\n@@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n \t\t\t&head_commit->object.oid);\n \n \t/*\n-\t * If the merge head is different from the original one, we cannot\n+\t * If any merge head is different from the original one, we cannot\n \t * fast-forward.\n \t */\n \tif (can_fast_forward) {\n-\t\tstruct commit_list *second_parent = commit->parents->next;\n+\t\tstruct commit_list *p = commit->parents->next;\n \n-\t\tif (second_parent && !second_parent->next &&\n-\t\t oidcmp(&merge_commit->object.oid,\n-\t\t\t &second_parent->item->object.oid))\n+\t\tfor (j = to_merge; j && p; j = j->next, p = p->next)\n+\t\t\tif (oidcmp(&j->item->object.oid,\n+\t\t\t\t &p->item->object.oid)) {\n+\t\t\t\tcan_fast_forward = 0;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t/*\n+\t\t * If the number of merge heads differs from the original merge\n+\t\t * commit, we cannot fast-forward.\n+\t\t */\n+\t\tif (j || p)\n \t\t\tcan_fast_forward = 0;\n \t}\n \n-\tif (can_fast_forward && commit->parents->next &&\n-\t !commit->parents->next->next &&\n-\t !oidcmp(&commit->parents->next->item->object.oid,\n-\t\t &merge_commit->object.oid)) {\n+\tif (can_fast_forward) {\n \t\trollback_lock_file(&lock);\n \t\tret = fast_forward_to(&commit->object.oid,\n \t\t\t\t &head_commit->object.oid, 0, opts);\n \t\tgoto leave_merge;\n \t}\n \n+\tif (to_merge->next) {\n+\t\t/* Octopus merge */\n+\t\tstruct child_process cmd = CHILD_PROCESS_INIT;\n+\n+\t\tif (read_env_script(&cmd.env_array)) {\n+\t\t\tconst char *gpg_opt = gpg_sign_opt_quoted(opts);\n+\n+\t\t\tret = error(_(staged_changes_advice), gpg_opt, gpg_opt);\n+\t\t\tgoto leave_merge;\n+\t\t}\n+\n+\t\tcmd.git_cmd = 1;\n+\t\targv_array_push(&cmd.args, \"merge\");\n+\t\targv_array_push(&cmd.args, \"-s\");\n+\t\targv_array_push(&cmd.args, \"octopus\");\n+\t\targv_array_push(&cmd.args, \"--no-edit\");\n+\t\targv_array_push(&cmd.args, \"--no-ff\");\n+\t\targv_array_push(&cmd.args, \"--no-log\");\n+\t\targv_array_push(&cmd.args, \"--no-stat\");\n+\t\targv_array_push(&cmd.args, \"-F\");\n+\t\targv_array_push(&cmd.args, git_path_merge_msg());\n+\t\tif (opts->gpg_sign)\n+\t\t\targv_array_push(&cmd.args, opts->gpg_sign);\n+\n+\t\t/* Add the tips to be merged */\n+\t\tfor (j = to_merge; j; j = j->next)\n+\t\t\targv_array_push(&cmd.args,\n+\t\t\t\t\toid_to_hex(&j->item->object.oid));\n+\n+\t\tstrbuf_release(&ref_name);\n+\t\tunlink(git_path_cherry_pick_head());\n+\t\trollback_lock_file(&lock);\n+\n+\t\trollback_lock_file(&lock);\n+\t\tret = run_command(&cmd);\n+\n+\t\t/* force re-reading of the cache */\n+\t\tif (!ret && (discard_cache() < 0 || read_cache() < 0))\n+\t\t\tret = error(_(\"could not read index\"));\n+\t\tgoto leave_merge;\n+\t}\n+\n+\tmerge_commit = to_merge->item;\n \twrite_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,\n \t\t git_path_merge_head(), 0);\n \twrite_message(\"no-ff\", 5, git_path_merge_mode(), 0);\n@@ -3040,6 +3123,7 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n leave_merge:\n \tstrbuf_release(&ref_name);\n \trollback_lock_file(&lock);\n+\tfree_commit_list(to_merge);\n \treturn ret;\n }\n \n@@ -3877,7 +3961,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t */\n \twhile ((commit = get_revision(revs))) {\n \t\tstruct commit_list *to_merge;\n-\t\tint is_octopus;\n \t\tconst char *p1, *p2;\n \t\tstruct object_id *oid;\n \t\tint is_empty;\n@@ -3909,11 +3992,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\t\tcontinue;\n \t\t}\n \n-\t\tis_octopus = to_merge && to_merge->next;\n-\n-\t\tif (is_octopus)\n-\t\t\tBUG(\"Octopus merges not yet supported\");\n-\n \t\t/* Create a label */\n \t\tstrbuf_reset(&label);\n \t\tif (skip_prefix(oneline.buf, \"Merge \", &p1) &&\n@@ -3935,13 +4013,17 @@ static int make_script_with_merges(struct pretty_print_context *pp,\n \t\tstrbuf_addf(&buf, \"%s -C %s\",\n \t\t\t cmd_merge, oid_to_hex(&commit->object.oid));\n \n-\t\t/* label the tip of merged branch */\n-\t\toid = &to_merge->item->object.oid;\n-\t\tstrbuf_addch(&buf, ' ');\n+\t\t/* label the tips of merged branches */\n+\t\tfor (; to_merge; to_merge = to_merge->next) {\n+\t\t\toid = &to_merge->item->object.oid;\n+\t\t\tstrbuf_addch(&buf, ' ');\n+\n+\t\t\tif (!oidset_contains(&interesting, oid)) {\n+\t\t\t\tstrbuf_addstr(&buf, label_oid(oid, NULL,\n+\t\t\t\t\t\t\t &state));\n+\t\t\t\tcontinue;\n+\t\t\t}\n \n-\t\tif (!oidset_contains(&interesting, oid))\n-\t\t\tstrbuf_addstr(&buf, label_oid(oid, NULL, &state));\n-\t\telse {\n \t\t\ttips_tail = &commit_list_insert(to_merge->item,\n \t\t\t\t\t\t\ttips_tail)->next;\n \ndiff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\nindex 78f7c9958..9e6229727 100755\n--- a/t/t3430-rebase-merges.sh\n+++ b/t/t3430-rebase-merges.sh\n@@ -329,4 +329,38 @@ test_expect_success 'labels that are object IDs are rewritten' '\n \t! grep \"^label $third$\" .git/ORIGINAL-TODO\n '\n \n+test_expect_success 'octopus merges' '\n+\tgit checkout -b three &&\n+\ttest_commit before-octopus &&\n+\ttest_commit three &&\n+\tgit checkout -b two HEAD^ &&\n+\ttest_commit two &&\n+\tgit checkout -b one HEAD^ &&\n+\ttest_commit one &&\n+\ttest_tick &&\n+\t(GIT_AUTHOR_NAME=\"Hank\" GIT_AUTHOR_EMAIL=\"hank@sea.world\" \\\n+\t git merge -m \"Tüntenfüsch\" two three) &&\n+\n+\t: fast forward if possible &&\n+\tbefore=\"$(git rev-parse --verify HEAD)\" &&\n+\ttest_tick &&\n+\tgit rebase -i -r HEAD^^ &&\n+\ttest_cmp_rev HEAD $before &&\n+\n+\ttest_tick &&\n+\tgit rebase -i --force -r HEAD^^ &&\n+\ttest \"Hank\" = \"$(git show -s --format=%an HEAD)\" &&\n+\ttest \"$before\" != $(git rev-parse HEAD) &&\n+\ttest_cmp_graph HEAD^^.. <<-\\EOF\n+\t*-. Tüntenfüsch\n+\t|\\ \\\n+\t| | * three\n+\t| * | two\n+\t| |/\n+\t* | one\n+\t|/\n+\to before-octopus\n+\tEOF\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"352234","messageId":"9e2c1fe83178f7047d8f2aacaa961381a4894081.1531312689.git.gitgitgadget@gmail.com","threadId":"48862","inReplyTo":"pull.8.git.gitgitgadget@gmail.com","subject":"[PATCH 1/3] merge: allow reading the merge commit message from a file","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2017-12-22T14:10:02Z","receivedAt":"2017-12-22T14:10:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThis is consistent with `git commit` which, like `git merge`, supports\npassing the commit message via `-m ` and, unlike `git merge` before\nthis patch, via `-F `.\n\nIt is useful to allow this for scripted use, or for the upcoming patch\nto allow (re-)creating octopus merges in `git rebase --rebase-merges`.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-merge.txt | 10 +++++++++-\n builtin/merge.c | 32 ++++++++++++++++++++++++++++++++\n 2 files changed, 41 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\nindex 6a5c00e2c..537dca7fa 100644\n--- a/Documentation/git-merge.txt\n+++ b/Documentation/git-merge.txt\n@@ -12,7 +12,7 @@ SYNOPSIS\n 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n \t[-s ] [-X ] [-S[]]\n \t[--[no-]allow-unrelated-histories]\n-\t[--[no-]rerere-autoupdate] [-m ] [...]\n+\t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n 'git merge' --abort\n 'git merge' --continue\n \n@@ -75,6 +75,14 @@ The 'git fmt-merge-msg' command can be\n used to give a good default for automated 'git merge'\n invocations. The automated message can include the branch description.\n \n+-F ::\n+--file=::\n+\tRead the commit message to be used for the merge commit (in\n+\tcase one is created).\n++\n+If `--log` is specified, a shortlog of the commits being merged\n+will be appended to the specified message.\n+\n --[no-]rerere-autoupdate::\n \tAllow the rerere mechanism to update the index with the\n \tresult of auto-conflict resolution if possible.\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4a4c09496..b0e907751 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,\n \treturn 0;\n }\n \n+static int option_read_message(struct parse_opt_ctx_t *ctx,\n+\t\t\t const struct option *opt, int unset)\n+{\n+\tstruct strbuf *buf = opt->value;\n+\tconst char *arg;\n+\n+\tif (unset)\n+\t\tBUG(\"-F cannot be negated\");\n+\n+\tif (ctx->opt) {\n+\t\targ = ctx->opt;\n+\t\tctx->opt = NULL;\n+\t} else if (ctx->argc > 1) {\n+\t\tctx->argc--;\n+\t\targ = *++ctx->argv;\n+\t} else\n+\t\treturn opterror(opt, \"requires a value\", 0);\n+\n+\tif (buf->len)\n+\t\tstrbuf_addch(buf, '\\n');\n+\tif (ctx->prefix && !is_absolute_path(arg))\n+\t\targ = prefix_filename(ctx->prefix, arg);\n+\tif (strbuf_read_file(buf, arg, 0) < 0)\n+\t\treturn error(_(\"could not read file '%s'\"), arg);\n+\thave_message = 1;\n+\n+\treturn 0;\n+}\n+\n static struct strategy *get_strategy(const char *name)\n {\n \tint i;\n@@ -228,6 +257,9 @@ static struct option builtin_merge_options[] = {\n \tOPT_CALLBACK('m', \"message\", &merge_msg, N_(\"message\"),\n \t\tN_(\"merge commit message (for a non-fast-forward merge)\"),\n \t\toption_parse_message),\n+\t{ OPTION_LOWLEVEL_CALLBACK, 'F', \"file\", &merge_msg, N_(\"path\"),\n+\t\tN_(\"read message from file\"), PARSE_OPT_NONEG,\n+\t\t(parse_opt_cb *) option_read_message },\n \tOPT__VERBOSITY(&verbosity),\n \tOPT_BOOL(0, \"abort\", &abort_current_merge,\n \t\tN_(\"abort the current in-progress merge\")),\n-- \ngitgitgadget\n\n"},{"id":"352235","messageId":"20cd1b914e74c8f45e5cc9ed82d83eee1463cd91.1531312689.git.gitgitgadget@gmail.com","threadId":"48862","inReplyTo":"pull.8.git.gitgitgadget@gmail.com","subject":"[PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-03-09T16:36:47Z","receivedAt":"2018-03-09T16:36:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nNow that we support octopus merges in the `--rebase-merges` mode,\nwe should give users who actually read the manuals a chance to know\nabout this fact.\n\nSigned-off-by: Johannes Schindelin \n---\n Documentation/git-rebase.txt | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\nindex 0e20a66e7..c4bcd24bb 100644\n--- a/Documentation/git-rebase.txt\n+++ b/Documentation/git-rebase.txt\n@@ -879,8 +879,8 @@ rescheduled immediately, with a helpful message how to edit the todo list\n (this typically happens when a `reset` command was inserted into the todo\n list manually and contains a typo).\n \n-The `merge` command will merge the specified revision into whatever is\n-HEAD at that time. With `-C `, the commit message of\n+The `merge` command will merge the specified revision(s) into whatever\n+is HEAD at that time. With `-C `, the commit message of\n the specified merge commit will be used. When the `-C` is changed to\n a lower-case `-c`, the message will be opened in an editor after a\n successful merge so that the user can edit the message.\n@@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.\n when the merge operation did not even start), it is rescheduled immediately.\n \n At this time, the `merge` command will *always* use the `recursive`\n-merge strategy, with no way to choose a different one. To work around\n+merge strategy for regular merges, and `octopus` for octopus merges,\n+strategy, with no way to choose a different one. To work around\n this, an `exec` command can be used to call `git merge` explicitly,\n using the fact that the labels are worktree-local refs (the ref\n `refs/rewritten/onto` would correspond to the label `onto`, for example).\n-- \ngitgitgadget\n"},{"id":"352232","messageId":"pull.8.git.gitgitgadget@gmail.com","threadId":"48862","inReplyTo":null,"subject":"[PATCH 0/3] rebase -r: support octopus merges","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2018-07-11T12:38:09Z","receivedAt":"2018-07-11T12:38:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"The `--rebase-merges` option of `git rebase` generates todo lists that include the merge commits that are to be rebased.\n\nTo keep things simpler to review, I decided to support only regular, 2-parent merge commits first.\n\nWith this patch series, support is extended to cover also octopus merges.\n\nJohannes Schindelin (3):\n merge: allow reading the merge commit message from a file\n rebase --rebase-merges: add support for octopus merges\n rebase --rebase-merges: adjust man page for octopus support\n\n Documentation/git-merge.txt | 10 ++-\n Documentation/git-rebase.txt | 7 +-\n builtin/merge.c | 32 +++++++\n sequencer.c | 168 ++++++++++++++++++++++++++---------\n t/t3430-rebase-merges.sh | 34 +++++++\n 5 files changed, 204 insertions(+), 47 deletions(-)\n\n\nbase-commit: e3331758f12da22f4103eec7efe1b5304a9be5e9\nPublished-As: https://github.com/gitgitgadget/git/releases/tags/pr-8%2Fdscho%2Fsequencer-and-octopus-merges-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-8/dscho/sequencer-and-octopus-merges-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/8\n-- \ngitgitgadget\n"},{"id":"352273","messageId":"CABPp-BFMKnTMYTd3mK69wZ7__TZTwoF9NmX+RXwyqgUj7NJ0Kw@mail.gmail.com","threadId":"48862","inReplyTo":"20cd1b914e74c8f45e5cc9ed82d83eee1463cd91.1531312689.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2018-07-11T17:05:00Z","receivedAt":"2018-07-11T17:05:04Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi Dscho,\n\nOn Fri, Mar 9, 2018 at 8:36 AM, Johannes Schindelin via GitGitGadget\n wrote:\n> From: Johannes Schindelin \n>\n> Now that we support octopus merges in the `--rebase-merges` mode,\n> we should give users who actually read the manuals a chance to know\n> about this fact.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/git-rebase.txt | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> index 0e20a66e7..c4bcd24bb 100644\n> --- a/Documentation/git-rebase.txt\n> +++ b/Documentation/git-rebase.txt\n> @@ -879,8 +879,8 @@ rescheduled immediately, with a helpful message how to edit the todo list\n> (this typically happens when a `reset` command was inserted into the todo\n> list manually and contains a typo).\n>\n> -The `merge` command will merge the specified revision into whatever is\n> -HEAD at that time. With `-C `, the commit message of\n> +The `merge` command will merge the specified revision(s) into whatever\n> +is HEAD at that time. With `-C `, the commit message of\n> the specified merge commit will be used. When the `-C` is changed to\n> a lower-case `-c`, the message will be opened in an editor after a\n> successful merge so that the user can edit the message.\n> @@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.\n> when the merge operation did not even start), it is rescheduled immediately.\n>\n> At this time, the `merge` command will *always* use the `recursive`\n> -merge strategy, with no way to choose a different one. To work around\n> +merge strategy for regular merges, and `octopus` for octopus merges,\n> +strategy, with no way to choose a different one. To work around\n\nThe \"...merges, strategy, with...\" looks like an incomplete edit.\nPerhaps \"...and `octopus` strategy for octopus merges, with no way...\"\n?\n\n> this, an `exec` command can be used to call `git merge` explicitly,\n> using the fact that the labels are worktree-local refs (the ref\n> `refs/rewritten/onto` would correspond to the label `onto`, for example).\n> --\n> gitgitgadget\n"},{"id":"352277","messageId":"xmqqbmbd3cc3.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"pull.8.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-11T17:35:08Z","receivedAt":"2018-07-11T17:35:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> The `--rebase-merges` option of `git rebase` generates todo lists that include the merge commits that are to be rebased.\n>\n> To keep things simpler to review, I decided to support only regular, 2-parent merge commits first.\n>\n> With this patch series, support is extended to cover also octopus merges.\n\n;-)\n\nTo be honest, I am not sure if there still are people who use\noctopus (even though I freely admit that I am guilty of inventing\nthe term and the mechanism), given its downsides, primary one of\nwhich is that it makes bisection less efficient.\n\nNevertheless, this *is* the right thing to do from feature\ncompleteness point of view. Thanks for following it through.\n\n>\n> Johannes Schindelin (3):\n> merge: allow reading the merge commit message from a file\n> rebase --rebase-merges: add support for octopus merges\n> rebase --rebase-merges: adjust man page for octopus support\n>\n> Documentation/git-merge.txt | 10 ++-\n> Documentation/git-rebase.txt | 7 +-\n> builtin/merge.c | 32 +++++++\n> sequencer.c | 168 ++++++++++++++++++++++++++---------\n> t/t3430-rebase-merges.sh | 34 +++++++\n> 5 files changed, 204 insertions(+), 47 deletions(-)\n>\n>\n> base-commit: e3331758f12da22f4103eec7efe1b5304a9be5e9\n> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-8%2Fdscho%2Fsequencer-and-octopus-merges-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-8/dscho/sequencer-and-octopus-merges-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/8\n"},{"id":"352280","messageId":"CAGZ79kaYpw9t3kCvPnDiLc5z7cLFsVQv4XJDAff_T-bShEjPoQ@mail.gmail.com","threadId":"48862","inReplyTo":"xmqqbmbd3cc3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-07-11T17:47:34Z","receivedAt":"2018-07-11T17:47:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jul 11, 2018 at 10:35 AM Junio C Hamano wrote:\n> To be honest, I am not sure if there still are people who use\n> octopus\n\nThe latest merge with more than 2 parents in linux.git is\ndf958569dbaa (Merge branches 'acpi-tables' and 'acpica', 2018-07-05),\nalthough looking through the log of octopusses I get the impression that\nmostly Rafael J. Wysocki is really keen on\nthese. :-)\n"},{"id":"352286","messageId":"CAPig+cS1KQseynMCGC_6FZwzB0waJX8+C7CVKNqACzOhjB4uSg@mail.gmail.com","threadId":"48862","inReplyTo":"03d0907ec61f0ea53b59659c84b8a6662e9e7607.1531312689.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-07-11T18:49:03Z","receivedAt":"2018-07-11T18:49:17Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget\n wrote:\n> Previously, we introduced the `merge` command for use in todo lists,\n> to allow to recreate and modify branch topology.\n>\n> For ease of implementation, and to make review easier, the initial\n> implementation only supported merge commits with exactly two parents.\n>\n> This patch adds support for octopus merges, making use of the\n> just-introduced `-F ` option for the `git merge` command: to keep\n> things simple, we spawn a new Git command instead of trying to call a\n> library function, also opening an easier door to enhance `rebase\n> --rebase-merges` to optionally use a merge strategy different from\n> `recursive` for regular merges: this feature would use the same code\n> path as octopus merges and simply spawn a `git merge`.\n>\n> Signed-off-by: Johannes Schindelin \n\nA few minor notes below (not a proper review)...\n\n> diff --git a/sequencer.c b/sequencer.c\n> @@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n> - strbuf_addf(&buf, \"Merge branch '%.*s'\",\n> + strbuf_addf(&buf, \"Merge %s '%.*s'\",\n> + to_merge->next ? \"branches\" : \"branch\",\n\nError messages in this file are already localizable. If this text ever\nbecomes localizable, then this \"sentence lego\" could be problematic\nfor translators.\n\n> @@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n> + cmd.git_cmd = 1;\n> + argv_array_push(&cmd.args, \"merge\");\n> + argv_array_push(&cmd.args, \"-s\");\n> + argv_array_push(&cmd.args, \"octopus\");\n\nargv_array_pushl(&cmd.args, \"-s\", \"octopus\", NULL);\n\nwhich would make it clear that these two arguments must remain\ntogether, and prevent someone from coming along and inserting a new\nargument between them.\n\n> + argv_array_push(&cmd.args, \"--no-edit\");\n> + argv_array_push(&cmd.args, \"--no-ff\");\n> + argv_array_push(&cmd.args, \"--no-log\");\n> + argv_array_push(&cmd.args, \"--no-stat\");\n> + argv_array_push(&cmd.args, \"-F\");\n> + argv_array_push(&cmd.args, git_path_merge_msg());\n\nDitto:\nargv_array_pushl(&cmd.args, \"-L\", git_path_merge_msg(), NULL);\n\n> diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> @@ -329,4 +329,38 @@ test_expect_success 'labels that are object IDs are rewritten' '\n> +test_expect_success 'octopus merges' '\n> + git checkout -b three &&\n> + test_commit before-octopus &&\n> + test_commit three &&\n> + git checkout -b two HEAD^ &&\n> + test_commit two &&\n> + git checkout -b one HEAD^ &&\n> + test_commit one &&\n> + test_tick &&\n> + (GIT_AUTHOR_NAME=\"Hank\" GIT_AUTHOR_EMAIL=\"hank@sea.world\" \\\n> + git merge -m \"Tüntenfüsch\" two three) &&\n\nUnnecessary subshell?\n\n> + : fast forward if possible &&\n> + before=\"$(git rev-parse --verify HEAD)\" &&\n> + test_tick &&\n> + git rebase -i -r HEAD^^ &&\n> + test_cmp_rev HEAD $before &&\n> +\n> + test_tick &&\n> + git rebase -i --force -r HEAD^^ &&\n> + test \"Hank\" = \"$(git show -s --format=%an HEAD)\" &&\n> + test \"$before\" != $(git rev-parse HEAD) &&\n> + test_cmp_graph HEAD^^.. <<-\\EOF\n> + *-. Tüntenfüsch\n> + |\\ \\\n> + | | * three\n> + | * | two\n> + | |/\n> + * | one\n> + |/\n> + o before-octopus\n> + EOF\n> +'\n"},{"id":"352307","messageId":"xmqqfu0p1luz.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"CAPig+cS1KQseynMCGC_6FZwzB0waJX8+C7CVKNqACzOhjB4uSg@mail.gmail.com","subject":"Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-11T21:52:20Z","receivedAt":"2018-07-11T21:52:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> diff --git a/sequencer.c b/sequencer.c\n>> @@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n>> - strbuf_addf(&buf, \"Merge branch '%.*s'\",\n>> + strbuf_addf(&buf, \"Merge %s '%.*s'\",\n>> + to_merge->next ? \"branches\" : \"branch\",\n>\n> Error messages in this file are already localizable. If this text ever\n> becomes localizable, then this \"sentence lego\" could be problematic\n> for translators.\n\nI do not think we'd want to localize these default merge messages,\nthough.\n\n>> @@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n>> + cmd.git_cmd = 1;\n>> + argv_array_push(&cmd.args, \"merge\");\n>> + argv_array_push(&cmd.args, \"-s\");\n>> + argv_array_push(&cmd.args, \"octopus\");\n>\n> argv_array_pushl(&cmd.args, \"-s\", \"octopus\", NULL);\n>\n> which would make it clear that these two arguments must remain\n> together, and prevent someone from coming along and inserting a new\n> argument between them.\n\nA valid point. It is OK to break \"merge\" and \"-s octopus\" into\nseparate push invocations, but not \"-s\" and \"octopus\". Or perhaps\npush it as a single \"--strategy=octopus\" argument, which would be\na better approach anyway.\n\n>> + (GIT_AUTHOR_NAME=\"Hank\" GIT_AUTHOR_EMAIL=\"hank@sea.world\" \\\n>> + git merge -m \"Tüntenfüsch\" two three) &&\n>\n> Unnecessary subshell?\n\nRight.\n"},{"id":"352308","messageId":"xmqqbmbd1l7i.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"9e2c1fe83178f7047d8f2aacaa961381a4894081.1531312689.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] merge: allow reading the merge commit message from a file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-11T22:06:25Z","receivedAt":"2018-07-11T22:06:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" \nwrites:\n\n> From: Johannes Schindelin \n>\n> This is consistent with `git commit` which, like `git merge`, supports\n> passing the commit message via `-m ` and, unlike `git merge` before\n> this patch, via `-F `.\n>\n> It is useful to allow this for scripted use, or for the upcoming patch\n> to allow (re-)creating octopus merges in `git rebase --rebase-merges`.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> Documentation/git-merge.txt | 10 +++++++++-\n> builtin/merge.c | 32 ++++++++++++++++++++++++++++++++\n> 2 files changed, 41 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt\n> index 6a5c00e2c..537dca7fa 100644\n> --- a/Documentation/git-merge.txt\n> +++ b/Documentation/git-merge.txt\n> @@ -12,7 +12,7 @@ SYNOPSIS\n> 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]\n> \t[-s ] [-X ] [-S[]]\n> \t[--[no-]allow-unrelated-histories]\n> -\t[--[no-]rerere-autoupdate] [-m ] [...]\n> +\t[--[no-]rerere-autoupdate] [-m ] [-F ] [...]\n> 'git merge' --abort\n> 'git merge' --continue\n> \n> @@ -75,6 +75,14 @@ The 'git fmt-merge-msg' command can be\n> used to give a good default for automated 'git merge'\n> invocations. The automated message can include the branch description.\n> \n> +-F ::\n> +--file=::\n> +\tRead the commit message to be used for the merge commit (in\n> +\tcase one is created).\n> ++\n> +If `--log` is specified, a shortlog of the commits being merged\n> +will be appended to the specified message.\n> +\n> --[no-]rerere-autoupdate::\n> \tAllow the rerere mechanism to update the index with the\n> \tresult of auto-conflict resolution if possible.\n> diff --git a/builtin/merge.c b/builtin/merge.c\n> index 4a4c09496..b0e907751 100644\n> --- a/builtin/merge.c\n> +++ b/builtin/merge.c\n> @@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,\n> \treturn 0;\n> }\n> \n> +static int option_read_message(struct parse_opt_ctx_t *ctx,\n> +\t\t\t const struct option *opt, int unset)\n> +{\n> +\tstruct strbuf *buf = opt->value;\n> +\tconst char *arg;\n> +\n> +\tif (unset)\n> +\t\tBUG(\"-F cannot be negated\");\n\nThe message \"-F cannot be negated\" looks as if it is pointing out a\nmistake by the end user, and does not mesh well with the real reason\nwhy this is BUG() and is not die().\n\nI understand that this is BUG() not die() because options[] array\ntells this callback not to be called with unset by having the\nPARSE_OPT_NONEG bit there.\n\n> +\tif (ctx->opt) {\n> +\t\targ = ctx->opt;\n> +\t\tctx->opt = NULL;\n> +\t} else if (ctx->argc > 1) {\n> +\t\tctx->argc--;\n> +\t\targ = *++ctx->argv;\n> +\t} else\n> +\t\treturn opterror(opt, \"requires a value\", 0);\n> +\n> +\tif (buf->len)\n> +\t\tstrbuf_addch(buf, '\\n');\n\nDo we assume that buf, if it is not empty, is properly terminated\nwith LF already? I am wondering if the real reason we do these two\nlines is to make sure we have a separating blank line between what\nis already there (if there already is something) and what we add, in\nwhich case the above would want to say\n\n\tif (buf->len) {\n\t\tstrbuf_complete_line(buf);\n\t\tstrbuf_addch(buf, '\\n');\n\t}\n\ninstead.\n\n> +\tif (ctx->prefix && !is_absolute_path(arg))\n> +\t\targ = prefix_filename(ctx->prefix, arg);\n> +\tif (strbuf_read_file(buf, arg, 0) < 0)\n> +\t\treturn error(_(\"could not read file '%s'\"), arg);\n> +\thave_message = 1;\n\nA similar question is what we would want to do when the file ends\nwith an incomplete line. With \"--log\", we would be appending more\nstuff to buf, and we'd want to complete such an incomplete line\nbefore that happens, either here or in the code immediately before\n\"--log\" is processed.\n\n> +\n> +\treturn 0;\n> +}\n> +\n> static struct strategy *get_strategy(const char *name)\n> {\n> \tint i;\n> @@ -228,6 +257,9 @@ static struct option builtin_merge_options[] = {\n> \tOPT_CALLBACK('m', \"message\", &merge_msg, N_(\"message\"),\n> \t\tN_(\"merge commit message (for a non-fast-forward merge)\"),\n> \t\toption_parse_message),\n> +\t{ OPTION_LOWLEVEL_CALLBACK, 'F', \"file\", &merge_msg, N_(\"path\"),\n> +\t\tN_(\"read message from file\"), PARSE_OPT_NONEG,\n> +\t\t(parse_opt_cb *) option_read_message },\n> \tOPT__VERBOSITY(&verbosity),\n> \tOPT_BOOL(0, \"abort\", &abort_current_merge,\n> \t\tN_(\"abort the current in-progress merge\")),\n"},{"id":"352330","messageId":"nycvar.QRO.7.76.6.1807121447240.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"CABPp-BFMKnTMYTd3mK69wZ7__TZTwoF9NmX+RXwyqgUj7NJ0Kw@mail.gmail.com","subject":"Re: [PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T12:48:01Z","receivedAt":"2018-07-12T12:48:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Elijah,\n\nOn Wed, 11 Jul 2018, Elijah Newren wrote:\n\n> On Fri, Mar 9, 2018 at 8:36 AM, Johannes Schindelin via GitGitGadget\n> wrote:\n>\n> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt\n> > index 0e20a66e7..c4bcd24bb 100644\n> > --- a/Documentation/git-rebase.txt\n> > +++ b/Documentation/git-rebase.txt\n> > @@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.\n> > when the merge operation did not even start), it is rescheduled immediately.\n> >\n> > At this time, the `merge` command will *always* use the `recursive`\n> > -merge strategy, with no way to choose a different one. To work around\n> > +merge strategy for regular merges, and `octopus` for octopus merges,\n> > +strategy, with no way to choose a different one. To work around\n> \n> The \"...merges, strategy, with...\" looks like an incomplete edit.\n> Perhaps \"...and `octopus` strategy for octopus merges, with no way...\"\n> ?\n\nOkay. Will be fixed in v2.\n\nCiao,\nDscho\n"},{"id":"352331","messageId":"nycvar.QRO.7.76.6.1807121448530.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"xmqqbmbd3cc3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T12:49:48Z","receivedAt":"2018-07-12T12:50:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 11 Jul 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > The `--rebase-merges` option of `git rebase` generates todo lists that\n> > include the merge commits that are to be rebased.\n> >\n> > To keep things simpler to review, I decided to support only regular,\n> > 2-parent merge commits first.\n> >\n> > With this patch series, support is extended to cover also octopus\n> > merges.\n> \n> ;-)\n> \n> To be honest, I am not sure if there still are people who use octopus\n> (even though I freely admit that I am guilty of inventing the term and\n> the mechanism), given its downsides, primary one of which is that it\n> makes bisection less efficient.\n> \n> Nevertheless, this *is* the right thing to do from feature completeness\n> point of view. Thanks for following it through.\n\n--preserve-merges supports octopus merges, IIRC. And as I want to\ndeprecate --preserve-merges, --rebase-merges *has* to support octopus\nmerges, whether you or I would ever use them or not.\n\nCiao,\nDscho\n"},{"id":"352332","messageId":"nycvar.QRO.7.76.6.1807121450100.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"CAGZ79kaYpw9t3kCvPnDiLc5z7cLFsVQv4XJDAff_T-bShEjPoQ@mail.gmail.com","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T12:54:38Z","receivedAt":"2018-07-12T12:54:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 11 Jul 2018, Stefan Beller wrote:\n\n> On Wed, Jul 11, 2018 at 10:35 AM Junio C Hamano wrote:\n> > To be honest, I am not sure if there still are people who use\n> > octopus\n> \n> The latest merge with more than 2 parents in linux.git is df958569dbaa\n> (Merge branches 'acpi-tables' and 'acpica', 2018-07-05), although\n> looking through the log of octopusses I get the impression that mostly\n> Rafael J. Wysocki is really keen on these.\n> :-)\n\nIMO core Git contributors seriously need to forget about using the Linux\nkernel repository as the gold standard when looking how Git is used.\n\nGit is used in *so many* different scenarios, and both in terms of\ncommits/day as well as overall repository size *and* development speed,\nLinux is not even in the \"smack down the middle\" category. Compared to\nwhat is being done with Git on a daily basis, the Linux kernel repository\n(and project structure) is relatively small.\n\nA much more meaningful measure would be: how many octopus merge commits\nhave been pushed to GitHub in the past two weeks. I don't think I have the\ntechnical means to answer that question, though.\n\nIn any case, the Git project is run in such a way that even having a\nfeature used even by just single user whose name happens to be Andrew\nMorton declares that feature off-limits for deprecation.\n\nWhen applying this context to `--rebase-merges` and Octopus merges, even a\nsingle user would be sufficient for us to support that feature. And I am\nsure that there are more than just a dozen users of this feature.\n\nCiao,\nDscho\n"},{"id":"352333","messageId":"nycvar.QRO.7.76.6.1807121455130.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"xmqqbmbd1l7i.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/3] merge: allow reading the merge commit message from a file","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T12:58:59Z","receivedAt":"2018-07-12T12:59:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 11 Jul 2018, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" \n> writes:\n> \n> > diff --git a/builtin/merge.c b/builtin/merge.c\n> > index 4a4c09496..b0e907751 100644\n> > --- a/builtin/merge.c\n> > +++ b/builtin/merge.c\n> > @@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,\n> > \treturn 0;\n> > }\n> > \n> > +static int option_read_message(struct parse_opt_ctx_t *ctx,\n> > +\t\t\t const struct option *opt, int unset)\n> > +{\n> > +\tstruct strbuf *buf = opt->value;\n> > +\tconst char *arg;\n> > +\n> > +\tif (unset)\n> > +\t\tBUG(\"-F cannot be negated\");\n> \n> The message \"-F cannot be negated\" looks as if it is pointing out a\n> mistake by the end user, and does not mesh well with the real reason\n> why this is BUG() and is not die().\n> \n> I understand that this is BUG() not die() because options[] array\n> tells this callback not to be called with unset by having the\n> PARSE_OPT_NONEG bit there.\n\nOkay. I would have appreciated some sort of indication what you prefer\ninstead. I went with \"--no-file?!?\"\n\n> > +\tif (ctx->opt) {\n> > +\t\targ = ctx->opt;\n> > +\t\tctx->opt = NULL;\n> > +\t} else if (ctx->argc > 1) {\n> > +\t\tctx->argc--;\n> > +\t\targ = *++ctx->argv;\n> > +\t} else\n> > +\t\treturn opterror(opt, \"requires a value\", 0);\n> > +\n> > +\tif (buf->len)\n> > +\t\tstrbuf_addch(buf, '\\n');\n> \n> Do we assume that buf, if it is not empty, is properly terminated\n> with LF already? I am wondering if the real reason we do these two\n> lines is to make sure we have a separating blank line between what\n> is already there (if there already is something) and what we add, in\n> which case the above would want to say\n> \n> \tif (buf->len) {\n> \t\tstrbuf_complete_line(buf);\n> \t\tstrbuf_addch(buf, '\\n');\n> \t}\n> \n> instead.\n\nTrue. Thanks for the suggestion!\n\n> > +\tif (ctx->prefix && !is_absolute_path(arg))\n> > +\t\targ = prefix_filename(ctx->prefix, arg);\n> > +\tif (strbuf_read_file(buf, arg, 0) < 0)\n> > +\t\treturn error(_(\"could not read file '%s'\"), arg);\n> > +\thave_message = 1;\n> \n> A similar question is what we would want to do when the file ends\n> with an incomplete line. With \"--log\", we would be appending more\n> stuff to buf, and we'd want to complete such an incomplete line\n> before that happens, either here or in the code immediately before\n> \"--log\" is processed.\n\nThis is what I inserted here:\n\n\tstrbuf_complete_line(buf);\n\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > static struct strategy *get_strategy(const char *name)\n> > {\n> > \tint i;\n\nThanks for the review, and especially for the suggestions how to improve\nthe code.\n\nCiao,\nDscho\n"},{"id":"352335","messageId":"nycvar.QRO.7.76.6.1807121503220.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"CAPig+cS1KQseynMCGC_6FZwzB0waJX8+C7CVKNqACzOhjB4uSg@mail.gmail.com","subject":"Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T13:08:33Z","receivedAt":"2018-07-12T13:08:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Eric,\n\nOn Wed, 11 Jul 2018, Eric Sunshine wrote:\n\n> On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget\n> wrote:\n\n> > diff --git a/sequencer.c b/sequencer.c\n> > @@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n> > - strbuf_addf(&buf, \"Merge branch '%.*s'\",\n> > + strbuf_addf(&buf, \"Merge %s '%.*s'\",\n> > + to_merge->next ? \"branches\" : \"branch\",\n> \n> Error messages in this file are already localizable. If this text ever\n> becomes localizable, then this \"sentence lego\" could be problematic\n> for translators.\n\nAs pointed out by Junio, this is not really a message eligible for\nlocalization, at least not right now. If it becomes localized later, it\nwill be very easy to change the sentence lego then.\n\nI am just wary of making life harder for the compiler that wants to verify\nthat the printf() style parameters match the format.\n\n> > @@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n> > + cmd.git_cmd = 1;\n> > + argv_array_push(&cmd.args, \"merge\");\n> > + argv_array_push(&cmd.args, \"-s\");\n> > + argv_array_push(&cmd.args, \"octopus\");\n> \n> argv_array_pushl(&cmd.args, \"-s\", \"octopus\", NULL);\n> \n> which would make it clear that these two arguments must remain\n> together, and prevent someone from coming along and inserting a new\n> argument between them.\n\nI use a single `argv_array_pushl()` call now, with intuitive line\nwrapping.\n\n> > + argv_array_push(&cmd.args, \"--no-edit\");\n> > + argv_array_push(&cmd.args, \"--no-ff\");\n> > + argv_array_push(&cmd.args, \"--no-log\");\n> > + argv_array_push(&cmd.args, \"--no-stat\");\n> > + argv_array_push(&cmd.args, \"-F\");\n> > + argv_array_push(&cmd.args, git_path_merge_msg());\n> \n> Ditto:\n> argv_array_pushl(&cmd.args, \"-L\", git_path_merge_msg(), NULL);\n> \n> > diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh\n> > @@ -329,4 +329,38 @@ test_expect_success 'labels that are object IDs are rewritten' '\n> > +test_expect_success 'octopus merges' '\n> > + git checkout -b three &&\n> > + test_commit before-octopus &&\n> > + test_commit three &&\n> > + git checkout -b two HEAD^ &&\n> > + test_commit two &&\n> > + git checkout -b one HEAD^ &&\n> > + test_commit one &&\n> > + test_tick &&\n> > + (GIT_AUTHOR_NAME=\"Hank\" GIT_AUTHOR_EMAIL=\"hank@sea.world\" \\\n> > + git merge -m \"Tüntenfüsch\" two three) &&\n> \n> Unnecessary subshell?\n\nYes, I agree. I don't recall why I did it that way, probably I tried\nsomething else originally that required a subshell, or something.\n\nThank you for helping me make the patches better.\n\nCiao,\nDscho"},{"id":"352336","messageId":"nycvar.QRO.7.76.6.1807121509550.75@tvgsbejvaqbjf.bet","threadId":"48862","inReplyTo":"xmqqfu0p1luz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-07-12T13:11:17Z","receivedAt":"2018-07-12T13:11:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 11 Jul 2018, Junio C Hamano wrote:\n\n> Eric Sunshine writes:\n> \n> >> @@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,\n> >> + cmd.git_cmd = 1;\n> >> + argv_array_push(&cmd.args, \"merge\");\n> >> + argv_array_push(&cmd.args, \"-s\");\n> >> + argv_array_push(&cmd.args, \"octopus\");\n> >\n> > argv_array_pushl(&cmd.args, \"-s\", \"octopus\", NULL);\n> >\n> > which would make it clear that these two arguments must remain\n> > together, and prevent someone from coming along and inserting a new\n> > argument between them.\n> \n> A valid point. It is OK to break \"merge\" and \"-s octopus\" into\n> separate push invocations, but not \"-s\" and \"octopus\". Or perhaps\n> push it as a single \"--strategy=octopus\" argument, which would be\n> a better approach anyway.\n\nI had missed that in the documentation, as the synopsis does not mention\nit.\n\nThank you!\nDscho\n"},{"id":"352360","messageId":"xmqqpnzszar0.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"nycvar.QRO.7.76.6.1807121455130.75@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/3] merge: allow reading the merge commit message from a file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-12T16:20:35Z","receivedAt":"2018-07-12T16:20:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> > +static int option_read_message(struct parse_opt_ctx_t *ctx,\n>> > +\t\t\t const struct option *opt, int unset)\n>> > +{\n>> > +\tstruct strbuf *buf = opt->value;\n>> > +\tconst char *arg;\n>> > +\n>> > +\tif (unset)\n>> > +\t\tBUG(\"-F cannot be negated\");\n>> \n>> The message \"-F cannot be negated\" looks as if it is pointing out a\n>> mistake by the end user, and does not mesh well with the real reason\n>> why this is BUG() and is not die().\n>> \n>> I understand that this is BUG() not die() because options[] array\n>> tells this callback not to be called with unset by having the\n>> PARSE_OPT_NONEG bit there.\n>\n> Okay. I would have appreciated some sort of indication what you prefer\n> instead. I went with \"--no-file?!?\"\n\nI have no strong preference; anything is OK as long as the message\nis unique and points reading developer in the right direction, and\n\"--no-file?!?\" signals quite strongly that the code is not expected\nthat it has to handle that option at this point (instead, it expects\nsomebody else has dealt with it), so it sounds fine.\n\nI think doing all of these inside parse_options callback means that\nyou can have \"merge -F file1 -F file2\" and slurp contents from both\nfiles as separate paragraphs. I briefly wondered if --no-file is\nsomething the end user might want to be able to use to discard what\nhas been read so far, but \"merge -m msg -F file --no-file\" would\nhave to discard everything, not just what we read from the file, so\nit would not be useful with the structure of the message assembly\nwe have today, which this code builds on.\n\n>> > +\tif (ctx->opt) {\n>> > +\t\targ = ctx->opt;\n>> > +\t\tctx->opt = NULL;\n>> > +\t} else if (ctx->argc > 1) {\n>> > +\t\tctx->argc--;\n>> > +\t\targ = *++ctx->argv;\n>> > +\t} else\n>> > +\t\treturn opterror(opt, \"requires a value\", 0);\n>> > +\n>> > +\tif (buf->len)\n>> > +\t\tstrbuf_addch(buf, '\\n');\n>> \n>> Do we assume that buf, if it is not empty, is properly terminated\n>> with LF already? I am wondering if the real reason we do these two\n>> lines is to make sure we have a separating blank line between what\n>> is already there (if there already is something) and what we add, in\n>> which case the above would want to say\n>> \n>> \tif (buf->len) {\n>> \t\tstrbuf_complete_line(buf);\n>> \t\tstrbuf_addch(buf, '\\n');\n>> \t}\n>> \n>> instead.\n>\n> True. Thanks for the suggestion!\n>\n>> > +\tif (ctx->prefix && !is_absolute_path(arg))\n>> > +\t\targ = prefix_filename(ctx->prefix, arg);\n>> > +\tif (strbuf_read_file(buf, arg, 0) < 0)\n>> > +\t\treturn error(_(\"could not read file '%s'\"), arg);\n>> > +\thave_message = 1;\n>> \n>> A similar question is what we would want to do when the file ends\n>> with an incomplete line. With \"--log\", we would be appending more\n>> stuff to buf, and we'd want to complete such an incomplete line\n>> before that happens, either here or in the code immediately before\n>> \"--log\" is processed.\n>\n> This is what I inserted here:\n>\n> \tstrbuf_complete_line(buf);\n\nI had a slight suspicion that completing immediately before we\nappend anything in a later step in the codepath would be safer.\nWhen we get a complaint: \n\n 'merge -F file' when I am not using '--log' or adding sign-off,\n adds an extra newline at the end when I deliberately give a file\n that ends with an incomplete line for such and such reasons.\n\nI do not think I would have a good argument why the then-current\nbehaviour is not a bug but an intended behaviour.\n\nAnd I do not think the fact that I am unable to fill \"such and such\"\nabove means such a complaint is nonsense---it merely indicates that\nI lack imagination and that I am not thinking enough to accomodate\nother people's needs.\n"},{"id":"352361","messageId":"xmqqlgagzah1.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"nycvar.QRO.7.76.6.1807121450100.75@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-12T16:26:34Z","receivedAt":"2018-07-12T16:26:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Git is used in *so many* different scenarios, and both in terms of\n> commits/day as well as overall repository size *and* development speed,\n\nThat misses another important factor, though. How well the project\nuses the tool, iow, how canonical should its way of using the tool\nbe considered and encouraged to be imitated by others.\n\n> A much more meaningful measure would be: how many octopus merge commits\n> have been pushed to GitHub in the past two weeks. I don't think I have the\n> technical means to answer that question, though.\n\nIt does not mean that misusing a feature is a good thing and should\nbe encouraged if many misguided people do so.\n"},{"id":"352476","messageId":"8dbaefe4-778e-1e83-4beb-0261699cc483@kdbg.org","threadId":"48862","inReplyTo":"xmqqlgagzah1.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2018-07-13T16:42:07Z","receivedAt":"2018-07-13T16:42:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.07.2018 um 18:26 schrieb Junio C Hamano:\n> Johannes Schindelin writes:\n>> A much more meaningful measure would be: how many octopus merge commits\n>> have been pushed to GitHub in the past two weeks. I don't think I have the\n>> technical means to answer that question, though.\n> \n> It does not mean that misusing a feature is a good thing and should\n> be encouraged if many misguided people do so.\n\nJust recently I had to rebuild the version of git-gui that comes with \nGit 2.18.0 before it was released:\n\nhttps://github.com/j6t/git-gui-ng/commit/f07ae1d7f07b036d78a3d4706e6cb4102e623fb3\n\nI think that an octopus merge is the right tool for the task. Am I \nmisguided?\n\n-- Hannes\n"},{"id":"352638","messageId":"xmqq1sc3ukry.fsf@gitster-ct.c.googlers.com","threadId":"48862","inReplyTo":"8dbaefe4-778e-1e83-4beb-0261699cc483@kdbg.org","subject":"Re: [PATCH 0/3] rebase -r: support octopus merges","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-16T17:56:33Z","receivedAt":"2018-07-16T17:56:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 12.07.2018 um 18:26 schrieb Junio C Hamano:\n>> Johannes Schindelin writes:\n>>> A much more meaningful measure would be: how many octopus merge commits\n>>> have been pushed to GitHub in the past two weeks. I don't think I have the\n>>> technical means to answer that question, though.\n>>\n>> It does not mean that misusing a feature is a good thing and should\n>> be encouraged if many misguided people do so.\n>\n> Just recently I had to rebuild the version of git-gui that comes with\n> Git 2.18.0 before it was released:\n>\n> https://github.com/j6t/git-gui-ng/commit/f07ae1d7f07b036d78a3d4706e6cb4102e623fb3\n>\n> I think that an octopus merge is the right tool for the task. Am I\n> misguided?\n\nIt could be used and it is the right tool are two different things,\nI would think.\n\n"}]}