D4576: localrepo: extract resolving of opener options to standalone functions

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


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

REVISION SUMMARY
  Requirements and config options are converted into a dict which is
  available to the store vfs to consult. This is how storage options
  are communicated from the repo layer to the storage layer.
  
  Currently, we do that option resolution in a private method on the
  repo instance. And there is a single method doing that resolution.
  
  Opener options are logically specific to the storage backend they
  apply to. And, opener options may wish to influence how the repo
  object/type is constructed. So it makes sense to have more granular
  storage option resolution that occurs before the repo object is
  instantiated.
  
  This commit extracts the code for resolving opener options into new
  module-level functions. These functions are run before the repo
  instance is constructed.
  
  As part of the code move, we split the option resolution into
  generic and revlog-specific options. After this commit, we no longer
  add revlog-specific options to repos that don't have a revlog
  requirement.
  
  Some of these opener options and associated config options might make
  sense on alternate storage backends. We can always reuse config
  options and opener option names for other backends. But we shouldn't
  be passing opener options to storage backends that won't recognize
  them. I haven't done it here, but after this commit it should be
  possible for store backends to validate the set of opener options
  it receives.
  
  Because localrepository.openerreqs is no longer used after this commit,
  it has been removed.
  
  I'm not super thrilled about the code outside of localrepo that is
  adding requirements and updating opener options. We'll probably want
  to create a more formal API for that use case that constructs a new
  repo instance and poisons the old repo object. But this was a
  pre-existing issue and can be dealt with later. I have little doubt
  it will cause me troubles as I continue to refactor how repository
  objects are instantiated.
  
  .. api::
  
    ``localrepository.openerreqs`` has been removed. Override
    ``localrepo.resolvestorevfsoptions()`` to add custom opener options.
  
  .. api::
  
    ``localrepository._applyopenerreqs()`` has been removed. Use
    ``localrepo.resolvestorevfsoptions()`` to add custom opener options.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/localrepo.py
  mercurial/repository.py
  mercurial/streamclone.py

CHANGE DETAILS

diff --git a/mercurial/streamclone.py b/mercurial/streamclone.py
--- a/mercurial/streamclone.py
+++ b/mercurial/streamclone.py
@@ -114,6 +114,8 @@
     A legacy stream clone will not be performed if a bundle2 stream clone is
     supported.
     """
+    from . import localrepo
+
     supported, requirements = canperformstreamclone(pullop)
 
     if not supported:
@@ -166,7 +168,8 @@
         # requirements from the streamed-in repository
         repo.requirements = requirements | (
                 repo.requirements - repo.supportedformats)
-        repo._applyopenerreqs()
+        repo.svfs.options = localrepo.resolvestorevfsoptions(
+            repo.ui, repo.requirements)
         repo._writerequirements()
 
         if rbranchmap:
@@ -624,6 +627,8 @@
         progress.complete()
 
 def applybundlev2(repo, fp, filecount, filesize, requirements):
+    from . import localrepo
+
     missingreqs = [r for r in requirements if r not in repo.supported]
     if missingreqs:
         raise error.Abort(_('unable to apply stream clone: '
@@ -637,5 +642,6 @@
     # requirements from the streamed-in repository
     repo.requirements = set(requirements) | (
             repo.requirements - repo.supportedformats)
-    repo._applyopenerreqs()
+    repo.svfs.options = localrepo.resolvestorevfsoptions(
+        repo.ui, repo.requirements)
     repo._writerequirements()
diff --git a/mercurial/repository.py b/mercurial/repository.py
--- a/mercurial/repository.py
+++ b/mercurial/repository.py
@@ -1219,12 +1219,6 @@
         This is actually a class attribute and is shared among all instances.
         """)
 
-    openerreqs = interfaceutil.Attribute(
-        """Set of requirements that are passed to the opener.
-
-        This is actually a class attribute and is shared among all instances.
-        """)
-
     supported = interfaceutil.Attribute(
         """Set of requirements that this repo is capable of opening.""")
 
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -481,9 +481,11 @@
     # of them.
     store = makestore(requirements, storebasepath,
                       lambda base: vfsmod.vfs(base, cacheaudited=True))
-
     hgvfs.createmode = store.createmode
 
+    storevfs = store.vfs
+    storevfs.options = resolvestorevfsoptions(ui, requirements)
+
     # The cache vfs is used to manage cache files.
     cachevfs = vfsmod.vfs(cachepath, cacheaudited=True)
     cachevfs.createmode = store.createmode
@@ -578,6 +580,92 @@
 
     return storemod.basicstore(path, vfstype)
 
+def resolvestorevfsoptions(ui, requirements):
+    """Resolve the options to pass to the store vfs opener.
+
+    The returned dict is used to influence behavior of the storage layer.
+    """
+    options = {}
+
+    if b'treemanifest' in requirements:
+        options[b'treemanifest'] = True
+
+    # experimental config: format.manifestcachesize
+    manifestcachesize = ui.configint(b'format', b'manifestcachesize')
+    if manifestcachesize is not None:
+        options[b'manifestcachesize'] = manifestcachesize
+
+    # In the absence of another requirement superseding a revlog-related
+    # requirement, we have to assume the repo is using revlog version 0.
+    # This revlog format is super old and we don't bother trying to parse
+    # opener options for it because those options wouldn't do anything
+    # meaningful on such old repos.
+    if b'revlogv1' in requirements or REVLOGV2_REQUIREMENT in requirements:
+        options.update(resolverevlogstorevfsoptions(ui, requirements))
+
+    return options
+
+def resolverevlogstorevfsoptions(ui, requirements):
+    """Resolve opener options specific to revlogs."""
+
+    options = {}
+
+    if b'revlogv1' in requirements:
+        options[b'revlogv1'] = True
+    if REVLOGV2_REQUIREMENT in requirements:
+        options[b'revlogv2'] = True
+
+    if b'generaldelta' in requirements:
+        options[b'generaldelta'] = True
+
+    # experimental config: format.chunkcachesize
+    chunkcachesize = ui.configint(b'format', b'chunkcachesize')
+    if chunkcachesize is not None:
+        options[b'chunkcachesize'] = chunkcachesize
+
+    deltabothparents = ui.configbool(b'storage',
+                                     b'revlog.optimize-delta-parent-choice')
+    options[b'deltabothparents'] = deltabothparents
+
+    options[b'lazydeltabase'] = not scmutil.gddeltaconfig(ui)
+
+    chainspan = ui.configbytes(b'experimental', b'maxdeltachainspan')
+    if 0 <= chainspan:
+        options[b'maxdeltachainspan'] = chainspan
+
+    mmapindexthreshold = ui.configbytes(b'experimental',
+                                        b'mmapindexthreshold')
+    if mmapindexthreshold is not None:
+        options[b'mmapindexthreshold'] = mmapindexthreshold
+
+    withsparseread = ui.configbool(b'experimental', b'sparse-read')
+    srdensitythres = float(ui.config(b'experimental',
+                                     b'sparse-read.density-threshold'))
+    srmingapsize = ui.configbytes(b'experimental',
+                                  b'sparse-read.min-gap-size')
+    options[b'with-sparse-read'] = withsparseread
+    options[b'sparse-read-density-threshold'] = srdensitythres
+    options[b'sparse-read-min-gap-size'] = srmingapsize
+
+    sparserevlog = SPARSEREVLOG_REQUIREMENT in requirements
+    options[b'sparse-revlog'] = sparserevlog
+    if sparserevlog:
+        options[b'generaldelta'] = True
+
+    maxchainlen = None
+    if sparserevlog:
+        maxchainlen = revlogconst.SPARSE_REVLOG_MAX_CHAIN_LENGTH
+    # experimental config: format.maxchainlen
+    maxchainlen = ui.configint(b'format', b'maxchainlen', maxchainlen)
+    if maxchainlen is not None:
+        options[b'maxchainlen'] = maxchainlen
+
+    for r in requirements:
+        if r.startswith(b'exp-compression-'):
+            options[b'compengine'] = r[len(b'exp-compression-'):]
+
+    return options
+
 @interfaceutil.implementer(repository.completelocalrepository)
 class localrepository(object):
 
@@ -602,11 +690,6 @@
         'exp-sparse',
         'internal-phase'
     }
-    openerreqs = {
-        'revlogv1',
-        'generaldelta',
-        'treemanifest',
-    }
 
     # list of prefix for file which can be written without 'wlock'
     # Extensions should extend this list when needed
@@ -712,7 +795,6 @@
                 self.svfs.vfs.audit = self._getsvfsward(self.svfs.vfs.audit)
             else: # standard vfs
                 self.svfs.audit = self._getsvfsward(self.svfs.audit)
-        self._applyopenerreqs()
 
         self._dirstatevalidatewarned = False
 
@@ -817,56 +899,6 @@
             caps.add('bundle2=' + urlreq.quote(capsblob))
         return caps
 
-    def _applyopenerreqs(self):
-        self.svfs.options = {r: True for r in self.requirements
-                             if r in self.openerreqs}
-        # experimental config: format.chunkcachesize
-        chunkcachesize = self.ui.configint('format', 'chunkcachesize')
-        if chunkcachesize is not None:
-            self.svfs.options['chunkcachesize'] = chunkcachesize
-        # experimental config: format.manifestcachesize
-        manifestcachesize = self.ui.configint('format', 'manifestcachesize')
-        if manifestcachesize is not None:
-            self.svfs.options['manifestcachesize'] = manifestcachesize
-        deltabothparents = self.ui.configbool('storage',
-            'revlog.optimize-delta-parent-choice')
-        self.svfs.options['deltabothparents'] = deltabothparents
-        self.svfs.options['lazydeltabase'] = not scmutil.gddeltaconfig(self.ui)
-        chainspan = self.ui.configbytes('experimental', 'maxdeltachainspan')
-        if 0 <= chainspan:
-            self.svfs.options['maxdeltachainspan'] = chainspan
-        mmapindexthreshold = self.ui.configbytes('experimental',
-                                                 'mmapindexthreshold')
-        if mmapindexthreshold is not None:
-            self.svfs.options['mmapindexthreshold'] = mmapindexthreshold
-        withsparseread = self.ui.configbool('experimental', 'sparse-read')
-        srdensitythres = float(self.ui.config('experimental',
-                                              'sparse-read.density-threshold'))
-        srmingapsize = self.ui.configbytes('experimental',
-                                           'sparse-read.min-gap-size')
-        self.svfs.options['with-sparse-read'] = withsparseread
-        self.svfs.options['sparse-read-density-threshold'] = srdensitythres
-        self.svfs.options['sparse-read-min-gap-size'] = srmingapsize
-        sparserevlog = SPARSEREVLOG_REQUIREMENT in self.requirements
-        self.svfs.options['sparse-revlog'] = sparserevlog
-        if sparserevlog:
-            self.svfs.options['generaldelta'] = True
-        maxchainlen = None
-        if sparserevlog:
-            maxchainlen = revlogconst.SPARSE_REVLOG_MAX_CHAIN_LENGTH
-        # experimental config: format.maxchainlen
-        maxchainlen = self.ui.configint('format', 'maxchainlen', maxchainlen)
-        if maxchainlen is not None:
-            self.svfs.options['maxchainlen'] = maxchainlen
-
-        for r in self.requirements:
-            if r.startswith('exp-compression-'):
-                self.svfs.options['compengine'] = r[len('exp-compression-'):]
-
-        # TODO move "revlogv2" to openerreqs once finalized.
-        if REVLOGV2_REQUIREMENT in self.requirements:
-            self.svfs.options['revlogv2'] = True
-
     def _writerequirements(self):
         scmutil.writerequires(self.vfs, self.requirements)
 
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1779,6 +1779,8 @@
     This is a very early implementation that will massive rework before being
     inflicted to any end-user.
     """
+    from . import localrepo
+
     tr = op.gettransaction()
     unpackerversion = inpart.params.get('version', '01')
     # We should raise an appropriate exception here
@@ -1795,7 +1797,8 @@
                 "bundle contains tree manifests, but local repo is "
                 "non-empty and does not use tree manifests"))
         op.repo.requirements.add('treemanifest')
-        op.repo._applyopenerreqs()
+        op.repo.svfs.options = localrepo.resolvestorevfsoptions(
+            op.repo.ui, op.repo.requirements)
         op.repo._writerequirements()
     extrakwargs = {}
     targetphase = inpart.params.get('targetphase')



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


More information about the Mercurial-devel mailing list