| Submitter | phabricator |
|---|---|
| Date | April 14, 2020, 3:49 p.m. |
| Message ID | <differential-rev-PHID-DREV-r76ns2pakcmqlu5jh4i5-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/46092/ |
| State | Superseded |
| Headers | show |
Comments
pulkit added inline comments. INLINE COMMENTS > localrepo.py:941 > + epnm = ui.config(b'experimental', b'exp-persistent-nodemap.mode') > + options[b'exp-persistent-nodemap.mode'] = epnm > if ui.configbool(b'devel', b'persistent-nodemap'): nit: we can directly assign the value instead of temp variable > nodemap.py:148 > + if not can_incremental: > + msg = _("persistent nodemap in strict mode without efficient method") > + if mode == b'warn': needs a `b''` prefix REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8414/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8414 To: marmoute, #hg-reviewers Cc: pulkit, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > configitems.py:684 > coreconfigitem( > + b'experimental', b'exp-persistent-nodemap.mode', default=b'compat', > +) another minor nit: since the config section is `experimental`, we can skip the `exp-` prefix. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8414/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8414 To: marmoute, #hg-reviewers Cc: pulkit, mercurial-devel
marmoute added inline comments. INLINE COMMENTS > pulkit wrote in configitems.py:684 > another minor nit: since the config section is `experimental`, we can skip the `exp-` prefix. This matcht he pattern used above. In addition the `exp-` prefix make sense here` the feature is (was) in a state were any actual usage is out of question. Using a specific prefix for this prevent people using older version to step into a non-working experimental feature when t hey try to use an experimental feature that works fine in a later version. > pulkit wrote in localrepo.py:941 > nit: we can directly assign the value instead of temp variable Then the line is too long an need wrapping. I would rather have the small tmp variable than complex line wrapping here. > pulkit wrote in nodemap.py:148 > needs a `b''` prefix good catch, thanks REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D8414/new/ REVISION DETAIL https://phab.mercurial-scm.org/D8414 To: marmoute, #hg-reviewers Cc: pulkit, mercurial-devel
Patch
diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t --- a/tests/test-persistent-nodemap.t +++ b/tests/test-persistent-nodemap.t @@ -10,7 +10,9 @@ > [devel] > persistent-nodemap=yes > EOF - $ hg debugbuilddag .+5000 --new-file + $ hg debugbuilddag .+5000 --new-file --config "experimental.exp-persistent-nodemap.mode=warn" + persistent nodemap in strict mode without efficient method (no-rust no-pure !) + persistent nodemap in strict mode without efficient method (no-rust no-pure !) $ hg debugnodemap --metadata uid: ???????????????? (glob) tip-rev: 5000 diff --git a/mercurial/revlogutils/nodemap.py b/mercurial/revlogutils/nodemap.py --- a/mercurial/revlogutils/nodemap.py +++ b/mercurial/revlogutils/nodemap.py @@ -13,6 +13,8 @@ import re import struct +from ..i18n import _ + from .. import ( error, node as nodemod, @@ -105,6 +107,9 @@ def addabort(self, *args, **kwargs): pass + def _report(self, *args): + pass + def update_persistent_nodemap(revlog): """update the persistent nodemap right now @@ -138,6 +143,11 @@ ondisk_docket = revlog._nodemap_docket feed_data = util.safehasattr(revlog.index, "update_nodemap_data") use_mmap = revlog.opener.options.get(b"exp-persistent-nodemap.mmap") + mode = revlog.opener.options.get(b"exp-persistent-nodemap.mode") + if not can_incremental: + msg = _("persistent nodemap in strict mode without efficient method") + if mode == b'warn': + tr._report(b"%s\n" % msg) data = None # first attemp an incremental update of the data diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py --- a/mercurial/localrepo.py +++ b/mercurial/localrepo.py @@ -937,6 +937,8 @@ options[b'exp-persistent-nodemap'] = True if ui.configbool(b'experimental', b'exp-persistent-nodemap.mmap'): options[b'exp-persistent-nodemap.mmap'] = True + epnm = ui.config(b'experimental', b'exp-persistent-nodemap.mode') + options[b'exp-persistent-nodemap.mode'] = epnm if ui.configbool(b'devel', b'persistent-nodemap'): options[b'devel-force-nodemap'] = True diff --git a/mercurial/configitems.py b/mercurial/configitems.py --- a/mercurial/configitems.py +++ b/mercurial/configitems.py @@ -681,6 +681,9 @@ b'experimental', b'exp-persistent-nodemap.mmap', default=True, ) coreconfigitem( + b'experimental', b'exp-persistent-nodemap.mode', default=b'compat', +) +coreconfigitem( b'experimental', b'server.filesdata.recommended-batch-size', default=50000, ) coreconfigitem(