Patchwork [6,of,6,ctx-minor-fixes] context: define contract for parents

login
register
mail settings
Submitter Sean Farley
Date Sept. 19, 2018, 5:35 a.m.
Message ID <58496e395dd324297748.1537335348@1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa>
Download mbox | patch
Permalink /patch/34824/
State Accepted
Headers show

Comments

Sean Farley - Sept. 19, 2018, 5:35 a.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1528497109 25200
#      Fri Jun 08 15:31:49 2018 -0700
# Branch ctx-minor-fixes
# Node ID 58496e395dd324297748551297d1d572ab762496
# Parent  56222df6e5c8738b9b04881477c166540c50b17a
context: define contract for parents

From now on, context._parents will be a list of length two, containing
the parent revs while context.parents() will return a list of the parent
contexts for each parent that isn't null.
Yuya Nishihara - Sept. 19, 2018, 1:27 p.m.
On Tue, 18 Sep 2018 22:35:48 -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1528497109 25200
> #      Fri Jun 08 15:31:49 2018 -0700
> # Branch ctx-minor-fixes
> # Node ID 58496e395dd324297748551297d1d572ab762496
> # Parent  56222df6e5c8738b9b04881477c166540c50b17a
> context: define contract for parents

>      def parents(self):
>          """return contexts for each parent changeset"""
> -        return self._parents
> +        pl = self._parents
> +
> +        # maybe should assert that we expect _parents to always be both parents
> +        # (even when p2 is null) and to always be revs (e.g. ints)
> +        if pl[1] == nullrev:
> +            return [changectx(self._repo, pl[0])]
> +        return [changectx(self._repo, pl[0]), changectx(self._repo, pl[1])]
>  
>      def p1(self):
>          return self.parents()[0]
>  
>      def p2(self):
> @@ -479,15 +485,11 @@ class changectx(basectx):
>      def _manifestdelta(self):
>          return self._manifestctx.readdelta()
>  
>      @propertycache
>      def _parents(self):
> -        repo = self._repo
> -        p1, p2 = repo.changelog.parentrevs(self._rev)
> -        if p2 == nullrev:
> -            return [changectx(repo, p1)]
> -        return [changectx(repo, p1), changectx(repo, p2)]
> +        return self._repo.changelog.parentrevs(self._rev)

So, this removes the cache of p1/p2 ctxs. IIUC, that's the reason we have
self._parents as an interface for sub classes.
Sean Farley - Sept. 19, 2018, 9:38 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Tue, 18 Sep 2018 22:35:48 -0700, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1528497109 25200
>> #      Fri Jun 08 15:31:49 2018 -0700
>> # Branch ctx-minor-fixes
>> # Node ID 58496e395dd324297748551297d1d572ab762496
>> # Parent  56222df6e5c8738b9b04881477c166540c50b17a
>> context: define contract for parents
>
>>      def parents(self):
>>          """return contexts for each parent changeset"""
>> -        return self._parents
>> +        pl = self._parents
>> +
>> +        # maybe should assert that we expect _parents to always be both parents
>> +        # (even when p2 is null) and to always be revs (e.g. ints)
>> +        if pl[1] == nullrev:
>> +            return [changectx(self._repo, pl[0])]
>> +        return [changectx(self._repo, pl[0]), changectx(self._repo, pl[1])]
>>  
>>      def p1(self):
>>          return self.parents()[0]
>>  
>>      def p2(self):
>> @@ -479,15 +485,11 @@ class changectx(basectx):
>>      def _manifestdelta(self):
>>          return self._manifestctx.readdelta()
>>  
>>      @propertycache
>>      def _parents(self):
>> -        repo = self._repo
>> -        p1, p2 = repo.changelog.parentrevs(self._rev)
>> -        if p2 == nullrev:
>> -            return [changectx(repo, p1)]
>> -        return [changectx(repo, p1), changectx(repo, p2)]
>> +        return self._repo.changelog.parentrevs(self._rev)
>
> So, this removes the cache of p1/p2 ctxs. IIUC, that's the reason we have
> self._parents as an interface for sub classes.

Oh, ho, good catch. My motivation here is to make substituting contexts
a bit more uniform. I'll try to keep the changectx cache and see how
that works.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
index 1871edb..ca54843 100644
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -231,11 +231,17 @@  class basectx(object):
             instabilities.append('content-divergent')
         return instabilities
 
     def parents(self):
         """return contexts for each parent changeset"""
-        return self._parents
+        pl = self._parents
+
+        # maybe should assert that we expect _parents to always be both parents
+        # (even when p2 is null) and to always be revs (e.g. ints)
+        if pl[1] == nullrev:
+            return [changectx(self._repo, pl[0])]
+        return [changectx(self._repo, pl[0]), changectx(self._repo, pl[1])]
 
     def p1(self):
         return self.parents()[0]
 
     def p2(self):
@@ -479,15 +485,11 @@  class changectx(basectx):
     def _manifestdelta(self):
         return self._manifestctx.readdelta()
 
     @propertycache
     def _parents(self):
-        repo = self._repo
-        p1, p2 = repo.changelog.parentrevs(self._rev)
-        if p2 == nullrev:
-            return [changectx(repo, p1)]
-        return [changectx(repo, p1), changectx(repo, p2)]
+        return self._repo.changelog.parentrevs(self._rev)
 
     def changeset(self):
         c = self._changeset
         return (
             c.manifest,
@@ -1298,14 +1300,11 @@  class workingctx(committablectx):
     def hex(self):
         return hex(wdirid)
 
     @propertycache
     def _parents(self):
-        p = self._repo.dirstate.parents()
-        if p[1] == nullid:
-            p = p[:-1]
-        return [changectx(self._repo, x) for x in p]
+        return [self._repo[p].rev() for p in self._repo.dirstate.parents()]
 
     def _fileinfo(self, path):
         # populate __dict__['_manifest'] as workingctx has no _manifestdelta
         self._manifest
         return super(workingctx, self)._fileinfo(path)
@@ -1777,11 +1776,11 @@  class overlayworkingctx(committablectx):
         super(overlayworkingctx, self).__init__(repo)
         self.clean()
 
     def setbase(self, wrappedctx):
         self._wrappedctx = wrappedctx
-        self._parents = [wrappedctx]
+        self._parents = [wrappedctx.rev(), nullrev]
         # Drop old manifest cache as it is now out of date.
         # This is necessary when, e.g., rebasing several nodes with one
         # ``overlayworkingctx`` (e.g. with --collapse).
         util.clearcachedproperty(self, '_manifest')
 
@@ -2245,11 +2244,11 @@  class memctx(committablectx):
         super(memctx, self).__init__(repo, text, user, date, extra)
         self._rev = None
         self._node = None
         parents = [(p or nullid) for p in parents]
         p1, p2 = parents
-        self._parents = [self._repo[p] for p in (p1, p2)]
+        self._parents = [self._repo[p].rev() for p in (p1, p2)]
         files = sorted(set(files))
         self._files = files
         if branch is not None:
             self._extra['branch'] = encoding.fromlocal(branch)
         self.substate = {}
@@ -2391,19 +2390,19 @@  class metadataonlyctx(committablectx):
         else:
             parents = [repo[p] for p in parents if p is not None]
         parents = parents[:]
         while len(parents) < 2:
             parents.append(repo[nullid])
-        p1, p2 = self._parents = parents
+        p1, p2 = self._parents = parents[0].rev(), parents[1].rev()
 
         # sanity check to ensure that the reused manifest parents are
         # manifests of our commit parents
         mp1, mp2 = self.manifestctx().parents
-        if p1 != nullid and p1.manifestnode() != mp1:
+        if p1 != nullrev and self.p1().manifestnode() != mp1:
             raise RuntimeError('can\'t reuse the manifest: '
                                'its p1 doesn\'t match the new ctx p1')
-        if p2 != nullid and p2.manifestnode() != mp2:
+        if p2 != nullrev and self.p2().manifestnode() != mp2:
             raise RuntimeError('can\'t reuse the manifest: '
                                'its p2 doesn\'t match the new ctx p2')
 
         self._files = originalctx.files()
         self.substate = {}
diff --git a/tests/drawdag.py b/tests/drawdag.py
index c417856..2f784c7 100644
--- a/tests/drawdag.py
+++ b/tests/drawdag.py
@@ -288,13 +288,13 @@  class simplecommitctx(context.committabl
             'date': b'0 0',
             'extra': {b'branch': b'default'},
         }
         super(simplecommitctx, self).__init__(repo, name, **opts)
         self._added = added
-        self._parents = parentctxs
+        self._parents = [p.rev() for p in parentctxs]
         while len(self._parents) < 2:
-            self._parents.append(repo[node.nullid])
+            self._parents.append(node.nullrev)
 
     def filectx(self, key):
         return simplefilectx(key, self._added[key])
 
     def commit(self):