[PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>
---
 sequencer.c              | 168 +++++++++++++++++++++++++++++----------
 t/t3430-rebase-merges.sh |  34 ++++++++
 2 files changed, 159 insertions(+), 43 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5354d4d51..bcc43699c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2841,6 +2841,26 @@ static int do_reset(const char *name, int len, struct replay_opts *opts)
 	return ret;
 }
 
+static struct commit *lookup_label(const char *label, int len,
+				   struct strbuf *buf)
+{
+	struct commit *commit;
+
+	strbuf_reset(buf);
+	strbuf_addf(buf, "refs/rewritten/%.*s", len, label);
+	commit = lookup_commit_reference_by_name(buf->buf);
+	if (!commit) {
+		/* fall back to non-rewritten ref or commit */
+		strbuf_splice(buf, 0, strlen("refs/rewritten/"), "", 0);
+		commit = lookup_commit_reference_by_name(buf->buf);
+	}
+
+	if (!commit)
+		error(_("could not resolve '%s'"), buf->buf);
+
+	return commit;
+}
+
 static int do_merge(struct commit *commit, const char *arg, int arg_len,
 		    int flags, struct replay_opts *opts)
 {
@@ -2849,8 +2869,9 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 	struct strbuf ref_name = STRBUF_INIT;
 	struct commit *head_commit, *merge_commit, *i;
 	struct commit_list *bases, *j, *reversed = NULL;
+	struct commit_list *to_merge = NULL, **tail = &to_merge;
 	struct merge_options o;
-	int merge_arg_len, oneline_offset, can_fast_forward, ret;
+	int merge_arg_len, oneline_offset, can_fast_forward, ret, k;
 	static struct lock_file lock;
 	const char *p;
 
@@ -2865,26 +2886,34 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 		goto leave_merge;
 	}
 
-	oneline_offset = arg_len;
-	merge_arg_len = strcspn(arg, " \t\n");
-	p = arg + merge_arg_len;
-	p += strspn(p, " \t\n");
-	if (*p == '#' && (!p[1] || isspace(p[1]))) {
-		p += 1 + strspn(p + 1, " \t\n");
-		oneline_offset = p - arg;
-	} else if (p - arg < arg_len)
-		BUG("octopus merges are not supported yet: '%s'", p);
-
-	strbuf_addf(&ref_name, "refs/rewritten/%.*s", merge_arg_len, arg);
-	merge_commit = lookup_commit_reference_by_name(ref_name.buf);
-	if (!merge_commit) {
-		/* fall back to non-rewritten ref or commit */
-		strbuf_splice(&ref_name, 0, strlen("refs/rewritten/"), "", 0);
-		merge_commit = lookup_commit_reference_by_name(ref_name.buf);
+	/*
+	 * For octopus merges, the arg starts with the list of revisions to be
+	 * merged. The list is optionally followed by '#' and the oneline.
+	 */
+	merge_arg_len = oneline_offset = arg_len;
+	for (p = arg; p - arg < arg_len; p += strspn(p, " \t\n")) {
+		if (!*p)
+			break;
+		if (*p == '#' && (!p[1] || isspace(p[1]))) {
+			p += 1 + strspn(p + 1, " \t\n");
+			oneline_offset = p - arg;
+			break;
+		}
+		k = strcspn(p, " \t\n");
+		if (!k)
+			continue;
+		merge_commit = lookup_label(p, k, &ref_name);
+		if (!merge_commit) {
+			ret = error(_("unable to parse '%.*s'"), k, p);
+			goto leave_merge;
+		}
+		tail = &commit_list_insert(merge_commit, tail)->next;
+		p += k;
+		merge_arg_len = p - arg;
 	}
 
-	if (!merge_commit) {
-		ret = error(_("could not resolve '%s'"), ref_name.buf);
+	if (!to_merge) {
+		ret = error(_("nothing to merge: '%.*s'"), arg_len, arg);
 		goto leave_merge;
 	}
 
@@ -2895,8 +2924,13 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 		 * "[new root]", let's simply fast-forward to the merge head.
 		 */
 		rollback_lock_file(&lock);
-		ret = fast_forward_to(&merge_commit->object.oid,
-				       &head_commit->object.oid, 0, opts);
+		if (to_merge->next)
+			ret = error(_("octopus merge cannot be executed on "
+				      "top of a [new root]"));
+		else
+			ret = fast_forward_to(&to_merge->item->object.oid,
+					      &head_commit->object.oid, 0,
+					      opts);
 		goto leave_merge;
 	}
 
@@ -2932,7 +2966,8 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 			p = arg + oneline_offset;
 			len = arg_len - oneline_offset;
 		} else {
-			strbuf_addf(&buf, "Merge branch '%.*s'",
+			strbuf_addf(&buf, "Merge %s '%.*s'",
+				    to_merge->next ? "branches" : "branch",
 				    merge_arg_len, arg);
 			p = buf.buf;
 			len = buf.len;
@@ -2956,28 +2991,76 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 			&head_commit->object.oid);
 
 	/*
-	 * If the merge head is different from the original one, we cannot
+	 * If any merge head is different from the original one, we cannot
 	 * fast-forward.
 	 */
 	if (can_fast_forward) {
-		struct commit_list *second_parent = commit->parents->next;
+		struct commit_list *p = commit->parents->next;
 
-		if (second_parent && !second_parent->next &&
-		    oidcmp(&merge_commit->object.oid,
-			   &second_parent->item->object.oid))
+		for (j = to_merge; j && p; j = j->next, p = p->next)
+			if (oidcmp(&j->item->object.oid,
+				   &p->item->object.oid)) {
+				can_fast_forward = 0;
+				break;
+			}
+		/*
+		 * If the number of merge heads differs from the original merge
+		 * commit, we cannot fast-forward.
+		 */
+		if (j || p)
 			can_fast_forward = 0;
 	}
 
-	if (can_fast_forward && commit->parents->next &&
-	    !commit->parents->next->next &&
-	    !oidcmp(&commit->parents->next->item->object.oid,
-		    &merge_commit->object.oid)) {
+	if (can_fast_forward) {
 		rollback_lock_file(&lock);
 		ret = fast_forward_to(&commit->object.oid,
 				      &head_commit->object.oid, 0, opts);
 		goto leave_merge;
 	}
 
+	if (to_merge->next) {
+		/* Octopus merge */
+		struct child_process cmd = CHILD_PROCESS_INIT;
+
+		if (read_env_script(&cmd.env_array)) {
+			const char *gpg_opt = gpg_sign_opt_quoted(opts);
+
+			ret = error(_(staged_changes_advice), gpg_opt, gpg_opt);
+			goto leave_merge;
+		}
+
+		cmd.git_cmd = 1;
+		argv_array_push(&cmd.args, "merge");
+		argv_array_push(&cmd.args, "-s");
+		argv_array_push(&cmd.args, "octopus");
+		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());
+		if (opts->gpg_sign)
+			argv_array_push(&cmd.args, opts->gpg_sign);
+
+		/* Add the tips to be merged */
+		for (j = to_merge; j; j = j->next)
+			argv_array_push(&cmd.args,
+					oid_to_hex(&j->item->object.oid));
+
+		strbuf_release(&ref_name);
+		unlink(git_path_cherry_pick_head());
+		rollback_lock_file(&lock);
+
+		rollback_lock_file(&lock);
+		ret = run_command(&cmd);
+
+		/* force re-reading of the cache */
+		if (!ret && (discard_cache() < 0 || read_cache() < 0))
+			ret = error(_("could not read index"));
+		goto leave_merge;
+	}
+
+	merge_commit = to_merge->item;
 	write_message(oid_to_hex(&merge_commit->object.oid), GIT_SHA1_HEXSZ,
 		      git_path_merge_head(), 0);
 	write_message("no-ff", 5, git_path_merge_mode(), 0);
@@ -3040,6 +3123,7 @@ static int do_merge(struct commit *commit, const char *arg, int arg_len,
 leave_merge:
 	strbuf_release(&ref_name);
 	rollback_lock_file(&lock);
+	free_commit_list(to_merge);
 	return ret;
 }
 
@@ -3877,7 +3961,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 	 */
 	while ((commit = get_revision(revs))) {
 		struct commit_list *to_merge;
-		int is_octopus;
 		const char *p1, *p2;
 		struct object_id *oid;
 		int is_empty;
@@ -3909,11 +3992,6 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 			continue;
 		}
 
-		is_octopus = to_merge && to_merge->next;
-
-		if (is_octopus)
-			BUG("Octopus merges not yet supported");
-
 		/* Create a label */
 		strbuf_reset(&label);
 		if (skip_prefix(oneline.buf, "Merge ", &p1) &&
@@ -3935,13 +4013,17 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 		strbuf_addf(&buf, "%s -C %s",
 			    cmd_merge, oid_to_hex(&commit->object.oid));
 
-		/* label the tip of merged branch */
-		oid = &to_merge->item->object.oid;
-		strbuf_addch(&buf, ' ');
+		/* label the tips of merged branches */
+		for (; to_merge; to_merge = to_merge->next) {
+			oid = &to_merge->item->object.oid;
+			strbuf_addch(&buf, ' ');
+
+			if (!oidset_contains(&interesting, oid)) {
+				strbuf_addstr(&buf, label_oid(oid, NULL,
+							      &state));
+				continue;
+			}
 
-		if (!oidset_contains(&interesting, oid))
-			strbuf_addstr(&buf, label_oid(oid, NULL, &state));
-		else {
 			tips_tail = &commit_list_insert(to_merge->item,
 							tips_tail)->next;
 
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index 78f7c9958..9e6229727 100755
--- 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' '
 	! grep "^label $third$" .git/ORIGINAL-TODO
 '
 
+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) &&
+
+	: 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
+'
+
 test_done
-- 
gitgitgadget

[PATCH 1/3] merge: allow reading the merge commit message from a file

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

This is consistent with `git commit` which, like `git merge`, supports
passing the commit message via `-m <msg>` and, unlike `git merge` before
this patch, via `-F <file>`.

It is useful to allow this for scripted use, or for the upcoming patch
to allow (re-)creating octopus merges in `git rebase --rebase-merges`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-merge.txt | 10 +++++++++-
 builtin/merge.c             | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 6a5c00e2c..537dca7fa 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
 	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
 	[--[no-]allow-unrelated-histories]
-	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
+	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
 'git merge' --abort
 'git merge' --continue
 
@@ -75,6 +75,14 @@ The 'git fmt-merge-msg' command can be
 used to give a good default for automated 'git merge'
 invocations. The automated message can include the branch description.
 
+-F <file>::
+--file=<file>::
+	Read the commit message to be used for the merge commit (in
+	case one is created).
++
+If `--log` is specified, a shortlog of the commits being merged
+will be appended to the specified message.
+
 --[no-]rerere-autoupdate::
 	Allow the rerere mechanism to update the index with the
 	result of auto-conflict resolution if possible.
diff --git a/builtin/merge.c b/builtin/merge.c
index 4a4c09496..b0e907751 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,
 	return 0;
 }
 
+static int option_read_message(struct parse_opt_ctx_t *ctx,
+			       const struct option *opt, int unset)
+{
+	struct strbuf *buf = opt->value;
+	const char *arg;
+
+	if (unset)
+		BUG("-F cannot be negated");
+
+	if (ctx->opt) {
+		arg = ctx->opt;
+		ctx->opt = NULL;
+	} else if (ctx->argc > 1) {
+		ctx->argc--;
+		arg = *++ctx->argv;
+	} else
+		return opterror(opt, "requires a value", 0);
+
+	if (buf->len)
+		strbuf_addch(buf, '\n');
+	if (ctx->prefix && !is_absolute_path(arg))
+		arg = prefix_filename(ctx->prefix, arg);
+	if (strbuf_read_file(buf, arg, 0) < 0)
+		return error(_("could not read file '%s'"), arg);
+	have_message = 1;
+
+	return 0;
+}
+
 static struct strategy *get_strategy(const char *name)
 {
 	int i;
@@ -228,6 +257,9 @@ static struct option builtin_merge_options[] = {
 	OPT_CALLBACK('m', "message", &merge_msg, N_("message"),
 		N_("merge commit message (for a non-fast-forward merge)"),
 		option_parse_message),
+	{ OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
+		N_("read message from file"), PARSE_OPT_NONEG,
+		(parse_opt_cb *) option_read_message },
 	OPT__VERBOSITY(&verbosity),
 	OPT_BOOL(0, "abort", &abort_current_merge,
 		N_("abort the current in-progress merge")),
-- 
gitgitgadget

[PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

Now that we support octopus merges in the `--rebase-merges` mode,
we should give users who actually read the manuals a chance to know
about this fact.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git-rebase.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0e20a66e7..c4bcd24bb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -879,8 +879,8 @@ rescheduled immediately, with a helpful message how to edit the todo list
 (this typically happens when a `reset` command was inserted into the todo
 list manually and contains a typo).
 
-The `merge` command will merge the specified revision into whatever is
-HEAD at that time. With `-C <original-commit>`, the commit message of
+The `merge` command will merge the specified revision(s) into whatever
+is HEAD at that time. With `-C <original-commit>`, the commit message of
 the specified merge commit will be used. When the `-C` is changed to
 a lower-case `-c`, the message will be opened in an editor after a
 successful merge so that the user can edit the message.
@@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.
 when the merge operation did not even start), it is rescheduled immediately.
 
 At this time, the `merge` command will *always* use the `recursive`
-merge strategy, with no way to choose a different one. To work around
+merge strategy for regular merges, and `octopus` for octopus merges,
+strategy, with no way to choose a different one. To work around
 this, an `exec` command can be used to call `git merge` explicitly,
 using the fact that the labels are worktree-local refs (the ref
 `refs/rewritten/onto` would correspond to the label `onto`, for example).
-- 
gitgitgadget

[PATCH 0/3] rebase -r: support octopus merges

[Johannes Schindelin via GitGitGadget]

The `--rebase-merges` option of `git rebase` generates todo lists that include the merge commits that are to be rebased.

To keep things simpler to review, I decided to support only regular, 2-parent merge commits first.

With this patch series, support is extended to cover also octopus merges.

Johannes Schindelin (3):
  merge: allow reading the merge commit message from a file
  rebase --rebase-merges: add support for octopus merges
  rebase --rebase-merges: adjust man page for octopus support

 Documentation/git-merge.txt  |  10 ++-
 Documentation/git-rebase.txt |   7 +-
 builtin/merge.c              |  32 +++++++
 sequencer.c                  | 168 ++++++++++++++++++++++++++---------
 t/t3430-rebase-merges.sh     |  34 +++++++
 5 files changed, 204 insertions(+), 47 deletions(-)


base-commit: e3331758f12da22f4103eec7efe1b5304a9be5e9
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-8%2Fdscho%2Fsequencer-and-octopus-merges-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-8/dscho/sequencer-and-octopus-merges-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/8
-- 
gitgitgadget

Re: [PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support

[Elijah Newren]

Hi Dscho,

On Fri, Mar 9, 2018 at 8:36 AM, Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> Now that we support octopus merges in the `--rebase-merges` mode,
> we should give users who actually read the manuals a chance to know
> about this fact.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-rebase.txt | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 0e20a66e7..c4bcd24bb 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -879,8 +879,8 @@ rescheduled immediately, with a helpful message how to edit the todo list
>  (this typically happens when a `reset` command was inserted into the todo
>  list manually and contains a typo).
>
> -The `merge` command will merge the specified revision into whatever is
> -HEAD at that time. With `-C <original-commit>`, the commit message of
> +The `merge` command will merge the specified revision(s) into whatever
> +is HEAD at that time. With `-C <original-commit>`, the commit message of
>  the specified merge commit will be used. When the `-C` is changed to
>  a lower-case `-c`, the message will be opened in an editor after a
>  successful merge so that the user can edit the message.
> @@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.
>  when the merge operation did not even start), it is rescheduled immediately.
>
>  At this time, the `merge` command will *always* use the `recursive`
> -merge strategy, with no way to choose a different one. To work around
> +merge strategy for regular merges, and `octopus` for octopus merges,
> +strategy, with no way to choose a different one. To work around

The "...merges, strategy, with..." looks like an incomplete edit.
Perhaps "...and `octopus` strategy for octopus merges, with no way..."
?

>  this, an `exec` command can be used to call `git merge` explicitly,
>  using the fact that the labels are worktree-local refs (the ref
>  `refs/rewritten/onto` would correspond to the label `onto`, for example).
> --
> gitgitgadget

Re: [PATCH 0/3] rebase -r: support octopus merges

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> The `--rebase-merges` option of `git rebase` generates todo lists that include the merge commits that are to be rebased.
>
> To keep things simpler to review, I decided to support only regular, 2-parent merge commits first.
>
> With this patch series, support is extended to cover also octopus merges.

;-)

To be honest, I am not sure if there still are people who use
octopus (even though I freely admit that I am guilty of inventing
the term and the mechanism), given its downsides, primary one of
which is that it makes bisection less efficient.

Nevertheless, this *is* the right thing to do from feature
completeness point of view.  Thanks for following it through.

>
> Johannes Schindelin (3):
>   merge: allow reading the merge commit message from a file
>   rebase --rebase-merges: add support for octopus merges
>   rebase --rebase-merges: adjust man page for octopus support
>
>  Documentation/git-merge.txt  |  10 ++-
>  Documentation/git-rebase.txt |   7 +-
>  builtin/merge.c              |  32 +++++++
>  sequencer.c                  | 168 ++++++++++++++++++++++++++---------
>  t/t3430-rebase-merges.sh     |  34 +++++++
>  5 files changed, 204 insertions(+), 47 deletions(-)
>
>
> base-commit: e3331758f12da22f4103eec7efe1b5304a9be5e9
> Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-8%2Fdscho%2Fsequencer-and-octopus-merges-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-8/dscho/sequencer-and-octopus-merges-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/8

Re: [PATCH 0/3] rebase -r: support octopus merges

[Stefan Beller]

On Wed, Jul 11, 2018 at 10:35 AM Junio C Hamano <gitster@pobox.com> wrote:
> To be honest, I am not sure if there still are people who use
> octopus

The latest merge with more than 2 parents in linux.git is
df958569dbaa (Merge branches 'acpi-tables' and 'acpica', 2018-07-05),
although looking through the log of octopusses I get the impression that
mostly Rafael J. Wysocki <rafael.j.wysocki@intel.com> is really keen on
these. :-)

Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[Eric Sunshine]

On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget
<gitgitgadget@gmail.com> 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 <johannes.schindelin@gmx.de>

A few minor notes below (not a proper review)...

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

> @@ -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);

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

Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

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

I do not think we'd want to localize these default merge messages,
though.

>> @@ -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.

A valid point.  It is OK to break "merge" and "-s octopus" into
separate push invocations, but not "-s" and "octopus".  Or perhaps
push it as a single "--strategy=octopus" argument, which would be
a better approach anyway.

>> +       (GIT_AUTHOR_NAME="Hank" GIT_AUTHOR_EMAIL="hank@sea.world" \
>> +        git merge -m "Tüntenfüsch" two three) &&
>
> Unnecessary subshell?

Right.

Re: [PATCH 1/3] merge: allow reading the merge commit message from a file

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> This is consistent with `git commit` which, like `git merge`, supports
> passing the commit message via `-m <msg>` and, unlike `git merge` before
> this patch, via `-F <file>`.
>
> It is useful to allow this for scripted use, or for the upcoming patch
> to allow (re-)creating octopus merges in `git rebase --rebase-merges`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  Documentation/git-merge.txt | 10 +++++++++-
>  builtin/merge.c             | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index 6a5c00e2c..537dca7fa 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -12,7 +12,7 @@ SYNOPSIS
>  'git merge' [-n] [--stat] [--no-commit] [--squash] [--[no-]edit]
>  	[-s <strategy>] [-X <strategy-option>] [-S[<keyid>]]
>  	[--[no-]allow-unrelated-histories]
> -	[--[no-]rerere-autoupdate] [-m <msg>] [<commit>...]
> +	[--[no-]rerere-autoupdate] [-m <msg>] [-F <file>] [<commit>...]
>  'git merge' --abort
>  'git merge' --continue
>  
> @@ -75,6 +75,14 @@ The 'git fmt-merge-msg' command can be
>  used to give a good default for automated 'git merge'
>  invocations. The automated message can include the branch description.
>  
> +-F <file>::
> +--file=<file>::
> +	Read the commit message to be used for the merge commit (in
> +	case one is created).
> ++
> +If `--log` is specified, a shortlog of the commits being merged
> +will be appended to the specified message.
> +
>  --[no-]rerere-autoupdate::
>  	Allow the rerere mechanism to update the index with the
>  	result of auto-conflict resolution if possible.
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 4a4c09496..b0e907751 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,
>  	return 0;
>  }
>  
> +static int option_read_message(struct parse_opt_ctx_t *ctx,
> +			       const struct option *opt, int unset)
> +{
> +	struct strbuf *buf = opt->value;
> +	const char *arg;
> +
> +	if (unset)
> +		BUG("-F cannot be negated");

The message "-F cannot be negated" looks as if it is pointing out a
mistake by the end user, and does not mesh well with the real reason
why this is BUG() and is not die().

I understand that this is BUG() not die() because options[] array
tells this callback not to be called with unset by having the
PARSE_OPT_NONEG bit there.

> +	if (ctx->opt) {
> +		arg = ctx->opt;
> +		ctx->opt = NULL;
> +	} else if (ctx->argc > 1) {
> +		ctx->argc--;
> +		arg = *++ctx->argv;
> +	} else
> +		return opterror(opt, "requires a value", 0);
> +
> +	if (buf->len)
> +		strbuf_addch(buf, '\n');

Do we assume that buf, if it is not empty, is properly terminated
with LF already?  I am wondering if the real reason we do these two
lines is to make sure we have a separating blank line between what
is already there (if there already is something) and what we add, in
which case the above would want to say

	if (buf->len) {
		strbuf_complete_line(buf);
		strbuf_addch(buf, '\n');
	}

instead.

> +	if (ctx->prefix && !is_absolute_path(arg))
> +		arg = prefix_filename(ctx->prefix, arg);
> +	if (strbuf_read_file(buf, arg, 0) < 0)
> +		return error(_("could not read file '%s'"), arg);
> +	have_message = 1;

A similar question is what we would want to do when the file ends
with an incomplete line.  With "--log", we would be appending more
stuff to buf, and we'd want to complete such an incomplete line
before that happens, either here or in the code immediately before
"--log" is processed.

> +
> +	return 0;
> +}
> +
>  static struct strategy *get_strategy(const char *name)
>  {
>  	int i;
> @@ -228,6 +257,9 @@ static struct option builtin_merge_options[] = {
>  	OPT_CALLBACK('m', "message", &merge_msg, N_("message"),
>  		N_("merge commit message (for a non-fast-forward merge)"),
>  		option_parse_message),
> +	{ OPTION_LOWLEVEL_CALLBACK, 'F', "file", &merge_msg, N_("path"),
> +		N_("read message from file"), PARSE_OPT_NONEG,
> +		(parse_opt_cb *) option_read_message },
>  	OPT__VERBOSITY(&verbosity),
>  	OPT_BOOL(0, "abort", &abort_current_merge,
>  		N_("abort the current in-progress merge")),

Re: [PATCH 3/3] rebase --rebase-merges: adjust man page for octopus support

[Johannes Schindelin]

Hi Elijah,

On Wed, 11 Jul 2018, Elijah Newren wrote:

> On Fri, Mar 9, 2018 at 8:36 AM, Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>
> > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> > index 0e20a66e7..c4bcd24bb 100644
> > --- a/Documentation/git-rebase.txt
> > +++ b/Documentation/git-rebase.txt
> > @@ -889,7 +889,8 @@ If a `merge` command fails for any reason other than merge conflicts (i.e.
> >  when the merge operation did not even start), it is rescheduled immediately.
> >
> >  At this time, the `merge` command will *always* use the `recursive`
> > -merge strategy, with no way to choose a different one. To work around
> > +merge strategy for regular merges, and `octopus` for octopus merges,
> > +strategy, with no way to choose a different one. To work around
> 
> The "...merges, strategy, with..." looks like an incomplete edit.
> Perhaps "...and `octopus` strategy for octopus merges, with no way..."
> ?

Okay. Will be fixed in v2.

Ciao,
Dscho

Re: [PATCH 0/3] rebase -r: support octopus merges

[Johannes Schindelin]

Hi Junio,

On Wed, 11 Jul 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > The `--rebase-merges` option of `git rebase` generates todo lists that
> > include the merge commits that are to be rebased.
> >
> > To keep things simpler to review, I decided to support only regular,
> > 2-parent merge commits first.
> >
> > With this patch series, support is extended to cover also octopus
> > merges.
> 
> ;-)
> 
> To be honest, I am not sure if there still are people who use octopus
> (even though I freely admit that I am guilty of inventing the term and
> the mechanism), given its downsides, primary one of which is that it
> makes bisection less efficient.
> 
> Nevertheless, this *is* the right thing to do from feature completeness
> point of view.  Thanks for following it through.

--preserve-merges supports octopus merges, IIRC. And as I want to
deprecate --preserve-merges, --rebase-merges *has* to support octopus
merges, whether you or I would ever use them or not.

Ciao,
Dscho

Re: [PATCH 0/3] rebase -r: support octopus merges

[Johannes Schindelin]

Hi Stefan,

On Wed, 11 Jul 2018, Stefan Beller wrote:

> On Wed, Jul 11, 2018 at 10:35 AM Junio C Hamano <gitster@pobox.com> wrote:
> > To be honest, I am not sure if there still are people who use
> > octopus
> 
> The latest merge with more than 2 parents in linux.git is df958569dbaa
> (Merge branches 'acpi-tables' and 'acpica', 2018-07-05), although
> looking through the log of octopusses I get the impression that mostly
> Rafael J. Wysocki <rafael.j.wysocki@intel.com> is really keen on these.
> :-)

IMO core Git contributors seriously need to forget about using the Linux
kernel repository as the gold standard when looking how Git is used.

Git is used in *so many* different scenarios, and both in terms of
commits/day as well as overall repository size *and* development speed,
Linux is not even in the "smack down the middle" category. Compared to
what is being done with Git on a daily basis, the Linux kernel repository
(and project structure) is relatively small.

A much more meaningful measure would be: how many octopus merge commits
have been pushed to GitHub in the past two weeks. I don't think I have the
technical means to answer that question, though.

In any case, the Git project is run in such a way that even having a
feature used even by just single user whose name happens to be Andrew
Morton declares that feature off-limits for deprecation.

When applying this context to `--rebase-merges` and Octopus merges, even a
single user would be sufficient for us to support that feature. And I am
sure that there are more than just a dozen users of this feature.

Ciao,
Dscho

Re: [PATCH 1/3] merge: allow reading the merge commit message from a file

[Johannes Schindelin]

Hi Junio,

On Wed, 11 Jul 2018, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > diff --git a/builtin/merge.c b/builtin/merge.c
> > index 4a4c09496..b0e907751 100644
> > --- a/builtin/merge.c
> > +++ b/builtin/merge.c
> > @@ -111,6 +111,35 @@ static int option_parse_message(const struct option *opt,
> >  	return 0;
> >  }
> >  
> > +static int option_read_message(struct parse_opt_ctx_t *ctx,
> > +			       const struct option *opt, int unset)
> > +{
> > +	struct strbuf *buf = opt->value;
> > +	const char *arg;
> > +
> > +	if (unset)
> > +		BUG("-F cannot be negated");
> 
> The message "-F cannot be negated" looks as if it is pointing out a
> mistake by the end user, and does not mesh well with the real reason
> why this is BUG() and is not die().
> 
> I understand that this is BUG() not die() because options[] array
> tells this callback not to be called with unset by having the
> PARSE_OPT_NONEG bit there.

Okay. I would have appreciated some sort of indication what you prefer
instead. I went with "--no-file?!?"

> > +	if (ctx->opt) {
> > +		arg = ctx->opt;
> > +		ctx->opt = NULL;
> > +	} else if (ctx->argc > 1) {
> > +		ctx->argc--;
> > +		arg = *++ctx->argv;
> > +	} else
> > +		return opterror(opt, "requires a value", 0);
> > +
> > +	if (buf->len)
> > +		strbuf_addch(buf, '\n');
> 
> Do we assume that buf, if it is not empty, is properly terminated
> with LF already?  I am wondering if the real reason we do these two
> lines is to make sure we have a separating blank line between what
> is already there (if there already is something) and what we add, in
> which case the above would want to say
> 
> 	if (buf->len) {
> 		strbuf_complete_line(buf);
> 		strbuf_addch(buf, '\n');
> 	}
> 
> instead.

True. Thanks for the suggestion!

> > +	if (ctx->prefix && !is_absolute_path(arg))
> > +		arg = prefix_filename(ctx->prefix, arg);
> > +	if (strbuf_read_file(buf, arg, 0) < 0)
> > +		return error(_("could not read file '%s'"), arg);
> > +	have_message = 1;
> 
> A similar question is what we would want to do when the file ends
> with an incomplete line.  With "--log", we would be appending more
> stuff to buf, and we'd want to complete such an incomplete line
> before that happens, either here or in the code immediately before
> "--log" is processed.

This is what I inserted here:

	strbuf_complete_line(buf);

> > +
> > +	return 0;
> > +}
> > +
> >  static struct strategy *get_strategy(const char *name)
> >  {
> >  	int i;

Thanks for the review, and especially for the suggestions how to improve
the code.

Ciao,
Dscho

Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[Johannes Schindelin]

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

Hi Eric,

On Wed, 11 Jul 2018, Eric Sunshine wrote:

> On Wed, Jul 11, 2018 at 8:38 AM Johannes Schindelin via GitGitGadget
> <gitgitgadget@gmail.com> wrote:

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

As pointed out by Junio, this is not really a message eligible for
localization, at least not right now. If it becomes localized later, it
will be very easy to change the sentence lego then.

I am just wary of making life harder for the compiler that wants to verify
that the printf() style parameters match the format.

> > @@ -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.

I use a single `argv_array_pushl()` call now, with intuitive line
wrapping.

> > +               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);
> 
> > 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?

Yes, I agree. I don't recall why I did it that way, probably I tried
something else originally that required a subshell, or something.

Thank you for helping me make the patches better.

Ciao,
Dscho

Re: [PATCH 2/3] rebase --rebase-merges: add support for octopus merges

[Johannes Schindelin]

Hi Junio,

On Wed, 11 Jul 2018, Junio C Hamano wrote:

> Eric Sunshine <sunshine@sunshineco.com> writes:
> 
> >> @@ -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.
> 
> A valid point.  It is OK to break "merge" and "-s octopus" into
> separate push invocations, but not "-s" and "octopus".  Or perhaps
> push it as a single "--strategy=octopus" argument, which would be
> a better approach anyway.

I had missed that in the documentation, as the synopsis does not mention
it.

Thank you!
Dscho

Re: [PATCH 1/3] merge: allow reading the merge commit message from a file

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> > +static int option_read_message(struct parse_opt_ctx_t *ctx,
>> > +			       const struct option *opt, int unset)
>> > +{
>> > +	struct strbuf *buf = opt->value;
>> > +	const char *arg;
>> > +
>> > +	if (unset)
>> > +		BUG("-F cannot be negated");
>> 
>> The message "-F cannot be negated" looks as if it is pointing out a
>> mistake by the end user, and does not mesh well with the real reason
>> why this is BUG() and is not die().
>> 
>> I understand that this is BUG() not die() because options[] array
>> tells this callback not to be called with unset by having the
>> PARSE_OPT_NONEG bit there.
>
> Okay. I would have appreciated some sort of indication what you prefer
> instead. I went with "--no-file?!?"

I have no strong preference; anything is OK as long as the message
is unique and points reading developer in the right direction, and
"--no-file?!?" signals quite strongly that the code is not expected
that it has to handle that option at this point (instead, it expects
somebody else has dealt with it), so it sounds fine.

I think doing all of these inside parse_options callback means that
you can have "merge -F file1 -F file2" and slurp contents from both
files as separate paragraphs.  I briefly wondered if --no-file is
something the end user might want to be able to use to discard what
has been read so far, but "merge -m msg -F file --no-file" would
have to discard everything, not just what we read from the file, so
it would not be useful with the structure of the message assembly
we have today, which this code builds on.

>> > +	if (ctx->opt) {
>> > +		arg = ctx->opt;
>> > +		ctx->opt = NULL;
>> > +	} else if (ctx->argc > 1) {
>> > +		ctx->argc--;
>> > +		arg = *++ctx->argv;
>> > +	} else
>> > +		return opterror(opt, "requires a value", 0);
>> > +
>> > +	if (buf->len)
>> > +		strbuf_addch(buf, '\n');
>> 
>> Do we assume that buf, if it is not empty, is properly terminated
>> with LF already?  I am wondering if the real reason we do these two
>> lines is to make sure we have a separating blank line between what
>> is already there (if there already is something) and what we add, in
>> which case the above would want to say
>> 
>> 	if (buf->len) {
>> 		strbuf_complete_line(buf);
>> 		strbuf_addch(buf, '\n');
>> 	}
>> 
>> instead.
>
> True. Thanks for the suggestion!
>
>> > +	if (ctx->prefix && !is_absolute_path(arg))
>> > +		arg = prefix_filename(ctx->prefix, arg);
>> > +	if (strbuf_read_file(buf, arg, 0) < 0)
>> > +		return error(_("could not read file '%s'"), arg);
>> > +	have_message = 1;
>> 
>> A similar question is what we would want to do when the file ends
>> with an incomplete line.  With "--log", we would be appending more
>> stuff to buf, and we'd want to complete such an incomplete line
>> before that happens, either here or in the code immediately before
>> "--log" is processed.
>
> This is what I inserted here:
>
> 	strbuf_complete_line(buf);

I had a slight suspicion that completing immediately before we
append anything in a later step in the codepath would be safer.
When we get a complaint: 

    'merge -F file' when I am not using '--log' or adding sign-off,
    adds an extra newline at the end when I deliberately give a file
    that ends with an incomplete line for such and such reasons.

I do not think I would have a good argument why the then-current
behaviour is not a bug but an intended behaviour.

And I do not think the fact that I am unable to fill "such and such"
above means such a complaint is nonsense---it merely indicates that
I lack imagination and that I am not thinking enough to accomodate
other people's needs.

Re: [PATCH 0/3] rebase -r: support octopus merges

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Git is used in *so many* different scenarios, and both in terms of
> commits/day as well as overall repository size *and* development speed,

That misses another important factor, though.  How well the project
uses the tool, iow, how canonical should its way of using the tool
be considered and encouraged to be imitated by others.

> A much more meaningful measure would be: how many octopus merge commits
> have been pushed to GitHub in the past two weeks. I don't think I have the
> technical means to answer that question, though.

It does not mean that misusing a feature is a good thing and should
be encouraged if many misguided people do so.

Re: [PATCH 0/3] rebase -r: support octopus merges

[Johannes Sixt]

Am 12.07.2018 um 18:26 schrieb Junio C Hamano:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> A much more meaningful measure would be: how many octopus merge commits
>> have been pushed to GitHub in the past two weeks. I don't think I have the
>> technical means to answer that question, though.
> 
> It does not mean that misusing a feature is a good thing and should
> be encouraged if many misguided people do so.

Just recently I had to rebuild the version of git-gui that comes with 
Git 2.18.0 before it was released:

https://github.com/j6t/git-gui-ng/commit/f07ae1d7f07b036d78a3d4706e6cb4102e623fb3

I think that an octopus merge is the right tool for the task. Am I 
misguided?

-- Hannes

Re: [PATCH 0/3] rebase -r: support octopus merges

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 12.07.2018 um 18:26 schrieb Junio C Hamano:
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>> A much more meaningful measure would be: how many octopus merge commits
>>> have been pushed to GitHub in the past two weeks. I don't think I have the
>>> technical means to answer that question, though.
>>
>> It does not mean that misusing a feature is a good thing and should
>> be encouraged if many misguided people do so.
>
> Just recently I had to rebuild the version of git-gui that comes with
> Git 2.18.0 before it was released:
>
> https://github.com/j6t/git-gui-ng/commit/f07ae1d7f07b036d78a3d4706e6cb4102e623fb3
>
> I think that an octopus merge is the right tool for the task. Am I
> misguided?

It could be used and it is the right tool are two different things,
I would think.