Patchwork [09,of,10] bundlerepo: make __init__ more like revlog.addgroup

login
register
mail settings
Submitter Mads Kiilerich
Date Jan. 16, 2013, 7:57 p.m.
Message ID <52c0ec0dbbe5cae4daeb.1358366253@mk-desktop>
Download mbox | patch
Permalink /patch/669/
State Deferred
Headers show

Comments

Mads Kiilerich - Jan. 16, 2013, 7:57 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1358365301 -3600
# Node ID 52c0ec0dbbe5cae4daeb33d0a3591e6240e9054e
# Parent  61031b4e9ac4ab381814398489771f53f0ca9751
bundlerepo: make __init__ more like revlog.addgroup

- just to prove there is duplicate code.
tonfa - Jan. 16, 2013, 8:32 p.m.
LGTM


On Wed, Jan 16, 2013 at 8:57 PM, Mads Kiilerich <mads@kiilerich.com> wrote:

> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1358365301 -3600
> # Node ID 52c0ec0dbbe5cae4daeb33d0a3591e6240e9054e
> # Parent  61031b4e9ac4ab381814398489771f53f0ca9751
> bundlerepo: make __init__ more like revlog.addgroup
>
> - just to prove there is duplicate code.
>
> diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
> --- a/mercurial/bundlerepo.py
> +++ b/mercurial/bundlerepo.py
> @@ -33,8 +33,9 @@
>          self.bundle = bundle
>          self.basemap = {} # mapping rev to delta base rev
>          n = len(self)
> +        self.bundlerevs = set() # used by 'bundle()' revset expression
> +
>          chain = None
> -        self.bundlerevs = set() # used by 'bundle()' revset expression
>          while True:
>              chunkdata = bundle.deltachunk(chain)
>              if not chunkdata:
> @@ -46,9 +47,6 @@
>              deltabase = chunkdata['deltabase']
>              delta = chunkdata['delta']
>
> -            size = len(delta)
> -            start = bundle.tell() - size
> -
>              link = linkmapper(cs)
>              if node in self.nodemap:
>                  # this can happen if two branches make the same change
> @@ -59,13 +57,18 @@
>              for p in (p1, p2):
>                  if p not in self.nodemap:
>                      raise error.LookupError(p, self.indexfile,
> -                                            _("unknown parent"))
> +                                            _('unknown parent'))
>
>              if deltabase not in self.nodemap:
>                  raise LookupError(deltabase, self.indexfile,
>                                    _('unknown delta base'))
>
>              baserev = self.rev(deltabase)
> +            chain = node
> +
> +            size = len(delta)
> +            start = bundle.tell() - size
> +
>              # start, size, full unc. size, base (unused), link, p1, p2,
> node
>              e = (revlog.offset_type(start, 0), size, -1, -1, link,
>                   self.rev(p1), self.rev(p2), node)
> @@ -73,7 +76,6 @@
>              self.index.insert(-1, e)
>              self.nodemap[node] = n
>              self.bundlerevs.add(n)
> -            chain = node
>              n += 1
>
>      def _chunk(self, rev):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Mads Kiilerich - Jan. 16, 2013, 11:51 p.m.
Benoit Boissinot wrote, On 01/16/2013 09:32 PM:
> LGTM

Thanks for the quick review.

Series pushed to crew before spotting Matt's mail.

... except this patch. There is really no reason to make the duplication 
obvious without also removing it.
'
/Mads

>
>
> On Wed, Jan 16, 2013 at 8:57 PM, Mads Kiilerich <mads@kiilerich.com 
> <mailto:mads@kiilerich.com>> wrote:
>
>     # HG changeset patch
>     # User Mads Kiilerich <madski@unity3d.com <mailto:madski@unity3d.com>>
>     # Date 1358365301 -3600
>     # Node ID 52c0ec0dbbe5cae4daeb33d0a3591e6240e9054e
>     # Parent  61031b4e9ac4ab381814398489771f53f0ca9751
>     bundlerepo: make __init__ more like revlog.addgroup
>
>     - just to prove there is duplicate code.
>

Patch

diff --git a/mercurial/bundlerepo.py b/mercurial/bundlerepo.py
--- a/mercurial/bundlerepo.py
+++ b/mercurial/bundlerepo.py
@@ -33,8 +33,9 @@ 
         self.bundle = bundle
         self.basemap = {} # mapping rev to delta base rev
         n = len(self)
+        self.bundlerevs = set() # used by 'bundle()' revset expression
+
         chain = None
-        self.bundlerevs = set() # used by 'bundle()' revset expression
         while True:
             chunkdata = bundle.deltachunk(chain)
             if not chunkdata:
@@ -46,9 +47,6 @@ 
             deltabase = chunkdata['deltabase']
             delta = chunkdata['delta']
 
-            size = len(delta)
-            start = bundle.tell() - size
-
             link = linkmapper(cs)
             if node in self.nodemap:
                 # this can happen if two branches make the same change
@@ -59,13 +57,18 @@ 
             for p in (p1, p2):
                 if p not in self.nodemap:
                     raise error.LookupError(p, self.indexfile,
-                                            _("unknown parent"))
+                                            _('unknown parent'))
 
             if deltabase not in self.nodemap:
                 raise LookupError(deltabase, self.indexfile,
                                   _('unknown delta base'))
 
             baserev = self.rev(deltabase)
+            chain = node
+
+            size = len(delta)
+            start = bundle.tell() - size
+
             # start, size, full unc. size, base (unused), link, p1, p2, node
             e = (revlog.offset_type(start, 0), size, -1, -1, link,
                  self.rev(p1), self.rev(p2), node)
@@ -73,7 +76,6 @@ 
             self.index.insert(-1, e)
             self.nodemap[node] = n
             self.bundlerevs.add(n)
-            chain = node
             n += 1
 
     def _chunk(self, rev):