D4354: cmdutil: return a revlog from openrevlog() and split function

[indygreg (Gregory Szorc)]

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

REVISION SUMMARY
  The filelog class is a wrapper around a revlog instance. I have plans
  to give manifests and the changelog a similar treatment.
  
  When filelog was ported away from revlog and when I started writing
  patches to do the same for manifests, I noticed that a lot of
  debug* and perf* commands were relying on low-level revlog APIs
  like start(), end(), deltaparent(), etc. For filelog, I added these
  to the interface, even though I didn't want to because they don't
  belong on a generic storage interface.
  
  For manifest (and eventually changelog), the pain is too much to bear.
  We need to cut the tight coupling.
  
  These debug* and perf* commands use cmdutil.openrevlog() to obtain
  a revlog instance.
  
  This commit effectively renames openrevlog() to openstorage(), adds
  an argument to ensure a revlog instance is returned, and introduces a
  replacement openrevlog() that calls openstorage() such that a revlog
  instance is returned.
  
  By doing things this way, we allow the debug* and perf* commands to
  still work on revlog-based repositories without having to expose
  low-level revlog APIs in the storage interfaces.
  
  The practical side-effect of this on the current code base is we return
  a revlog instance instead of a filelog. The manifest and changelog are
  not affected at this time.
  
  Some of filelog's storage APIs are different from revlog. For example,
  read() strips the optional header containing copy/rename metadata. This
  may impact some perf* commands. But I don't think the impact is
  worth worrying about.
  
  Upcoming commits will port existing consumers to openstorage(), where
  appropriate.
  
  This commit does cause some test regressions when using the simple
  store. These will be fixed as commands are ported to use storage APIs.
  
  .. api:: cmdutil.openrevlog() now returns a revlog instance or aborts
  
    Previously, it would return a storage object, which may not be a
    revlog instance.
    
    Use the new cmdutil.openstorage() API to return an object conforming
    to the storage interface of the thing you are accessing if you don't
    need a revlog instance.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/cmdutil.py

CHANGE DETAILS

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1054,7 +1054,7 @@
     fn = makefilename(ctx, pat, **props)
     return open(fn, mode)
 
-def openrevlog(repo, cmd, file_, opts):
+def openstorage(repo, cmd, file_, opts, returnrevlog=False):
     """opens the changelog, manifest, a filelog or a given revlog"""
     cl = opts['changelog']
     mf = opts['manifest']
@@ -1092,15 +1092,41 @@
             filelog = repo.file(file_)
             if len(filelog):
                 r = filelog
+
+        # Not all storage may be revlogs. If requested, try to return an actual
+        # revlog instance.
+        if returnrevlog:
+            if isinstance(r, revlog.revlog):
+                pass
+            elif util.safehasattr(r, '_revlog'):
+                r = r._revlog
+            elif r is not None:
+                raise error.Abort(_('%r does not appear to be a revlog') % r)
+
     if not r:
+        if not returnrevlog:
+            raise error.Abort(_('cannot give path to non-revlog'))
+
         if not file_:
             raise error.CommandError(cmd, _('invalid arguments'))
         if not os.path.isfile(file_):
             raise error.Abort(_("revlog '%s' not found") % file_)
         r = revlog.revlog(vfsmod.vfs(pycompat.getcwd(), audit=False),
                           file_[:-2] + ".i")
     return r
 
+def openrevlog(repo, cmd, file_, opts):
+    """Obtain a revlog backing storage of an item.
+
+    This is similar to ``openstorage()`` except it always returns a revlog.
+
+    In most cases, a caller cares about the main storage object - not the
+    revlog backing it. Therefore, this function should only be used by code
+    that needs to examine low-level revlog implementation details. e.g. debug
+    commands.
+    """
+    return openstorage(repo, cmd, file_, opts, returnrevlog=True)
+
 def copy(ui, repo, pats, opts, rename=False):
     # called with the repo lock held
     #



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