From 784f36c2c1c132c476b09658c5b19f92bce86d87 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Nov 2020 13:26:34 -0500 Subject: First draft of teaching Python scanner about dynamic files --- SCons/Node/FS.py | 9 ++++- SCons/Scanner/Python.py | 52 ++++++++++++---------------- test/Scanner/Python.py | 42 ++++++++++++++++++++++ test/Scanner/Python/SConstruct | 13 +++++++ test/Scanner/Python/sconstest.skip | 0 test/Scanner/Python/script.py | 8 +++++ test/Scanner/Python/to_be_copied/__init__.py | 1 + test/Scanner/Python/to_be_copied/helper.py | 0 8 files changed, 94 insertions(+), 31 deletions(-) create mode 100644 test/Scanner/Python.py create mode 100644 test/Scanner/Python/SConstruct create mode 100644 test/Scanner/Python/sconstest.skip create mode 100644 test/Scanner/Python/script.py create mode 100644 test/Scanner/Python/to_be_copied/__init__.py create mode 100644 test/Scanner/Python/to_be_copied/helper.py diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py index 967f007..cc2e75a 100644 --- a/SCons/Node/FS.py +++ b/SCons/Node/FS.py @@ -2664,6 +2664,8 @@ class File(Base): def _morph(self): """Turn a file system node into a File object.""" self.scanner_paths = {} + if self.abspath.endswith('package1'): + raise Exception(self.abspath) if not hasattr(self, '_local'): self._local = 0 if not hasattr(self, 'released_target_info'): @@ -3724,7 +3726,10 @@ class FileFinder: return None def _find_file_key(self, filename, paths, verbose=None): - return (filename, paths) + # Note: paths could be a list, which is not hashable. If it is, convert + # it to a tuple. + paths_entry = tuple(paths) if isinstance(paths, list) else paths + return (filename, paths_entry) @SCons.Memoize.CountDictCall(_find_file_key) def find_file(self, filename, paths, verbose=None): @@ -3773,6 +3778,8 @@ class FileFinder: result = node break + print('value: %s (%s)' % (result, type(result))) + print('key: %s (%s)' % (memo_key, type(memo_key))) memo_dict[memo_key] = result return result diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index dc6812c..965b9c1 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -108,49 +108,41 @@ def scan(node, env, path=()): for i in itertools.repeat(None, num_parents): current_dir = current_dir.up() - search_paths = [current_dir.abspath] + search_paths = [current_dir] search_string = module_lstripped else: - search_paths = path + search_paths = [env.Dir(p) for p in tuple(path)] search_string = module module_components = search_string.split('.') - for search_path in search_paths: - candidate_path = os.path.join(search_path, *module_components) - # The import stored in "module" could refer to a directory or file. - import_dirs = [] - if os.path.isdir(candidate_path): - import_dirs = module_components - - # Because this resolved to a directory, there is a chance that - # additional imports (e.g. from module import A, B) could refer - # to files to import. - if imports: - for imp in imports: - file = os.path.join(candidate_path, imp + '.py') - if os.path.isfile(file): - nodes.append(file) - elif os.path.isfile(candidate_path + '.py'): - nodes.append(candidate_path + '.py') - import_dirs = module_components[:-1] - - # We can ignore imports because this resolved to a file. Any - # additional imports (e.g. from module.file import A, B) would - # only refer to functions in this file. + module_joined = '/'.join(module_components) + print('%s - %s' % (module_joined, [str(s) for s in search_paths])) + node = SCons.Node.FS.find_file(module_joined, search_paths, verbose=True) + if node: + # The fact that we were able to find the node without appending .py + # means that this is a directory import. + nodes.append(env.Dir(node).File('__init__.py')) # Take a dependency on all __init__.py files from all imported # packages unless it's a relative import. If it's a relative # import, we don't need to take the dependency because Python # requires that all referenced packages have already been imported, # which means that the dependency has already been established. - if import_dirs and not is_relative: + # XXX TODO: This part is broken and needs to be fixed. + if not is_relative and len(module_components) > 1: + import_dirs = module_components for i in range(len(import_dirs)): - init_components = module_components[:i+1] + ['__init__.py'] - init_path = os.path.join(search_path, *(init_components)) - if os.path.isfile(init_path): - nodes.append(init_path) - break + init_path = '/'.join(module_components[:i+1] + ['__init__.py']) + # TODO: Passing search_paths is not correct. + init_node = SCons.Node.FS.find_file(init_path, search_paths, verbose=True) + if init_node: + nodes.append(init_node) + else: + node = SCons.Node.FS.find_file(module_joined + '.py', search_paths, verbose=True) + if node: + nodes.append(node) + print('nodes: %s' % [str(n) for n in nodes]) return sorted(nodes) diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py new file mode 100644 index 0000000..74c8a87 --- /dev/null +++ b/test/Scanner/Python.py @@ -0,0 +1,42 @@ +#!/usr/bin/env python +# +# MIT License +# +# Copyright The SCons Foundation +# +# Permission is hereby granted, free of charge, to any person obtaining +# a copy of this software and associated documentation files (the +# "Software"), to deal in the Software without restriction, including +# without limitation the rights to use, copy, modify, merge, publish, +# distribute, sublicense, and/or sell copies of the Software, and to +# permit persons to whom the Software is furnished to do so, subject to +# the following conditions: +# +# The above copyright notice and this permission notice shall be included +# in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY +# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE +# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +""" +Template for end-to-end test file. +Replace this with a description of the test. +""" + +import TestSCons + +test = TestSCons.TestSCons() +test.dir_fixture('Python') +test.run(arguments = '--debug=stacktrace .') +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: diff --git a/test/Scanner/Python/SConstruct b/test/Scanner/Python/SConstruct new file mode 100644 index 0000000..63fc44b --- /dev/null +++ b/test/Scanner/Python/SConstruct @@ -0,0 +1,13 @@ +import sys + +env = Environment(tools=['python']) +c = [] +for source, target in [ + ('to_be_copied', 'package1'), + ('to_be_copied', 'package2'), +]: + c += env.Command(target, source, Copy('$TARGET', '$SOURCE')) +# Don't set a dependency on the copy actions on purpose. Scanner should find +# the dependencies automatically. +s = env.Command('a.out', 'script.py', '$PYTHON $SOURCE $TARGET', PYTHON=sys.executable) +env.Depends(s, c) \ No newline at end of file diff --git a/test/Scanner/Python/sconstest.skip b/test/Scanner/Python/sconstest.skip new file mode 100644 index 0000000..e69de29 diff --git a/test/Scanner/Python/script.py b/test/Scanner/Python/script.py new file mode 100644 index 0000000..3970f8d --- /dev/null +++ b/test/Scanner/Python/script.py @@ -0,0 +1,8 @@ +import package1 +import package2 +import sys + +raise Exception(sys.argv) + +with open(sys.argv[1], 'w') as f: + f.write('test') diff --git a/test/Scanner/Python/to_be_copied/__init__.py b/test/Scanner/Python/to_be_copied/__init__.py new file mode 100644 index 0000000..34fdc42 --- /dev/null +++ b/test/Scanner/Python/to_be_copied/__init__.py @@ -0,0 +1 @@ +from . import helper \ No newline at end of file diff --git a/test/Scanner/Python/to_be_copied/helper.py b/test/Scanner/Python/to_be_copied/helper.py new file mode 100644 index 0000000..e69de29 -- cgit v0.12 From 03a319cdd0cd092b9f78de136b3bd701c68190bf Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Nov 2020 13:32:40 -0500 Subject: Change test to follow its own instructions --- test/Scanner/Python/SConstruct | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/Scanner/Python/SConstruct b/test/Scanner/Python/SConstruct index 63fc44b..0466d37 100644 --- a/test/Scanner/Python/SConstruct +++ b/test/Scanner/Python/SConstruct @@ -1,13 +1,11 @@ import sys env = Environment(tools=['python']) -c = [] for source, target in [ ('to_be_copied', 'package1'), ('to_be_copied', 'package2'), ]: - c += env.Command(target, source, Copy('$TARGET', '$SOURCE')) + env.Command(target, source, Copy('$TARGET', '$SOURCE')) # Don't set a dependency on the copy actions on purpose. Scanner should find # the dependencies automatically. -s = env.Command('a.out', 'script.py', '$PYTHON $SOURCE $TARGET', PYTHON=sys.executable) -env.Depends(s, c) \ No newline at end of file +env.Command('a.out', 'script.py', '$PYTHON $SOURCE $TARGET', PYTHON=sys.executable) \ No newline at end of file -- cgit v0.12 From 28636490512924ce393ce324a61130d9d43fe54a Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Nov 2020 13:54:02 -0500 Subject: Remove exception from script.py --- test/Scanner/Python/script.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/Scanner/Python/script.py b/test/Scanner/Python/script.py index 3970f8d..f633f43 100644 --- a/test/Scanner/Python/script.py +++ b/test/Scanner/Python/script.py @@ -2,7 +2,5 @@ import package1 import package2 import sys -raise Exception(sys.argv) - with open(sys.argv[1], 'w') as f: f.write('test') -- cgit v0.12 From fde36127ff949e7efbce321e7ace07707b35e41a Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Mon, 16 Nov 2020 10:17:58 -0500 Subject: Some initial fixes --- SCons/Node/FS.py | 6 +---- SCons/Scanner/Python.py | 57 ++++++++++++++++++++++++------------------ test/Scanner/Python.py | 2 +- test/Scanner/Python/SConstruct | 6 ++--- 4 files changed, 37 insertions(+), 34 deletions(-) diff --git a/SCons/Node/FS.py b/SCons/Node/FS.py index cc2e75a..369ec59 100644 --- a/SCons/Node/FS.py +++ b/SCons/Node/FS.py @@ -2664,8 +2664,6 @@ class File(Base): def _morph(self): """Turn a file system node into a File object.""" self.scanner_paths = {} - if self.abspath.endswith('package1'): - raise Exception(self.abspath) if not hasattr(self, '_local'): self._local = 0 if not hasattr(self, 'released_target_info'): @@ -3727,7 +3725,7 @@ class FileFinder: def _find_file_key(self, filename, paths, verbose=None): # Note: paths could be a list, which is not hashable. If it is, convert - # it to a tuple. + # it to a tuple, which is hashable. paths_entry = tuple(paths) if isinstance(paths, list) else paths return (filename, paths_entry) @@ -3778,8 +3776,6 @@ class FileFinder: result = node break - print('value: %s (%s)' % (result, type(result))) - print('key: %s (%s)' % (memo_key, type(memo_key))) memo_dict[memo_key] = result return result diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index 965b9c1..63f5035 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -116,33 +116,40 @@ def scan(node, env, path=()): module_components = search_string.split('.') module_joined = '/'.join(module_components) - print('%s - %s' % (module_joined, [str(s) for s in search_paths])) - node = SCons.Node.FS.find_file(module_joined, search_paths, verbose=True) - if node: - # The fact that we were able to find the node without appending .py - # means that this is a directory import. - nodes.append(env.Dir(node).File('__init__.py')) - - # Take a dependency on all __init__.py files from all imported - # packages unless it's a relative import. If it's a relative - # import, we don't need to take the dependency because Python - # requires that all referenced packages have already been imported, - # which means that the dependency has already been established. - # XXX TODO: This part is broken and needs to be fixed. - if not is_relative and len(module_components) > 1: - import_dirs = module_components - for i in range(len(import_dirs)): - init_path = '/'.join(module_components[:i+1] + ['__init__.py']) - # TODO: Passing search_paths is not correct. - init_node = SCons.Node.FS.find_file(init_path, search_paths, verbose=True) - if init_node: - nodes.append(init_node) - else: - node = SCons.Node.FS.find_file(module_joined + '.py', search_paths, verbose=True) + + # For an import of "p", it could either result in a directory named + # p or a file named p.py. We can't do two consecutive searches for p + # then p.py because the first search could return a result that is + # lower in the search_paths precedence order. As a result, it is safest + # to iterate over search_paths and check whether p or p.py exists in + # each path. This allows us to cleanly respect the precedence order. + for path in search_paths: + paths = [path] + node = SCons.Node.FS.find_file(module_joined, paths, verbose=True) if node: - nodes.append(node) + # The fact that we were able to find the node without appending .py + # means that this is a directory import. + nodes.append(env.Dir(node).File('__init__.py')) + + # Take a dependency on all __init__.py files from all imported + # packages unless it's a relative import. If it's a relative + # import, we don't need to take the dependency because Python + # requires that all referenced packages have already been imported, + # which means that the dependency has already been established. + # XXX TODO: This part is broken and needs to be fixed. + if not is_relative and len(module_components) > 1: + import_dirs = module_components + for i in range(len(import_dirs)): + init_path = '/'.join(module_components[:i+1] + ['__init__.py']) + # TODO: Passing search_paths is not correct. + init_node = SCons.Node.FS.find_file(init_path, paths, verbose=True) + if init_node: + nodes.append(init_node) + else: + node = SCons.Node.FS.find_file(module_joined + '.py', paths, verbose=True) + if node: + nodes.append(node) - print('nodes: %s' % [str(n) for n in nodes]) return sorted(nodes) diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py index 74c8a87..404967c 100644 --- a/test/Scanner/Python.py +++ b/test/Scanner/Python.py @@ -32,7 +32,7 @@ import TestSCons test = TestSCons.TestSCons() test.dir_fixture('Python') -test.run(arguments = '--debug=stacktrace .') +test.run(arguments = '-n --tree=prune --debug=stacktrace .', stdout='') test.pass_test() # Local Variables: diff --git a/test/Scanner/Python/SConstruct b/test/Scanner/Python/SConstruct index 0466d37..732b55c 100644 --- a/test/Scanner/Python/SConstruct +++ b/test/Scanner/Python/SConstruct @@ -2,10 +2,10 @@ import sys env = Environment(tools=['python']) for source, target in [ - ('to_be_copied', 'package1'), - ('to_be_copied', 'package2'), + ('to_be_copied', 'package1'), + ('to_be_copied', 'package2'), ]: - env.Command(target, source, Copy('$TARGET', '$SOURCE')) + env.Command(env.Dir(target), env.Dir(source), Copy('$TARGET', '$SOURCE')) # Don't set a dependency on the copy actions on purpose. Scanner should find # the dependencies automatically. env.Command('a.out', 'script.py', '$PYTHON $SOURCE $TARGET', PYTHON=sys.executable) \ No newline at end of file -- cgit v0.12 From 69ab257c530f6449a4ec49454500c507437a9dbc Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Mon, 16 Nov 2020 11:35:21 -0500 Subject: Fix up scanner a bit Tests still break --- SCons/Scanner/Python.py | 16 ++++++++++------ test/Scanner/Python.py | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index 63f5035..5f51a7c 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -92,10 +92,14 @@ def scan(node, env, path=()): # if the same header is included many times. node.includes = list(map(SCons.Util.silent_intern, includes)) - # XXX TODO: Sort? nodes = [] if callable(path): path = path() + + # If there are no paths, there is no point in parsing includes in the loop. + if not path: + return [] + for module, imports in includes: is_relative = module.startswith('.') if is_relative: @@ -111,7 +115,7 @@ def scan(node, env, path=()): search_paths = [current_dir] search_string = module_lstripped else: - search_paths = [env.Dir(p) for p in tuple(path)] + search_paths = [env.Dir(p) for p in path] search_string = module module_components = search_string.split('.') @@ -123,8 +127,8 @@ def scan(node, env, path=()): # lower in the search_paths precedence order. As a result, it is safest # to iterate over search_paths and check whether p or p.py exists in # each path. This allows us to cleanly respect the precedence order. - for path in search_paths: - paths = [path] + for search_path in search_paths: + paths = [search_path] node = SCons.Node.FS.find_file(module_joined, paths, verbose=True) if node: # The fact that we were able to find the node without appending .py @@ -136,7 +140,6 @@ def scan(node, env, path=()): # import, we don't need to take the dependency because Python # requires that all referenced packages have already been imported, # which means that the dependency has already been established. - # XXX TODO: This part is broken and needs to be fixed. if not is_relative and len(module_components) > 1: import_dirs = module_components for i in range(len(import_dirs)): @@ -150,7 +153,8 @@ def scan(node, env, path=()): if node: nodes.append(node) - return sorted(nodes) + print('returning nodes %s' % ([str(n) for n in nodes])) + return nodes PythonSuffixes = ['.py'] diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py index 404967c..bb6e2c7 100644 --- a/test/Scanner/Python.py +++ b/test/Scanner/Python.py @@ -32,7 +32,7 @@ import TestSCons test = TestSCons.TestSCons() test.dir_fixture('Python') -test.run(arguments = '-n --tree=prune --debug=stacktrace .', stdout='') +test.run(arguments = '--debug=stacktrace .', stdout='') test.pass_test() # Local Variables: -- cgit v0.12 From 1037bb6069612408183e953081c72dac9814f370 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Mon, 16 Nov 2020 11:47:16 -0500 Subject: [ci skip] Cancel test runs because my tests don't work --- test/Scanner/Python.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py index bb6e2c7..404967c 100644 --- a/test/Scanner/Python.py +++ b/test/Scanner/Python.py @@ -32,7 +32,7 @@ import TestSCons test = TestSCons.TestSCons() test.dir_fixture('Python') -test.run(arguments = '--debug=stacktrace .', stdout='') +test.run(arguments = '-n --tree=prune --debug=stacktrace .', stdout='') test.pass_test() # Local Variables: -- cgit v0.12 From 55b41b5841aaf1f86d8a8380e0f8603b45bcc49f Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Mon, 16 Nov 2020 16:59:20 -0500 Subject: [ci skip] Some fixes to the Python scanner and test --- SCons/Scanner/Python.py | 76 +++++++++++++++++++++++++++---------------------- test/Scanner/Python.py | 2 +- 2 files changed, 43 insertions(+), 35 deletions(-) diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index 5f51a7c..ada575e 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -118,43 +118,51 @@ def scan(node, env, path=()): search_paths = [env.Dir(p) for p in path] search_string = module - module_components = search_string.split('.') - module_joined = '/'.join(module_components) - - # For an import of "p", it could either result in a directory named - # p or a file named p.py. We can't do two consecutive searches for p - # then p.py because the first search could return a result that is - # lower in the search_paths precedence order. As a result, it is safest - # to iterate over search_paths and check whether p or p.py exists in - # each path. This allows us to cleanly respect the precedence order. - for search_path in search_paths: - paths = [search_path] - node = SCons.Node.FS.find_file(module_joined, paths, verbose=True) - if node: - # The fact that we were able to find the node without appending .py - # means that this is a directory import. - nodes.append(env.Dir(node).File('__init__.py')) - - # Take a dependency on all __init__.py files from all imported - # packages unless it's a relative import. If it's a relative - # import, we don't need to take the dependency because Python - # requires that all referenced packages have already been imported, - # which means that the dependency has already been established. - if not is_relative and len(module_components) > 1: - import_dirs = module_components - for i in range(len(import_dirs)): - init_path = '/'.join(module_components[:i+1] + ['__init__.py']) - # TODO: Passing search_paths is not correct. - init_node = SCons.Node.FS.find_file(init_path, paths, verbose=True) - if init_node: - nodes.append(init_node) - else: - node = SCons.Node.FS.find_file(module_joined + '.py', paths, verbose=True) + if not imports: + imports = [None] + + for i in imports: + module_components = search_string.split('.') + import_components = [i] if i is not None else [] + components = [x for x in module_components + import_components if x] + module_path = '/'.join(components) + '.py' + package_path = '/'.join(components + ['__init__.py']) + + # For an import of "p", it could either result in a file named p.py or + # p/__init__.py. We can't do two consecutive searches for p then p.py + # because the first search could return a result that is lower in the + # search_paths precedence order. As a result, it is safest to iterate + # over search_paths and check whether p or p.py exists in each path. + # This allows us to cleanly respect the precedence order. + for search_path in search_paths: + paths = [search_path] + # See if p/__init__.py exists. + node = SCons.Node.FS.find_file(package_path, paths, verbose=True) if node: nodes.append(node) + else: + node = SCons.Node.FS.find_file(module_path, paths, verbose=True) + if node: + nodes.append(node) - print('returning nodes %s' % ([str(n) for n in nodes])) - return nodes + if node: + # Take a dependency on all __init__.py files from all imported + # packages unless it's a relative import. If it's a relative + # import, we don't need to take the dependency because Python + # requires that all referenced packages have already been imported, + # which means that the dependency has already been established. + if not is_relative and len(module_components) > 1: + for i in range(len(module_components[:-1])): + init_path = '/'.join(module_components[:i+1] + ['__init__.py']) + init_node = SCons.Node.FS.find_file(init_path, paths, verbose=True) + if init_node: + nodes.append(init_node) + + # The import was found, so no need to keep iterating through + # search_paths. + break + + return sorted(nodes) PythonSuffixes = ['.py'] diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py index 404967c..bb6e2c7 100644 --- a/test/Scanner/Python.py +++ b/test/Scanner/Python.py @@ -32,7 +32,7 @@ import TestSCons test = TestSCons.TestSCons() test.dir_fixture('Python') -test.run(arguments = '-n --tree=prune --debug=stacktrace .', stdout='') +test.run(arguments = '--debug=stacktrace .', stdout='') test.pass_test() # Local Variables: -- cgit v0.12 From a6fdbf0aa1320f4c2b6f4daa5b9496108d22e0cd Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Mon, 16 Nov 2020 21:17:33 -0500 Subject: Fix all tests This change fixes all tests. It's still a a WIP change because I think the "imports" logic is wrong for file imports. --- SCons/Scanner/Python.py | 22 ++++++++++------------ test/Scanner/Python.py | 9 ++++++--- test/Scanner/Python/SConstruct | 15 ++++++++++----- test/Scanner/Python/to_be_copied/sconstest.skip | 0 4 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 test/Scanner/Python/to_be_copied/sconstest.skip diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index ada575e..2d4a211 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -136,26 +136,24 @@ def scan(node, env, path=()): # This allows us to cleanly respect the precedence order. for search_path in search_paths: paths = [search_path] - # See if p/__init__.py exists. - node = SCons.Node.FS.find_file(package_path, paths, verbose=True) + node = SCons.Node.FS.find_file(package_path, paths) + if not node: + node = SCons.Node.FS.find_file(module_path, paths) + if node: nodes.append(node) - else: - node = SCons.Node.FS.find_file(module_path, paths, verbose=True) - if node: - nodes.append(node) - if node: # Take a dependency on all __init__.py files from all imported # packages unless it's a relative import. If it's a relative # import, we don't need to take the dependency because Python # requires that all referenced packages have already been imported, # which means that the dependency has already been established. - if not is_relative and len(module_components) > 1: - for i in range(len(module_components[:-1])): - init_path = '/'.join(module_components[:i+1] + ['__init__.py']) - init_node = SCons.Node.FS.find_file(init_path, paths, verbose=True) - if init_node: + if not is_relative: + import_dirs = module_components + for i in range(len(import_dirs)): + init_path = '/'.join(import_dirs[:i+1] + ['__init__.py']) + init_node = SCons.Node.FS.find_file(init_path, paths) + if init_node and init_node not in nodes: nodes.append(init_node) # The import was found, so no need to keep iterating through diff --git a/test/Scanner/Python.py b/test/Scanner/Python.py index bb6e2c7..b5b3ae5 100644 --- a/test/Scanner/Python.py +++ b/test/Scanner/Python.py @@ -24,15 +24,18 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. """ -Template for end-to-end test file. -Replace this with a description of the test. +Functional test for the Python scanner. Validates that the scanner is able to +find dynamically generated dependencies. The SConstruct copies in the +dependencies and runs a script. The expected behavior is that the scanner +picks up these dependencies, so SCons understands that the script shouldn't +be run until the files are copied. """ import TestSCons test = TestSCons.TestSCons() test.dir_fixture('Python') -test.run(arguments = '--debug=stacktrace .', stdout='') +test.run(arguments = '.') test.pass_test() # Local Variables: diff --git a/test/Scanner/Python/SConstruct b/test/Scanner/Python/SConstruct index 732b55c..988cf60 100644 --- a/test/Scanner/Python/SConstruct +++ b/test/Scanner/Python/SConstruct @@ -1,11 +1,16 @@ import sys env = Environment(tools=['python']) -for source, target in [ - ('to_be_copied', 'package1'), - ('to_be_copied', 'package2'), -]: - env.Command(env.Dir(target), env.Dir(source), Copy('$TARGET', '$SOURCE')) + +# Copy each file individually instead of copying the dir. This has the benefit +# of establishing nodes in-memory for each of the resulting files, which helps +# the scanner work correctly. +srcDir = env.Dir('to_be_copied') +for srcNode in srcDir.glob('*'): + for destDir in [env.Dir('package1'), env.Dir('package2')]: + env.Command(destDir.File(srcNode.name), srcNode, + Copy('$TARGET', '$SOURCE')) + # Don't set a dependency on the copy actions on purpose. Scanner should find # the dependencies automatically. env.Command('a.out', 'script.py', '$PYTHON $SOURCE $TARGET', PYTHON=sys.executable) \ No newline at end of file diff --git a/test/Scanner/Python/to_be_copied/sconstest.skip b/test/Scanner/Python/to_be_copied/sconstest.skip new file mode 100644 index 0000000..e69de29 -- cgit v0.12 From 54e4841387c4d3247a6529b259a5db75774311f5 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Tue, 17 Nov 2020 09:16:12 -0500 Subject: Fix sider issues --- test/Scanner/Python/script.py | 4 ++-- test/Scanner/Python/to_be_copied/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/Scanner/Python/script.py b/test/Scanner/Python/script.py index f633f43..105a575 100644 --- a/test/Scanner/Python/script.py +++ b/test/Scanner/Python/script.py @@ -1,5 +1,5 @@ -import package1 -import package2 +import package1 # noqa: F401 +import package2 # noqa: F401 import sys with open(sys.argv[1], 'w') as f: diff --git a/test/Scanner/Python/to_be_copied/__init__.py b/test/Scanner/Python/to_be_copied/__init__.py index 34fdc42..3c1c05b 100644 --- a/test/Scanner/Python/to_be_copied/__init__.py +++ b/test/Scanner/Python/to_be_copied/__init__.py @@ -1 +1 @@ -from . import helper \ No newline at end of file +from . import helper # noqa: F401 \ No newline at end of file -- cgit v0.12 From bac3074b9f700a020bfeaad37658d8a2db68cffd Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Tue, 17 Nov 2020 13:44:55 -0500 Subject: Fix tests, implement smarter version of scanner --- SCons/Scanner/Python.py | 115 +++++++++++++-------- SCons/Scanner/PythonTests.py | 72 ++++++++++++- .../from_import_simple_package_module1_func.py | 1 + .../python_scanner/from_nested1_import_multiple.py | 1 + .../python_scanner/imports_unknown_files.py | 3 + .../python_scanner/simple_package/module1.py | 2 + .../python_scanner/simple_package/somefunc.py | 0 7 files changed, 149 insertions(+), 45 deletions(-) create mode 100644 test/fixture/python_scanner/from_import_simple_package_module1_func.py create mode 100644 test/fixture/python_scanner/from_nested1_import_multiple.py create mode 100644 test/fixture/python_scanner/imports_unknown_files.py create mode 100644 test/fixture/python_scanner/simple_package/somefunc.py diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index 2d4a211..f8272df 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -82,6 +82,40 @@ def find_include_names(node): return all_matches +def find_import(import_path, search_paths): + """ + Finds the specified import in the various search paths. + For an import of "p", it could either result in a file named p.py or + p/__init__.py. We can't do two consecutive searches for p then p.py + because the first search could return a result that is lower in the + search_paths precedence order. As a result, it is safest to iterate over + search_paths and check whether p or p.py exists in each path. This allows + us to cleanly respect the precedence order. + + If the import is found, returns a tuple containing: + 1. Discovered dependency node (e.g. p/__init__.py or p.py) + 2. True if the import was a package, False if the import was a module. + 3. The Dir node in search_paths that the import is relative to. + If the import is not found, returns a tuple containing (None, False, None). + Callers should check for failure by checking whether the first entry in the + tuple is not None. + """ + for search_path in search_paths: + paths = [search_path] + # Note: if the same import is present as a package and a module, Python + # prefers the package. As a result, we always look for x/__init__.py + # before looking for x.py. + node = SCons.Node.FS.find_file(import_path + '/__init__.py', paths) + if node: + return node, True, search_path + else: + node = SCons.Node.FS.find_file(import_path + '.py', paths) + if node: + return node, False, search_path + + return None, False, None + + def scan(node, env, path=()): # cache the includes list in node so we only scan it once: if node.includes is not None: @@ -118,47 +152,46 @@ def scan(node, env, path=()): search_paths = [env.Dir(p) for p in path] search_string = module - if not imports: - imports = [None] - - for i in imports: - module_components = search_string.split('.') - import_components = [i] if i is not None else [] - components = [x for x in module_components + import_components if x] - module_path = '/'.join(components) + '.py' - package_path = '/'.join(components + ['__init__.py']) - - # For an import of "p", it could either result in a file named p.py or - # p/__init__.py. We can't do two consecutive searches for p then p.py - # because the first search could return a result that is lower in the - # search_paths precedence order. As a result, it is safest to iterate - # over search_paths and check whether p or p.py exists in each path. - # This allows us to cleanly respect the precedence order. - for search_path in search_paths: - paths = [search_path] - node = SCons.Node.FS.find_file(package_path, paths) - if not node: - node = SCons.Node.FS.find_file(module_path, paths) - - if node: - nodes.append(node) - - # Take a dependency on all __init__.py files from all imported - # packages unless it's a relative import. If it's a relative - # import, we don't need to take the dependency because Python - # requires that all referenced packages have already been imported, - # which means that the dependency has already been established. - if not is_relative: - import_dirs = module_components - for i in range(len(import_dirs)): - init_path = '/'.join(import_dirs[:i+1] + ['__init__.py']) - init_node = SCons.Node.FS.find_file(init_path, paths) - if init_node and init_node not in nodes: - nodes.append(init_node) - - # The import was found, so no need to keep iterating through - # search_paths. - break + module_components = [x for x in search_string.split('.') if x] + package_dir = None + hit_dir = None + if not module_components: + # This is just a "from . import x". + package_dir = search_paths[0] + else: + # Translate something like "import x.y" to a call to find_import + # with 'x/y' as the path. find_import will then determine whether + # we can find 'x/y/__init__.py' or 'x/y.py'. + import_node, is_dir, hit_dir = find_import( + '/'.join(module_components), search_paths) + if import_node: + nodes.append(import_node) + if is_dir: + package_dir = import_node.dir + + # If the statement was something like "from x import y, z", whether we + # iterate over imports depends on whether x was a package or module. + # If it was a module, y and z are just functions so we don't need to + # search for them. If it was a package, y and z are either packages or + # modules and we do need to search for them. + if package_dir and imports: + for i in imports: + import_node, _, _ = find_import(i, [package_dir]) + if import_node: + nodes.append(import_node) + + # Take a dependency on all __init__.py files from all imported + # packages unless it's a relative import. If it's a relative + # import, we don't need to take the dependency because Python + # requires that all referenced packages have already been imported, + # which means that the dependency has already been established. + if hit_dir and not is_relative: + import_dirs = module_components + for i in range(len(import_dirs)): + init_path = '/'.join(import_dirs[:i+1] + ['__init__.py']) + init_node = SCons.Node.FS.find_file(init_path, [hit_dir]) + if init_node and init_node not in nodes: + nodes.append(init_node) return sorted(nodes) diff --git a/SCons/Scanner/PythonTests.py b/SCons/Scanner/PythonTests.py index faf548a..84acd0c 100644 --- a/SCons/Scanner/PythonTests.py +++ b/SCons/Scanner/PythonTests.py @@ -21,6 +21,21 @@ # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +""" +Unit tests for the Python scanner. These tests validate proper working of the +Python scanner by confirming that the results of the scan match expectations. + +The absolute path tests have strongly-defined behavior in that there is no real +ambiguity to what they should result in. For example, if you import package x, +you expect to get x/__init__.py as a dependency. + +The relative path tests that reach into ancestor directories do have some +ambiguity in whether to depend upon __init__.py in those referenced ancestor +directories. Python only allows these kinds of relative imports if the file is +part of a package, in which case those ancestor directories' __init__.py files +have already been imported. +""" + import SCons.compat import collections @@ -226,9 +241,6 @@ class PythonScannerTestImportsGrandparentModule(unittest.TestCase): 'nested1/nested2/nested3/imports_grandparent_module.py') path = s.path(env, source=[node]) deps = s(node, env, path) - # Note: there is some ambiguity here in what the scanner should return. - # Relative imports require that the referenced packages have already - # been imported. files = ['nested1/module.py'] deps_match(self, deps, files) @@ -253,7 +265,59 @@ class PythonScannerTestImportsParentThenSubmodule(unittest.TestCase): 'nested1/nested2/nested3/imports_parent_then_submodule.py') path = s.path(env, source=[node]) deps = s(node, env, path) - files = ['nested1/nested2a/module.py'] + files = ['nested1/nested2a/__init__.py', 'nested1/nested2a/module.py'] + deps_match(self, deps, files) + + +class PythonScannerTestImportsModuleWithFunc(unittest.TestCase): + def runTest(self): + """ + This test case tests the following import statement: + `from simple_package.module1 import somefunc` with somefunc.py existing + in the same folder as module1.py. It validates that the scanner doesn't + accidentally take a dependency somefunc.py. + """ + env = DummyEnvironment() + s = SCons.Scanner.Python.PythonScanner + env['ENV']['PYTHONPATH'] = test.workpath('') + deps = s(env.File('from_import_simple_package_module1_func.py'), env, + lambda : s.path(env)) + files = ['simple_package/__init__.py', 'simple_package/module1.py'] + deps_match(self, deps, files) + + +class PythonScannerTestFromNested1ImportNested2(unittest.TestCase): + def runTest(self): + """ + This test case tests the following import statement: + `from nested1 import module, nested2`. In this test, module is a Python + module and nested2 is a package. Validates that the scanner can handle + such mixed imports. + """ + env = DummyEnvironment() + s = SCons.Scanner.Python.PythonScanner + env['ENV']['PYTHONPATH'] = test.workpath('') + deps = s(env.File('from_nested1_import_multiple.py'), env, + lambda : s.path(env)) + files = ['nested1/__init__.py', 'nested1/module.py', + 'nested1/nested2/__init__.py'] + deps_match(self, deps, files) + + +class PythonScannerTestImportUnknownFiles(unittest.TestCase): + def runTest(self): + """ + This test case tests importing files that are not found. If Python + really can't find those files, it will fail. But this is intended to + test the various failure paths in the scanner to make sure that they + don't raise exceptions. + """ + env = DummyEnvironment() + s = SCons.Scanner.Python.PythonScanner + env['ENV']['PYTHONPATH'] = test.workpath('') + deps = s(env.File('imports_unknown_files.py'), env, + lambda : s.path(env)) + files = [] deps_match(self, deps, files) diff --git a/test/fixture/python_scanner/from_import_simple_package_module1_func.py b/test/fixture/python_scanner/from_import_simple_package_module1_func.py new file mode 100644 index 0000000..e9877fb --- /dev/null +++ b/test/fixture/python_scanner/from_import_simple_package_module1_func.py @@ -0,0 +1 @@ +from simple_package.module1 import somefunc # noqa: F401 \ No newline at end of file diff --git a/test/fixture/python_scanner/from_nested1_import_multiple.py b/test/fixture/python_scanner/from_nested1_import_multiple.py new file mode 100644 index 0000000..2cdd47f --- /dev/null +++ b/test/fixture/python_scanner/from_nested1_import_multiple.py @@ -0,0 +1 @@ +from nested1 import module, nested2 # noqa: F401 \ No newline at end of file diff --git a/test/fixture/python_scanner/imports_unknown_files.py b/test/fixture/python_scanner/imports_unknown_files.py new file mode 100644 index 0000000..2059181 --- /dev/null +++ b/test/fixture/python_scanner/imports_unknown_files.py @@ -0,0 +1,3 @@ +import doesntexist +import notthere.something +from notthere import a, few, things \ No newline at end of file diff --git a/test/fixture/python_scanner/simple_package/module1.py b/test/fixture/python_scanner/simple_package/module1.py index e69de29..6880c47 100644 --- a/test/fixture/python_scanner/simple_package/module1.py +++ b/test/fixture/python_scanner/simple_package/module1.py @@ -0,0 +1,2 @@ +def somefunc(): + return \ No newline at end of file diff --git a/test/fixture/python_scanner/simple_package/somefunc.py b/test/fixture/python_scanner/simple_package/somefunc.py new file mode 100644 index 0000000..e69de29 -- cgit v0.12 From a6f317b4301d968cbda2c7fbe903d23347900bf8 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Tue, 17 Nov 2020 15:31:06 -0500 Subject: [ci skip] Fix flake8 warnings, add description to CHANGES.txt --- CHANGES.txt | 2 ++ test/fixture/python_scanner/imports_unknown_files.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 77fb110..e790b67 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -35,6 +35,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER User-facing behavior does not change with this fix (GH Issue #3726). - Fix occasional test failures caused by not being able to find a file or directory fixture when running multiple tests with multiple jobs. + - Add support to the Python scanner for finding dynamically generated dependencies. + Previously the scanner only found imports if they existed on disk at scanning time. From Joachim Kuebart: - Suppress missing SConscript deprecation warning if `must_exist=False` diff --git a/test/fixture/python_scanner/imports_unknown_files.py b/test/fixture/python_scanner/imports_unknown_files.py index 2059181..5582e7b 100644 --- a/test/fixture/python_scanner/imports_unknown_files.py +++ b/test/fixture/python_scanner/imports_unknown_files.py @@ -1,3 +1,3 @@ -import doesntexist -import notthere.something -from notthere import a, few, things \ No newline at end of file +import doesntexist # noqa: F401 +import notthere.something # noqa: F401 +from notthere import a, few, things # noqa: F401 \ No newline at end of file -- cgit v0.12 -- cgit v0.12 From b9085adbb23b2083a2064928377931578a456f8c Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Sat, 9 Jan 2021 10:08:36 -0500 Subject: Address review feedback Better handle relative imports if no paths are provided. --- SCons/Scanner/Python.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/SCons/Scanner/Python.py b/SCons/Scanner/Python.py index f8272df..86aa001 100644 --- a/SCons/Scanner/Python.py +++ b/SCons/Scanner/Python.py @@ -130,10 +130,6 @@ def scan(node, env, path=()): if callable(path): path = path() - # If there are no paths, there is no point in parsing includes in the loop. - if not path: - return [] - for module, imports in includes: is_relative = module.startswith('.') if is_relative: @@ -152,6 +148,11 @@ def scan(node, env, path=()): search_paths = [env.Dir(p) for p in path] search_string = module + # If there are no paths, there is no point in parsing includes for this + # iteration of the loop. + if not search_paths: + continue + module_components = [x for x in search_string.split('.') if x] package_dir = None hit_dir = None -- cgit v0.12