Patchwork [9,of,9,v3] bundlerepo: add support for treemanifests in cg3 bundles

login
register
mail settings
Submitter Augie Fackler
Date Aug. 5, 2016, 9:45 p.m.
Message ID <397d6a2f1f2bf957c458.1470433510@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/16141/
State Accepted
Headers show

Comments

Augie Fackler - Aug. 5, 2016, 9:45 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1470416891 14400
#      Fri Aug 05 13:08:11 2016 -0400
# Node ID 397d6a2f1f2bf957c4586da116128cf2f23c1ce1
# Parent  7ec7023b4cb4acaeb42b4fa4ed28b82ebed41b71
bundlerepo: add support for treemanifests in cg3 bundles

This is a little messier than I'd like, and I'll probably come back
and do some more refactoring later, but as it is this unblocks
narrowhg. An alternative approach (which I may do as part of the
mentioned refactoring) would be to construct *all* dirlog instances up
front, so that we don't have to keep track of the linkmapper
method. This would avoid a reference cycle between the bundlemanifest
and the bundlerepository, but I was hesitant to do all the work up
front like that.

With this change, it's possible to do 'hg incoming' and 'hg pull' from
bundles in .hg/strip-backup in a treemanifest repository. Sadly, this
doesn't make it possible to 'hg clone' one of those (if you do 'hg
strip 0'), because the cg3 in the bundle gets written without a
treemanifest flag. Since that's going to be an involved refactor in a
different part of the code (which I *suspect* won't touch any of the
code I've just written here), let's leave it as an idea for Later.
via Mercurial-devel - Aug. 5, 2016, 10:24 p.m.
Thanks for fixing this! Pushing to 'committed' with some minor changes
in flight once tests have finished.

On Fri, Aug 5, 2016 at 2:45 PM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1470416891 14400
> #      Fri Aug 05 13:08:11 2016 -0400
> # Node ID 397d6a2f1f2bf957c4586da116128cf2f23c1ce1
> # Parent  7ec7023b4cb4acaeb42b4fa4ed28b82ebed41b71
> bundlerepo: add support for treemanifests in cg3 bundles
>
> This is a little messier than I'd like, and I'll probably come back
> and do some more refactoring later, but as it is this unblocks
> narrowhg. An alternative approach (which I may do as part of the
> mentioned refactoring) would be to construct *all* dirlog instances up
> front, so that we don't have to keep track of the linkmapper
> method. This would avoid a reference cycle between the bundlemanifest
> and the bundlerepository, but I was hesitant to do all the work up
> front like that.
>
> With this change, it's possible to do 'hg incoming' and 'hg pull' from
> bundles in .hg/strip-backup in a treemanifest repository. Sadly, this
> doesn't make it possible to 'hg clone' one of those (if you do 'hg
> strip 0'), because the cg3 in the bundle gets written without a
> treemanifest flag. Since that's going to be an involved refactor in a
> different part of the code (which I *suspect* won't touch any of the
> code I've just written here), let's leave it as an idea for Later.
>
> diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
> --- a/mercurial/bundlerepo.py
> +++ b/mercurial/bundlerepo.py
> @@ -188,10 +188,17 @@ class bundlechangelog(bundlerevlog, chan
>              self.filteredrevs = oldfilter
>
>  class bundlemanifest(bundlerevlog, manifest.manifest):
> -    def __init__(self, opener, bundle, linkmapper):
> -        manifest.manifest.__init__(self, opener)
> +    def __init__(self, opener, bundle, linkmapper, dirlogstarts=None, dir=''):
> +        manifest.manifest.__init__(self, opener, dir=dir)
>          bundlerevlog.__init__(self, opener, self.indexfile, bundle,
>                                linkmapper)
> +        if dirlogstarts is None:
> +            if self.bundle.version == "03":
> +                dirlogstarts = _getfilestarts(self.bundle)
> +            else:
> +                dirlogstarts = {}

Nit: I think I've heard that we prefer reassignment (of dirlogstarts
in this case) to 'else' blocks. Changing in flight.

> +        self._dirlogstarts = dirlogstarts
> +        self.linkmapper = linkmapper

Only used within this class, so adding '_' prefix in flight.

>
>      def baserevision(self, nodeorrev):
>          node = nodeorrev
> @@ -204,6 +211,14 @@ class bundlemanifest(bundlerevlog, manif
>              result = manifest.manifest.revision(self, nodeorrev)
>          return result
>
> +    def dirlog(self, d):
> +        if d in self._dirlogstarts:
> +            self.bundle.seek(self._dirlogstarts[d])
> +            return bundlemanifest(
> +                self.opener, self.bundle, self.linkmapper,
> +                self._dirlogstarts, dir=d)
> +        return super(bundlemanifest, self).dirlog(d)
> +
>  class bundlefilelog(bundlerevlog, filelog.filelog):
>      def __init__(self, opener, path, bundle, linkmapper):
>          filelog.filelog.__init__(self, opener, path)
> @@ -310,6 +325,8 @@ class bundlerepository(localrepo.localre
>                                                                  bundlename,
>                                                                  self.vfs)
>
> +        # map dirlog name to position in the bundle
> +        self.bundledirstarts = {} # type: Dict[bytes, int]

I was going to complain about the inconsistency with the name below,
but it seems unused now, so I'll just remove it instead. And the
_dirlogstarts is too far away too bother me :-)

>          # dict with the mapping 'filename' -> position in the bundle
>          self.bundlefilespos = {}
>
> @@ -336,10 +353,6 @@ class bundlerepository(localrepo.localre
>          self.bundle.manifestheader()
>          linkmapper = self.unfiltered().changelog.rev
>          m = bundlemanifest(self.svfs, self.bundle, linkmapper)
> -        # XXX: hack to work with changegroup3, but we still don't handle
> -        # tree manifests correctly
> -        if self.bundle.version == "03":
> -            self.bundle.filelogheader()
>          self.filestart = self.bundle.tell()
>          return m
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -913,7 +913,7 @@ class manifest(revlog.revlog):
>          self._usemanifestv2 = usemanifestv2
>          indexfile = "00manifest.i"
>          if dir:
> -            assert self._treeondisk
> +            assert self._treeondisk, 'opts is %r' % opts
>              if not dir.endswith('/'):
>                  dir = dir + '/'
>              indexfile = "meta/" + dir + "00manifest.i"
> diff --git a/tests/test-treemanifest.t b/tests/test-treemanifest.t
> --- a/tests/test-treemanifest.t
> +++ b/tests/test-treemanifest.t
> @@ -326,6 +326,25 @@ Stripping and recovering changes should
>       rev    offset  length  delta linkrev nodeid       p1           p2
>         0         0     127     -1       4 064927a0648a 000000000000 000000000000
>         1       127     111      0       5 25ecb8cb8618 000000000000 000000000000
> +  $ hg incoming .hg/strip-backup/*
> +  comparing with .hg/strip-backup/*-backup.hg (glob)
> +  searching for changes
> +  changeset:   6:51cfd7b1e13b
> +  tag:         tip
> +  user:        test
> +  date:        Thu Jan 01 00:00:00 1970 +0000
> +  summary:     modify dir1/a
> +
> +  $ hg pull .hg/strip-backup/*
> +  pulling from .hg/strip-backup/51cfd7b1e13b-78a2f3ed-backup.hg
> +  searching for changes
> +  adding changesets
> +  adding manifests
> +  adding file changes
> +  added 1 changesets with 1 changes to 1 files
> +  (run 'hg update' to get a working copy)
> +  $ hg --config extensions.strip= strip tip
> +  saved backup bundle to $TESTTMP/repo-mixed/.hg/strip-backup/*-backup.hg (glob)
>    $ hg unbundle -q .hg/strip-backup/*
>    $ hg debugindex --dir dir1
>       rev    offset  length  delta linkrev nodeid       p1           p2
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -188,10 +188,17 @@  class bundlechangelog(bundlerevlog, chan
             self.filteredrevs = oldfilter
 
 class bundlemanifest(bundlerevlog, manifest.manifest):
-    def __init__(self, opener, bundle, linkmapper):
-        manifest.manifest.__init__(self, opener)
+    def __init__(self, opener, bundle, linkmapper, dirlogstarts=None, dir=''):
+        manifest.manifest.__init__(self, opener, dir=dir)
         bundlerevlog.__init__(self, opener, self.indexfile, bundle,
                               linkmapper)
+        if dirlogstarts is None:
+            if self.bundle.version == "03":
+                dirlogstarts = _getfilestarts(self.bundle)
+            else:
+                dirlogstarts = {}
+        self._dirlogstarts = dirlogstarts
+        self.linkmapper = linkmapper
 
     def baserevision(self, nodeorrev):
         node = nodeorrev
@@ -204,6 +211,14 @@  class bundlemanifest(bundlerevlog, manif
             result = manifest.manifest.revision(self, nodeorrev)
         return result
 
+    def dirlog(self, d):
+        if d in self._dirlogstarts:
+            self.bundle.seek(self._dirlogstarts[d])
+            return bundlemanifest(
+                self.opener, self.bundle, self.linkmapper,
+                self._dirlogstarts, dir=d)
+        return super(bundlemanifest, self).dirlog(d)
+
 class bundlefilelog(bundlerevlog, filelog.filelog):
     def __init__(self, opener, path, bundle, linkmapper):
         filelog.filelog.__init__(self, opener, path)
@@ -310,6 +325,8 @@  class bundlerepository(localrepo.localre
                                                                 bundlename,
                                                                 self.vfs)
 
+        # map dirlog name to position in the bundle
+        self.bundledirstarts = {} # type: Dict[bytes, int]
         # dict with the mapping 'filename' -> position in the bundle
         self.bundlefilespos = {}
 
@@ -336,10 +353,6 @@  class bundlerepository(localrepo.localre
         self.bundle.manifestheader()
         linkmapper = self.unfiltered().changelog.rev
         m = bundlemanifest(self.svfs, self.bundle, linkmapper)
-        # XXX: hack to work with changegroup3, but we still don't handle
-        # tree manifests correctly
-        if self.bundle.version == "03":
-            self.bundle.filelogheader()
         self.filestart = self.bundle.tell()
         return m
 
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -913,7 +913,7 @@  class manifest(revlog.revlog):
         self._usemanifestv2 = usemanifestv2
         indexfile = "00manifest.i"
         if dir:
-            assert self._treeondisk
+            assert self._treeondisk, 'opts is %r' % opts
             if not dir.endswith('/'):
                 dir = dir + '/'
             indexfile = "meta/" + dir + "00manifest.i"
diff --git a/tests/test-treemanifest.t b/tests/test-treemanifest.t
--- a/tests/test-treemanifest.t
+++ b/tests/test-treemanifest.t
@@ -326,6 +326,25 @@  Stripping and recovering changes should 
      rev    offset  length  delta linkrev nodeid       p1           p2
        0         0     127     -1       4 064927a0648a 000000000000 000000000000
        1       127     111      0       5 25ecb8cb8618 000000000000 000000000000
+  $ hg incoming .hg/strip-backup/*
+  comparing with .hg/strip-backup/*-backup.hg (glob)
+  searching for changes
+  changeset:   6:51cfd7b1e13b
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     modify dir1/a
+  
+  $ hg pull .hg/strip-backup/*
+  pulling from .hg/strip-backup/51cfd7b1e13b-78a2f3ed-backup.hg
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  (run 'hg update' to get a working copy)
+  $ hg --config extensions.strip= strip tip
+  saved backup bundle to $TESTTMP/repo-mixed/.hg/strip-backup/*-backup.hg (glob)
   $ hg unbundle -q .hg/strip-backup/*
   $ hg debugindex --dir dir1
      rev    offset  length  delta linkrev nodeid       p1           p2