scons
scons copied to clipboard
Fix issue causing scanners to receive incorrect paths
This pull request resolves an issue where scanners receive an incorrect path if they use PathList
as their path_function
. It also includes a test to demonstrate the issue.
PathList
can be used by scanners to easily get a list of paths corresponding to a specific variable in the environment. For example, the ProgramScanner
sets its path_function
to the result of the call SCons.Scanner.FindPathDirs('LIBPATH')
.
The issue happens when the following conditions are all true (demonstrated by the included test):
- 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. - A SConscript file is being run that is not in the source root directory.
Here is a summary of why the issue is happening. The PathList
class converts the directory to a string, thus turning it into a relative path to the source root. When the PathList
class evaluates the string (e.g. "$SDKROOT/include"
) it substitutes the directory to a string and ends up with a path that is correct relative to the source root. However, it then applies that path as being relative to the SConscript file, which is not correct.
I am not wedded to the current fix but it is the most compelling fix that I have been able to find. In this case it is dangerous to operate on relative paths, so the fix is to operate on absolute paths when doing string substitution in the PathList
class. Object substitution (e.g. if the string being substituted is "$SDKROOT"
instead of "$SDKROOT/include"
) continues to work as before.
Contributor Checklist:
- [x] I have created a new test or updated the unit tests to cover the new/changed functionality.
- [x] I have updated
master/src/CHANGES.txt
directory (and read theREADME.txt
in that directory) - [ ] I have updated the appropriate documentation
Thoughts on this patch? I'm open to alternative solutions if it has risks that I am unaware of.
O.k. did some digging.
So the issue is if the value returned from _PathList.subst_path()
is a string and not a node, then Base.RFindalldirs()
will append it to the current dir (via Dir.get_all_rdirs()
)
If you change your test to the following:
env['SDKROOT'] = env.Dir('sdk')
env.Replace(
PYTHON=sys.executable,
PYPATH=[env.Dir('$SDKROOT'), env.Dir('$SDKROOT/sdk_subdir')],
Then your test passes without any code modifications.
Note that you need to set SDKROOT
before env.Replace
's env.Dir
with $SDKROOT
in it.
This is in line with the expected expansion of any env variable which refers to a File Node from any other directory but the base directory if I'm remembering correctly.
Is that a viable change for you?
Seems like the best fix would be to pass the cwd into the subst_path()
and then have subst itself do the right thing with any nodes it evaluates..
I think your fix, while working for you is not a general purpose fix because it overrides the existing node_conv() function which will call str(arg) in some cases which is not happening with your lambda.
Lemme ponder a bit more. but let us know if you can use actual nodes in your path list?
Also if you allow me write access to this PR branch I'll push fixes to the merge conflicts.