On 16/10/2018 18:41, Gregory Szorc wrote: > On Tue, Oct 16, 2018 at 10:38 AM Boris Feld <boris.f...@octobus.net > <mailto:boris.f...@octobus.net>> wrote: > > # HG changeset patch > # User Boris Feld <boris.f...@octobus.net > <mailto:boris.f...@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. Okay, make sense, I'm sending a V2 with this updated. > > > > 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 I'm not sure what you are trying to address here. Could you elaborate? > > > 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@mercurial-scm.org > <mailto:Mercurial-devel@mercurial-scm.org> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
