This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1679

rebase: fix for hgsubversion
ClosedPublic

Authored by phillco on Dec 13 2017, 1:10 AM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG83014fa95435: rebase: fix for hgsubversion
Summary

5c25fe7fb1e broke something in the hgsubversion test path, causing it raise an
abort (Abort: nothing to merge) during a perfectly good rebase. I tracked it
down to this change. It's probably not hgsubversion related.

I suspect that using the same wctx from before the initial update causes
problems with the wctx's cached manifest property. I noticed we also sometimes
stick random gunk on the wctx object in other places (like in copies.py) so
it's probably best to reset it for now.

The line I added before was actually useless since we don't pass wctx to the
initial merge.update, so it defaults to repo[None]. So I just removed it.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

phillco created this revision.Dec 13 2017, 1:10 AM
phillco added a comment.EditedDec 13 2017, 12:55 PM

A better way might just be to use None for self.wtcx when rebasing on disk, which will cause fresh wctxs to be used every time and feel less icky. We could use self.inmemory instead of self.wctx.isinmemory().

yuja accepted this revision.Dec 14 2017, 7:19 AM
yuja added a subscriber: yuja.

Queued, thanks.

In D1679#28741, @phillco wrote:

A better way might just be to use None for self.wtcx when rebasing on disk,

Yeah, it might be. workingctx object must be recreated when dirstate changes.

This revision is now accepted and ready to land.Dec 14 2017, 7:19 AM
Closed by commit rHG83014fa95435: rebase: fix for hgsubversion (authored by phillco). · Explain WhyDec 14 2017, 7:30 AM
This revision was automatically updated to reflect the committed changes.
phillco added a comment.Dec 14 2017, 2:01 PM

@yuja is that just because of the _manifest property cache, or are there others as well?

yuja added a comment.Dec 15 2017, 7:50 AM
In D1679#28814, @phillco wrote:

@yuja is that just because of the _manifest property cache, or are there others as well?

Others, too. workingctx._parents() for example.

Revision Contents
Changeset List

PathPackages
Mhgext/rebase.py (5 lines)

Diff 4415

hgext/rebase.py

def rebasenode(repo, rev, p1, base, state, collapse, dest, wctx): def rebasenode(repo, rev, p1, base, state, collapse, dest, wctx):
'Rebase a single revision rev on top of p1 using base as merge ancestor' 'Rebase a single revision rev on top of p1 using base as merge ancestor'
# Merge phase # Merge phase
# Update to destination and merge it with local # Update to destination and merge it with local
if wctx.isinmemory(): if wctx.isinmemory():
wctx.setbase(repo[p1]) wctx.setbase(repo[p1])
else: else:
# This is necessary to invalidate workingctx's caches.
wctx = repo[None]
if repo['.'].rev() != p1: if repo['.'].rev() != p1:
repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1])) repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1]))
mergemod.update(repo, p1, False, True) mergemod.update(repo, p1, False, True)
else: else:
repo.ui.debug(" already in destination\n") repo.ui.debug(" already in destination\n")
# This is, alas, necessary to invalidate workingctx's manifest cache,
# as well as other data we litter on it in other places.
wctx = repo[None]
repo.dirstate.write(repo.currenttransaction()) repo.dirstate.write(repo.currenttransaction())
repo.ui.debug(" merge against %d:%s\n" % (rev, repo[rev])) repo.ui.debug(" merge against %d:%s\n" % (rev, repo[rev]))
if base is not None: if base is not None:
repo.ui.debug(" detach base %d:%s\n" % (base, repo[base])) repo.ui.debug(" detach base %d:%s\n" % (base, repo[base]))
# When collapsing in-place, the parent is the common ancestor, we # When collapsing in-place, the parent is the common ancestor, we
# have to allow merging with it. # have to allow merging with it.
stats = mergemod.update(repo, rev, True, True, base, collapse, stats = mergemod.update(repo, rev, True, True, base, collapse,
labels=['dest', 'source'], wc=wctx) labels=['dest', 'source'], wc=wctx)