Re: [PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Johannes Schindelin]

Hi Philip,

On Thu, 18 Jan 2018, Philip Oakley wrote:

> From: "Johannes Schindelin" 
> > This one is a bit tricky to explain, so let's try with a diagram:
> >
> >C
> >  /   \
> > A - B - E - F
> >  \   /
> >D
> >
> > To illustrate what this new mode is all about, let's consider what
> > happens upon `git rebase -i --recreate-merges B`, in particular to
> > the commit `D`. In the default mode, the new branch structure is:
> >
> >  --- C' --
> >  / \
> > A - B -- E' - F'
> >  \/
> >D'
> >
> > This is not really preserving the branch topology from before! The
> > reason is that the commit `D` does not have `B` as ancestor, and
> > therefore it gets rebased onto `B`.
> >
> > However, when recreating branch structure, there are legitimate use
> > cases where one might want to preserve the branch points of commits that
> > do not descend from the  commit that was passed to the rebase
> > command, e.g. when a branch from core Git's `next` was merged into Git
> > for Windows' master we will not want to rebase those commits on top of a
> > Windows-specific commit. In the example above, the desired outcome would
> > look like this:
> >
> >  --- C' --
> >  / \
> > A - B -- E' - F'
> >  \/
> >   -- D' --
> 
> I'm not understanding this. I see that D properly starts from A, but
> don't see why it is now D'. Surely it's unchanged.

It is not necessarily unchanged, because this is an *interactive* rebase.
If you mark `D` for `reword`, for example, it may be changed.

I use the label D' in the mathematical sense, to indicate that D' is
derived from D. It may even be identical to D, but the point is that it is
in the todo list of the interactive rebase, so it can be changed. As
opposed to, say, A and B. Those cannot be changed in this interactive
rebase.

> Maybe it's the arc/node confusion. Maybe even spell out that the rebased
> commits from the command are B..HEAD, but that includes D, which may not
> be what folk had expected. (not even sure if the reflog comes into
> determining merge-bases here..)
> 
> I do think an exact definition is needed (e.g. via --ancestry-path or
> its equivalent?).

I don't find "ancestry path" any more intuitive a term than the
mathematically correct "uncomparable".

If you have a better way to explain this (without devolving into
mathematical terminology), please let's hear it.

Don't get me wrong, as a mathematician I am comfortable with very precise
descriptions involving plenty of Greek symbols.

But this documentation, and these commit messages do not target myself. I
know perfectly well what I am talking about here. The target audience are
software developers who may not have a background in mathematics, who do
not even want to fully understand what the heck constitutes a Directed
Acyclic Graph.

So what we need here is plain English. And I had thought that the analogy
with the family tree would be intuitive enough for even math haters to
understand easily and quickly...

Ciao,
Dscho

Re: [PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Eric Sunshine]

On Thu, Jan 18, 2018 at 10:36 AM, Johannes Schindelin
 wrote:
> This one is a bit tricky to explain, so let's try with a diagram:
> [...]
> Signed-off-by: Johannes Schindelin 
> ---
> diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
> @@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, const 
> char *prefix)
> +   if (no_rebase_cousins >= 0&& !recreate_merges)

Style: space before &&

> +   warning(_("--[no-]rebase-cousins has no effect without "
> + "--recreate-merges"));

Re: [PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Philip Oakley]

From: "Johannes Schindelin" 

This one is a bit tricky to explain, so let's try with a diagram:

   C
 /   \
A - B - E - F
 \   /
   D

To illustrate what this new mode is all about, let's consider what
happens upon `git rebase -i --recreate-merges B`, in particular to
the commit `D`. In the default mode, the new branch structure is:

  --- C' --
 / \
A - B -- E' - F'
 \/
   D'

This is not really preserving the branch topology from before! The
reason is that the commit `D` does not have `B` as ancestor, and
therefore it gets rebased onto `B`.

However, when recreating branch structure, there are legitimate use
cases where one might want to preserve the branch points of commits that
do not descend from the  commit that was passed to the rebase
command, e.g. when a branch from core Git's `next` was merged into Git
for Windows' master we will not want to rebase those commits on top of a
Windows-specific commit. In the example above, the desired outcome would
look like this:

  --- C' --
 / \
A - B -- E' - F'
 \/
  -- D' --


I'm not understanding this. I see that D properly starts from A, but don't 
see why it is now D'. Surely it's unchanged.
Maybe it's the arc/node confusion. Maybe even spell out that the rebased 
commits from the command are B..HEAD, but that includes D, which may not be 
what folk had expected. (not even sure if the reflog comes into determining 
merge-bases here..)


I do think an exact definition is needed (e.g. via --ancestry-path or its 
equivalent?).




Let's introduce the term "cousins" for such commits ("D" in the
example), and the "no-rebase-cousins" mode of the merge-recreating
rebase, to help those use cases.

Signed-off-by: Johannes Schindelin 
---
Documentation/git-rebase.txt  |  7 ++-
builtin/rebase--helper.c  |  9 -
git-rebase--interactive.sh|  1 +
git-rebase.sh | 12 +++-
sequencer.c   |  4 
sequencer.h   |  8 
t/t3430-rebase-recreate-merges.sh | 23 +++
7 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d061373288..ac07a5c3fc9 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,10 +368,15 @@ The commit list format can be changed by setting the 
configuration option
rebase.instructionFormat.  A customized instruction format will 
automatically

have the long commit hash prepended to the format.

---recreate-merges::
+--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
 Recreate merge commits instead of flattening the history by replaying
 merges. Merge conflict resolutions or manual amendments to merge
 commits are not preserved.
++
+By default, or when `rebase-cousins` was specified, commits which do not 
have
+`` as direct ancestor are rebased onto `` (or 
``,
+if specified). If the `rebase-cousins` mode is turned off, such commits 
will

+retain their original branch point.

-p::
--preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..ef08fef4d14 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

{
 struct replay_opts opts = REPLAY_OPTS_INIT;
 unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
- int abbreviate_commands = 0;
+ int abbreviate_commands = 0, no_rebase_cousins = -1;
 enum {
 CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
 CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
@@ -23,6 +23,8 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

 OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
 OPT_BOOL(0, "keep-empty", _empty, N_("keep empty commits")),
 OPT_BOOL(0, "recreate-merges", _merges, N_("recreate merge 
commits")),

+ OPT_BOOL(0, "no-rebase-cousins", _rebase_cousins,
+ N_("keep original branch points of cousins")),
 OPT_CMDMODE(0, "continue", , N_("continue rebase"),
 CONTINUE),
 OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, 
const char *prefix)

 flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
 flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
 flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+ flags |= no_rebase_cousins > 0 ? TODO_LIST_NO_REBASE_COUSINS : 0;
 flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;

+ if (no_rebase_cousins >= 0&& !recreate_merges)
+ warning(_("--[no-]rebase-cousins has no effect without "
+   "--recreate-merges"));
+
 if (command == CONTINUE && argc == 1)
 return !!sequencer_continue();
 if (command == ABORT && argc == 1)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3459ec5a018..23184c77e88 100644
--- 

[PATCH 8/8] rebase -i: introduce --recreate-merges=no-rebase-cousins

[Johannes Schindelin]

This one is a bit tricky to explain, so let's try with a diagram:

C
  /   \
A - B - E - F
  \   /
D

To illustrate what this new mode is all about, let's consider what
happens upon `git rebase -i --recreate-merges B`, in particular to
the commit `D`. In the default mode, the new branch structure is:

   --- C' --
  / \
A - B -- E' - F'
  \/
D'

This is not really preserving the branch topology from before! The
reason is that the commit `D` does not have `B` as ancestor, and
therefore it gets rebased onto `B`.

However, when recreating branch structure, there are legitimate use
cases where one might want to preserve the branch points of commits that
do not descend from the  commit that was passed to the rebase
command, e.g. when a branch from core Git's `next` was merged into Git
for Windows' master we will not want to rebase those commits on top of a
Windows-specific commit. In the example above, the desired outcome would
look like this:

   --- C' --
  / \
A - B -- E' - F'
  \/
   -- D' --

Let's introduce the term "cousins" for such commits ("D" in the
example), and the "no-rebase-cousins" mode of the merge-recreating
rebase, to help those use cases.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt  |  7 ++-
 builtin/rebase--helper.c  |  9 -
 git-rebase--interactive.sh|  1 +
 git-rebase.sh | 12 +++-
 sequencer.c   |  4 
 sequencer.h   |  8 
 t/t3430-rebase-recreate-merges.sh | 23 +++
 7 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 1d061373288..ac07a5c3fc9 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -368,10 +368,15 @@ The commit list format can be changed by setting the 
configuration option
 rebase.instructionFormat.  A customized instruction format will automatically
 have the long commit hash prepended to the format.
 
---recreate-merges::
+--recreate-merges[=(rebase-cousins|no-rebase-cousins)]::
Recreate merge commits instead of flattening the history by replaying
merges. Merge conflict resolutions or manual amendments to merge
commits are not preserved.
++
+By default, or when `rebase-cousins` was specified, commits which do not have
+`` as direct ancestor are rebased onto `` (or ``,
+if specified). If the `rebase-cousins` mode is turned off, such commits will
+retain their original branch point.
 
 -p::
 --preserve-merges::
diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c
index a34ab5c0655..ef08fef4d14 100644
--- a/builtin/rebase--helper.c
+++ b/builtin/rebase--helper.c
@@ -13,7 +13,7 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
 {
struct replay_opts opts = REPLAY_OPTS_INIT;
unsigned flags = 0, keep_empty = 0, recreate_merges = 0;
-   int abbreviate_commands = 0;
+   int abbreviate_commands = 0, no_rebase_cousins = -1;
enum {
CONTINUE = 1, ABORT, MAKE_SCRIPT, SHORTEN_OIDS, EXPAND_OIDS,
CHECK_TODO_LIST, SKIP_UNNECESSARY_PICKS, REARRANGE_SQUASH,
@@ -23,6 +23,8 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")),
OPT_BOOL(0, "keep-empty", _empty, N_("keep empty 
commits")),
OPT_BOOL(0, "recreate-merges", _merges, N_("recreate 
merge commits")),
+   OPT_BOOL(0, "no-rebase-cousins", _rebase_cousins,
+N_("keep original branch points of cousins")),
OPT_CMDMODE(0, "continue", , N_("continue rebase"),
CONTINUE),
OPT_CMDMODE(0, "abort", , N_("abort rebase"),
@@ -57,8 +59,13 @@ int cmd_rebase__helper(int argc, const char **argv, const 
char *prefix)
flags |= keep_empty ? TODO_LIST_KEEP_EMPTY : 0;
flags |= abbreviate_commands ? TODO_LIST_ABBREVIATE_CMDS : 0;
flags |= recreate_merges ? TODO_LIST_RECREATE_MERGES : 0;
+   flags |= no_rebase_cousins > 0 ? TODO_LIST_NO_REBASE_COUSINS : 0;
flags |= command == SHORTEN_OIDS ? TODO_LIST_SHORTEN_IDS : 0;
 
+   if (no_rebase_cousins >= 0&& !recreate_merges)
+   warning(_("--[no-]rebase-cousins has no effect without "
+ "--recreate-merges"));
+
if (command == CONTINUE && argc == 1)
return !!sequencer_continue();
if (command == ABORT && argc == 1)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 3459ec5a018..23184c77e88 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -901,6 +901,7 @@ if test t != "$preserve_merges"
 then
git rebase--helper --make-script ${keep_empty:+--keep-empty} \