Re: [PATCH v5 00/12] rebase -i: offer to recreate merge commits
Hi Buga, On Tue, 6 Mar 2018, Igor Djordjevic wrote: > [...] > > But before elaborating, I would like to hear your opinion on whether you > find it worth to pursue that goal here, before `--recreate-merges` hits > the mainstream, or it might be just fine as a possible later > improvement, too (if accepted, that is). As I suggested in another sub-thread, I think the best way forward is to use your idea to make the 'rebase original merge commits' strategy explicit. That would not actually hold up the current --recreate-merges patch series, but would mean to provide an add-on patch series to add support for `merge -R` and then use that from the generated todo list. For implementation detail reasons, it may actually make sense to integrate those patches into the --recreate-merges patch series, though. Should not be hard (except during GitMerge). > p.s. lol, now that I said it, and after writing all this, I might > actually even like the idea of (later) having `--rebase-merges` > alongside `--recreate-merges`, too, each one clearly communicating > its default mode of operation - rebase merges vs. recreate merges... > as one might rightfully expect ;) Eh :P Hehe... Ciao, Dscho
Re: [PATCH v5 00/12] rebase -i: offer to recreate merge commits
Hi Johannes, On 26/02/2018 22:29, Johannes Schindelin wrote: > > Once upon a time, I dreamt of an interactive rebase that would not > flatten branch structure, 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 --recreate-merges as "--preserve-merges done right". First of all, thanks for this wonderful improvement to existing `git rebase` functionality, I`m really excited to have this in the mainline! :) But in the light of "--preserve-merges done right", I would like to hear your opinion on a topic that might be considered more or less important, and thus tackled in a few different ways... :$ Rebasing amended merges :( Even though documentation is quite clear about merge conflicts and manual amendments not recreated automatically, this might be considered quite an issue (a bug, even), as even in case of non-interactive rebase, amended content will be dropped - and even worse, it all happens silently, without alerting the user (for whom we presume to know what he`s doing, I guess). Now, might be this is considered the least interesting use case, in comparison to all the power of more serious interactive rebases, but I would argue it could be the one most used by less adventurous users that would simply like to stay up-to-date with upstream, rebasing their current work on top of it (think `git pull --rebase=recreate`, even). As it currently is, and that was the case with `--preserve-merges`, too, this will cause them to silently lose their work (amended merge content). And while documentation is clear about it, these might be less knowledgeable users, too, and thus potentially be the ones we should (try to) protect even more, if possible. Now, in the light of that other, ongoing "merge rebasing" topic[1], it seems we really might be able to do much better, actually _rebasing_ merges (and keeping manual conflict resolutions/amendments), instead of _recreating_ them (and silently loosing content), and doing so reliably (or stopping for possible user inspection, but not silently doing the wrong thing, even if documented). This concerns non-interactive rebase the most, but I have ideas on making it aligned with interactive one, too, where user could actually decide whether to rebase or (re)create the merge (rebase becoming the default, intuitively aligned with non-interactive rebase). But before elaborating, I would like to hear your opinion on whether you find it worth to pursue that goal here, before `--recreate-merges` hits the mainstream, or it might be just fine as a possible later improvement, too (if accepted, that is). My main concern, and why I raised the question inside this topic in the first place, is default behavior. With `--recreate-merges` just being introduced, we have no backwards compatibility to think about, being a unique chance to make default behave as needed (not to say "correct", even), and might be really ticking one more of "--preserve-merges done right" boxes, and could be a pretty important one, too. But once this becomes widely available, I guess it will be hard to improve (fix?) this merge rebasing silent content losing behavior (even if we would acknowledge it as a bug), without introducing additional options - and without a possibility to make possibly "right" behavior a default one, thus further complicating user experience. So, I wanted to hear your stance on this :( Knowing how much this means to you, it is really not my wish to drag this topic further, and if you find it that we`re good here as it is, I wouldn`t have any objections - I guess later new `--rebase-merges` option is a possibility, too, might be a wrapper around `--recreate-merges`, but with actual merge rebasing being a default (where merge recreation would still be possible, too)... Otherwise, if you have any interest in it now, I can further elaborate what I`m thinking about, where it might help improve both user experience and rebase possibilities, for what might not be too much extra work... hopefully :P Whatever ends up being your response, I`m really grateful for your work on this matter so far, and thank you for everything you did. p.s. lol, now that I said it, and after writing all this, I might actually even like the idea of (later) having `--rebase-merges` alongside `--recreate-merges`, too, each one clearly communicating its
[PATCH v5 00/12] rebase -i: offer to recreate merge commits
Once upon a time, I dreamt of an interactive rebase that would not
flatten branch structure, 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 --recreate-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 --recreate-merges.
Changes since v3:
- (sorry for the broken iteration v4)
- fixed a grammar error in "introduce the `merge` command"'s commit message.
- fixed a couple of resource leaks in safe_append() and do_reset(), pointed
out by Eric Sunshine.
Johannes Schindelin (11):
sequencer: avoid using errno clobbered by rollback_lock_file()
sequencer: make rearrange_squash() a bit more obvious
sequencer: introduce new commands to reset the revision
sequencer: introduce the `merge` command
sequencer: fast-forward merge commits, if possible
rebase-helper --make-script: introduce a flag to recreate merges
rebase: introduce the --recreate-merges option
sequencer: make refs generated by the `label` command worktree-local
sequencer: handle post-rewrite for merge commands
pull: accept --rebase=recreate to recreate the branch topology
rebase -i: introduce --recreate-merges=[no-]rebase-cousins
Stefan Beller (1):
git-rebase--interactive: clarify arguments
Documentation/config.txt | 8 +
Documentation/git-pull.txt | 5 +-
Documentation/git-rebase.txt | 14 +-
builtin/pull.c | 14 +-
builtin/rebase--helper.c | 13 +-
builtin/remote.c | 2 +
contrib/completion/git-completion.bash | 4 +-
git-rebase--interactive.sh | 22 +-
git-rebase.sh | 16 +
refs.c | 3 +-
sequencer.c| 742 -
sequencer.h| 7 +
t/t3430-rebase-recreate-merges.sh | 208 +
13 files changed, 1027 insertions(+), 31 deletions(-)
create mode 100755 t/t3430-rebase-recreate-merges.sh
base-commit: e3a80781f5932f5fea12a49eb06f3ade4ed8945c
Published-As: https://github.com/dscho/git/releases/tag/recreate-merges-v5
Fetch-It-Via: git fetch https://github.com/dscho/git recreate-merges-v5
Interdiff vs v4:
diff --git a/sequencer.c b/sequencer.c
index 63ae71a7512..b2bf63029d4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2514,14 +2514,17 @@ static int safe_append(const char *filename, const
char *fmt, ...)
if (write_in_full(fd, buf.buf, buf.len) < 0) {
error_errno(_("could not write to '%s'"), filename);
+ strbuf_release();
rollback_lock_file();
return -1;
}
if (commit_lock_file() < 0) {
+ strbuf_release();
rollback_lock_file();
return error(_("failed to finalize '%s'"), filename);
}
+ strbuf_release();
return 0;
}
@@ -2601,8 +2604,11 @@ static int do_reset(const char *name, int len, struct
replay_opts *opts)
unpack_tree_opts.update = 1;
unpack_tree_opts.reset = 1;
- if (read_cache_unmerged())
+ if (read_cache_unmerged()) {
+ rollback_lock_file();
+ strbuf_release(_name);
return error_resolve_conflict(_(action_name(opts)));
+ }
if (!fill_tree_descriptor(, )) {
error(_("failed to find tree of %s"), oid_to_hex());
--
2.16.1.windows.4
