Patchwork D1679: rebase: fix for hgsubversion

login
register
mail settings
Submitter phabricator
Date Dec. 13, 2017, 6:10 a.m.
Message ID <differential-rev-PHID-DREV-b32wx2dz74elrwcu573p-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26263/
State Superseded
Headers show

Comments

phabricator - Dec. 13, 2017, 6:10 a.m.
phillco created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  https://phab.mercurial-scm.org/rHG5c25fe7fb1e63a31ed082dfcb93673fa79cf081f 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1679

AFFECTED FILES
  hgext/rebase.py

CHANGE DETAILS




To: phillco, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 13, 2017, 5:55 p.m.
phillco added a comment.


  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 `sel.inmemory` instead of `self.wctx.isinmemory()`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1679

To: phillco, #hg-reviewers
Cc: mercurial-devel
phabricator - Dec. 14, 2017, 12:19 p.m.
yuja accepted this revision.
yuja added a comment.
This revision is now accepted and ready to land.


  Queued, thanks.
  
  In https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1679

To: phillco, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 14, 2017, 7:01 p.m.
phillco added a comment.


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

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1679

To: phillco, #hg-reviewers, yuja
Cc: yuja, mercurial-devel
phabricator - Dec. 15, 2017, 12:50 p.m.
yuja added a comment.


  In https://phab.mercurial-scm.org/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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1679

To: phillco, #hg-reviewers, yuja
Cc: yuja, mercurial-devel

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1102,13 +1102,14 @@ 
     if wctx.isinmemory():
         wctx.setbase(repo[p1])
     else:
-        # This is necessary to invalidate workingctx's caches.
-        wctx = repo[None]
         if repo['.'].rev() != p1:
             repo.ui.debug(" update to %d:%s\n" % (p1, repo[p1]))
             mergemod.update(repo, p1, False, True)
         else:
             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.ui.debug(" merge against %d:%s\n" % (rev, repo[rev]))
     if base is not None: