Patchwork D8417: nodemap: gate the feature behind a new requirement

login
register
mail settings
Submitter phabricator
Date April 14, 2020, 3:50 p.m.
Message ID <differential-rev-PHID-DREV-oskkw6txbscvuzy3r5tu-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46093/
State Superseded
Headers show

Comments

phabricator - April 14, 2020, 3:50 p.m.
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Now that the feature is working smoothly, a question was still open, should we
  gate the feature behind a new requirement or just treat it as a cache to be
  warmed by those who can and ignored by other.
  
  The advantage of using the cache approach is a transparent upgrade/downgrade
  story, making the feature easier to move to. However having out of date cache
  can come with a significant performance hit for process who expect an up to
  date cache but found none. In this case the file needs to be stored under
  `.hg/cache`.
  
  The "requirement" approach guarantee that the persistent nodemap is up to date.
  However, it comes with a less flexible activation story since an explicite
  upgrade is required. In this case the file can be stored in `.hg/store`.
  
  This wiki page is relevant to this questions:
  
    https://www.mercurial-scm.org/wiki/ComputedIndexPlan
  
  So which one should we take? Another element came into plan, the persistent
  nodemap use the `add` method of the transaction, it is used to keep track of a
  file content before a transaction in case we need to rollback it back. It turns
  out that the `transaction.add` API does not support file stored anywhere than
  `.hg/store`. Making it support file stored elsewhere is possible, require a
  change in on disk transaction format. Updating on disk file requires…
  introducing a new requirements.
  
  As a result, we pick the second option "gating the persistent nodemap behind a
  new requirements".

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8417

AFFECTED FILES
  mercurial/localrepo.py
  tests/test-persistent-nodemap.t

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: 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
@@ -2,14 +2,14 @@ 
 Test the persistent on-disk nodemap
 ===================================
 
-  $ hg init test-repo
-  $ cd test-repo
-  $ cat << EOF >> .hg/hgrc
+  $ cat << EOF >> $HGRCPATH
   > [experimental]
   > exp-persistent-nodemap=yes
   > [devel]
   > persistent-nodemap=yes
   > EOF
+  $ hg init test-repo
+  $ cd test-repo
   $ 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 !)
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -445,6 +445,9 @@ 
 # copies related information in changeset's sidedata.
 COPIESSDC_REQUIREMENT = b'exp-copies-sidedata-changeset'
 
+# The repository use persistent nodemap for the changelog and the manifest.
+NODEMAP_REQUIREMENT = b'persistent-nodemap'
+
 # Functions receiving (ui, features) that extensions can register to impact
 # the ability to load repositories with custom requirements. Only
 # functions defined in loaded extensions are called.
@@ -933,7 +936,7 @@ 
 
     if ui.configbool(b'experimental', b'rust.index'):
         options[b'rust.index'] = True
-    if ui.configbool(b'experimental', b'exp-persistent-nodemap'):
+    if NODEMAP_REQUIREMENT in requirements:
         options[b'exp-persistent-nodemap'] = True
     if ui.configbool(b'experimental', b'exp-persistent-nodemap.mmap'):
         options[b'exp-persistent-nodemap.mmap'] = True
@@ -1023,6 +1026,7 @@ 
         REVLOGV2_REQUIREMENT,
         SIDEDATA_REQUIREMENT,
         SPARSEREVLOG_REQUIREMENT,
+        NODEMAP_REQUIREMENT,
         bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT,
     }
     _basesupported = supportedformats | {
@@ -3654,6 +3658,9 @@ 
     if ui.configbool(b'format', b'bookmarks-in-store'):
         requirements.add(bookmarks.BOOKMARKS_IN_STORE_REQUIREMENT)
 
+    if ui.configbool(b'experimental', b'exp-persistent-nodemap'):
+        requirements.add(NODEMAP_REQUIREMENT)
+
     return requirements