Patchwork [01,of,11] bundle2: move function building obsmarker-part in the bundle2 module

login
register
mail settings
Submitter Pierre-Yves David
Date May 28, 2017, 4:32 p.m.
Message ID <b513db11cf13c11be714.1495989122@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/20986/
State Accepted
Headers show

Comments

Pierre-Yves David - May 28, 2017, 4:32 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1494064548 -7200
#      Sat May 06 11:55:48 2017 +0200
# Node ID b513db11cf13c11be714bfcd9ba8f6cf2d872c31
# Parent  f2116efd2c3abae0f99e2f610c2ff2feb237b628
# EXP-Topic obsstrip
# Available At https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
#              hg pull https://www.mercurial-scm.org/repo/users/marmoute/mercurial/ -r b513db11cf13
bundle2: move function building obsmarker-part in the bundle2 module

We move it next to similar part building functions. We will need it for the
"writenewbundle" logic. This will allow us to easily include obsmarkers in
on-disk bundle, a necessary step before having `hg strip` also operate on
markers.

(Yes, the bundle2 module was already too large, but there any many
interdependencies between its components so it is non-trivial to split, this is
a quest for another adventure.)
Gregory Szorc - May 28, 2017, 6:53 p.m.
On Sun, May 28, 2017 at 9:32 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1494064548 -7200
> #      Sat May 06 11:55:48 2017 +0200
> # Node ID b513db11cf13c11be714bfcd9ba8f6cf2d872c31
> # Parent  f2116efd2c3abae0f99e2f610c2ff2feb237b628
> # EXP-Topic obsstrip
> # Available At https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/
> #              hg pull https://www.mercurial-scm.org/
> repo/users/marmoute/mercurial/ -r b513db11cf13
> bundle2: move function building obsmarker-part in the bundle2 module
>

I queued parts 1 and 2.

I refactored the logic in buildobsmarkerspart() in part 1 to use early
return and avoid excessive indent.

I fixed some English in part 2.


>
> We move it next to similar part building functions. We will need it for the
> "writenewbundle" logic. This will allow us to easily include obsmarkers in
> on-disk bundle, a necessary step before having `hg strip` also operate on
> markers.
>
> (Yes, the bundle2 module was already too large, but there any many
> interdependencies between its components so it is non-trivial to split,
> this is
> a quest for another adventure.)
>
> diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
> --- a/mercurial/bundle2.py
> +++ b/mercurial/bundle2.py
> @@ -1401,6 +1401,21 @@ def addparttagsfnodescache(repo, bundler
>      if chunks:
>          bundler.newpart('hgtagsfnodes', data=''.join(chunks))
>
> +def buildobsmarkerspart(bundler, markers):
> +    """add an obsmarker part to the bundler with <markers>
> +
> +    No part is created if markers is empty.
> +    Raises ValueError if the bundler doesn't support any known obsmarker
> format.
> +    """
> +    if markers:
> +        remoteversions = obsmarkersversion(bundler.capabilities)
> +        version = obsolete.commonversion(remoteversions)
> +        if version is None:
> +            raise ValueError('bundler does not support common obsmarker
> format')
> +        stream = obsolete.encodemarkers(markers, True, version=version)
> +        return bundler.newpart('obsmarkers', data=stream)
> +    return None
> +
>  def writebundle(ui, cg, filename, bundletype, vfs=None, compression=None,
>                  compopts=None):
>      """Write a bundle file and return its filename.
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -248,21 +248,6 @@ def getbundlespec(ui, fh):
>      else:
>          raise error.Abort(_('unknown bundle type: %s') % b)
>
> -def buildobsmarkerspart(bundler, markers):
> -    """add an obsmarker part to the bundler with <markers>
> -
> -    No part is created if markers is empty.
> -    Raises ValueError if the bundler doesn't support any known obsmarker
> format.
> -    """
> -    if markers:
> -        remoteversions = bundle2.obsmarkersversion(bundler.capabilities)
> -        version = obsolete.commonversion(remoteversions)
> -        if version is None:
> -            raise ValueError('bundler does not support common obsmarker
> format')
> -        stream = obsolete.encodemarkers(markers, True, version=version)
> -        return bundler.newpart('obsmarkers', data=stream)
> -    return None
> -
>  def _computeoutgoing(repo, heads, common):
>      """Computes which revs are outgoing given a set of common
>      and a set of heads.
> @@ -822,7 +807,7 @@ def _pushb2obsmarkers(pushop, bundler):
>      pushop.stepsdone.add('obsmarkers')
>      if pushop.outobsmarkers:
>          markers = sorted(pushop.outobsmarkers)
> -        buildobsmarkerspart(bundler, markers)
> +        bundle2.buildobsmarkerspart(bundler, markers)
>
>  @b2partsgenerator('bookmarks')
>  def _pushb2bookmarks(pushop, bundler):
> @@ -1648,7 +1633,7 @@ def _getbundleobsmarkerpart(bundler, rep
>          subset = [c.node() for c in repo.set('::%ln', heads)]
>          markers = repo.obsstore.relevantmarkers(subset)
>          markers = sorted(markers)
> -        buildobsmarkerspart(bundler, markers)
> +        bundle2.buildobsmarkerspart(bundler, markers)
>
>  @getbundle2partsgenerator('hgtagsfnodes')
>  def _getbundletagsfnodes(bundler, repo, source, bundlecaps=None,
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - May 29, 2017, 6:45 a.m.
On 05/28/2017 08:53 PM, Gregory Szorc wrote:
> On Sun, May 28, 2017 at 9:32 AM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     # HG changeset patch
>     # User Pierre-Yves David <pierre-yves.david@octobus.net
>     <mailto:pierre-yves.david@octobus.net>>
>     # Date 1494064548 -7200
>     #      Sat May 06 11:55:48 2017 +0200
>     # Node ID b513db11cf13c11be714bfcd9ba8f6cf2d872c31
>     # Parent  f2116efd2c3abae0f99e2f610c2ff2feb237b628
>     # EXP-Topic obsstrip
>     # Available At
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/>
>     #              hg pull
>     https://www.mercurial-scm.org/repo/users/marmoute/mercurial/
>     <https://www.mercurial-scm.org/repo/users/marmoute/mercurial/> -r
>     b513db11cf13
>     bundle2: move function building obsmarker-part in the bundle2 module
>
>
> I queued parts 1 and 2.
>
> I refactored the logic in buildobsmarkerspart() in part 1 to use early
> return and avoid excessive indent.

Thanks for the queuing.

Patch 1 used to be a pure code movement patch, it is now both a code 
movement and a refactoring. That makes the code changes harder to track.
I would prefer to see such refactoring done in a follow up patch.

Cheers,

Patch

diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1401,6 +1401,21 @@  def addparttagsfnodescache(repo, bundler
     if chunks:
         bundler.newpart('hgtagsfnodes', data=''.join(chunks))
 
+def buildobsmarkerspart(bundler, markers):
+    """add an obsmarker part to the bundler with <markers>
+
+    No part is created if markers is empty.
+    Raises ValueError if the bundler doesn't support any known obsmarker format.
+    """
+    if markers:
+        remoteversions = obsmarkersversion(bundler.capabilities)
+        version = obsolete.commonversion(remoteversions)
+        if version is None:
+            raise ValueError('bundler does not support common obsmarker format')
+        stream = obsolete.encodemarkers(markers, True, version=version)
+        return bundler.newpart('obsmarkers', data=stream)
+    return None
+
 def writebundle(ui, cg, filename, bundletype, vfs=None, compression=None,
                 compopts=None):
     """Write a bundle file and return its filename.
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -248,21 +248,6 @@  def getbundlespec(ui, fh):
     else:
         raise error.Abort(_('unknown bundle type: %s') % b)
 
-def buildobsmarkerspart(bundler, markers):
-    """add an obsmarker part to the bundler with <markers>
-
-    No part is created if markers is empty.
-    Raises ValueError if the bundler doesn't support any known obsmarker format.
-    """
-    if markers:
-        remoteversions = bundle2.obsmarkersversion(bundler.capabilities)
-        version = obsolete.commonversion(remoteversions)
-        if version is None:
-            raise ValueError('bundler does not support common obsmarker format')
-        stream = obsolete.encodemarkers(markers, True, version=version)
-        return bundler.newpart('obsmarkers', data=stream)
-    return None
-
 def _computeoutgoing(repo, heads, common):
     """Computes which revs are outgoing given a set of common
     and a set of heads.
@@ -822,7 +807,7 @@  def _pushb2obsmarkers(pushop, bundler):
     pushop.stepsdone.add('obsmarkers')
     if pushop.outobsmarkers:
         markers = sorted(pushop.outobsmarkers)
-        buildobsmarkerspart(bundler, markers)
+        bundle2.buildobsmarkerspart(bundler, markers)
 
 @b2partsgenerator('bookmarks')
 def _pushb2bookmarks(pushop, bundler):
@@ -1648,7 +1633,7 @@  def _getbundleobsmarkerpart(bundler, rep
         subset = [c.node() for c in repo.set('::%ln', heads)]
         markers = repo.obsstore.relevantmarkers(subset)
         markers = sorted(markers)
-        buildobsmarkerspart(bundler, markers)
+        bundle2.buildobsmarkerspart(bundler, markers)
 
 @getbundle2partsgenerator('hgtagsfnodes')
 def _getbundletagsfnodes(bundler, repo, source, bundlecaps=None,