Re: [PATCH v9 00/17] rebase -i: offer to recreate commit topology by rebasing merges

[Sergey Organov]

This has been sent by mistake, I'm sorry, please disregard.

Sergey Organov  writes:

> Johannes Schindelin  writes:
>
>> Junio, I think this is now ready for `next`. Thank you for your patience
>> and help with this.

[...]

Re: [PATCH v9 00/17] rebase -i: offer to recreate commit topology by rebasing merges

[Sergey Organov]

Johannes Schindelin  writes:

> Junio, I think this is now ready for `next`. Thank you for your patience
> and help with this.
>
> Once upon a time, I dreamed of an interactive rebase that would not
> linearize all patches and drop all merge commits, but instead recreate
> the commit topology faithfully.
>
> My original attempt was --preserve-merges, but that design was so
> limited that I did not even enable it in interactive mode.
>
> Subsequently, it *was* enabled in interactive mode, with the predictable
> consequences: as the --preserve-merges design does not allow for
> specifying the parents of merge commits explicitly, all the new commits'
> parents are defined *implicitly* by the previous commit history, and
> hence it is *not possible to even reorder commits*.
>
> This design flaw cannot be fixed. Not without a complete re-design, at
> least. This patch series offers such a re-design.
>
> Think of --rebase-merges as "--preserve-merges done right". It
> introduces new verbs for the todo list, `label`, `reset` and `merge`.
> For a commit topology like this:
>
> A - B - C
>   \   /
> D
>
> the generated todo list would look like this:
>
> # branch D
> pick 0123 A
> label branch-point
> pick 1234 D
> label D
>
> reset branch-point
> pick 2345 B
> merge -C 3456 D # C
>
> There are more patches in the pipeline, based on this patch series, but
> left for later in the interest of reviewable patch series: one mini
> series to use the sequencer even for `git rebase -i --root`, and another
> one to add support for octopus merges to --rebase-merges. And then one
> to allow for rebasing merge commits in a smarter way (this one will need
> a bit more work, though, as it can result in very complicated, nested
> merge conflicts *very* easily).
>
> Changes since v8:
>
> - Disentangled the patch introducing `label`/`reset` from the one
>   introducing `merge` again (this was one stupid, tired `git commit
>   --amend` too many).
>
> - Augmented the commit message of "introduce the `merge` command" to
>   describe what the `label onto` is all about.
>
> - Fixed the error message when `reset` would overwrite untracked files to
>   actually say that a "reset" failed (not a "merge").
>
> - Clarified the rationale for `label onto` in the commit message of
>   "rebase-helper --make-script: introduce a flag to rebase merges".
>
> - Edited the description of `--rebase-merges` heavily, for clarity, in
>   "rebase: introduce the --rebase-merges option".
>
> - Edited the commit message of (and the documentation introduced by) " rebase
>   -i: introduce --rebase-merges=[no-]rebase-cousins" for clarity (also
>   mentioning the `--ancestry-path` option).
>
> - When run_git_commit() fails after a successful merge, we now take pains
>   not to reschedule the `merge` command.
>
> - Rebased the patch series on top of current `master`, i.e. both
>   `pw/rebase-keep-empty-fixes` and `pw/rebase-signoff`, to resolve merge
>   conflicts myself.
>
>
> Johannes Schindelin (15):
>   sequencer: avoid using errno clobbered by rollback_lock_file()
>   sequencer: make rearrange_squash() a bit more obvious
>   sequencer: refactor how original todo list lines are accessed
>   sequencer: offer helpful advice when a command was rescheduled
>   sequencer: introduce new commands to reset the revision
>   sequencer: introduce the `merge` command
>   sequencer: fast-forward `merge` commands, if possible
>   rebase-helper --make-script: introduce a flag to rebase merges
>   rebase: introduce the --rebase-merges option
>   sequencer: make refs generated by the `label` command worktree-local
>   sequencer: handle post-rewrite for merge commands
>   rebase --rebase-merges: avoid "empty merges"
>   pull: accept --rebase=merges to recreate the branch topology
>   rebase -i: introduce --rebase-merges=[no-]rebase-cousins
>   rebase -i --rebase-merges: add a section to the man page
>
> Phillip Wood (1):
>   rebase --rebase-merges: add test for --keep-empty
>
> Stefan Beller (1):
>   git-rebase--interactive: clarify arguments
>
>  Documentation/config.txt   |   8 +
>  Documentation/git-pull.txt |   6 +-
>  Documentation/git-rebase.txt   | 163 -
>  builtin/pull.c |  14 +-
>  builtin/rebase--helper.c   |  13 +-
>  builtin/remote.c   |  18 +-
>  contrib/completion/git-completion.bash |   4 +-
>  git-rebase--interactive.sh |  22 +-
>  git-rebase.sh  |  16 +
>  refs.c |   3 +-
>  sequencer.c| 892 -
>  sequencer.h|   7 +
>  t/t3421-rebase-topology-linear.sh  |   1 +
>  t/t3430-rebase-merges.sh   | 244 +++
>  14 files changed, 1352 insertions(+), 59 

Re: [PATCH v9 00/17] rebase -i: offer to recreate commit topology by rebasing merges

[Junio C Hamano]

Junio C Hamano  writes:

>> - Rebased the patch series on top of current `master`, i.e. both
>>   `pw/rebase-keep-empty-fixes` and `pw/rebase-signoff`, to resolve merge
>>   conflicts myself.
>
> Good to see the last item, as this gave me a chance to make sure
> that the conflict resolution I've been carrying matches how you
> would have resolved as the original author.  Applying these on the
> old base (with minor conflict resolution) to match the old iteration
> and merging the result to the new base1f1cddd5 ("The fourth batch
> for 2.18", 2018-04-25) resulted in the same tree as the tree that
> results from applying these on top of the new base.
>
> That was done only to validate the result of the past resolution
> (and also seeing the interdiff from the old iteration).  There is no
> reason to keep this series back-portable to older tip of 'master',
> so I'll queue the result of applying the patches to the new base.

By the way, the rebasing made the topic textually merge cleanly to
the tip of 'pu' which made it slightly more cumbersome to deal with
a semantic conflict the topic has with another topic that modifies
the function signature of get_main_ref_store().  This topic adds a
new callsite in sequencer.c to this function.

The old base that forced the integrator to resolve conflicts in
sequencer.c with some other topic, thanks to that exact textual
conflicts, gave rerere a chance to record the adjustment for this
semantic conflict.

Now because the series applied to new base does not have textual
conflicts in sequencer.c when merged to 'pu', the adjustment for the
semantic conflict needs to be carried by a different mechanism.

Side note.  Do not take the above as a complaint.  Dealing with
interactions among various topics in flight while keeping them
as straight and clean topic is what I do.  It is a normal part
of running an active project.

It may be an interesting exercise to attempt to rebase tonight's
'pu' onto something younger in 'pu', say 'pu~4', without changing
anything in "pu^2" (which is the tip of this topic) and see how well
the merge recreation feature of this topic handles the evil merge.

The gist of the evil merge looks like this:

diff --cc sequencer.c
index a428fc7db7,e2f8394284..729cf05768
--- a/sequencer.c
+++ b/sequencer.c
@@@ -2483,6 -2527,349 +2556,349 @@@ static int do_exec(const char *command_
 ...
+ 
+ static int do_label(const char *name, int len)
+ {
 -  struct ref_store *refs = get_main_ref_store();
++  struct ref_store *refs = get_main_ref_store(the_repository);
+   struct ref_transaction *transaction;
+   struct strbuf ref_name = STRBUF_INIT, err = STRBUF_INIT;
+   struct strbuf msg = STRBUF_INIT;
+...

Re: [PATCH v9 00/17] rebase -i: offer to recreate commit topology by rebasing merges

[Junio C Hamano]

Johannes Schindelin  writes:

> Changes since v8:
>
> - Disentangled the patch introducing `label`/`reset` from the one
>   introducing `merge` again (this was one stupid, tired `git commit
>   --amend` too many).
>
> - Augmented the commit message of "introduce the `merge` command" to
>   describe what the `label onto` is all about.
>
> - Fixed the error message when `reset` would overwrite untracked files to
>   actually say that a "reset" failed (not a "merge").
>
> - Clarified the rationale for `label onto` in the commit message of
>   "rebase-helper --make-script: introduce a flag to rebase merges".
>
> - Edited the description of `--rebase-merges` heavily, for clarity, in
>   "rebase: introduce the --rebase-merges option".
>
> - Edited the commit message of (and the documentation introduced by) " rebase
>   -i: introduce --rebase-merges=[no-]rebase-cousins" for clarity (also
>   mentioning the `--ancestry-path` option).
>
> - When run_git_commit() fails after a successful merge, we now take pains
>   not to reschedule the `merge` command.
>
> - Rebased the patch series on top of current `master`, i.e. both
>   `pw/rebase-keep-empty-fixes` and `pw/rebase-signoff`, to resolve merge
>   conflicts myself.

Good to see the last item, as this gave me a chance to make sure
that the conflict resolution I've been carrying matches how you
would have resolved as the original author.  Applying these on the
old base (with minor conflict resolution) to match the old iteration
and merging the result to the new base1f1cddd5 ("The fourth batch
for 2.18", 2018-04-25) resulted in the same tree as the tree that
results from applying these on top of the new base.

That was done only to validate the result of the past resolution
(and also seeing the interdiff from the old iteration).  There is no
reason to keep this series back-portable to older tip of 'master',
so I'll queue the result of applying the patches to the new base.

[PATCH v9 00/17] rebase -i: offer to recreate commit topology by rebasing merges

[Johannes Schindelin]

Junio, I think this is now ready for `next`. Thank you for your patience
and help with this.

Once upon a time, I dreamed of an interactive rebase that would not
linearize all patches and drop all merge commits, but instead recreate
the commit topology faithfully.

My original attempt was --preserve-merges, but that design was so
limited that I did not even enable it in interactive mode.

Subsequently, it *was* enabled in interactive mode, with the predictable
consequences: as the --preserve-merges design does not allow for
specifying the parents of merge commits explicitly, all the new commits'
parents are defined *implicitly* by the previous commit history, and
hence it is *not possible to even reorder commits*.

This design flaw cannot be fixed. Not without a complete re-design, at
least. This patch series offers such a re-design.

Think of --rebase-merges as "--preserve-merges done right". It
introduces new verbs for the todo list, `label`, `reset` and `merge`.
For a commit topology like this:

A - B - C
  \   /
D

the generated todo list would look like this:

# branch D
pick 0123 A
label branch-point
pick 1234 D
label D

reset branch-point
pick 2345 B
merge -C 3456 D # C

There are more patches in the pipeline, based on this patch series, but
left for later in the interest of reviewable patch series: one mini
series to use the sequencer even for `git rebase -i --root`, and another
one to add support for octopus merges to --rebase-merges. And then one
to allow for rebasing merge commits in a smarter way (this one will need
a bit more work, though, as it can result in very complicated, nested
merge conflicts *very* easily).

Changes since v8:

- Disentangled the patch introducing `label`/`reset` from the one
  introducing `merge` again (this was one stupid, tired `git commit
  --amend` too many).

- Augmented the commit message of "introduce the `merge` command" to
  describe what the `label onto` is all about.

- Fixed the error message when `reset` would overwrite untracked files to
  actually say that a "reset" failed (not a "merge").

- Clarified the rationale for `label onto` in the commit message of
  "rebase-helper --make-script: introduce a flag to rebase merges".

- Edited the description of `--rebase-merges` heavily, for clarity, in
  "rebase: introduce the --rebase-merges option".

- Edited the commit message of (and the documentation introduced by) " rebase
  -i: introduce --rebase-merges=[no-]rebase-cousins" for clarity (also
  mentioning the `--ancestry-path` option).

- When run_git_commit() fails after a successful merge, we now take pains
  not to reschedule the `merge` command.

- Rebased the patch series on top of current `master`, i.e. both
  `pw/rebase-keep-empty-fixes` and `pw/rebase-signoff`, to resolve merge
  conflicts myself.


Johannes Schindelin (15):
  sequencer: avoid using errno clobbered by rollback_lock_file()
  sequencer: make rearrange_squash() a bit more obvious
  sequencer: refactor how original todo list lines are accessed
  sequencer: offer helpful advice when a command was rescheduled
  sequencer: introduce new commands to reset the revision
  sequencer: introduce the `merge` command
  sequencer: fast-forward `merge` commands, if possible
  rebase-helper --make-script: introduce a flag to rebase merges
  rebase: introduce the --rebase-merges option
  sequencer: make refs generated by the `label` command worktree-local
  sequencer: handle post-rewrite for merge commands
  rebase --rebase-merges: avoid "empty merges"
  pull: accept --rebase=merges to recreate the branch topology
  rebase -i: introduce --rebase-merges=[no-]rebase-cousins
  rebase -i --rebase-merges: add a section to the man page

Phillip Wood (1):
  rebase --rebase-merges: add test for --keep-empty

Stefan Beller (1):
  git-rebase--interactive: clarify arguments

 Documentation/config.txt   |   8 +
 Documentation/git-pull.txt |   6 +-
 Documentation/git-rebase.txt   | 163 -
 builtin/pull.c |  14 +-
 builtin/rebase--helper.c   |  13 +-
 builtin/remote.c   |  18 +-
 contrib/completion/git-completion.bash |   4 +-
 git-rebase--interactive.sh |  22 +-
 git-rebase.sh  |  16 +
 refs.c |   3 +-
 sequencer.c| 892 -
 sequencer.h|   7 +
 t/t3421-rebase-topology-linear.sh  |   1 +
 t/t3430-rebase-merges.sh   | 244 +++
 14 files changed, 1352 insertions(+), 59 deletions(-)
 create mode 100755 t/t3430-rebase-merges.sh


base-commit: 1f1cddd558b54bb0ce19c8ace353fd07b758510d
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v9
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v9