This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8085

manifest: move matches method to be outside the interface
ClosedPublic

Authored by durin42 on Feb 5 2020, 5:34 PM.

Details

Reviewers
martinvonz
pulkit
Group Reviewers
hg-reviewers
Commits
rHG0bf3b5e80d30: manifest: move matches method to be outside the interface
Summary

In order to adequately smoke out any legacy consumers of the method, we rename
it to _matches so it's clear that it's class-private. To my amazement, all
consumers of this method really only wanted matching filenames, not a full
filtered manifest.

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

durin42 created this revision.Feb 5 2020, 5:34 PM
martinvonz requested changes to this revision.Feb 7 2020, 5:08 PM
martinvonz added a subscriber: martinvonz.
martinvonz added inline comments.
mercurial/manifest.py
1207

This now checks match.always() recursively for every tree manifest. Seems wasteful, but maybe not noticeable? Could you test (or just revert that part of the patch)?

This revision now requires changes to proceed.Feb 7 2020, 5:08 PM
durin42 updated this revision to Diff 20101.Feb 10 2020, 7:49 PM
pulkit accepted this revision.Feb 10 2020, 9:33 PM
durin42 added a commit: rHG0bf3b5e80d30: manifest: move matches method to be outside the interface.Feb 10 2020, 9:43 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG0bf3b5e80d30: manifest: move matches method to be outside the interface (authored by durin42). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 20142

mercurial/interfaces/repository.py

""" """
def dirs(): def dirs():
"""Returns an object implementing the ``idirs`` interface.""" """Returns an object implementing the ``idirs`` interface."""
def hasdir(dir): def hasdir(dir):
"""Returns a bool indicating if a directory is in this manifest.""" """Returns a bool indicating if a directory is in this manifest."""
def matches(match):
"""Generate a new manifest filtered through a matcher.
Returns an object conforming to the ``imanifestdict`` interface.
"""
def walk(match): def walk(match):
"""Generator of paths in manifest satisfying a matcher. """Generator of paths in manifest satisfying a matcher.
This is equivalent to ``self.matches(match).iterkeys()`` except a new
manifest object is not created.
If the matcher has explicit files listed and they don't exist in If the matcher has explicit files listed and they don't exist in
the manifest, ``match.bad()`` is called for each missing file. the manifest, ``match.bad()`` is called for each missing file.
""" """
def diff(other, match=None, clean=False): def diff(other, match=None, clean=False):
"""Find differences between this manifest and another. """Find differences between this manifest and another.
This manifest is compared to ``other``. This manifest is compared to ``other``.

mercurial/manifest.py

# for dirstate.walk, files=[''] means "walk the whole tree". # for dirstate.walk, files=[''] means "walk the whole tree".
# follow that here, too # follow that here, too
fset.discard(b'') fset.discard(b'')
for fn in sorted(fset): for fn in sorted(fset):
if not self.hasdir(fn): if not self.hasdir(fn):
match.bad(fn, None) match.bad(fn, None)
def matches(self, match): def _matches(self, match):
'''generate a new manifest filtered by the match argument''' '''generate a new manifest filtered by the match argument'''
if match.always(): if match.always():
return self.copy() return self.copy()
if self._filesfastpath(match): if self._filesfastpath(match):
m = manifestdict() m = manifestdict()
lm = self._lm lm = self._lm
for fn in match.files(): for fn in match.files():
The result is returned as a dict with filename as key and The result is returned as a dict with filename as key and
values of the form ((n1,fl1),(n2,fl2)), where n1/n2 is the values of the form ((n1,fl1),(n2,fl2)), where n1/n2 is the
nodeid in the current/other manifest and fl1/fl2 is the flag nodeid in the current/other manifest and fl1/fl2 is the flag
in the current/other manifest. Where the file does not exist, in the current/other manifest. Where the file does not exist,
the nodeid will be None and the flags will be the empty the nodeid will be None and the flags will be the empty
string. string.
''' '''
if match: if match:
m1 = self.matches(match) m1 = self._matches(match)
m2 = m2.matches(match) m2 = m2._matches(match)
return m1.diff(m2, clean=clean) return m1.diff(m2, clean=clean)
return self._lm.diff(m2._lm, clean) return self._lm.diff(m2._lm, clean)
def setflag(self, key, flag): def setflag(self, key, flag):
self._lm[key] = self[key], flag self._lm[key] = self[key], flag
def get(self, key, default=None): def get(self, key, default=None):
try: try:
copy._copyfunc = _copyfunc copy._copyfunc = _copyfunc
else: else:
copy._copyfunc = self._copyfunc copy._copyfunc = self._copyfunc
return copy return copy
def filesnotin(self, m2, match=None): def filesnotin(self, m2, match=None):
'''Set of files in this manifest that are not in the other''' '''Set of files in this manifest that are not in the other'''
if match and not match.always(): if match and not match.always():
m1 = self.matches(match) m1 = self._matches(match)
m2 = m2.matches(match) m2 = m2._matches(match)
return m1.filesnotin(m2) return m1.filesnotin(m2)
files = set() files = set()
def _filesnotin(t1, t2): def _filesnotin(t1, t2):
if t1._node == t2._node and not t1._dirty and not t2._dirty: if t1._node == t2._node and not t1._dirty and not t2._dirty:
return return
t1._load() t1._load()
return self._dirs[topdir].hasdir(subdir) return self._dirs[topdir].hasdir(subdir)
return False return False
dirslash = dir + b'/' dirslash = dir + b'/'
return dirslash in self._dirs or dirslash in self._lazydirs return dirslash in self._dirs or dirslash in self._lazydirs
def walk(self, match): def walk(self, match):
'''Generates matching file names. '''Generates matching file names.
Equivalent to manifest.matches(match).iterkeys(), but without creating
an entirely new manifest.
It also reports nonexistent files by marking them bad with match.bad(). It also reports nonexistent files by marking them bad with match.bad().
''' '''
if match.always(): if match.always():
for f in iter(self): for f in iter(self):
yield f yield f
return return
fset = set(match.files()) fset = set(match.files())
fullp = self._subpath(p) fullp = self._subpath(p)
if match(fullp): if match(fullp):
yield fullp yield fullp
else: else:
if not visit or p[:-1] in visit: if not visit or p[:-1] in visit:
for f in self._dirs[p]._walk(match): for f in self._dirs[p]._walk(match):
yield f yield f
def matches(self, match):
'''generate a new manifest filtered by the match argument'''
if match.always():
return self.copy()
return self._matches(match)
def _matches(self, match): def _matches(self, match):
'''recursively generate a new manifest filtered by the match argument. '''recursively generate a new manifest filtered by the match argument.
''' '''
if match.always():
return self.copy()
return self._matches_inner(match)
def _matches_inner(self, match):
if match.always():
return self.copy()
visit = match.visitchildrenset(self._dir[:-1]) visit = match.visitchildrenset(self._dir[:-1])
if visit == b'all': if visit == b'all':
return self.copy() return self.copy()
ret = treemanifest(self._dir) ret = treemanifest(self._dir)
if not visit: if not visit:
return ret return ret
ret._files[fn] = self._files[fn] ret._files[fn] = self._files[fn]
if fn in self._flags: if fn in self._flags:
ret._flags[fn] = self._flags[fn] ret._flags[fn] = self._flags[fn]
visit = self._loadchildrensetlazy(visit) visit = self._loadchildrensetlazy(visit)
for dir, subm in pycompat.iteritems(self._dirs): for dir, subm in pycompat.iteritems(self._dirs):
if visit and dir[:-1] not in visit: if visit and dir[:-1] not in visit:
continue continue
m = subm._matches(match) m = subm._matches_inner(match)
martinvonzUnsubmitted
Not Done

This now checks match.always() recursively for every tree manifest. Seems wasteful, but maybe not noticeable? Could you test (or just revert that part of the patch)?

martinvonz: This now checks `match.always()` recursively for every tree manifest. Seems wasteful, but maybe…
if not m._isempty(): if not m._isempty():
ret._dirs[dir] = m ret._dirs[dir] = m
if not ret._isempty(): if not ret._isempty():
ret._dirty = True ret._dirty = True
return ret return ret
def diff(self, m2, match=None, clean=False): def diff(self, m2, match=None, clean=False):
'''Finds changes between the current manifest and m2. '''Finds changes between the current manifest and m2.
Args: Args:
m2: the manifest to which this manifest should be compared. m2: the manifest to which this manifest should be compared.
clean: if true, include files unchanged between these manifests clean: if true, include files unchanged between these manifests
with a None value in the returned dictionary. with a None value in the returned dictionary.
The result is returned as a dict with filename as key and The result is returned as a dict with filename as key and
values of the form ((n1,fl1),(n2,fl2)), where n1/n2 is the values of the form ((n1,fl1),(n2,fl2)), where n1/n2 is the
nodeid in the current/other manifest and fl1/fl2 is the flag nodeid in the current/other manifest and fl1/fl2 is the flag
in the current/other manifest. Where the file does not exist, in the current/other manifest. Where the file does not exist,
the nodeid will be None and the flags will be the empty the nodeid will be None and the flags will be the empty
string. string.
''' '''
if match and not match.always(): if match and not match.always():
m1 = self.matches(match) m1 = self._matches(match)
m2 = m2.matches(match) m2 = m2._matches(match)
return m1.diff(m2, clean=clean) return m1.diff(m2, clean=clean)
result = {} result = {}
emptytree = treemanifest() emptytree = treemanifest()
def _iterativediff(t1, t2, stack): def _iterativediff(t1, t2, stack):
"""compares two tree manifests and append new tree-manifests which """compares two tree manifests and append new tree-manifests which
needs to be compared to stack""" needs to be compared to stack"""
if t1._node == t2._node and not t1._dirty and not t2._dirty: if t1._node == t2._node and not t1._dirty and not t2._dirty:

tests/test-manifest.py

list(m.items()), list(m.items()),
) )
# Sometimes it even tries a 22-byte fake hash, but we can # Sometimes it even tries a 22-byte fake hash, but we can
# return 21 and it'll work out # return 21 and it'll work out
m[b'foo'] = want + b'+' m[b'foo'] = want + b'+'
self.assertEqual(want, m[b'foo']) self.assertEqual(want, m[b'foo'])
# make sure the suffix survives a copy # make sure the suffix survives a copy
match = matchmod.match(util.localpath(b'/repo'), b'', [b're:foo']) match = matchmod.match(util.localpath(b'/repo'), b'', [b're:foo'])
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual(want, m2[b'foo']) self.assertEqual(want, m2[b'foo'])
self.assertEqual(1, len(m2)) self.assertEqual(1, len(m2))
m2 = m.copy() m2 = m.copy()
self.assertEqual(want, m2[b'foo']) self.assertEqual(want, m2[b'foo'])
# suffix with iteration # suffix with iteration
self.assertEqual( self.assertEqual(
[(b'bar/baz/qux.py', BIN_HASH_2), (b'foo', want)], list(m.items()) [(b'bar/baz/qux.py', BIN_HASH_2), (b'foo', want)], list(m.items())
) )
# shows up in diff # shows up in diff
self.assertEqual({b'foo': ((want, f), (h, b''))}, m.diff(clean)) self.assertEqual({b'foo': ((want, f), (h, b''))}, m.diff(clean))
self.assertEqual({b'foo': ((h, b''), (want, f))}, clean.diff(m)) self.assertEqual({b'foo': ((h, b''), (want, f))}, clean.diff(m))
def testMatchException(self): def testMatchException(self):
m = self.parsemanifest(A_SHORT_MANIFEST) m = self.parsemanifest(A_SHORT_MANIFEST)
match = matchmod.match(util.localpath(b'/repo'), b'', [b're:.*']) match = matchmod.match(util.localpath(b'/repo'), b'', [b're:.*'])
def filt(path): def filt(path):
if path == b'foo': if path == b'foo':
assert False assert False
return True return True
match.matchfn = filt match.matchfn = filt
with self.assertRaises(AssertionError): with self.assertRaises(AssertionError):
m.matches(match) m._matches(match)
def testRemoveItem(self): def testRemoveItem(self):
m = self.parsemanifest(A_SHORT_MANIFEST) m = self.parsemanifest(A_SHORT_MANIFEST)
del m[b'foo'] del m[b'foo']
with self.assertRaises(KeyError): with self.assertRaises(KeyError):
m[b'foo'] m[b'foo']
self.assertEqual(1, len(m)) self.assertEqual(1, len(m))
self.assertEqual(1, len(list(m))) self.assertEqual(1, len(list(m)))
def testMatchesMetadata(self): def testMatchesMetadata(self):
'''Tests matches() for a few specific files to make sure that both '''Tests matches() for a few specific files to make sure that both
the set of files as well as their flags and nodeids are correct in the set of files as well as their flags and nodeids are correct in
the resulting manifest.''' the resulting manifest.'''
m = self.parsemanifest(A_HUGE_MANIFEST) m = self.parsemanifest(A_HUGE_MANIFEST)
match = matchmod.exact([b'file1', b'file200', b'file300']) match = matchmod.exact([b'file1', b'file200', b'file300'])
m2 = m.matches(match) m2 = m._matches(match)
w = (b'file1\0%sx\n' b'file200\0%sl\n' b'file300\0%s\n') % ( w = (b'file1\0%sx\n' b'file200\0%sl\n' b'file300\0%s\n') % (
HASH_2, HASH_2,
HASH_1, HASH_1,
HASH_1, HASH_1,
) )
self.assertEqual(w, m2.text()) self.assertEqual(w, m2.text())
def testMatchesNonexistentFile(self): def testMatchesNonexistentFile(self):
'''Tests matches() for a small set of specific files, including one '''Tests matches() for a small set of specific files, including one
nonexistent file to make sure in only matches against existing files. nonexistent file to make sure in only matches against existing files.
''' '''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.exact( match = matchmod.exact(
[b'a/b/c/bar.txt', b'a/b/d/qux.py', b'readme.txt', b'nonexistent'] [b'a/b/c/bar.txt', b'a/b/d/qux.py', b'readme.txt', b'nonexistent']
) )
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual( self.assertEqual(
[b'a/b/c/bar.txt', b'a/b/d/qux.py', b'readme.txt'], m2.keys() [b'a/b/c/bar.txt', b'a/b/d/qux.py', b'readme.txt'], m2.keys()
) )
def testMatchesNonexistentDirectory(self): def testMatchesNonexistentDirectory(self):
'''Tests matches() for a relpath match on a directory that doesn't '''Tests matches() for a relpath match on a directory that doesn't
actually exist.''' actually exist.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.match( match = matchmod.match(
util.localpath(b'/repo'), b'', [b'a/f'], default=b'relpath' util.localpath(b'/repo'), b'', [b'a/f'], default=b'relpath'
) )
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual([], m2.keys()) self.assertEqual([], m2.keys())
def testMatchesExactLarge(self): def testMatchesExactLarge(self):
'''Tests matches() for files matching a large list of exact files. '''Tests matches() for files matching a large list of exact files.
''' '''
m = self.parsemanifest(A_HUGE_MANIFEST) m = self.parsemanifest(A_HUGE_MANIFEST)
flist = m.keys()[80:300] flist = m.keys()[80:300]
match = matchmod.exact(flist) match = matchmod.exact(flist)
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual(flist, m2.keys()) self.assertEqual(flist, m2.keys())
def testMatchesFull(self): def testMatchesFull(self):
'''Tests matches() for what should be a full match.''' '''Tests matches() for what should be a full match.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.match(util.localpath(b'/repo'), b'', [b'']) match = matchmod.match(util.localpath(b'/repo'), b'', [b''])
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual(m.keys(), m2.keys()) self.assertEqual(m.keys(), m2.keys())
def testMatchesDirectory(self): def testMatchesDirectory(self):
'''Tests matches() on a relpath match on a directory, which should '''Tests matches() on a relpath match on a directory, which should
match against all files within said directory.''' match against all files within said directory.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.match( match = matchmod.match(
util.localpath(b'/repo'), b'', [b'a/b'], default=b'relpath' util.localpath(b'/repo'), b'', [b'a/b'], default=b'relpath'
) )
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual( self.assertEqual(
[ [
b'a/b/c/bar.py', b'a/b/c/bar.py',
b'a/b/c/bar.txt', b'a/b/c/bar.txt',
b'a/b/c/foo.py', b'a/b/c/foo.py',
b'a/b/c/foo.txt', b'a/b/c/foo.txt',
b'a/b/d/baz.py', b'a/b/d/baz.py',
b'a/b/d/qux.py', b'a/b/d/qux.py',
b'a/b/d/ten.txt', b'a/b/d/ten.txt',
b'a/b/dog.py', b'a/b/dog.py',
b'a/b/fish.py', b'a/b/fish.py',
], ],
m2.keys(), m2.keys(),
) )
def testMatchesExactPath(self): def testMatchesExactPath(self):
'''Tests matches() on an exact match on a directory, which should '''Tests matches() on an exact match on a directory, which should
result in an empty manifest because you can't perform an exact match result in an empty manifest because you can't perform an exact match
against a directory.''' against a directory.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.exact([b'a/b']) match = matchmod.exact([b'a/b'])
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual([], m2.keys()) self.assertEqual([], m2.keys())
def testMatchesCwd(self): def testMatchesCwd(self):
'''Tests matches() on a relpath match with the current directory ('.') '''Tests matches() on a relpath match with the current directory ('.')
when not in the root directory.''' when not in the root directory.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.match( match = matchmod.match(
util.localpath(b'/repo'), b'a/b', [b'.'], default=b'relpath' util.localpath(b'/repo'), b'a/b', [b'.'], default=b'relpath'
) )
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual( self.assertEqual(
[ [
b'a/b/c/bar.py', b'a/b/c/bar.py',
b'a/b/c/bar.txt', b'a/b/c/bar.txt',
b'a/b/c/foo.py', b'a/b/c/foo.py',
b'a/b/c/foo.txt', b'a/b/c/foo.txt',
b'a/b/d/baz.py', b'a/b/d/baz.py',
b'a/b/d/qux.py', b'a/b/d/qux.py',
b'a/b/d/ten.txt', b'a/b/d/ten.txt',
b'a/b/dog.py', b'a/b/dog.py',
b'a/b/fish.py', b'a/b/fish.py',
], ],
m2.keys(), m2.keys(),
) )
def testMatchesWithPattern(self): def testMatchesWithPattern(self):
'''Tests matches() for files matching a pattern that reside '''Tests matches() for files matching a pattern that reside
deeper than the specified directory.''' deeper than the specified directory.'''
m = self.parsemanifest(A_DEEPER_MANIFEST) m = self.parsemanifest(A_DEEPER_MANIFEST)
match = matchmod.match(util.localpath(b'/repo'), b'', [b'a/b/*/*.txt']) match = matchmod.match(util.localpath(b'/repo'), b'', [b'a/b/*/*.txt'])
m2 = m.matches(match) m2 = m._matches(match)
self.assertEqual( self.assertEqual(
[b'a/b/c/bar.txt', b'a/b/c/foo.txt', b'a/b/d/ten.txt'], m2.keys() [b'a/b/c/bar.txt', b'a/b/c/foo.txt', b'a/b/d/ten.txt'], m2.keys()
) )
class testmanifestdict(unittest.TestCase, basemanifesttests): class testmanifestdict(unittest.TestCase, basemanifesttests):
def parsemanifest(self, text): def parsemanifest(self, text):