This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3559

narrow: only wrap dirstate functions once, instead of per-reposetup
ClosedPublic

Authored by spectral on May 14 2018, 6:53 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG1cba497491be: narrow: only wrap dirstate functions once, instead of per-reposetup
Summary

chg will call reposetup multiple times, and we would end up double-wrapping (or
worse) the dirstate functions; this can cause issues like OSError 'No such file
or directory' during rebase operations, when we go to double-delete our
narrowspec backup file.

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

spectral created this revision.May 14 2018, 6:53 PM
yuja added a subscriber: yuja.May 16 2018, 8:07 AM

-def setup(repo):
+# Mapping of root:str to repo for repos that have the narrow requirement
+# specified.
+_rootrepomap = {}
+
+def _getrepo(ds):
+ """Check if narrow is enabled for repo associated with ds; return repo."""
+ return _rootrepomap.get(ds._root, None)

This might cause problem on long-running processes such as hgweb and
commandserver.

Instead, maybe we can extract a factory function of repo.dirstate() so that
the narrowrepo can easily override it.

class localrepository(object):
    @repofilecache('dirstate')
    def dirstate(self):
        return self._makedirstate()

...
    class narrowrepository(repo.__class__):
        def _makedirstate(self):
            d = super(...)
            return wrapdirstate(d)
spectral edited the summary of this revision. (Show Details)May 18 2018, 5:53 PM
spectral updated this revision to Diff 8744.
yuja added a comment.May 22 2018, 8:34 AM

Queued, thanks.

  • a/mercurial/localrepo.py

+++ b/mercurial/localrepo.py
@@ -778,6 +778,9 @@

@repofilecache('dirstate')
def dirstate(self):

+ return self._makedirstate()
+
+ def _makedirstate(self):

Can you add a docstring so we wouldn't remove this function by mistake?

  • # Prevent adding files that are outside the sparse checkout
  • editfuncs = ['normal', 'add', 'normallookup', 'copy', 'remove', 'merge']

+ @_editfunc
+ def remove(self, *args):
+ return super(narrowdirstate, self).remove(*args)
+
+ @_editfunc
+ def remove(self, *args):
+ return super(narrowdirstate, self).remove(*args)

s/remove/merge/ the last function in flight.

Closed by commit rHG1cba497491be: narrow: only wrap dirstate functions once, instead of per-reposetup (authored by spectral). · Explain WhyMay 22 2018, 8:37 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 8871

hgext/narrow/__init__.py

verify as verifymod, verify as verifymod,
) )
from . import ( from . import (
narrowbundle2, narrowbundle2,
narrowchangegroup, narrowchangegroup,
narrowcommands, narrowcommands,
narrowcopies, narrowcopies,
narrowdirstate,
narrowpatch, narrowpatch,
narrowrepo, narrowrepo,
narrowrevlog, narrowrevlog,
narrowtemplates, narrowtemplates,
narrowwirepeer, narrowwirepeer,
) )
configtable = {} configtable = {}
narrowchangegroup.setup() narrowchangegroup.setup()
narrowwirepeer.uisetup() narrowwirepeer.uisetup()
def reposetup(ui, repo): def reposetup(ui, repo):
"""Wraps local repositories with narrow repo support.""" """Wraps local repositories with narrow repo support."""
if not repo.local(): if not repo.local():
return return
narrowrepo.wraprepo(repo)
if changegroup.NARROW_REQUIREMENT in repo.requirements: if changegroup.NARROW_REQUIREMENT in repo.requirements:
narrowrepo.wraprepo(repo)
narrowcopies.setup(repo) narrowcopies.setup(repo)
narrowdirstate.setup(repo)
narrowpatch.setup(repo) narrowpatch.setup(repo)
narrowwirepeer.reposetup(repo) narrowwirepeer.reposetup(repo)
def _verifierinit(orig, self, repo, matcher=None): def _verifierinit(orig, self, repo, matcher=None):
# The verifier's matcher argument was desgined for narrowhg, so it should # The verifier's matcher argument was desgined for narrowhg, so it should
# be None from core. If another extension passes a matcher (unlikely), # be None from core. If another extension passes a matcher (unlikely),
# we'll have to fail until matchers can be composed more easily. # we'll have to fail until matchers can be composed more easily.
assert matcher is None assert matcher is None

hgext/narrow/narrowdirstate.py

# narrowdirstate.py - extensions to mercurial dirstate to support narrow clones # narrowdirstate.py - extensions to mercurial dirstate to support narrow clones
# #
# Copyright 2017 Google, Inc. # Copyright 2017 Google, Inc.
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
from mercurial.i18n import _ from mercurial.i18n import _
from mercurial import ( from mercurial import (
dirstate,
error, error,
extensions,
match as matchmod, match as matchmod,
narrowspec, narrowspec,
util as hgutil, util as hgutil,
) )
def setup(repo): def wrapdirstate(repo, dirstate):
"""Add narrow spec dirstate ignore, block changes outside narrow spec.""" """Add narrow spec dirstate ignore, block changes outside narrow spec."""
def walk(orig, self, match, subrepos, unknown, ignored, full=True, def _editfunc(fn):
narrowonly=True): def _wrapper(self, *args):
if narrowonly:
# hack to not exclude explicitly-specified paths so that they can
# be warned later on e.g. dirstate.add()
em = matchmod.exact(match._root, match._cwd, match.files())
nm = matchmod.unionmatcher([repo.narrowmatch(), em])
match = matchmod.intersectmatchers(match, nm)
return orig(self, match, subrepos, unknown, ignored, full)
extensions.wrapfunction(dirstate.dirstate, 'walk', walk)
# Prevent adding files that are outside the sparse checkout
editfuncs = ['normal', 'add', 'normallookup', 'copy', 'remove', 'merge']
for func in editfuncs:
def _wrapper(orig, self, *args):
dirstate = repo.dirstate dirstate = repo.dirstate
narrowmatch = repo.narrowmatch() narrowmatch = repo.narrowmatch()
for f in args: for f in args:
if f is not None and not narrowmatch(f) and f not in dirstate: if f is not None and not narrowmatch(f) and f not in dirstate:
raise error.Abort(_("cannot track '%s' - it is outside " + raise error.Abort(_("cannot track '%s' - it is outside " +
"the narrow clone") % f) "the narrow clone") % f)
return orig(self, *args) return fn(self, *args)
extensions.wrapfunction(dirstate.dirstate, func, _wrapper) return _wrapper
def filterrebuild(orig, self, parent, allfiles, changedfiles=None):
if changedfiles is None:
# Rebuilding entire dirstate, let's filter allfiles to match the
# narrowspec.
allfiles = [f for f in allfiles if repo.narrowmatch()(f)]
orig(self, parent, allfiles, changedfiles)
extensions.wrapfunction(dirstate.dirstate, 'rebuild', filterrebuild)
def _narrowbackupname(backupname): def _narrowbackupname(backupname):
assert 'dirstate' in backupname assert 'dirstate' in backupname
return backupname.replace('dirstate', narrowspec.FILENAME) return backupname.replace('dirstate', narrowspec.FILENAME)
def restorebackup(orig, self, tr, backupname): class narrowdirstate(dirstate.__class__):
self._opener.rename(_narrowbackupname(backupname), narrowspec.FILENAME, def walk(self, match, subrepos, unknown, ignored, full=True,
checkambig=True) narrowonly=True):
orig(self, tr, backupname) if narrowonly:
# hack to not exclude explicitly-specified paths so that they
# can be warned later on e.g. dirstate.add()
em = matchmod.exact(match._root, match._cwd, match.files())
nm = matchmod.unionmatcher([repo.narrowmatch(), em])
match = matchmod.intersectmatchers(match, nm)
return super(narrowdirstate, self).walk(match, subrepos, unknown,
ignored, full)
# Prevent adding/editing/copying/deleting files that are outside the
# sparse checkout
@_editfunc
def normal(self, *args):
return super(narrowdirstate, self).normal(*args)
@_editfunc
def add(self, *args):
return super(narrowdirstate, self).add(*args)
@_editfunc
def normallookup(self, *args):
return super(narrowdirstate, self).normallookup(*args)
@_editfunc
def copy(self, *args):
return super(narrowdirstate, self).copy(*args)
@_editfunc
def remove(self, *args):
return super(narrowdirstate, self).remove(*args)
@_editfunc
def merge(self, *args):
return super(narrowdirstate, self).merge(*args)
extensions.wrapfunction(dirstate.dirstate, 'restorebackup', restorebackup) def rebuild(self, parent, allfiles, changedfiles=None):
if changedfiles is None:
# Rebuilding entire dirstate, let's filter allfiles to match the
# narrowspec.
allfiles = [f for f in allfiles if repo.narrowmatch()(f)]
super(narrowdirstate, self).rebuild(parent, allfiles, changedfiles)
def restorebackup(self, tr, backupname):
self._opener.rename(_narrowbackupname(backupname),
narrowspec.FILENAME, checkambig=True)
super(narrowdirstate, self).restorebackup(tr, backupname)
def savebackup(orig, self, tr, backupname): def savebackup(self, tr, backupname):
orig(self, tr, backupname) super(narrowdirstate, self).savebackup(tr, backupname)
narrowbackupname = _narrowbackupname(backupname) narrowbackupname = _narrowbackupname(backupname)
self._opener.tryunlink(narrowbackupname) self._opener.tryunlink(narrowbackupname)
hgutil.copyfile(self._opener.join(narrowspec.FILENAME), hgutil.copyfile(self._opener.join(narrowspec.FILENAME),
self._opener.join(narrowbackupname), hardlink=True) self._opener.join(narrowbackupname), hardlink=True)
extensions.wrapfunction(dirstate.dirstate, 'savebackup', savebackup) def clearbackup(self, tr, backupname):
super(narrowdirstate, self).clearbackup(tr, backupname)
def clearbackup(orig, self, tr, backupname):
orig(self, tr, backupname)
self._opener.unlink(_narrowbackupname(backupname)) self._opener.unlink(_narrowbackupname(backupname))
extensions.wrapfunction(dirstate.dirstate, 'clearbackup', clearbackup) dirstate.__class__ = narrowdirstate
return dirstate

hgext/narrow/narrowrepo.py

from mercurial import ( from mercurial import (
changegroup, changegroup,
hg, hg,
narrowspec, narrowspec,
scmutil, scmutil,
) )
from . import ( from . import (
narrowdirstate,
narrowrevlog, narrowrevlog,
) )
def wrappostshare(orig, sourcerepo, destrepo, **kwargs): def wrappostshare(orig, sourcerepo, destrepo, **kwargs):
orig(sourcerepo, destrepo, **kwargs) orig(sourcerepo, destrepo, **kwargs)
if changegroup.NARROW_REQUIREMENT in sourcerepo.requirements: if changegroup.NARROW_REQUIREMENT in sourcerepo.requirements:
with destrepo.wlock(): with destrepo.wlock():
with destrepo.vfs('shared', 'a') as fp: with destrepo.vfs('shared', 'a') as fp:
removed = list(filter(narrowmatch, s.removed)) removed = list(filter(narrowmatch, s.removed))
deleted = list(filter(narrowmatch, s.deleted)) deleted = list(filter(narrowmatch, s.deleted))
unknown = list(filter(narrowmatch, s.unknown)) unknown = list(filter(narrowmatch, s.unknown))
ignored = list(filter(narrowmatch, s.ignored)) ignored = list(filter(narrowmatch, s.ignored))
clean = list(filter(narrowmatch, s.clean)) clean = list(filter(narrowmatch, s.clean))
return scmutil.status(modified, added, removed, deleted, unknown, return scmutil.status(modified, added, removed, deleted, unknown,
ignored, clean) ignored, clean)
def _makedirstate(self):
dirstate = super(narrowrepository, self)._makedirstate()
return narrowdirstate.wrapdirstate(self, dirstate)
repo.__class__ = narrowrepository repo.__class__ = narrowrepository

mercurial/localrepo.py

return manifest.manifestrevlog(self.svfs) return manifest.manifestrevlog(self.svfs)
@storecache('00manifest.i') @storecache('00manifest.i')
def manifestlog(self): def manifestlog(self):
return manifest.manifestlog(self.svfs, self) return manifest.manifestlog(self.svfs, self)
@repofilecache('dirstate') @repofilecache('dirstate')
def dirstate(self): def dirstate(self):
return self._makedirstate()
def _makedirstate(self):
sparsematchfn = lambda: sparse.matcher(self) sparsematchfn = lambda: sparse.matcher(self)
return dirstate.dirstate(self.vfs, self.ui, self.root, return dirstate.dirstate(self.vfs, self.ui, self.root,
self._dirstatevalidate, sparsematchfn) self._dirstatevalidate, sparsematchfn)
def _dirstatevalidate(self, node): def _dirstatevalidate(self, node):
try: try:
self.changelog.rev(node) self.changelog.rev(node)

tests/test-narrow-expanddirstate.t

> if not repo.currenttransaction(): > if not repo.currenttransaction():
> ui.develwarn(b'expandnarrowspec called outside of transaction!') > ui.develwarn(b'expandnarrowspec called outside of transaction!')
> repo.setnarrowpats(includes, excludes) > repo.setnarrowpats(includes, excludes)
> newmatcher = narrowspec.match(repo.root, includes, excludes) > newmatcher = narrowspec.match(repo.root, includes, excludes)
> added = matchmod.differencematcher(newmatcher, currentmatcher) > added = matchmod.differencematcher(newmatcher, currentmatcher)
> for f in repo[b'.'].manifest().walk(added): > for f in repo[b'.'].manifest().walk(added):
> repo.dirstate.normallookup(f) > repo.dirstate.normallookup(f)
> >
> def makeds(ui, repo): > def wrapds(ui, repo, ds):
> def wrapds(orig, self):
> ds = orig(self)
> class expandingdirstate(ds.__class__): > class expandingdirstate(ds.__class__):
> @hgutil.propertycache > @hgutil.propertycache
> def _map(self): > def _map(self):
> ret = super(expandingdirstate, self)._map > ret = super(expandingdirstate, self)._map
> with repo.wlock(), repo.lock(), repo.transaction( > with repo.wlock(), repo.lock(), repo.transaction(
> b'expandnarrowspec'): > b'expandnarrowspec'):
> expandnarrowspec(ui, repo, > expandnarrowspec(ui, repo,
> encoding.environ.get(b'DIRSTATEINCLUDES')) > encoding.environ.get(b'DIRSTATEINCLUDES'))
> return ret > return ret
> ds.__class__ = expandingdirstate > ds.__class__ = expandingdirstate
> return ds > return ds
> return wrapds
> >
> def reposetup(ui, repo): > def reposetup(ui, repo):
> extensions.wrapfilecache(localrepo.localrepository, b'dirstate', > class expandingrepo(repo.__class__):
> makeds(ui, repo)) > def _makedirstate(self):
> def overridepatch(orig, *args, **kwargs): > dirstate = super(expandingrepo, self)._makedirstate()
> return wrapds(ui, repo, dirstate)
> repo.__class__ = expandingrepo
>
> def extsetup(unused_ui):
> def overridepatch(orig, ui, repo, *args, **kwargs):
> with repo.wlock(): > with repo.wlock():
> expandnarrowspec(ui, repo, encoding.environ.get(b'PATCHINCLUDES')) > expandnarrowspec(ui, repo, encoding.environ.get(b'PATCHINCLUDES'))
> return orig(*args, **kwargs) > return orig(ui, repo, *args, **kwargs)
> >
> extensions.wrapfunction(patch, b'patch', overridepatch) > extensions.wrapfunction(patch, b'patch', overridepatch)
> EOF > EOF
$ cat >> ".hg/hgrc" <<EOF $ cat >> ".hg/hgrc" <<EOF
> [extensions] > [extensions]
> expand_extension = $TESTTMP/expand_extension.py > expand_extension = $TESTTMP/expand_extension.py
> EOF > EOF