D4571: localrepo: move requirements reasonability testing to own function

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Thu Sep 13 16:30:59 UTC 2018


indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Just because we know how to handle each listed requirement doesn't
  mean that set of requirements is reasonable.
  
  This commit introduces an extension-wrappable function to validate
  that a set of requirements makes sense.
  
  We could combine this with ensurerequirementsrecognized(). But I think
  having a line between basic membership testing and compatibility
  checking is more powerful as it will help differentiate between
  missing support and buggy behavior.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/localrepo.py
  mercurial/statichttprepo.py

CHANGE DETAILS

diff --git a/mercurial/statichttprepo.py b/mercurial/statichttprepo.py
--- a/mercurial/statichttprepo.py
+++ b/mercurial/statichttprepo.py
@@ -176,6 +176,7 @@
         supportedrequirements = localrepo.gathersupportedrequirements(ui)
         localrepo.ensurerequirementsrecognized(requirements,
                                                supportedrequirements)
+        localrepo.ensurerequirementscompatible(ui, requirements)
 
         # setup store
         self.store = store.store(requirements, self.path, vfsclass)
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -431,8 +431,13 @@
         extensions.loadall(ui)
 
     supportedrequirements = gathersupportedrequirements(ui)
+
+    # We first validate the requirements are known.
     ensurerequirementsrecognized(requirements, supportedrequirements)
 
+    # Then we validate that the known set is reasonable to use together.
+    ensurerequirementscompatible(ui, requirements)
+
     # At this point, we know we should be capable of opening the repository.
     # Now get on with doing that.
 
@@ -494,6 +499,24 @@
             hint=_(b'see https://mercurial-scm.org/wiki/MissingRequirement '
                    b'for more information'))
 
+def ensurerequirementscompatible(ui, requirements):
+    """Validates that a set of recognized requirements is mutually compatible.
+
+    Some requirements may not be compatible with others or require
+    config options that aren't enabled. This function is called during
+    repository opening to ensure that the set of requirements needed
+    to open a repository is sane and compatible with config options.
+
+    Extensions can monkeypatch this function to perform additional
+    checking.
+
+    ``error.RepoError`` should be raised on failure.
+    """
+    if b'exp-sparse' in requirements and not sparse.enabled:
+        raise error.RepoError(_(b'repository is using sparse feature but '
+                                b'sparse is not enabled; enable the '
+                                b'"sparse" extensions to access'))
+
 @interfaceutil.implementer(repository.completelocalrepository)
 class localrepository(object):
 
@@ -625,11 +648,6 @@
             if inst.errno != errno.ENOENT:
                 raise
 
-        if 'exp-sparse' in self.requirements and not sparse.enabled:
-            raise error.RepoError(_('repository is using sparse feature but '
-                                    'sparse is not enabled; enable the '
-                                    '"sparse" extensions to access'))
-
         self.store = store.store(
             self.requirements, self.sharedpath,
             lambda base: vfsmod.vfs(base, cacheaudited=True))



To: indygreg, #hg-reviewers
Cc: mercurial-devel


More information about the Mercurial-devel mailing list