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

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4354

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

Authored by indygreg on Aug 22 2018, 4:58 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG53e532007878: cmdutil: return a revlog from openrevlog() and split function
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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Aug 22 2018, 4:58 PM
Closed by commit rHG53e532007878: cmdutil: return a revlog from openrevlog() and split function (authored by indygreg). · Explain WhyAug 25 2018, 10:09 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/cmdutil.py (28 lines)

Diff 10566

mercurial/cmdutil.py

if writable: if writable:
fp = repo.ui.fout fp = repo.ui.fout
else: else:
fp = repo.ui.fin fp = repo.ui.fin
return _unclosablefile(fp) return _unclosablefile(fp)
fn = makefilename(ctx, pat, **props) fn = makefilename(ctx, pat, **props)
return open(fn, mode) 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""" """opens the changelog, manifest, a filelog or a given revlog"""
cl = opts['changelog'] cl = opts['changelog']
mf = opts['manifest'] mf = opts['manifest']
dir = opts['dir'] dir = opts['dir']
msg = None msg = None
if cl and mf: if cl and mf:
msg = _('cannot specify --changelog and --manifest at the same time') msg = _('cannot specify --changelog and --manifest at the same time')
elif cl and dir: elif cl and dir:
if len(dirlog): if len(dirlog):
r = dirlog r = dirlog
elif mf: elif mf:
r = repo.manifestlog.getstorage(b'') r = repo.manifestlog.getstorage(b'')
elif file_: elif file_:
filelog = repo.file(file_) filelog = repo.file(file_)
if len(filelog): if len(filelog):
r = 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 r:
if not returnrevlog:
raise error.Abort(_('cannot give path to non-revlog'))
if not file_: if not file_:
raise error.CommandError(cmd, _('invalid arguments')) raise error.CommandError(cmd, _('invalid arguments'))
if not os.path.isfile(file_): if not os.path.isfile(file_):
raise error.Abort(_("revlog '%s' not found") % file_) raise error.Abort(_("revlog '%s' not found") % file_)
r = revlog.revlog(vfsmod.vfs(pycompat.getcwd(), audit=False), r = revlog.revlog(vfsmod.vfs(pycompat.getcwd(), audit=False),
file_[:-2] + ".i") file_[:-2] + ".i")
return r 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): def copy(ui, repo, pats, opts, rename=False):
# called with the repo lock held # called with the repo lock held
# #
# hgsep => pathname that uses "/" to separate directories # hgsep => pathname that uses "/" to separate directories
# ossep => pathname that uses os.sep to separate directories # ossep => pathname that uses os.sep to separate directories
cwd = repo.getcwd() cwd = repo.getcwd()
targets = {} targets = {}
after = opts.get("after") after = opts.get("after")