match: teach diffmatcher.visitdir() to return 'all' if possible
This patch teaches differencematcher.visitdir() to return 'all' when
m1.visitdir() returns 'all' and m2 does not matches.
Before this patch, from a differencematcher.visitdir(), we always returned
either True or False. We never returned 'all' even when we can. This causes
problem when m1 and m2 of a differencematcher are themselves differencematcher.
In that case, we try to check:
`if self._m2_.visitdir(dir) == 'all'`
which will never be 'all' even though it can be.
This leads to iterating over a lot of sub-directory manifest, even though we
don't want to while extending a narrow clone. I am yet to measure the impact of
this but calculating manifest was taking ~50-60 seconds, so this should
definitely save some of time there.
Differential Revision: https://phab.mercurial-scm.org/D5814
--- a/mercurial/match.py Sun Feb 03 10:01:43 2019 +0100
+++ b/mercurial/match.py Mon Feb 04 18:14:03 2019 +0300
@@ -677,6 +677,9 @@
def visitdir(self, dir):
if self._m2.visitdir(dir) == 'all':
return False
+ elif not self._m2.visitdir(dir):
+ # m2 does not match dir, we can return 'all' here if possible
+ return self._m1.visitdir(dir)
return bool(self._m1.visitdir(dir))
def visitchildrenset(self, dir):
--- a/tests/test-match.py Sun Feb 03 10:01:43 2019 +0100
+++ b/tests/test-match.py Mon Feb 04 18:14:03 2019 +0300
@@ -255,20 +255,19 @@
m1 = matchmod.alwaysmatcher(b'', b'')
m2 = matchmod.nevermatcher(b'', b'')
dm = matchmod.differencematcher(m1, m2)
- # dm should be equivalent to a alwaysmatcher. OPT: if m2 is a
- # nevermatcher, we could return 'all' for these.
+ # dm should be equivalent to a alwaysmatcher.
#
# We're testing Equal-to-True instead of just 'assertTrue' since
# assertTrue does NOT verify that it's a bool, just that it's truthy.
# While we may want to eventually make these return 'all', they should
# not currently do so.
- self.assertEqual(dm.visitdir(b'.'), True)
- self.assertEqual(dm.visitdir(b'dir'), True)
- self.assertEqual(dm.visitdir(b'dir/subdir'), True)
- self.assertEqual(dm.visitdir(b'dir/subdir/z'), True)
- self.assertEqual(dm.visitdir(b'dir/foo'), True)
- self.assertEqual(dm.visitdir(b'dir/subdir/x'), True)
- self.assertEqual(dm.visitdir(b'folder'), True)
+ self.assertEqual(dm.visitdir(b'.'), 'all')
+ self.assertEqual(dm.visitdir(b'dir'), 'all')
+ self.assertEqual(dm.visitdir(b'dir/subdir'), 'all')
+ self.assertEqual(dm.visitdir(b'dir/subdir/z'), 'all')
+ self.assertEqual(dm.visitdir(b'dir/foo'), 'all')
+ self.assertEqual(dm.visitdir(b'dir/subdir/x'), 'all')
+ self.assertEqual(dm.visitdir(b'folder'), 'all')
def testVisitchildrensetM2never(self):
m1 = matchmod.alwaysmatcher(b'', b'')
@@ -295,9 +294,8 @@
# an 'all' pattern, just True.
self.assertEqual(dm.visitdir(b'dir/subdir/z'), True)
self.assertEqual(dm.visitdir(b'dir/subdir/x'), True)
- # OPT: We could return 'all' for these.
- self.assertEqual(dm.visitdir(b'dir/foo'), True)
- self.assertEqual(dm.visitdir(b'folder'), True)
+ self.assertEqual(dm.visitdir(b'dir/foo'), 'all')
+ self.assertEqual(dm.visitdir(b'folder'), 'all')
def testVisitchildrensetM2SubdirPrefix(self):
m1 = matchmod.alwaysmatcher(b'', b'')
@@ -322,7 +320,7 @@
dm = matchmod.differencematcher(m1, m2)
self.assertEqual(dm.visitdir(b'.'), True)
self.assertEqual(dm.visitdir(b'dir'), True)
- self.assertEqual(dm.visitdir(b'dir/subdir'), True)
+ self.assertEqual(dm.visitdir(b'dir/subdir'), 'all')
self.assertFalse(dm.visitdir(b'dir/foo'))
self.assertFalse(dm.visitdir(b'folder'))
# OPT: We should probably return False for these; we don't because