Re: [PATCH 3 of 4] storage: also use `deltamode argument` for ifiledata

2018-10-18 Thread Boris FELD 

On 16/10/2018 18:41, Gregory Szorc wrote:
> On Tue, Oct 16, 2018 at 10:38 AM Boris Feld  > wrote:
>
> # HG changeset patch
> # User Boris Feld  >
> # Date 1539120395 -7200
> #      Tue Oct 09 23:26:35 2018 +0200
> # Node ID c835d3020e536e8ef368223d628b9e0c6d0c8251
> # Parent  a075ac3487d6d266ec5f2dbd6901da21c38d4ed9
> # EXP-Topic slim-bundle
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull
> https://bitbucket.org/octobus/mercurial-devel/ -r c835d3020e53
> storage: also use `deltamode argument` for ifiledata
>
> Now that lower level uses such argument, we can propagate the
> change to higher
> layers. At some of the higher level, we use `None` as the default
> value to
> simplify imports and argument forwarding  (instead of
> `storageutil.DELTAMODE_STD`). Strictly speaking, we could import
> `storageutil`
> everywhere, however, it did not seem to help readability.
>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -29,6 +29,10 @@ from . import (
>      util,
>  )
>
> +from .utils import (
> +    storageutil,
> +)
> +
>  _CHANGEGROUPV1_DELTA_HEADER = struct.Struct("20s20s20s20s")
>  _CHANGEGROUPV2_DELTA_HEADER = struct.Struct("20s20s20s20s20s")
>  _CHANGEGROUPV3_DELTA_HEADER = struct.Struct(">20s20s20s20s20sH")
> @@ -697,12 +701,16 @@ def deltagroup(repo, store, nodes, ischa
>          progress = repo.ui.makeprogress(topic, unit=_('chunks'),
>                                          total=len(nodes))
>
> +    deltamode = storageutil.DELTAMODE_STD
> +    if forcedeltaparentprev:
> +        deltamode = storageutil.DELTAMODE_PREV
> +
>      revisions = store.emitrevisions(
>          nodes,
>          nodesorder=nodesorder,
>          revisiondata=True,
>          assumehaveparentrevisions=not ellipses,
> -        deltaprevious=forcedeltaparentprev)
> +        deltamode=deltamode)
>
>      for i, revision in enumerate(revisions):
>          if progress:
> diff --git a/mercurial/filelog.py b/mercurial/filelog.py
> --- a/mercurial/filelog.py
> +++ b/mercurial/filelog.py
> @@ -77,11 +77,11 @@ class filelog(object):
>
>      def emitrevisions(self, nodes, nodesorder=None,
>                        revisiondata=False,
> assumehaveparentrevisions=False,
> -                      deltaprevious=False):
> +                      deltamode=None):
>          return self._revlog.emitrevisions(
>              nodes, nodesorder=nodesorder, revisiondata=revisiondata,
>              assumehaveparentrevisions=assumehaveparentrevisions,
> -            deltaprevious=deltaprevious)
> +            deltamode=deltamode)
>
>      def addrevision(self, revisiondata, transaction, linkrev, p1, p2,
>                      node=None, flags=revlog.REVIDX_DEFAULT_FLAGS,
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -1575,11 +1575,11 @@ class manifestrevlog(object):
>
>      def emitrevisions(self, nodes, nodesorder=None,
>                        revisiondata=False,
> assumehaveparentrevisions=False,
> -                      deltaprevious=False):
> +                      deltamode=None):
>          return self._revlog.emitrevisions(
>              nodes, nodesorder=nodesorder, revisiondata=revisiondata,
>              assumehaveparentrevisions=assumehaveparentrevisions,
> -            deltaprevious=deltaprevious)
> +            deltamode=deltamode)
>
>      def addgroup(self, deltas, linkmapper, transaction,
> addrevisioncb=None):
>          return self._revlog.addgroup(deltas, linkmapper, transaction,
> diff --git a/mercurial/repository.py b/mercurial/repository.py
> --- a/mercurial/repository.py
> +++ b/mercurial/repository.py
> @@ -602,7 +602,7 @@ class ifiledata(interfaceutil.Interface)
>                        nodesorder=None,
>                        revisiondata=False,
>                        assumehaveparentrevisions=False,
> -                      deltaprevious=False):
> +                      deltamode=None):
>          """Produce ``irevisiondelta`` for revisions.
>
>          Given an iterable of nodes, emits objects conforming to the
> @@ -645,10 +645,10 @@ class ifiledata(interfaceutil.Interface)
>          The ``linknode`` attribute on the returned
> ``irevisiondelta`` may not
>          be set and it is the caller's responsibility to resolve
> it, if needed.
>
> -        If ``deltaprevious`` is True and revision data is
> requested, all
> -        revision data

Re: [PATCH 3 of 4] storage: also use `deltamode argument` for ifiledata

2018-10-16 Thread Gregory Szorc 
On Tue, Oct 16, 2018 at 10:38 AM Boris Feld  wrote:

> # HG changeset patch
> # User Boris Feld 
> # Date 1539120395 -7200
> #  Tue Oct 09 23:26:35 2018 +0200
> # Node ID c835d3020e536e8ef368223d628b9e0c6d0c8251
> # Parent  a075ac3487d6d266ec5f2dbd6901da21c38d4ed9
> # EXP-Topic slim-bundle
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #  hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> c835d3020e53
> storage: also use `deltamode argument` for ifiledata
>
> Now that lower level uses such argument, we can propagate the change to
> higher
> layers. At some of the higher level, we use `None` as the default value to
> simplify imports and argument forwarding  (instead of
> `storageutil.DELTAMODE_STD`). Strictly speaking, we could import
> `storageutil`
> everywhere, however, it did not seem to help readability.
>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -29,6 +29,10 @@ from . import (
>  util,
>  )
>
> +from .utils import (
> +storageutil,
> +)
> +
>  _CHANGEGROUPV1_DELTA_HEADER = struct.Struct("20s20s20s20s")
>  _CHANGEGROUPV2_DELTA_HEADER = struct.Struct("20s20s20s20s20s")
>  _CHANGEGROUPV3_DELTA_HEADER = struct.Struct(">20s20s20s20s20sH")
> @@ -697,12 +701,16 @@ def deltagroup(repo, store, nodes, ischa
>  progress = repo.ui.makeprogress(topic, unit=_('chunks'),
>  total=len(nodes))
>
> +deltamode = storageutil.DELTAMODE_STD
> +if forcedeltaparentprev:
> +deltamode = storageutil.DELTAMODE_PREV
> +
>  revisions = store.emitrevisions(
>  nodes,
>  nodesorder=nodesorder,
>  revisiondata=True,
>  assumehaveparentrevisions=not ellipses,
> -deltaprevious=forcedeltaparentprev)
> +deltamode=deltamode)
>
>  for i, revision in enumerate(revisions):
>  if progress:
> diff --git a/mercurial/filelog.py b/mercurial/filelog.py
> --- a/mercurial/filelog.py
> +++ b/mercurial/filelog.py
> @@ -77,11 +77,11 @@ class filelog(object):
>
>  def emitrevisions(self, nodes, nodesorder=None,
>revisiondata=False, assumehaveparentrevisions=False,
> -  deltaprevious=False):
> +  deltamode=None):
>  return self._revlog.emitrevisions(
>  nodes, nodesorder=nodesorder, revisiondata=revisiondata,
>  assumehaveparentrevisions=assumehaveparentrevisions,
> -deltaprevious=deltaprevious)
> +deltamode=deltamode)
>
>  def addrevision(self, revisiondata, transaction, linkrev, p1, p2,
>  node=None, flags=revlog.REVIDX_DEFAULT_FLAGS,
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -1575,11 +1575,11 @@ class manifestrevlog(object):
>
>  def emitrevisions(self, nodes, nodesorder=None,
>revisiondata=False, assumehaveparentrevisions=False,
> -  deltaprevious=False):
> +  deltamode=None):
>  return self._revlog.emitrevisions(
>  nodes, nodesorder=nodesorder, revisiondata=revisiondata,
>  assumehaveparentrevisions=assumehaveparentrevisions,
> -deltaprevious=deltaprevious)
> +deltamode=deltamode)
>
>  def addgroup(self, deltas, linkmapper, transaction,
> addrevisioncb=None):
>  return self._revlog.addgroup(deltas, linkmapper, transaction,
> diff --git a/mercurial/repository.py b/mercurial/repository.py
> --- a/mercurial/repository.py
> +++ b/mercurial/repository.py
> @@ -602,7 +602,7 @@ class ifiledata(interfaceutil.Interface)
>nodesorder=None,
>revisiondata=False,
>assumehaveparentrevisions=False,
> -  deltaprevious=False):
> +  deltamode=None):
>  """Produce ``irevisiondelta`` for revisions.
>
>  Given an iterable of nodes, emits objects conforming to the
> @@ -645,10 +645,10 @@ class ifiledata(interfaceutil.Interface)
>  The ``linknode`` attribute on the returned ``irevisiondelta`` may
> not
>  be set and it is the caller's responsibility to resolve it, if
> needed.
>
> -If ``deltaprevious`` is True and revision data is requested, all
> -revision data should be emitted as deltas against the revision
> -emitted just prior. The initial revision should be a delta against
> -its 1st parent.
> +If ``deltamode`` is storageutil.DELTAMODE_PREV and revision data
> is
> +requested, all revision data should be emitted as deltas against
> the
> +revision emitted just prior. The initial revision should be a
> delta
> +against its 1st parent.
>  """
>

I'm not thrilled about referencing a constant not in repository.py here: a

[PATCH 3 of 4] storage: also use `deltamode argument` for ifiledata

2018-10-16 Thread Boris Feld 
# HG changeset patch
# User Boris Feld 
# Date 1539120395 -7200
#  Tue Oct 09 23:26:35 2018 +0200
# Node ID c835d3020e536e8ef368223d628b9e0c6d0c8251
# Parent  a075ac3487d6d266ec5f2dbd6901da21c38d4ed9
# EXP-Topic slim-bundle
# Available At https://bitbucket.org/octobus/mercurial-devel/
#  hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 
c835d3020e53
storage: also use `deltamode argument` for ifiledata

Now that lower level uses such argument, we can propagate the change to higher
layers. At some of the higher level, we use `None` as the default value to
simplify imports and argument forwarding  (instead of
`storageutil.DELTAMODE_STD`). Strictly speaking, we could import `storageutil`
everywhere, however, it did not seem to help readability.

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -29,6 +29,10 @@ from . import (
 util,
 )
 
+from .utils import (
+storageutil,
+)
+
 _CHANGEGROUPV1_DELTA_HEADER = struct.Struct("20s20s20s20s")
 _CHANGEGROUPV2_DELTA_HEADER = struct.Struct("20s20s20s20s20s")
 _CHANGEGROUPV3_DELTA_HEADER = struct.Struct(">20s20s20s20s20sH")
@@ -697,12 +701,16 @@ def deltagroup(repo, store, nodes, ischa
 progress = repo.ui.makeprogress(topic, unit=_('chunks'),
 total=len(nodes))
 
+deltamode = storageutil.DELTAMODE_STD
+if forcedeltaparentprev:
+deltamode = storageutil.DELTAMODE_PREV
+
 revisions = store.emitrevisions(
 nodes,
 nodesorder=nodesorder,
 revisiondata=True,
 assumehaveparentrevisions=not ellipses,
-deltaprevious=forcedeltaparentprev)
+deltamode=deltamode)
 
 for i, revision in enumerate(revisions):
 if progress:
diff --git a/mercurial/filelog.py b/mercurial/filelog.py
--- a/mercurial/filelog.py
+++ b/mercurial/filelog.py
@@ -77,11 +77,11 @@ class filelog(object):
 
 def emitrevisions(self, nodes, nodesorder=None,
   revisiondata=False, assumehaveparentrevisions=False,
-  deltaprevious=False):
+  deltamode=None):
 return self._revlog.emitrevisions(
 nodes, nodesorder=nodesorder, revisiondata=revisiondata,
 assumehaveparentrevisions=assumehaveparentrevisions,
-deltaprevious=deltaprevious)
+deltamode=deltamode)
 
 def addrevision(self, revisiondata, transaction, linkrev, p1, p2,
 node=None, flags=revlog.REVIDX_DEFAULT_FLAGS,
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1575,11 +1575,11 @@ class manifestrevlog(object):
 
 def emitrevisions(self, nodes, nodesorder=None,
   revisiondata=False, assumehaveparentrevisions=False,
-  deltaprevious=False):
+  deltamode=None):
 return self._revlog.emitrevisions(
 nodes, nodesorder=nodesorder, revisiondata=revisiondata,
 assumehaveparentrevisions=assumehaveparentrevisions,
-deltaprevious=deltaprevious)
+deltamode=deltamode)
 
 def addgroup(self, deltas, linkmapper, transaction, addrevisioncb=None):
 return self._revlog.addgroup(deltas, linkmapper, transaction,
diff --git a/mercurial/repository.py b/mercurial/repository.py
--- a/mercurial/repository.py
+++ b/mercurial/repository.py
@@ -602,7 +602,7 @@ class ifiledata(interfaceutil.Interface)
   nodesorder=None,
   revisiondata=False,
   assumehaveparentrevisions=False,
-  deltaprevious=False):
+  deltamode=None):
 """Produce ``irevisiondelta`` for revisions.
 
 Given an iterable of nodes, emits objects conforming to the
@@ -645,10 +645,10 @@ class ifiledata(interfaceutil.Interface)
 The ``linknode`` attribute on the returned ``irevisiondelta`` may not
 be set and it is the caller's responsibility to resolve it, if needed.
 
-If ``deltaprevious`` is True and revision data is requested, all
-revision data should be emitted as deltas against the revision
-emitted just prior. The initial revision should be a delta against
-its 1st parent.
+If ``deltamode`` is storageutil.DELTAMODE_PREV and revision data is
+requested, all revision data should be emitted as deltas against the
+revision emitted just prior. The initial revision should be a delta
+against its 1st parent.
 """
 
 class ifilemutation(interfaceutil.Interface):
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -2221,7 +2221,8 @@ class revlog(object):
 return res
 
 def emitrevisions(self, nodes, nodesorder=None, revisiondata=False,
-