Patchwork [7,of,7] manifest: remove manifest.add and add memmfctx.write

login
register
mail settings
Submitter Durham Goode
Date Nov. 8, 2016, 4:43 p.m.
Message ID <23554dcfb163996fa003.1478623381@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/17412/
State Accepted
Headers show

Comments

Durham Goode - Nov. 8, 2016, 4:43 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1478621023 28800
#      Tue Nov 08 08:03:43 2016 -0800
# Node ID 23554dcfb163996fa003492385b367aa2c3c0bb1
# Parent  05bf15311d25783de82dcbe0f93df5963b504357
manifest: remove manifest.add and add memmfctx.write

This removes one more dependency on the manifest class by moving the write
functionality onto the memmanifestctx classes and changing the one consumer to
use the new API.

By moving the write path to a manifestctx, we now give the individual manifests
control over how they're read and serialized. This will be useful in developing
new manifest formats and storage systems.
Augie Fackler - Nov. 8, 2016, 6:40 p.m.
On Tue, Nov 08, 2016 at 08:43:01AM -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1478621023 28800
> #      Tue Nov 08 08:03:43 2016 -0800
> # Node ID 23554dcfb163996fa003492385b367aa2c3c0bb1
> # Parent  05bf15311d25783de82dcbe0f93df5963b504357
> manifest: remove manifest.add and add memmfctx.write

Series LG, but I want to let Martin look since he's been in here more frequently.

>
> This removes one more dependency on the manifest class by moving the write
> functionality onto the memmanifestctx classes and changing the one consumer to
> use the new API.
>
> By moving the write path to a manifestctx, we now give the individual manifests
> control over how they're read and serialized. This will be useful in developing
> new manifest formats and storage systems.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -1700,9 +1700,13 @@ class localrepository(object):
>              trp = weakref.proxy(tr)
>
>              if ctx.files():
> -                m1 = p1.manifest()
> -                m2 = p2.manifest()
> -                m = m1.copy()
> +                m1ctx = p1.manifestctx()
> +                m2ctx = p2.manifestctx()
> +                mctx = m1ctx.copy()
> +
> +                m = mctx.read()
> +                m1 = m1ctx.read()
> +                m2 = m2ctx.read()
>
>                  # check in files
>                  added = []
> @@ -1736,9 +1740,9 @@ class localrepository(object):
>                  drop = [f for f in removed if f in m]
>                  for f in drop:
>                      del m[f]
> -                mn = self.manifestlog.add(m, trp, linkrev,
> -                                          p1.manifestnode(), p2.manifestnode(),
> -                                          added, drop)
> +                mn = mctx.write(trp, linkrev,
> +                                p1.manifestnode(), p2.manifestnode(),
> +                                added, drop)
>                  files = changed + removed
>              else:
>                  mn = p1.manifestnode()
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -1315,14 +1315,14 @@ class manifestlog(object):
>              mancache[node] = m
>          return m
>
> -    def add(self, m, transaction, link, p1, p2, added, removed):
> -        return self._revlog.add(m, transaction, link, p1, p2, added, removed)
> -
>  class memmanifestctx(object):
>      def __init__(self, repo):
>          self._repo = repo
>          self._manifestdict = manifestdict()
>
> +    def _revlog(self):
> +        return self._repo.manifestlog._revlog
> +
>      def new(self):
>          return memmanifestctx(self._repo)
>
> @@ -1334,6 +1334,10 @@ class memmanifestctx(object):
>      def read(self):
>          return self._manifestdict
>
> +    def write(self, transaction, link, p1, p2, added, removed):
> +        return self._revlog().add(self._manifestdict, transaction, link, p1, p2,
> +                                  added, removed)
> +
>  class manifestctx(object):
>      """A class representing a single revision of a manifest, including its
>      contents, its parent revs, and its linkrev.
> @@ -1425,6 +1429,9 @@ class memtreemanifestctx(object):
>          self._dir = dir
>          self._treemanifest = treemanifest()
>
> +    def _revlog(self):
> +        return self._repo.manifestlog._revlog
> +
>      def new(self, dir=''):
>          return memtreemanifestctx(self._repo, dir=dir)
>
> @@ -1436,6 +1443,10 @@ class memtreemanifestctx(object):
>      def read(self):
>          return self._treemanifest
>
> +    def write(self, transaction, link, p1, p2, added, removed):
> +        return self._revlog().add(self._treemanifest, transaction, link, p1, p2,
> +                                  added, removed)
> +
>  class treemanifestctx(object):
>      def __init__(self, repo, dir, node):
>          self._repo = repo
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - Nov. 8, 2016, 7:05 p.m.
On Tue, Nov 8, 2016 at 10:40 AM, Augie Fackler <raf@durin42.com> wrote:

> On Tue, Nov 08, 2016 at 08:43:01AM -0800, Durham Goode wrote:
> > # HG changeset patch
> > # User Durham Goode <durham@fb.com>
> > # Date 1478621023 28800
> > #      Tue Nov 08 08:03:43 2016 -0800
> > # Node ID 23554dcfb163996fa003492385b367aa2c3c0bb1
> > # Parent  05bf15311d25783de82dcbe0f93df5963b504357
> > manifest: remove manifest.add and add memmfctx.write
>
> Series LG, but I want to let Martin look since he's been in here more
> frequently.
>

This series likely bitrots some of my in-flight series (notably perfbdiff
and possibly debugupgraderepo).

I've hogged the review queue a lot in this cycle so far. So please treat
Durham's stuff as more important and force me to rebase.


>
> >
> > This removes one more dependency on the manifest class by moving the
> write
> > functionality onto the memmanifestctx classes and changing the one
> consumer to
> > use the new API.
> >
> > By moving the write path to a manifestctx, we now give the individual
> manifests
> > control over how they're read and serialized. This will be useful in
> developing
> > new manifest formats and storage systems.
> >
> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> > --- a/mercurial/localrepo.py
> > +++ b/mercurial/localrepo.py
> > @@ -1700,9 +1700,13 @@ class localrepository(object):
> >              trp = weakref.proxy(tr)
> >
> >              if ctx.files():
> > -                m1 = p1.manifest()
> > -                m2 = p2.manifest()
> > -                m = m1.copy()
> > +                m1ctx = p1.manifestctx()
> > +                m2ctx = p2.manifestctx()
> > +                mctx = m1ctx.copy()
> > +
> > +                m = mctx.read()
> > +                m1 = m1ctx.read()
> > +                m2 = m2ctx.read()
> >
> >                  # check in files
> >                  added = []
> > @@ -1736,9 +1740,9 @@ class localrepository(object):
> >                  drop = [f for f in removed if f in m]
> >                  for f in drop:
> >                      del m[f]
> > -                mn = self.manifestlog.add(m, trp, linkrev,
> > -                                          p1.manifestnode(),
> p2.manifestnode(),
> > -                                          added, drop)
> > +                mn = mctx.write(trp, linkrev,
> > +                                p1.manifestnode(), p2.manifestnode(),
> > +                                added, drop)
> >                  files = changed + removed
> >              else:
> >                  mn = p1.manifestnode()
> > diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> > --- a/mercurial/manifest.py
> > +++ b/mercurial/manifest.py
> > @@ -1315,14 +1315,14 @@ class manifestlog(object):
> >              mancache[node] = m
> >          return m
> >
> > -    def add(self, m, transaction, link, p1, p2, added, removed):
> > -        return self._revlog.add(m, transaction, link, p1, p2, added,
> removed)
> > -
> >  class memmanifestctx(object):
> >      def __init__(self, repo):
> >          self._repo = repo
> >          self._manifestdict = manifestdict()
> >
> > +    def _revlog(self):
> > +        return self._repo.manifestlog._revlog
> > +
> >      def new(self):
> >          return memmanifestctx(self._repo)
> >
> > @@ -1334,6 +1334,10 @@ class memmanifestctx(object):
> >      def read(self):
> >          return self._manifestdict
> >
> > +    def write(self, transaction, link, p1, p2, added, removed):
> > +        return self._revlog().add(self._manifestdict, transaction,
> link, p1, p2,
> > +                                  added, removed)
> > +
> >  class manifestctx(object):
> >      """A class representing a single revision of a manifest, including
> its
> >      contents, its parent revs, and its linkrev.
> > @@ -1425,6 +1429,9 @@ class memtreemanifestctx(object):
> >          self._dir = dir
> >          self._treemanifest = treemanifest()
> >
> > +    def _revlog(self):
> > +        return self._repo.manifestlog._revlog
> > +
> >      def new(self, dir=''):
> >          return memtreemanifestctx(self._repo, dir=dir)
> >
> > @@ -1436,6 +1443,10 @@ class memtreemanifestctx(object):
> >      def read(self):
> >          return self._treemanifest
> >
> > +    def write(self, transaction, link, p1, p2, added, removed):
> > +        return self._revlog().add(self._treemanifest, transaction,
> link, p1, p2,
> > +                                  added, removed)
> > +
> >  class treemanifestctx(object):
> >      def __init__(self, repo, dir, node):
> >          self._repo = repo
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
via Mercurial-devel - Nov. 9, 2016, 7:09 p.m.
On Tue, Nov 8, 2016 at 11:05 AM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> On Tue, Nov 8, 2016 at 10:40 AM, Augie Fackler <raf@durin42.com> wrote:
>>
>> On Tue, Nov 08, 2016 at 08:43:01AM -0800, Durham Goode wrote:
>> > # HG changeset patch
>> > # User Durham Goode <durham@fb.com>
>> > # Date 1478621023 28800
>> > #      Tue Nov 08 08:03:43 2016 -0800
>> > # Node ID 23554dcfb163996fa003492385b367aa2c3c0bb1
>> > # Parent  05bf15311d25783de82dcbe0f93df5963b504357
>> > manifest: remove manifest.add and add memmfctx.write
>>
>> Series LG, but I want to let Martin look since he's been in here more
>> frequently.
>
>
> This series likely bitrots some of my in-flight series (notably perfbdiff
> and possibly debugupgraderepo).
>
> I've hogged the review queue a lot in this cycle so far. So please treat
> Durham's stuff as more important and force me to rebase.


Queued, thanks.

>
>>
>>
>> >
>> > This removes one more dependency on the manifest class by moving the
>> > write
>> > functionality onto the memmanifestctx classes and changing the one
>> > consumer to
>> > use the new API.
>> >
>> > By moving the write path to a manifestctx, we now give the individual
>> > manifests
>> > control over how they're read and serialized. This will be useful in
>> > developing
>> > new manifest formats and storage systems.
>> >
>> > diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> > --- a/mercurial/localrepo.py
>> > +++ b/mercurial/localrepo.py
>> > @@ -1700,9 +1700,13 @@ class localrepository(object):
>> >              trp = weakref.proxy(tr)
>> >
>> >              if ctx.files():
>> > -                m1 = p1.manifest()
>> > -                m2 = p2.manifest()
>> > -                m = m1.copy()
>> > +                m1ctx = p1.manifestctx()
>> > +                m2ctx = p2.manifestctx()
>> > +                mctx = m1ctx.copy()
>> > +
>> > +                m = mctx.read()
>> > +                m1 = m1ctx.read()
>> > +                m2 = m2ctx.read()
>> >
>> >                  # check in files
>> >                  added = []
>> > @@ -1736,9 +1740,9 @@ class localrepository(object):
>> >                  drop = [f for f in removed if f in m]
>> >                  for f in drop:
>> >                      del m[f]
>> > -                mn = self.manifestlog.add(m, trp, linkrev,
>> > -                                          p1.manifestnode(),
>> > p2.manifestnode(),
>> > -                                          added, drop)
>> > +                mn = mctx.write(trp, linkrev,
>> > +                                p1.manifestnode(), p2.manifestnode(),
>> > +                                added, drop)
>> >                  files = changed + removed
>> >              else:
>> >                  mn = p1.manifestnode()
>> > diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> > --- a/mercurial/manifest.py
>> > +++ b/mercurial/manifest.py
>> > @@ -1315,14 +1315,14 @@ class manifestlog(object):
>> >              mancache[node] = m
>> >          return m
>> >
>> > -    def add(self, m, transaction, link, p1, p2, added, removed):
>> > -        return self._revlog.add(m, transaction, link, p1, p2, added,
>> > removed)
>> > -
>> >  class memmanifestctx(object):
>> >      def __init__(self, repo):
>> >          self._repo = repo
>> >          self._manifestdict = manifestdict()
>> >
>> > +    def _revlog(self):
>> > +        return self._repo.manifestlog._revlog
>> > +
>> >      def new(self):
>> >          return memmanifestctx(self._repo)
>> >
>> > @@ -1334,6 +1334,10 @@ class memmanifestctx(object):
>> >      def read(self):
>> >          return self._manifestdict
>> >
>> > +    def write(self, transaction, link, p1, p2, added, removed):
>> > +        return self._revlog().add(self._manifestdict, transaction,
>> > link, p1, p2,
>> > +                                  added, removed)
>> > +
>> >  class manifestctx(object):
>> >      """A class representing a single revision of a manifest, including
>> > its
>> >      contents, its parent revs, and its linkrev.
>> > @@ -1425,6 +1429,9 @@ class memtreemanifestctx(object):
>> >          self._dir = dir
>> >          self._treemanifest = treemanifest()
>> >
>> > +    def _revlog(self):
>> > +        return self._repo.manifestlog._revlog
>> > +
>> >      def new(self, dir=''):
>> >          return memtreemanifestctx(self._repo, dir=dir)
>> >
>> > @@ -1436,6 +1443,10 @@ class memtreemanifestctx(object):
>> >      def read(self):
>> >          return self._treemanifest
>> >
>> > +    def write(self, transaction, link, p1, p2, added, removed):
>> > +        return self._revlog().add(self._treemanifest, transaction,
>> > link, p1, p2,
>> > +                                  added, removed)
>> > +
>> >  class treemanifestctx(object):
>> >      def __init__(self, repo, dir, node):
>> >          self._repo = repo
>> > _______________________________________________
>> > Mercurial-devel mailing list
>> > Mercurial-devel@mercurial-scm.org
>> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>

Patch

diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -1700,9 +1700,13 @@  class localrepository(object):
             trp = weakref.proxy(tr)
 
             if ctx.files():
-                m1 = p1.manifest()
-                m2 = p2.manifest()
-                m = m1.copy()
+                m1ctx = p1.manifestctx()
+                m2ctx = p2.manifestctx()
+                mctx = m1ctx.copy()
+
+                m = mctx.read()
+                m1 = m1ctx.read()
+                m2 = m2ctx.read()
 
                 # check in files
                 added = []
@@ -1736,9 +1740,9 @@  class localrepository(object):
                 drop = [f for f in removed if f in m]
                 for f in drop:
                     del m[f]
-                mn = self.manifestlog.add(m, trp, linkrev,
-                                          p1.manifestnode(), p2.manifestnode(),
-                                          added, drop)
+                mn = mctx.write(trp, linkrev,
+                                p1.manifestnode(), p2.manifestnode(),
+                                added, drop)
                 files = changed + removed
             else:
                 mn = p1.manifestnode()
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1315,14 +1315,14 @@  class manifestlog(object):
             mancache[node] = m
         return m
 
-    def add(self, m, transaction, link, p1, p2, added, removed):
-        return self._revlog.add(m, transaction, link, p1, p2, added, removed)
-
 class memmanifestctx(object):
     def __init__(self, repo):
         self._repo = repo
         self._manifestdict = manifestdict()
 
+    def _revlog(self):
+        return self._repo.manifestlog._revlog
+
     def new(self):
         return memmanifestctx(self._repo)
 
@@ -1334,6 +1334,10 @@  class memmanifestctx(object):
     def read(self):
         return self._manifestdict
 
+    def write(self, transaction, link, p1, p2, added, removed):
+        return self._revlog().add(self._manifestdict, transaction, link, p1, p2,
+                                  added, removed)
+
 class manifestctx(object):
     """A class representing a single revision of a manifest, including its
     contents, its parent revs, and its linkrev.
@@ -1425,6 +1429,9 @@  class memtreemanifestctx(object):
         self._dir = dir
         self._treemanifest = treemanifest()
 
+    def _revlog(self):
+        return self._repo.manifestlog._revlog
+
     def new(self, dir=''):
         return memtreemanifestctx(self._repo, dir=dir)
 
@@ -1436,6 +1443,10 @@  class memtreemanifestctx(object):
     def read(self):
         return self._treemanifest
 
+    def write(self, transaction, link, p1, p2, added, removed):
+        return self._revlog().add(self._treemanifest, transaction, link, p1, p2,
+                                  added, removed)
+
 class treemanifestctx(object):
     def __init__(self, repo, dir, node):
         self._repo = repo