[PATCH 3 of 4] storage: also use `deltamode argument` for ifiledata
Gregory Szorc
gregory.szorc at gmail.com
Tue Oct 16 12:41:58 EDT 2018
On Tue, Oct 16, 2018 at 10:38 AM Boris Feld <boris.feld at octobus.net> wrote:
> # HG changeset patch
> # User Boris Feld <boris.feld at octobus.net>
> # 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
point of repository.py is to have global constants defined in it and not in
other modules. See e.g. the changegroup flag constants.
>
> 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,
> - assumehaveparentrevisions=False,
> deltaprevious=False):
> + assumehaveparentrevisions=False,
> + deltamode=storageutil.DELTAMODE_STD):
> if nodesorder not in ('nodes', 'storage', None):
> raise error.ProgrammingError('unhandled value for nodesorder:
> %s' %
> nodesorder)
> @@ -2229,10 +2230,11 @@ class revlog(object):
> if nodesorder is None and not self._generaldelta:
> nodesorder = 'storage'
>
> - deltamode = storageutil.DELTAMODE_STD
> - if deltaprevious:
> - deltamode = storageutil.DELTAMODE_PREV
> - elif not self._storedeltachains:
> + if deltamode is None:
> + deltamode = storageutil.DELTAMODE_STD
> +
> + if (not deltamode == storageutil.DELTAMODE_PREV
> + and not self._storedeltachains):
>
"not X == Y" should be rewritten as "X != Y" for clarity.
I also think the logic in this block is a bit fragile. I think something
like the following is much clearer:
if not self._storedeltachains and deltamode != storageutil.DELTAMODE_PREV:
deltamode = storageutil.DELTAMODE_FULL
*or*
if deltamode == storageutil.DELTAMODE_PREV:
pass
elif not self._storedeltachains:
deltamode = storageutil.DELTAMODE_FULL
> deltamode = storageutil.DELTAMODE_FULL
>
> return storageutil.emitrevisions(
> diff --git a/mercurial/testing/storage.py b/mercurial/testing/storage.py
> --- a/mercurial/testing/storage.py
> +++ b/mercurial/testing/storage.py
> @@ -727,7 +727,8 @@ class ifiledatatests(basetestcase):
>
> # forceprevious=True forces a delta against the previous revision.
> # Special case for initial revision.
> - gen = f.emitrevisions([node0], revisiondata=True,
> deltaprevious=True)
> + gen = f.emitrevisions([node0], revisiondata=True,
> + deltamode=storageutil.DELTAMODE_PREV)
>
> rev = next(gen)
> self.assertEqual(rev.node, node0)
> @@ -744,7 +745,7 @@ class ifiledatatests(basetestcase):
> next(gen)
>
> gen = f.emitrevisions([node0, node2], revisiondata=True,
> - deltaprevious=True)
> + deltamode=storageutil.DELTAMODE_PREV)
>
> rev = next(gen)
> self.assertEqual(rev.node, node0)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20181016/0a274125/attachment.html>
More information about the Mercurial-devel
mailing list