D3376: registrar: replace "cmdtype" with an intent-based mechanism (API)

[indygreg (Gregory Szorc)]

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

REVISION SUMMARY
  Commands perform varied actions and repositories vary in their
  capabilities.
  
  Historically, the .hg/requires file has been used to lock out clients
  lacking a requirement. But this is a very heavy-handed approach and
  is typically reserved for cases where the on-disk storage format
  changes and we want to prevent incompatible clients from operating on
  a repo.
  
  Outside of the .hg/requires file, we tend to deal with things like
  optional, extension-provided features via checking at call sites.
  We'll either have checks in core or extensions will monkeypatch
  functions in core disabling incompatible features, enabling new
  features, etc.
  
  Things are somewhat tolerable today. But once we introduce alternate
  storage backends with varying support for repository features and
  vastly different modes of behavior, the current model will quickly
  grow unwieldy. For example, the implementation of the "simple store"
  required a lot of hacks to deal with stripping and verify because
  various parts of core assume things are implemented a certain way.
  Partial clone will require new ways of modeling file data retrieval,
  because we can no longer assume that all file data is already local.
  In this new world, some commands might not make any sense for certain
  types of repositories.
  
  What we need is a mechanism to affect the construction of repository
  (and eventually peer) instances so the requirements/capabilities
  needed for the current operation can be taken into account. "Current
  operation" can almost certainly be defined by a command. So it makes
  sense for commands to declare their intended actions.
  
  This commit introduces the "intents" concept on the command registrar.
  "intents" captures a set of strings that declare actions that are
  anticipated to be taken, requirements the repository must possess, etc.
  These intents will be passed into hg.repo(), which will pass them into
  localrepository, where they can be used to influence the object being
  created. Some use cases for this include:
  
  - For read-only intents, constructing a repository object that doesn't expose methods that can mutate the repository. Its VFS instances don't even allow opening a file with write access.
  - For read-only intents, constructing a repository object without cache invalidation logic. If the repo never changes during its lifetime, nothing ever needs to be invalidated and we don't need to do expensive things like verify the changelog's hidden revisions state is accurate every time we access repo.changelog.
  - We can automatically hide commands from `hg help` when the current repository doesn't provide that command. For example, an alternate storage backend may not support `hg commit`, so we can hide that command or anything else that would perform local commits.
  
  We already kind of had an "intents" mechanism on the registrar in the
  form of "cmdtype." However, it was never used. And it was limited to
  a single value. We really need something that supports multiple
  intents. And because intents may be defined by extensions and at this
  point are advisory, I think it is best to define them in a set rather
  than as separate arguments/attributes on the command.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/commands.py
  mercurial/dispatch.py
  mercurial/registrar.py
  tests/test-directaccess.t

CHANGE DETAILS

diff --git a/tests/test-directaccess.t b/tests/test-directaccess.t
--- a/tests/test-directaccess.t
+++ b/tests/test-directaccess.t
@@ -192,7 +192,7 @@
   $ hg log -qr 'null:wdir() & 2147483647'
   2147483647:ffffffffffff
 
-Commands with undefined cmdtype should not work right now
+Commands with undefined intent should not work right now
 
   $ hg phase -r 28ad74
   abort: hidden revision '28ad74' was rewritten as: 2443a0e66469!
diff --git a/mercurial/registrar.py b/mercurial/registrar.py
--- a/mercurial/registrar.py
+++ b/mercurial/registrar.py
@@ -138,15 +138,18 @@
     potential repository locations. See ``findrepo()``. If a repository is
     found, it will be used and passed to the decorated function.
 
-    There are three constants in the class which tells what type of the command
-    that is. That information will be helpful at various places. It will be also
-    be used to decide what level of access the command has on hidden commits.
-    The constants are:
+    The `intents` argument defines a set of intended actions or capabilities
+    the command is taking. These intents can be used to affect the construction
+    of the repository object passed to the command. For example, commands
+    declaring that they are read-only could receive a repository that doesn't
+    have any methods allowing repository mutation. Other intents could be used
+    to prevent the command from running if the requested intent could not be
+    fulfilled.
 
-    `unrecoverablewrite` is for those write commands which can't be recovered
-    like push.
-    `recoverablewrite` is for write commands which can be recovered like commit.
-    `readonly` is for commands which are read only.
+    The following intents are defined:
+
+    readonly
+       The command is read-only
 
     The signature of the decorated function looks like this:
         def cmd(ui[, repo] [, <args>] [, <options>])
@@ -161,29 +164,22 @@
     descriptions and examples.
     """
 
-    unrecoverablewrite = "unrecoverable"
-    recoverablewrite = "recoverable"
-    readonly = "readonly"
-
-    possiblecmdtypes = {unrecoverablewrite, recoverablewrite, readonly}
-
     def _doregister(self, func, name, options=(), synopsis=None,
                     norepo=False, optionalrepo=False, inferrepo=False,
-                    cmdtype=unrecoverablewrite):
+                    intents=None):
 
-        if cmdtype not in self.possiblecmdtypes:
-            raise error.ProgrammingError("unknown cmdtype value '%s' for "
-                                         "'%s' command" % (cmdtype, name))
         func.norepo = norepo
         func.optionalrepo = optionalrepo
         func.inferrepo = inferrepo
-        func.cmdtype = cmdtype
+        func.intents = intents or set()
         if synopsis:
             self._table[name] = func, list(options), synopsis
         else:
             self._table[name] = func, list(options)
         return func
 
+INTENT_READONLY = b'readonly'
+
 class revsetpredicate(_funcregistrarbase):
     """Decorator to register revset predicate
 
diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -35,7 +35,6 @@
     hook,
     profiling,
     pycompat,
-    registrar,
     scmutil,
     ui as uimod,
     util,
@@ -46,8 +45,6 @@
     stringutil,
 )
 
-unrecoverablewrite = registrar.command.unrecoverablewrite
-
 class request(object):
     def __init__(self, args, ui=None, repo=None, fin=None, fout=None,
                  ferr=None, prereposetups=None):
@@ -562,7 +559,7 @@
         return aliasargs(self.fn, args)
 
     def __getattr__(self, name):
-        adefaults = {r'norepo': True, r'cmdtype': unrecoverablewrite,
+        adefaults = {r'norepo': True, r'intents': set(),
                      r'optionalrepo': False, r'inferrepo': False}
         if name not in adefaults:
             raise AttributeError(name)
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -73,7 +73,7 @@
 table.update(debugcommandsmod.command._table)
 
 command = registrar.command(table)
-readonly = registrar.command.readonly
+INTENT_READONLY = registrar.INTENT_READONLY
 
 # common command options
 
@@ -1083,7 +1083,8 @@
       _('show only branches that have unmerged heads (DEPRECATED)')),
      ('c', 'closed', False, _('show normal and closed branches')),
     ] + formatteropts,
-    _('[-c]'), cmdtype=readonly)
+    _('[-c]'),
+    intents={INTENT_READONLY})
 def branches(ui, repo, active=False, closed=False, **opts):
     """list repository named branches
 
@@ -1282,7 +1283,8 @@
     ('', 'decode', None, _('apply any matching decode filter')),
     ] + walkopts + formatteropts,
     _('[OPTION]... FILE...'),
-    inferrepo=True, cmdtype=readonly)
+    inferrepo=True,
+    intents={INTENT_READONLY})
 def cat(ui, repo, file1, *pats, **opts):
     """output the current or given revision of files
 
@@ -1633,7 +1635,8 @@
      ('l', 'local', None, _('edit repository config')),
      ('g', 'global', None, _('edit global config'))] + formatteropts,
     _('[-u] [NAME]...'),
-    optionalrepo=True, cmdtype=readonly)
+    optionalrepo=True,
+    intents={INTENT_READONLY})
 def config(ui, repo, *values, **opts):
     """show combined config settings from all hgrc files
 
@@ -1802,7 +1805,8 @@
     ('c', 'change', '', _('change made by revision'), _('REV'))
     ] + diffopts + diffopts2 + walkopts + subrepoopts,
     _('[OPTION]... ([-c REV] | [-r REV1 [-r REV2]]) [FILE]...'),
-    inferrepo=True, cmdtype=readonly)
+    inferrepo=True,
+    intents={INTENT_READONLY})
 def diff(ui, repo, *pats, **opts):
     """diff repository (or selected files)
 
@@ -1895,7 +1899,8 @@
     ('', 'switch-parent', None, _('diff against the second parent')),
     ('r', 'rev', [], _('revisions to export'), _('REV')),
     ] + diffopts + formatteropts,
-    _('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'), cmdtype=readonly)
+    _('[OPTION]... [-o OUTFILESPEC] [-r] [REV]...'),
+    intents={INTENT_READONLY})
 def export(ui, repo, *changesets, **opts):
     """dump the header and diffs for one or more changesets
 
@@ -1990,7 +1995,8 @@
     [('r', 'rev', '', _('search the repository as it is in REV'), _('REV')),
      ('0', 'print0', None, _('end filenames with NUL, for use with xargs')),
     ] + walkopts + formatteropts + subrepoopts,
-    _('[OPTION]... [FILE]...'), cmdtype=readonly)
+    _('[OPTION]... [FILE]...'),
+    intents={INTENT_READONLY})
 def files(ui, repo, *pats, **opts):
     """list tracked files
 
@@ -2371,7 +2377,8 @@
     ('d', 'date', None, _('list the date (short with -q)')),
     ] + formatteropts + walkopts,
     _('[OPTION]... PATTERN [FILE]...'),
-    inferrepo=True, cmdtype=readonly)
+    inferrepo=True,
+    intents={INTENT_READONLY})
 def grep(ui, repo, pattern, *pats, **opts):
     """search revision history for a pattern in specified files
 
@@ -2617,7 +2624,8 @@
     ('a', 'active', False, _('show active branchheads only (DEPRECATED)')),
     ('c', 'closed', False, _('show normal and closed branch heads')),
     ] + templateopts,
-    _('[-ct] [-r STARTREV] [REV]...'), cmdtype=readonly)
+    _('[-ct] [-r STARTREV] [REV]...'),
+    intents={INTENT_READONLY})
 def heads(ui, repo, *branchrevs, **opts):
     """show branch heads
 
@@ -2693,7 +2701,8 @@
      ('s', 'system', [], _('show help for specific platform(s)')),
      ],
     _('[-ecks] [TOPIC]'),
-    norepo=True, cmdtype=readonly)
+    norepo=True,
+    intents={INTENT_READONLY})
 def help_(ui, name=None, **opts):
     """show help for a given topic or a help overview
 
@@ -2735,7 +2744,8 @@
     ('B', 'bookmarks', None, _('show bookmarks')),
     ] + remoteopts + formatteropts,
     _('[-nibtB] [-r REV] [SOURCE]'),
-    optionalrepo=True, cmdtype=readonly)
+    optionalrepo=True,
+    intents={INTENT_READONLY})
 def identify(ui, repo, source=None, rev=None,
              num=None, id=None, branch=None, tags=None, bookmarks=None, **opts):
     """identify the working directory or specified revision
@@ -3312,7 +3322,8 @@
      _('do not display revision or any of its ancestors'), _('REV')),
     ] + logopts + walkopts,
     _('[OPTION]... [FILE]'),
-    inferrepo=True, cmdtype=readonly)
+    inferrepo=True,
+    intents={INTENT_READONLY})
 def log(ui, repo, *pats, **opts):
     """show revision history of entire repository or files
 
@@ -3480,7 +3491,8 @@
     [('r', 'rev', '', _('revision to display'), _('REV')),
      ('', 'all', False, _("list files from all revisions"))]
          + formatteropts,
-    _('[-r REV]'), cmdtype=readonly)
+    _('[-r REV]'),
+    intents={INTENT_READONLY})
 def manifest(ui, repo, node=None, rev=None, **opts):
     """output the current or given revision of the project manifest
 
@@ -3758,7 +3770,7 @@
     displayer.close()
 
 @command('paths', formatteropts, _('[NAME]'), optionalrepo=True,
-    cmdtype=readonly)
+    intents={INTENT_READONLY})
 def paths(ui, repo, search=None, **opts):
     """show aliases for remote repositories
 
@@ -4696,7 +4708,7 @@
     return repo.rollback(dryrun=opts.get(r'dry_run'),
                          force=opts.get(r'force'))
 
- at command('root', [], cmdtype=readonly)
+ at command('root', [], intents={INTENT_READONLY})
 def root(ui, repo):
     """print the root (top) of the current working directory
 
@@ -4790,7 +4802,8 @@
     ('', 'change', '', _('list the changed files of a revision'), _('REV')),
     ] + walkopts + subrepoopts + formatteropts,
     _('[OPTION]... [FILE]...'),
-    inferrepo=True, cmdtype=readonly)
+    inferrepo=True,
+    intents={INTENT_READONLY})
 def status(ui, repo, *pats, **opts):
     """show changed files in the working directory
 
@@ -4958,7 +4971,8 @@
 
 @command('^summary|sum',
     [('', 'remote', None, _('check for push and pull'))],
-    '[--remote]', cmdtype=readonly)
+    '[--remote]',
+    intents={INTENT_READONLY})
 def summary(ui, repo, **opts):
     """summarize working directory state
 
@@ -5359,7 +5373,7 @@
     finally:
         release(lock, wlock)
 
- at command('tags', formatteropts, '', cmdtype=readonly)
+ at command('tags', formatteropts, '', intents={INTENT_READONLY})
 def tags(ui, repo, **opts):
     """list repository tags
 
@@ -5596,7 +5610,8 @@
     """
     return hg.verify(repo)
 
- at command('version', [] + formatteropts, norepo=True, cmdtype=readonly)
+ at command('version', [] + formatteropts, norepo=True,
+         intents={INTENT_READONLY})
 def version_(ui, **opts):
     """output version and copyright information"""
     opts = pycompat.byteskwargs(opts)



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