| Submitter | phabricator |
|---|---|
| Date | Jan. 16, 2020, 8:10 a.m. |
| Message ID | <differential-rev-PHID-DREV-wyjh3r4pzmjaex6k5qtv-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/44414/ |
| State | Superseded |
| Headers | show |
Comments
pulkit added inline comments. INLINE COMMENTS > rebase.py:1811 > + newps[0], newps[i] = newps[i], newps[0] > + bases[0], bases[i] = bases[i], bases[0] > + I am not sure why we are doing `bases[i] = bases[0]` here? (TBH I didn't try to understand the whole logic, mostly only the diff) REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7906/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7906 To: martinvonz, #hg-reviewers Cc: pulkit, mercurial-devel
martinvonz added inline comments. INLINE COMMENTS > pulkit wrote in rebase.py:1811 > I am not sure why we are doing `bases[i] = bases[0]` here? (TBH I didn't try to understand the whole logic, mostly only the diff) Doing it only for consistency with `newps`. `bases` and `newps` should be kept in sync (we don't do that in one other place, but that's pretty weird, IMO). So I think it's clearer this way even though it's unnecessary. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7906/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7906 To: martinvonz, #hg-reviewers Cc: pulkit, mercurial-devel
Patch
diff --git a/hgext/rebase.py b/hgext/rebase.py --- a/hgext/rebase.py +++ b/hgext/rebase.py @@ -1737,12 +1737,6 @@ if any(p != nullrev and isancestor(rev, p) for p in newps): raise error.Abort(_(b'source is ancestor of destination')) - # "rebasenode" updates to new p1, use the corresponding merge base. - if bases[0] != nullrev: - base = bases[0] - else: - base = None - # Check if the merge will contain unwanted changes. That may happen if # there are multiple special (non-changelog ancestor) merge bases, which # cannot be handled well by the 3-way merge algorithm. For example: @@ -1807,14 +1801,20 @@ % (rev, repo[rev], unwanteddesc) ) - base = bases[i] - # 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 and newps[i] != nullrev: - newps[0], newps[i] = newps[i], newps[0] + 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 repo.ui.debug(b" future parents are %d and %d\n" % tuple(newps))