From 2be6753441745d3e0859686593fe7748fe3c885c Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Dec 2019 09:55:29 -0500 Subject: [PATCH 1/5] Add test showing the wrong absolute path --- test/ScannerAbsPathCheck/fixture/SConstruct | 41 ++++++++++++++++ .../fixture/subdir/SConscript | 2 + .../fixture/subdir/empty.py | 0 .../fixture/subdir/sconstest.skip | 0 test/ScannerAbsPathCheck/test.py | 48 +++++++++++++++++++ 5 files changed, 91 insertions(+) create mode 100644 test/ScannerAbsPathCheck/fixture/SConstruct create mode 100644 test/ScannerAbsPathCheck/fixture/subdir/SConscript create mode 100644 test/ScannerAbsPathCheck/fixture/subdir/empty.py create mode 100644 test/ScannerAbsPathCheck/fixture/subdir/sconstest.skip create mode 100644 test/ScannerAbsPathCheck/test.py diff --git a/test/ScannerAbsPathCheck/fixture/SConstruct b/test/ScannerAbsPathCheck/fixture/SConstruct new file mode 100644 index 0000000000..abd1b0ab07 --- /dev/null +++ b/test/ScannerAbsPathCheck/fixture/SConstruct @@ -0,0 +1,41 @@ +import os +import sys +import SCons.Scanner + +sdk_root = None + +def verify_paths(node, env, scanpaths, arg): + """Verifies that the paths provided are as expected.""" + global sdk_root + sdk_root_subdir = os.path.join(sdk_root, 'sdk_subdir') + if len(scanpaths) != 2: + raise Exception('Expected two entries in scanpaths') + if scanpaths[0].abspath != sdk_root: + raise Exception('Expected first scanpath=%s, got %s.' % + (sdk_root, scanpaths[0].abspath)) + if scanpaths[1].abspath != sdk_root_subdir: + raise Exception('Expected second scanpath=%s, got %s.' % + (sdk_root_subdir, scanpaths[1].abspath)) + +# Create a scanner that generates a path using the variables in env['PYPATH']. +pyscan = Scanner(name='pythonfile', + function=verify_paths, + argument=None, + path_function=SCons.Scanner.FindPathDirs('PYPATH'), + skeys=['.py']) + +# Create a builder that uses that scanner. +b = Builder(action='$PYTHON $SOURCE', source_scanner=pyscan) + +env = Environment(BUILDERS={'DummyPythonBuilder': b}) + +# Now set some variables that are needed by the scanner and/or the SConscript. +env.Replace( + PYTHON=sys.executable, + PYPATH=['$SDKROOT', '$SDKROOT\\sdk_subdir'], + SDKROOT=env.Dir('sdk'), +) +sdk_root = env['SDKROOT'].abspath + +Export('env') +SConscript('subdir/SConscript', exports='env') diff --git a/test/ScannerAbsPathCheck/fixture/subdir/SConscript b/test/ScannerAbsPathCheck/fixture/subdir/SConscript new file mode 100644 index 0000000000..16f3513238 --- /dev/null +++ b/test/ScannerAbsPathCheck/fixture/subdir/SConscript @@ -0,0 +1,2 @@ +Import('env') +env.DummyPythonBuilder(target=['foo'], source=['empty.py']) diff --git a/test/ScannerAbsPathCheck/fixture/subdir/empty.py b/test/ScannerAbsPathCheck/fixture/subdir/empty.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/ScannerAbsPathCheck/fixture/subdir/sconstest.skip b/test/ScannerAbsPathCheck/fixture/subdir/sconstest.skip new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/ScannerAbsPathCheck/test.py b/test/ScannerAbsPathCheck/test.py new file mode 100644 index 0000000000..a6474d8653 --- /dev/null +++ b/test/ScannerAbsPathCheck/test.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python +# +# __COPYRIGHT__ +# +# 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. +# + +__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" + +""" +This test verifies that a scanner retrieves the correct paths when +path_function is set to SCons.Scanner.FindPathDirs(some_var) and env[some_var] +contains paths relative to the SConstruct root. +""" + +import os + +import TestSCons + +test = TestSCons.TestSCons() + +test.dir_fixture('fixture') +test.run() + +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: From 7a1f87aa740989e061dcaa9cea928b55eefc5ff2 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Dec 2019 10:06:51 -0500 Subject: [PATCH 2/5] Make test cross-platform by using a forward slash in env['PYPATH'] --- test/ScannerAbsPathCheck/fixture/SConstruct | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/ScannerAbsPathCheck/fixture/SConstruct b/test/ScannerAbsPathCheck/fixture/SConstruct index abd1b0ab07..435f44962b 100644 --- a/test/ScannerAbsPathCheck/fixture/SConstruct +++ b/test/ScannerAbsPathCheck/fixture/SConstruct @@ -32,7 +32,7 @@ env = Environment(BUILDERS={'DummyPythonBuilder': b}) # Now set some variables that are needed by the scanner and/or the SConscript. env.Replace( PYTHON=sys.executable, - PYPATH=['$SDKROOT', '$SDKROOT\\sdk_subdir'], + PYPATH=['$SDKROOT', '$SDKROOT/sdk_subdir'], SDKROOT=env.Dir('sdk'), ) sdk_root = env['SDKROOT'].abspath From 3847b1a03721dc38614a3a03a9835604aa0f40a0 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 13 Dec 2019 15:52:30 -0500 Subject: [PATCH 3/5] Fix sider failure --- test/ScannerAbsPathCheck/fixture/SConstruct | 1 + test/ScannerAbsPathCheck/test.py | 2 -- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/test/ScannerAbsPathCheck/fixture/SConstruct b/test/ScannerAbsPathCheck/fixture/SConstruct index 435f44962b..c9f542cc1c 100644 --- a/test/ScannerAbsPathCheck/fixture/SConstruct +++ b/test/ScannerAbsPathCheck/fixture/SConstruct @@ -16,6 +16,7 @@ def verify_paths(node, env, scanpaths, arg): if scanpaths[1].abspath != sdk_root_subdir: raise Exception('Expected second scanpath=%s, got %s.' % (sdk_root_subdir, scanpaths[1].abspath)) + return [] # Create a scanner that generates a path using the variables in env['PYPATH']. pyscan = Scanner(name='pythonfile', diff --git a/test/ScannerAbsPathCheck/test.py b/test/ScannerAbsPathCheck/test.py index a6474d8653..b94783bb20 100644 --- a/test/ScannerAbsPathCheck/test.py +++ b/test/ScannerAbsPathCheck/test.py @@ -30,8 +30,6 @@ contains paths relative to the SConstruct root. """ -import os - import TestSCons test = TestSCons.TestSCons() From 3e98ff77703c7cd186c6d0907a8cd748a2e3c332 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Sun, 22 Dec 2019 17:21:03 -0500 Subject: [PATCH 4/5] Try a possible fix This fix passed several tests so I am updating the PR with it to better understand the impact. --- src/engine/SCons/PathList.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/engine/SCons/PathList.py b/src/engine/SCons/PathList.py index 76cbeab873..fc9d535e7e 100644 --- a/src/engine/SCons/PathList.py +++ b/src/engine/SCons/PathList.py @@ -128,8 +128,13 @@ def subst_path(self, env, target, source): result = [] for type, value in self.pathlist: if type == TYPE_STRING_SUBST: - value = env.subst(value, target=target, source=source, - conv=node_conv) + # We override conv below to use absolute paths when possible. + # This avoids problems with relative paths when the target's + # working directory is different than the FS object's working + # directory. + value = env.subst( + value, target=target, source=source, + conv=lambda x: x.abspath if hasattr(x, 'abspath') else x) if SCons.Util.is_Sequence(value): result.extend(SCons.Util.flatten(value)) elif value: From ed3d04ed29e1fd4b8f637a3fed7a968cc6338c85 Mon Sep 17 00:00:00 2001 From: Adam Gross Date: Fri, 10 Jan 2020 11:46:02 -0500 Subject: [PATCH 5/5] Modify CHANGES.txt and apply preferred fix This change does the following: 1. Modifies CHANGES.txt to include a description of the fix. 2. Applies the preferred fix that I have been testing for a few weeks. --- src/CHANGES.txt | 7 +++++++ src/engine/SCons/PathList.py | 9 +++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index a9417c7853..68136af9c5 100755 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -16,6 +16,13 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER is a reasonable default and also aligns with changes in Appveyor's VS2019 image. - Drop support for Python 2.7. SCons will be Python 3.5+ going forward. + From Adam Gross: + - Fix an issue causing scanners to receive incorrect paths when the + environment contains a Directory node that is a subdirectory of the + source root directory, the environment variable used by the scanner + (e.g. env['LIBPATH']) references a subdirectory of that directory node, + and a SConscript file is being run that is not in the source root + directory. From Mathew Robinson: - Improve performance of Subst by preventing unnecessary frame diff --git a/src/engine/SCons/PathList.py b/src/engine/SCons/PathList.py index 76cbeab873..fc9d535e7e 100644 --- a/src/engine/SCons/PathList.py +++ b/src/engine/SCons/PathList.py @@ -128,8 +128,13 @@ def subst_path(self, env, target, source): result = [] for type, value in self.pathlist: if type == TYPE_STRING_SUBST: - value = env.subst(value, target=target, source=source, - conv=node_conv) + # We override conv below to use absolute paths when possible. + # This avoids problems with relative paths when the target's + # working directory is different than the FS object's working + # directory. + value = env.subst( + value, target=target, source=source, + conv=lambda x: x.abspath if hasattr(x, 'abspath') else x) if SCons.Util.is_Sequence(value): result.extend(SCons.Util.flatten(value)) elif value: