# HG changeset patch # User Pulkit Goyal # Date 2019-02-04 15:14:03 # Node ID 990aa150fd028d6ec84d3c703ab22995f016c38c # Parent 5718a7dec7364e225f72f476e84676d1005cb60a 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 diff --git a/mercurial/match.py b/mercurial/match.py --- a/mercurial/match.py +++ b/mercurial/match.py @@ -677,6 +677,9 @@ class differencematcher(basematcher): 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): diff --git a/tests/test-match.py b/tests/test-match.py --- a/tests/test-match.py +++ b/tests/test-match.py @@ -255,20 +255,19 @@ class DifferenceMatcherTests(unittest.Te 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 @@ class DifferenceMatcherTests(unittest.Te # 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 @@ class DifferenceMatcherTests(unittest.Te 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