( )⚙ D4365 match: make exactmatcher.visitchildrenset return file children as well

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4365

match: make exactmatcher.visitchildrenset return file children as well
ClosedPublic

Authored by spectral on Aug 24 2018, 1:19 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGc9a3f7f5c023: match: make exactmatcher.visitchildrenset return file children as well
Summary

Previously, if we had an exactmatcher like ['foo.txt', 'a/bar.txt', 'a/b/c/baz.txt'], we'd
get back the following data:

'.': {'a'}
'a': {'b'}
'a/b': {'c'}
'a/b/c': 'this'
'a/b/c/d': set()

This was incorrect, since visitchildrenset explicitly says not to pay attention
to 'foo.txt' and 'a/bar.txt' by not returning them or 'this'. Given the near
impossibility of making visitchildrenset reliabbly produce only subdirectories,
a previous commit has made it documented and expected that visitchildrenset can
return a set containing both files and subdirectories to visit, instead of
implying/requiring that visitchildrenset() return 'this' if there are files to
visit. This makes the code for exactmatcher match this clarified documentation.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

spectral created this revision.Aug 24 2018, 1:19 AM
Closed by commit rHGc9a3f7f5c023: match: make exactmatcher.visitchildrenset return file children as well (authored by spectral). · Explain WhyAug 24 2018, 1:21 PM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Aug 25 2018, 12:35 AM

+ if not self._fileset or dir not in self._dirs:

Just curious why you've added not self._fileset.

Except a vast style change, the important part is just this:

@@ -590,7 +590,7 @@
         if dir not in self._dirs:
             return set()
 
-        candidates = self._dirs - {'.'}
+        candidates = self._fileset | self._dirs - {'.'}
         if dir != '.':
             d = dir + '/'
             candidates = set(c[len(d):] for c in candidates if

Revision Contents
Changeset List

PathPackages
Mmercurial/match.py (35 lines)
Mtests/test-match.py (18 lines)

Diff 10543

mercurial/match.py

@propertycache @propertycache
def _dirs(self): def _dirs(self):
return set(util.dirs(self._fileset)) | {'.'} return set(util.dirs(self._fileset)) | {'.'}
def visitdir(self, dir): def visitdir(self, dir):
return dir in self._dirs return dir in self._dirs
def visitchildrenset(self, dir): def visitchildrenset(self, dir):
if dir in self._dirs: if not self._fileset or dir not in self._dirs:
candidates = self._dirs - {'.'} return set()
candidates = self._fileset | self._dirs - {'.'}
if dir != '.': if dir != '.':
d = dir + '/' d = dir + '/'
candidates = set(c[len(d):] for c in candidates if candidates = set(c[len(d):] for c in candidates if
c.startswith(d)) c.startswith(d))
# self._dirs includes all of the directories, recursively, so if # self._dirs includes all of the directories, recursively, so if
# we're attempting to match foo/bar/baz.txt, it'll have '.', 'foo', # we're attempting to match foo/bar/baz.txt, it'll have '.', 'foo',
# 'foo/bar' in it. Thus we can safely ignore a candidate that has a # 'foo/bar' in it. Thus we can safely ignore a candidate that has a
# '/' in it, indicating a it's for a subdir-of-a-subdir; the # '/' in it, indicating a it's for a subdir-of-a-subdir; the
# immediate subdir will be in there without a slash. # immediate subdir will be in there without a slash.
ret = set(c for c in candidates if '/' not in c) ret = set(c for c in candidates if '/' not in c)
# We need to emit 'this' for foo/bar, not set(), not {'baz.txt'}. # We really do not expect ret to be empty, since that would imply that
if not ret: # there's something in _dirs that didn't have a file in _fileset.
return 'this' assert ret
return ret return ret
return set()
def isexact(self): def isexact(self):
return True return True
@encoding.strmethod @encoding.strmethod
def __repr__(self): def __repr__(self):
return ('<exactmatcher files=%r>' % self._files) return ('<exactmatcher files=%r>' % self._files)

tests/test-match.py

self.assertFalse(m.visitdir(b'folder')) self.assertFalse(m.visitdir(b'folder'))
def testVisitchildrenset(self): def testVisitchildrenset(self):
m = matchmod.match(b'x', b'', patterns=[b'dir/subdir/foo.txt'], m = matchmod.match(b'x', b'', patterns=[b'dir/subdir/foo.txt'],
exact=True) exact=True)
assert isinstance(m, matchmod.exactmatcher) assert isinstance(m, matchmod.exactmatcher)
self.assertEqual(m.visitchildrenset(b'.'), {b'dir'}) self.assertEqual(m.visitchildrenset(b'.'), {b'dir'})
self.assertEqual(m.visitchildrenset(b'dir'), {b'subdir'}) self.assertEqual(m.visitchildrenset(b'dir'), {b'subdir'})
self.assertEqual(m.visitchildrenset(b'dir/subdir'), b'this') self.assertEqual(m.visitchildrenset(b'dir/subdir'), {b'foo.txt'})
self.assertEqual(m.visitchildrenset(b'dir/subdir/x'), set()) self.assertEqual(m.visitchildrenset(b'dir/subdir/x'), set())
self.assertEqual(m.visitchildrenset(b'dir/subdir/foo.txt'), set()) self.assertEqual(m.visitchildrenset(b'dir/subdir/foo.txt'), set())
self.assertEqual(m.visitchildrenset(b'folder'), set()) self.assertEqual(m.visitchildrenset(b'folder'), set())
def testVisitchildrensetFilesAndDirs(self):
m = matchmod.match(b'x', b'', patterns=[b'rootfile.txt',
b'a/file1.txt',
b'a/b/file2.txt',
# no file in a/b/c
b'a/b/c/d/file4.txt'],
exact=True)
assert isinstance(m, matchmod.exactmatcher)
self.assertEqual(m.visitchildrenset(b'.'), {b'a', b'rootfile.txt'})
self.assertEqual(m.visitchildrenset(b'a'), {b'b', b'file1.txt'})
self.assertEqual(m.visitchildrenset(b'a/b'), {b'c', b'file2.txt'})
self.assertEqual(m.visitchildrenset(b'a/b/c'), {b'd'})
self.assertEqual(m.visitchildrenset(b'a/b/c/d'), {b'file4.txt'})
self.assertEqual(m.visitchildrenset(b'a/b/c/d/e'), set())
self.assertEqual(m.visitchildrenset(b'folder'), set())
class DifferenceMatcherTests(unittest.TestCase): class DifferenceMatcherTests(unittest.TestCase):
def testVisitdirM2always(self): def testVisitdirM2always(self):
m1 = matchmod.alwaysmatcher(b'', b'') m1 = matchmod.alwaysmatcher(b'', b'')
m2 = matchmod.alwaysmatcher(b'', b'') m2 = matchmod.alwaysmatcher(b'', b'')
dm = matchmod.differencematcher(m1, m2) dm = matchmod.differencematcher(m1, m2)
# dm should be equivalent to a nevermatcher. # dm should be equivalent to a nevermatcher.
self.assertFalse(dm.visitdir(b'.')) self.assertFalse(dm.visitdir(b'.'))