Bug 1335309 - Add explicit find_executables arguments to every use of FileFinder. r?mshal draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 31 Jan 2017 13:01:34 +0900
changeset 468370 1fd4b9d395d43af02fad500984e1e37d59b65692
parent 468367 9c6057cde326f707076f4bda766f31df4888c5bb
child 468371 c292b0b2469259a45ae306836be5468cb772a784
push id43441
push userbmo:mh+mozilla@glandium.org
push dateTue, 31 Jan 2017 06:11:03 +0000
reviewersmshal
bugs1335309
milestone54.0a1
Bug 1335309 - Add explicit find_executables arguments to every use of FileFinder. r?mshal And make it an error not to give it. While the default is True, we do pass a value of False wherever it makes sense, which happens to be, in most places. This is an intermediate step to flip the default from True to False, ensuring that we don't unwantedly switch some calls to False.
python/mozbuild/mozbuild/action/generate_suggestedsites.py
python/mozbuild/mozbuild/action/zip.py
python/mozbuild/mozbuild/backend/android_eclipse.py
python/mozbuild/mozbuild/codecoverage/packager.py
python/mozbuild/mozbuild/test/backend/test_build.py
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/packager/unpack.py
python/mozbuild/mozpack/test/test_files.py
python/mozbuild/mozpack/test/test_mozjar.py
python/mozbuild/mozpack/test/test_unify.py
toolkit/mozapps/installer/packager.py
toolkit/mozapps/installer/strip.py
--- a/python/mozbuild/mozbuild/action/generate_suggestedsites.py
+++ b/python/mozbuild/mozbuild/action/generate_suggestedsites.py
@@ -97,17 +97,17 @@ def main(args):
             sites.append(site)
 
             # Now check for existence of an appropriately named drawable.  If none
             # exists, throw.  This stops a locale discovering, at runtime, that the
             # corresponding drawable was not added to en-US.
             if not opts.resources:
                 continue
             resources = os.path.abspath(opts.resources)
-            finder = FileFinder(resources)
+            finder = FileFinder(resources, find_executables=False)
             matches = [p for p, _ in finder.find(drawables_template.format(name=name))]
             if not matches:
                 raise Exception("Could not find drawable in '{resources}' for '{name}'"
                     .format(resources=resources, name=name))
             else:
                 if opts.verbose:
                     print("Found {len} drawables in '{resources}' for '{name}': {matches}"
                           .format(len=len(matches), resources=resources, name=name, matches=matches))
--- a/python/mozbuild/mozbuild/action/zip.py
+++ b/python/mozbuild/mozbuild/action/zip.py
@@ -23,17 +23,17 @@ def main(args):
     parser.add_argument("zip", help="Path to zip file to write")
     parser.add_argument("input", nargs="+",
                         help="Path to files to add to zip")
     args = parser.parse_args(args)
 
     jarrer = Jarrer(optimize=False)
 
     with errors.accumulate():
-        finder = FileFinder(args.C)
+        finder = FileFinder(args.C, find_executables=False)
         for path in args.input:
             for p, f in finder.find(path):
                 jarrer.add(p, f)
         jarrer.copy(mozpath.join(args.C, args.zip))
 
 
 if __name__ == '__main__':
     main(sys.argv[1:])
--- a/python/mozbuild/mozbuild/backend/android_eclipse.py
+++ b/python/mozbuild/mozbuild/backend/android_eclipse.py
@@ -242,17 +242,17 @@ class AndroidEclipseBackend(CommonBacken
             filteredResources = self._Element_for_filtered_resources(data.filtered_resources)
             defines['IDE_PROJECT_FILTERED_RESOURCES'] = pretty_print(filteredResources).strip()
         else:
             defines['IDE_PROJECT_FILTERED_RESOURCES'] = ''
         defines['ANDROID_TARGET_SDK'] = self.environment.substs['ANDROID_TARGET_SDK']
         defines['MOZ_ANDROID_MIN_SDK_VERSION'] = self.environment.defines['MOZ_ANDROID_MIN_SDK_VERSION']
 
         copier = FileCopier()
-        finder = FileFinder(template_directory)
+        finder = FileFinder(template_directory, find_executables=False)
         for input_filename, f in itertools.chain(finder.find('**'), finder.find('.**')):
             if input_filename == 'AndroidManifest.xml' and not data.is_library:
                 # Main projects supply their own manifests.
                 continue
             copier.add(input_filename, PreprocessedFile(
                 mozpath.join(finder.base, input_filename),
                 depfile_path=None,
                 marker='#',
--- a/python/mozbuild/mozbuild/codecoverage/packager.py
+++ b/python/mozbuild/mozbuild/codecoverage/packager.py
@@ -10,17 +10,17 @@ import sys
 from mozpack.files import FileFinder
 from mozpack.copier import Jarrer
 
 def package_gcno_tree(root, output_file):
     # XXX JarWriter doesn't support unicode strings, see bug 1056859
     if isinstance(root, unicode):
         root = root.encode('utf-8')
 
-    finder = FileFinder(root)
+    finder = FileFinder(root, find_executables=False)
     jarrer = Jarrer(optimize=False)
     for p, f in finder.find("**/*.gcno"):
         jarrer.add(p, f)
     jarrer.copy(output_file)
 
 
 def cli(args=sys.argv[1:]):
     parser = argparse.ArgumentParser()
--- a/python/mozbuild/mozbuild/test/backend/test_build.py
+++ b/python/mozbuild/mozbuild/test/backend/test_build.py
@@ -147,17 +147,17 @@ class TestBuild(unittest.TestCase):
         test_path = os.path.join(os.path.dirname(os.path.abspath(__file__)),
                                  'data', 'build') + os.sep
 
         # We want unicode instances out of the files, because having plain str
         # makes assertEqual diff output in case of error extra verbose because
         # of the difference in type.
         result = {
             p: f.open().read().decode('utf-8')
-            for p, f in FileFinder(mozpath.join(config.topobjdir, 'dist'))
+            for p, f in FileFinder(mozpath.join(config.topobjdir, 'dist'), find_executables=False)
         }
         self.assertTrue(len(result))
         self.assertEqual(result, {
             'bin/baz.ini': 'baz.ini: FOO is foo\n',
             'bin/child/bar.ini': 'bar.ini\n',
             'bin/child2/foo.css': 'foo.css: FOO is foo\n',
             'bin/child2/qux.ini': 'qux.ini: BAR is not defined\n',
             'bin/chrome.manifest':
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -836,31 +836,33 @@ class BaseFinder(object):
                 if mozpath.basedir(p, [pattern]) == pattern:
                     yield p, file_getter(p)
 
 
 class FileFinder(BaseFinder):
     '''
     Helper to get appropriate BaseFile instances from the file system.
     '''
-    def __init__(self, base, find_executables=True, ignore=(),
+    def __init__(self, base, ignore=(),
                  find_dotfiles=False, **kargs):
         '''
         Create a FileFinder for files under the given base directory.
 
         The find_executables argument determines whether the finder needs to
         try to guess whether files are executables. Disabling this guessing
         when not necessary can speed up the finder significantly.
 
         ``ignore`` accepts an iterable of patterns to ignore. Entries are
         strings that match paths relative to ``base`` using
         ``mozpath.match()``. This means if an entry corresponds
         to a directory, all files under that directory will be ignored. If
         an entry corresponds to a file, that particular file will be ignored.
         '''
+        assert 'find_executables' in kargs
+        find_executables = kargs.pop('find_executables')
         BaseFinder.__init__(self, base, **kargs)
         self.find_dotfiles = find_dotfiles
         self.find_executables = find_executables
         self.ignore = ignore
 
     def _find(self, pattern):
         '''
         Actual implementation of FileFinder.find(), dispatching to specialized
--- a/python/mozbuild/mozpack/packager/unpack.py
+++ b/python/mozbuild/mozpack/packager/unpack.py
@@ -42,17 +42,17 @@ class UnpackFinder(BaseFinder):
     The only argument to the constructor is a Finder instance or a path.
     The UnpackFinder is populated with files from this Finder instance,
     or with files from a FileFinder using the given path as its root.
     '''
     def __init__(self, source):
         if isinstance(source, BaseFinder):
             self._finder = source
         else:
-            self._finder = FileFinder(source)
+            self._finder = FileFinder(source, find_executables=False)
         self.base = self._finder.base
         self.files = FileRegistry()
         self.kind = 'flat'
         self.omnijar = None
         self.jarlogs = {}
         self.optimizedjars = False
         self.compressed = True
 
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -919,39 +919,39 @@ class TestFileFinder(MatchTestTemplate, 
         ensureParentDir(self.tmppath(path))
         open(self.tmppath(path), 'wb').write(path)
 
     def do_check(self, pattern, result):
         do_check(self, self.finder, pattern, result)
 
     def test_file_finder(self):
         self.prepare_match_test(with_dotfiles=True)
-        self.finder = FileFinder(self.tmpdir)
+        self.finder = FileFinder(self.tmpdir, find_executables=False)
         self.do_match_test()
         self.do_finder_test(self.finder)
 
     def test_get(self):
         self.prepare_match_test()
-        finder = FileFinder(self.tmpdir)
+        finder = FileFinder(self.tmpdir, find_executables=False)
 
         self.assertIsNone(finder.get('does-not-exist'))
         res = finder.get('bar')
         self.assertIsInstance(res, File)
         self.assertEqual(mozpath.normpath(res.path),
                          mozpath.join(self.tmpdir, 'bar'))
 
     def test_ignored_dirs(self):
         """Ignored directories should not have results returned."""
         self.prepare_match_test()
         self.add('fooz')
 
         # Present to ensure prefix matching doesn't exclude.
         self.add('foo/quxz')
 
-        self.finder = FileFinder(self.tmpdir, ignore=['foo/qux'])
+        self.finder = FileFinder(self.tmpdir, ignore=['foo/qux'], find_executables=False)
 
         self.do_check('**', ['bar', 'foo/bar', 'foo/baz', 'foo/quxz', 'fooz'])
         self.do_check('foo/*', ['foo/bar', 'foo/baz', 'foo/quxz'])
         self.do_check('foo/**', ['foo/bar', 'foo/baz', 'foo/quxz'])
         self.do_check('foo/qux/**', [])
         self.do_check('foo/qux/*', [])
         self.do_check('foo/qux/bar', [])
         self.do_check('foo/quxz', ['foo/quxz'])
@@ -959,44 +959,44 @@ class TestFileFinder(MatchTestTemplate, 
 
     def test_ignored_files(self):
         """Ignored files should not have results returned."""
         self.prepare_match_test()
 
         # Be sure prefix match doesn't get ignored.
         self.add('barz')
 
-        self.finder = FileFinder(self.tmpdir, ignore=['foo/bar', 'bar'])
+        self.finder = FileFinder(self.tmpdir, ignore=['foo/bar', 'bar'], find_executables=False)
         self.do_check('**', ['barz', 'foo/baz', 'foo/qux/1', 'foo/qux/2/test',
             'foo/qux/2/test2', 'foo/qux/bar'])
         self.do_check('foo/**', ['foo/baz', 'foo/qux/1', 'foo/qux/2/test',
             'foo/qux/2/test2', 'foo/qux/bar'])
 
     def test_ignored_patterns(self):
         """Ignore entries with patterns should be honored."""
         self.prepare_match_test()
 
         self.add('foo/quxz')
 
-        self.finder = FileFinder(self.tmpdir, ignore=['foo/qux/*'])
+        self.finder = FileFinder(self.tmpdir, ignore=['foo/qux/*'], find_executables=False)
         self.do_check('**', ['foo/bar', 'foo/baz', 'foo/quxz', 'bar'])
         self.do_check('foo/**', ['foo/bar', 'foo/baz', 'foo/quxz'])
 
     def test_dotfiles(self):
         """Finder can find files beginning with . is configured."""
         self.prepare_match_test(with_dotfiles=True)
-        self.finder = FileFinder(self.tmpdir, find_dotfiles=True)
+        self.finder = FileFinder(self.tmpdir, find_dotfiles=True, find_executables=False)
         self.do_check('**', ['bar', 'foo/.foo', 'foo/.bar/foo',
             'foo/bar', 'foo/baz', 'foo/qux/1', 'foo/qux/bar',
             'foo/qux/2/test', 'foo/qux/2/test2'])
 
     def test_dotfiles_plus_ignore(self):
         self.prepare_match_test(with_dotfiles=True)
         self.finder = FileFinder(self.tmpdir, find_dotfiles=True,
-                                 ignore=['foo/.bar/**'])
+                                 ignore=['foo/.bar/**'], find_executables=False)
         self.do_check('foo/**', ['foo/.foo', 'foo/bar', 'foo/baz',
             'foo/qux/1', 'foo/qux/bar', 'foo/qux/2/test', 'foo/qux/2/test2'])
 
 
 class TestJarFinder(MatchTestTemplate, TestWithTmpDir):
     def add(self, path):
         self.jar.add(path, path, compress=True)
 
@@ -1055,18 +1055,18 @@ class TestComposedFinder(MatchTestTempla
     def test_composed_finder(self):
         self.prepare_match_test()
         # Also add files in $tmp/a/foo/qux because ComposedFinder is
         # expected to mask foo/qux entirely with content from $tmp/b.
         ensureParentDir(self.tmppath('a/foo/qux/hoge'))
         open(self.tmppath('a/foo/qux/hoge'), 'wb').write('hoge')
         open(self.tmppath('a/foo/qux/bar'), 'wb').write('not the right content')
         self.finder = ComposedFinder({
-            '': FileFinder(self.tmppath('a')),
-            'foo/qux': FileFinder(self.tmppath('b')),
+            '': FileFinder(self.tmppath('a'), find_executables=False),
+            'foo/qux': FileFinder(self.tmppath('b'), find_executables=False),
         })
         self.do_match_test()
 
         self.assertIsNone(self.finder.get('does-not-exist'))
         self.assertIsInstance(self.finder.get('bar'), File)
 
 
 @unittest.skipUnless(hglib, 'hglib not available')
--- a/python/mozbuild/mozpack/test/test_mozjar.py
+++ b/python/mozbuild/mozpack/test/test_mozjar.py
@@ -240,17 +240,17 @@ class TestJar(unittest.TestCase):
 
         self.assertEqual(files[2].filename, 'baz/qux')
         self.assertTrue(files[2].compressed)
         self.assertEqual(files[2].read(), 'aaaaaaaaaaaaanopqrstuvwxyz')
 
     def test_add_from_finder(self):
         s = MockDest()
         with JarWriter(fileobj=s, optimize=self.optimize) as jar:
-            finder = FileFinder(test_data_path)
+            finder = FileFinder(test_data_path, find_executables=False)
             for p, f in finder.find('test_data'):
                 jar.add('test_data', f)
 
         jar = JarReader(fileobj=s)
         files = [j for j in jar]
 
         self.assertEqual(files[0].filename, 'test_data')
         self.assertFalse(files[0].compressed)
--- a/python/mozbuild/mozpack/test/test_unify.py
+++ b/python/mozbuild/mozpack/test/test_unify.py
@@ -41,35 +41,35 @@ class TestUnifiedFinder(TestUnified):
         self.create_one('a', 'bar', 'bar')
         self.create_one('b', 'baz', 'baz')
         self.create_one('a', 'qux', 'foobar')
         self.create_one('b', 'qux', 'baz')
         self.create_one('a', 'test/foo', 'a\nb\nc\n')
         self.create_one('b', 'test/foo', 'b\nc\na\n')
         self.create_both('test/bar', 'a\nb\nc\n')
 
-        finder = UnifiedFinder(FileFinder(self.tmppath('a')),
-                               FileFinder(self.tmppath('b')),
+        finder = UnifiedFinder(FileFinder(self.tmppath('a'), find_executables=False),
+                               FileFinder(self.tmppath('b'), find_executables=False),
                                sorted=['test'])
         self.assertEqual(sorted([(f, c.open().read())
                                  for f, c in finder.find('foo')]),
                          [('foo/bar', 'foobar'), ('foo/baz', 'foobaz')])
         self.assertRaises(ErrorMessage, any, finder.find('bar'))
         self.assertRaises(ErrorMessage, any, finder.find('baz'))
         self.assertRaises(ErrorMessage, any, finder.find('qux'))
         self.assertEqual(sorted([(f, c.open().read())
                                  for f, c in finder.find('test')]),
                          [('test/bar', 'a\nb\nc\n'),
                           ('test/foo', 'a\nb\nc\n')])
 
 
 class TestUnifiedBuildFinder(TestUnified):
     def test_unified_build_finder(self):
-        finder = UnifiedBuildFinder(FileFinder(self.tmppath('a')),
-                                    FileFinder(self.tmppath('b')))
+        finder = UnifiedBuildFinder(FileFinder(self.tmppath('a'), find_executables=False),
+                                    FileFinder(self.tmppath('b'), find_executables=False))
 
         # Test chrome.manifest unification
         self.create_both('chrome.manifest', 'a\nb\nc\n')
         self.create_one('a', 'chrome/chrome.manifest', 'a\nb\nc\n')
         self.create_one('b', 'chrome/chrome.manifest', 'b\nc\na\n')
         self.assertEqual(sorted([(f, c.open().read()) for f, c in
                                  finder.find('**/chrome.manifest')]),
                          [('chrome.manifest', 'a\nb\nc\n'),
--- a/toolkit/mozapps/installer/packager.py
+++ b/toolkit/mozapps/installer/packager.py
@@ -337,21 +337,24 @@ def main():
         )
         if args.js_binary:
             finder_args['minify_js_verify_command'] = [
                 args.js_binary,
                 os.path.join(os.path.abspath(os.path.dirname(__file__)),
                     'js-compare-ast.js')
             ]
         if args.unify:
-            finder = UnifiedBuildFinder(FileFinder(args.source),
-                                        FileFinder(args.unify),
+            finder = UnifiedBuildFinder(FileFinder(args.source,
+                                                   find_executables=True),
+                                        FileFinder(args.unify,
+                                                   find_executables=True),
                                         **finder_args)
         else:
-            finder = FileFinder(args.source, **finder_args)
+            finder = FileFinder(args.source, find_executables=True,
+                                **finder_args)
         if 'NO_PKG_FILES' in os.environ:
             sinkformatter = NoPkgFilesRemover(formatter,
                                               args.manifest is not None)
         else:
             sinkformatter = formatter
         sink = SimpleManifestSink(finder, sinkformatter)
         if args.manifest:
             preprocess_manifest(sink, args.manifest, defines)
--- a/toolkit/mozapps/installer/strip.py
+++ b/toolkit/mozapps/installer/strip.py
@@ -10,14 +10,14 @@ import sys
 from mozpack.files import FileFinder
 from mozpack.copier import FileCopier
 
 def strip(dir):
     copier = FileCopier()
     # The FileFinder will give use ExecutableFile instances for files
     # that can be stripped, and copying ExecutableFiles defaults to
     # stripping them unless buildconfig.substs['PKG_SKIP_STRIP'] is set.
-    for p, f in FileFinder(dir):
+    for p, f in FileFinder(dir, find_executables=True):
         copier.add(p, f)
     copier.copy(dir)
 
 if __name__ == '__main__':
     strip(sys.argv[1])