This is an archive of the discontinued Mercurial Phabricator instance.

Differential D57

match: write forceincludematcher using unionmatcher
ClosedPublic

Authored by martinvonz on Jul 11 2017, 8:01 PM.

Details

Reviewers
dsp
Group Reviewers
hg-reviewers
Commits
rHG6f4e5e5940a5: match: write forceincludematcher using unionmatcher
Summary

The forceincludematcher is simply a unionmatcher of a includematcher
(matching paths recursively) with the given matcher. Since the
forceincludematcher is only used by sparse, move it there.

I don't have a good sparse repo setup to test performance impact on.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Jul 11 2017, 8:01 PM
dsp added a subscriber: dsp.Jul 12 2017, 10:41 AM

I am slightly -1 on this, as it removes __repr__ for forceincludematcher. This would lead to remove information from hg debugwalk.

martinvonz added a comment.Jul 12 2017, 11:36 AM
In D57#731, @dsp wrote:

I am slightly -1 on this, as it removes __repr__ for forceincludematcher. This would lead to remove information from hg debugwalk.

What arguments do you pass to "hg debugwalk" to get a forceincludematcher?

durham added a subscriber: durham.Jul 12 2017, 12:29 PM

makeittso

Might be nice to have unit tests for matchers, now that they're getting some complications.

martinvonz added a comment.Jul 12 2017, 1:49 PM
In D57#743, @durham wrote:

makeittso
Might be nice to have unit tests for matchers, now that they're getting some complications.

I think the most interesting part to test would be visitdir(), because that's not tested much by our test suite ("def visitdir(self, dir): return True" would pass almost all tests). So it seems to me like D58 would benefit more from unit tests. patternmatcher and includematcher are more complex and would benefit more from unit tests.

dsp added a comment.Jul 12 2017, 7:28 PM
In D57#735, @martinvonz wrote:

What arguments do you pass to "hg debugwalk" to get a forceincludematcher?

You are right, as we don't use forceincludematcher inside match() it never gets into debugwalk. Your patch seems fine.

dsp accepted this revision.Jul 13 2017, 2:05 PM

accepting this for visibility to hg-reviewers.

This revision is now accepted and ready to land.Jul 13 2017, 2:05 PM
martinvonz updated this revision to Diff 128.Jul 14 2017, 2:47 AM
This revision now requires review to proceed.Jul 14 2017, 2:47 AM
Closed by commit rHG6f4e5e5940a5: match: write forceincludematcher using unionmatcher (authored by martinvonz). · Explain WhyJul 14 2017, 1:25 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/match.py (14 lines)
Mmercurial/sparse.py (11 lines)

Diff 128

mercurial/match.py

def prefix(self): def prefix(self):
return self._matcher.prefix() and not self._always return self._matcher.prefix() and not self._always
def __repr__(self): def __repr__(self):
return ('<subdirmatcher path=%r, matcher=%r>' % return ('<subdirmatcher path=%r, matcher=%r>' %
(self._path, self._matcher)) (self._path, self._matcher))
class forceincludematcher(basematcher):
"""A matcher that returns true for any of the forced includes before testing
against the actual matcher."""
def __init__(self, matcher, includes):
self._matcher = matcher
self._includes = includes
def matchfn(self, f):
return f in self._includes or self._matcher(f)
def __repr__(self):
return ('<forceincludematcher matcher=%r, includes=%r>' %
(self._matcher, sorted(self._includes)))
class unionmatcher(basematcher): class unionmatcher(basematcher):
"""A matcher that is the union of several matchers.""" """A matcher that is the union of several matchers."""
def __init__(self, matchers): def __init__(self, matchers):
self._matchers = matchers self._matchers = matchers
def matchfn(self, f): def matchfn(self, f):
for match in self._matchers: for match in self._matchers:
if match(f): if match(f):

mercurial/sparse.py

dirstate.drop(file) dirstate.drop(file)
repo.vfs.unlink('tempsparse') repo.vfs.unlink('tempsparse')
repo._sparsesignaturecache.clear() repo._sparsesignaturecache.clear()
msg = _('cleaned up %d temporarily added file(s) from the ' msg = _('cleaned up %d temporarily added file(s) from the '
'sparse checkout\n') 'sparse checkout\n')
repo.ui.status(msg % len(tempincludes)) repo.ui.status(msg % len(tempincludes))
def forceincludematcher(matcher, includes):
"""Returns a matcher that returns true for any of the forced includes
before testing against the actual matcher."""
kindpats = [('path', include, '') for include in includes]
includematcher = matchmod.includematcher('', '', kindpats)
return matchmod.unionmatcher([includematcher, matcher])
def matcher(repo, revs=None, includetemp=True): def matcher(repo, revs=None, includetemp=True):
"""Obtain a matcher for sparse working directories for the given revs. """Obtain a matcher for sparse working directories for the given revs.
If multiple revisions are specified, the matcher is the union of all If multiple revisions are specified, the matcher is the union of all
revs. revs.
``includetemp`` indicates whether to use the temporary sparse profile. ``includetemp`` indicates whether to use the temporary sparse profile.
""" """
while os.path.basename(dirname): while os.path.basename(dirname):
subdirs.add(dirname) subdirs.add(dirname)
dirname = os.path.dirname(dirname) dirname = os.path.dirname(dirname)
matcher = matchmod.match(repo.root, '', [], matcher = matchmod.match(repo.root, '', [],
include=includes, exclude=excludes, include=includes, exclude=excludes,
default='relpath') default='relpath')
if subdirs: if subdirs:
matcher = matchmod.forceincludematcher(matcher, subdirs) matcher = forceincludematcher(matcher, subdirs)
matchers.append(matcher) matchers.append(matcher)
except IOError: except IOError:
pass pass
if not matchers: if not matchers:
result = matchmod.always(repo.root, '') result = matchmod.always(repo.root, '')
elif len(matchers) == 1: elif len(matchers) == 1:
result = matchers[0] result = matchers[0]
else: else:
result = matchmod.unionmatcher(matchers) result = matchmod.unionmatcher(matchers)
if includetemp: if includetemp:
tempincludes = readtemporaryincludes(repo) tempincludes = readtemporaryincludes(repo)
result = matchmod.forceincludematcher(result, tempincludes) result = forceincludematcher(result, tempincludes)
repo._sparsematchercache[key] = result repo._sparsematchercache[key] = result
return result return result
def filterupdatesactions(repo, wctx, mctx, branchmerge, actions): def filterupdatesactions(repo, wctx, mctx, branchmerge, actions):
"""Filter updates to only lay out files that match the sparse rules.""" """Filter updates to only lay out files that match the sparse rules."""
if not enabled: if not enabled: