Commit 77bb38be00e by Martin von Zweigbergk

rebase: always be graft-like, not merge-like, also for merges

Rebase works by updating to a commit and then grafting changes on
top. However, before this patch, it would actually merge in changes
instead of grafting them in in some cases. That is, it would use the
common ancestor as base instead of using one of the parents. That
seems wrong to me, so I'm changing it so `defineparents()` always
returns a value for `base`.

This fixes the bad behavior in test-rebase-newancestor.t, which was
introduced in 65f215ea3e8e (tests: add test for rebasing merges with
ancestors of the rebase destination, 2014-11-30).

The difference in test-rebase-dest.t is because the files in the tip
revision were A, D, E, F before this patch and A, D, F, G after it. I
think both files should ideally be there.

Differential Revision: https://phab.mercurial-scm.org/D7907
parent 6ecc34b3113
......@@ -1678,22 +1678,6 @@ def defineparents(repo, rev, destmap, state, skipped, obsskipped):
elif p in state and state[p] > 0:
np = state[p]
# "bases" only record "special" merge bases that cannot be
# calculated from changelog DAG (i.e. isancestor(p, np) is False).
# For example:
#
# B' # rebase -s B -d D, when B was rebased to B'. dest for C
# | C # is B', but merge base for C is B, instead of
# D | # changelog.ancestor(C, B') == A. If changelog DAG and
# | B # "state" edges are merged (so there will be an edge from
# |/ # B to B'), the merge base is still ancestor(C, B') in
# A # the merged graph.
#
# Also see https://bz.mercurial-scm.org/show_bug.cgi?id=1950#c8
# which uses "virtual null merge" to explain this situation.
if isancestor(p, np):
bases[i] = nullrev
# If one parent becomes an ancestor of the other, drop the ancestor
for j, x in enumerate(newps[:i]):
if x == nullrev:
......@@ -1754,10 +1738,10 @@ def defineparents(repo, rev, destmap, state, skipped, obsskipped):
# But our merge base candidates (D and E in above case) could still be
# better than the default (ancestor(F, Z) == null). Therefore still
# pick one (so choose p1 above).
if sum(1 for b in set(bases) if b != nullrev) > 1:
if sum(1 for b in set(bases) if b != nullrev and b not in newps) > 1:
unwanted = [None, None] # unwanted[i]: unwanted revs if choose bases[i]
for i, base in enumerate(bases):
if base == nullrev:
if base == nullrev or base in newps:
continue
# Revisions in the side (not chosen as merge base) branch that
# might contain "surprising" contents
......@@ -1781,42 +1765,40 @@ def defineparents(repo, rev, destmap, state, skipped, obsskipped):
)
)
# Choose a merge base that has a minimal number of unwanted revs.
l, i = min(
(len(revs), i)
for i, revs in enumerate(unwanted)
if revs is not None
)
if any(revs is not None for revs in unwanted):
# Choose a merge base that has a minimal number of unwanted revs.
l, i = min(
(len(revs), i)
for i, revs in enumerate(unwanted)
if revs is not None
)
# The merge will include unwanted revisions. Abort now. Revisit this if
# we have a more advanced merge algorithm that handles multiple bases.
if l > 0:
unwanteddesc = _(b' or ').join(
(
b', '.join(b'%d:%s' % (r, repo[r]) for r in revs)
for revs in unwanted
if revs is not None
# The merge will include unwanted revisions. Abort now. Revisit this if
# we have a more advanced merge algorithm that handles multiple bases.
if l > 0:
unwanteddesc = _(b' or ').join(
(
b', '.join(b'%d:%s' % (r, repo[r]) for r in revs)
for revs in unwanted
if revs is not None
)
)
raise error.Abort(
_(b'rebasing %d:%s will include unwanted changes from %s')
% (rev, repo[rev], unwanteddesc)
)
)
raise error.Abort(
_(b'rebasing %d:%s will include unwanted changes from %s')
% (rev, repo[rev], unwanteddesc)
)
# newps[0] should match merge base if possible. Currently, if newps[i]
# is nullrev, the only case is newps[i] and newps[j] (j < i), one is
# the other's ancestor. In that case, it's fine to not swap newps here.
# (see CASE-1 and CASE-2 above)
if i != 0:
if newps[i] != nullrev:
newps[0], newps[i] = newps[i], newps[0]
bases[0], bases[i] = bases[i], bases[0]
# newps[0] should match merge base if possible. Currently, if newps[i]
# is nullrev, the only case is newps[i] and newps[j] (j < i), one is
# the other's ancestor. In that case, it's fine to not swap newps here.
# (see CASE-1 and CASE-2 above)
if i != 0:
if newps[i] != nullrev:
newps[0], newps[i] = newps[i], newps[0]
bases[0], bases[i] = bases[i], bases[0]
# "rebasenode" updates to new p1, use the corresponding merge base.
if bases[0] != nullrev:
base = bases[0]
else:
base = None
base = bases[0]
repo.ui.debug(b" future parents are %d and %d\n" % tuple(newps))
......
......@@ -256,7 +256,7 @@ Different destinations for merge changesets with a same root:
> EOS
rebasing 3:a4256619d830 "B" (B)
rebasing 6:8e139e245220 "C" (C tip)
o 8: 51e2ce92e06a C
o 8: d7d1169e9b1c C
|\
| o 7: 2ed0c8546285 B
| |\
......
......@@ -68,11 +68,6 @@ interesting conflict resolutions or additional changes in the merge commit, but
that is mixed up with the actual merge stuff and there is in general no way to
separate them.
Note: The dev branch contains _no_ changes to f-default. It might be unclear
how rebasing of ancestor merges should be handled, but the current behavior
with spurious prompts for conflicts in files that didn't change seems very
wrong.
$ hg init ancestor-merge
$ cd ancestor-merge
......@@ -133,16 +128,11 @@ Full rebase all the way back from branching point:
note: not rebasing 1:1d1a643d390e "dev: create branch", its destination already has all its changes
rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
rebasing 4:4b019212aaf6 "dev: merge default"
file 'f-default' was deleted in local [dest] but was modified in other [source].
You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
What do you want to do? c
note: not rebasing 4:4b019212aaf6 "dev: merge default", its destination already has all its changes
rebasing 6:010ced67e558 "dev: merge default"
note: not rebasing 6:010ced67e558 "dev: merge default", its destination already has all its changes
saved backup bundle to $TESTTMP/ancestor-merge/.hg/strip-backup/1d1a643d390e-4a6f6d17-rebase.hg
$ hg tglog
o 6: de147e4f69cf 'dev: merge default'
|
o 5: eda7b7f46f5d 'dev: merge default'
|
o 4: 3e075b1c0a40 'dev: f-dev stuff'
|
@ 3: e08089805d82 'default: f-other stuff'
......@@ -163,28 +153,8 @@ Grafty cherry picking rebasing:
> EOF
rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
rebasing 4:4b019212aaf6 "dev: merge default"
file 'f-default' was deleted in local [dest] but was modified in other [source].
You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
What do you want to do? c
rebasing 6:010ced67e558 "dev: merge default"
saved backup bundle to $TESTTMP/ancestor-merge-2/.hg/strip-backup/ec2c14fb2984-827d7a44-rebase.hg
$ hg tglog
o 7: de147e4f69cf 'dev: merge default'
|
o 6: eda7b7f46f5d 'dev: merge default'
|
o 5: 3e075b1c0a40 'dev: f-dev stuff'
|
o 4: e08089805d82 'default: f-other stuff'
|
o 3: 462860db70a1 'default: remove f-default'
|
o 2: f157ecfd2b6b 'default: f-default stuff'
|
| o 1: 1d1a643d390e 'dev: create branch' dev
|/
o 0: e90e8eb90b6f 'default: create f-default'
abort: rebasing 4:4b019212aaf6 will include unwanted changes from 1:1d1a643d390e
[255]
$ cd ..
......@@ -284,18 +254,7 @@ rebase of merge of ancestors
rebasing 6:4c5f12f25ebe "merge rebase ancestors" (tip)
resolving manifests
removing other
note: merging f9daf77ffe76+ and 4c5f12f25ebe using bids from ancestors a60552eb93fb and f59da8fc0fcf
calculating bids for ancestor a60552eb93fb
resolving manifests
calculating bids for ancestor f59da8fc0fcf
resolving manifests
auction for merging merge bids
other: consensus for g
end of auction
getting other
committing files:
other
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment