SCons
Fix 4609: MSVS tests and minor changes to msvs tool
Fix MSVS tests and minor changes to Tool/msvs.py.
testing/framework/TestSConsMSVS.py:
- Add default project GUID
- Pass MSVS_PROJECT_GUID via environment
- Add AdditionalOptions Condition to expected vcx project file
- Fix vs version number for vc version 14.3
- Fix expected platform toolset version
SCons/Tool/msvs.py:
- Use environment MSVS_PROJECT_GUID when creating project files info
- Fix writing Visual Studio 15 for VS2015
- Add .vcxprof as an expected suffix for assigning the name to the file base name
Fix early exit after vc version 8.0 (exit at the end of first loop execution) in:
- test/MSVS/vs-files.py
- test/MSVS/vs-scc-files.py
- test/MSVS/vs-scc-legacy-files.py
- test/MSVS/vs-variant_dir.py
Tests:
- Modify tests using TestSConsMSVS to add MSVS_PROJECT_GUID to the environment in the generated SConstruct/SConscript files.
- Fix: delete
env['PYTHON_ROOT']before next loop iteration in test/MSVS/vs-files.py. - Fix: add safe HOST_ARCH to sconstruct/sconscript files for vs*scc-files` and vs*-scc-legacy-files tests for non-windows, unsupported architectures.
Fixes #4609 Fixes #4608 Fixes #4612 Fixes #4613
Contributor Checklist:
- [x] I have created a new test or updated the unit tests to cover the new/changed functionality.
- [ ] I have updated
CHANGES.txtandRELEASE.txt(and read theREADME.rst). - [ ] I have updated the appropriate documentation
@mwichmann Need review of Tool/msvs.py changes to function GenerateProjectFilesInfo.
I believe all elements of the loop should share the same project GUID but I'm not sure. Passing a project GUID via the environment was already supported. However, I'm not convinced that it was ever used to test the newer generated files due to the early exit issue.
There may be a better/safer way by possibly mapping the project guid to the first generated loop GUID and using the project GUID only for matching generated loop GUIDs. The tests would certainly fail if there were different GUIDS in the loop.
There is an issue with the Tool/msvs.py changes to function GenerateProjectFilesInfo.
It works as expected for the test suite and would be fine in production if there is only one project.
It's not going to do the right thing in production for multiple projects (i.e., len(dspfiles) > 1) as they would all get assigned the same project GUID.
In this case, it would be more useful if MSVS_PROJECT_GUID was a scalar GUID or list of GUIDs.
@mwichmann The is an odd test/Docbook/basic/html/html_cmd.py failure.
350/1285 (27.24%) C:\Python38\python.exe test\Docbook\basic\html\html_cmd.py
STDOUT =========================================================================
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
C:\ProgramData\chocolatey\bin\xsltproc.EXE -o manual.html C:\projects\scons\SCons\Tool\docbook\docbook-xsl-1.76.1\html\docbook.xsl manual.xml
scons: done building targets.
STDERR =========================================================================
0a1,7
> error : No such file or directory
> http://www.oasis-open.org/docbook/xml/4.1.2/dbcentx.mod:192: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.1.2/ent/iso-pub.ent"
> %ISOpub;
> ^
> Entity: line 1:
> %ISOpub;
> ^
FAILED test of C:\projects\scons\scripts\scons.py
at line 734 of C:\projects\scons\testing\framework\TestCommon.py (_complete)
from line 835 of C:\projects\scons\testing\framework\TestCommon.py (run)
from line 459 of C:\projects\scons\testing\framework\TestSCons.py (run)
from line 41 of test\Docbook\basic\html\html_cmd.py
Test execution time: 25.5 seconds
#4609 failed the same test:
350/1285 (27.24%) C:\Python38\python.exe test\Docbook\basic\html\html_cmd.py
STDOUT =========================================================================
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
C:\ProgramData\chocolatey\bin\xsltproc.EXE -o manual.html C:\projects\scons\SCons\Tool\docbook\docbook-xsl-1.76.1\html\docbook.xsl manual.xml
scons: done building targets.
STDERR =========================================================================
0a1,7
> error : No such file or directory
> http://www.oasis-open.org/docbook/xml/4.1.2/dbcentx.mod:192: warning: failed to load external entity "http://www.oasis-open.org/docbook/xml/4.1.2/ent/iso-pub.ent"
> %ISOpub;
> ^
> Entity: line 1:
> %ISOpub;
> ^
FAILED test of C:\projects\scons\scripts\scons.py
at line 734 of C:\projects\scons\testing\framework\TestCommon.py (_complete)
from line 835 of C:\projects\scons\testing\framework\TestCommon.py (run)
from line 459 of C:\projects\scons\testing\framework\TestSCons.py (run)
from line 41 of test\Docbook\basic\html\html_cmd.py
Test execution time: 25.5 seconds
There's a range of things to do with docbook, that seem to get gradually worse over time. That one seems to indicate a missing file, but I've seen the same test fail other ways on systems that do have the file. It seems Debian/Ubuntu don't package it either (according to packages.ubuntu.com) - have no idea where it would come from if you're on Windows. Wouldn't worry too much. Someday, maybe someone will fix those.
Thank you.
There is a lengthy commit message for the last commit: https://github.com/SCons/scons/pull/4610/commits/76839d932e31aeb092f31fd1d81922aba193b446
I believe there were two unreported bug fixes and a minor change in behavior in the commit.
One of which appears to have happened in the real world: https://stackoverflow.com/questions/41637580/how-to-correclty-use-msvssolution-and-msvsproject
Please add a RELEASE/CHANGES.txt blurb.
@mwichmann & @jcbrill - do we need any doc updates here? Any functional changes not covered by docs?
do we need any doc updates here? Any functional changes not covered by docs?
YES!
An optional argument was added to MSVSProject (e.g., projectguid="{MYUSERGUID}"). It was either add an argument to MSVSProject or change the behavior of MSVS_PROJECT_GUID (which is documented) to be either a scalar or a list.
With the addition of the project argument, MSVS_PROJECT_GUID is somewhat obsolete and should only be used for single project solutions.
However, it is still used and may not do the what one would want if MSVS_PROJECT_GUID is defined and there are two or more MSVSProject instances without projectguid settings attached to MSVSSolution as multiple projects would be using the same GUID.
There is also a known change in behavior for standalone MSVSSolution generation: the solution file has the project files as sources due to the GUID assignments (i.e., the projects are generated before the solution. This was not the case before so it affects the behavior of clean: when the solution is cleaned the project files are also cleaned (this is the same behavior as auto_build_solution=1).
There is one more thing I want to look into involving cleaning with the variant-dir builds. The behavior is that the placeholder files are removed but not the files generated in the source folder. I think there is an old issue for this.
It would be nice if someone actually using MSVSProject and MSVSSolution tested the behavior.
You do realize if that:
env.MSVSProject(..., MSVS_PROJECT_GUID='unique to this builder call')
Will yield that builder getting it's own unique MSVS_PROJECT_GUID (via the OverrideEnvironment() which is always created when a builder is called with extra arguments)
So no need to add projectguid.
-Bill
The "projectguid" is retrieved from the environment. It needs to be added to the MSVSProject arguments documentation.
The project guid was implemented parallel to the MSVSProject argument slnguid which does not appear to documented anywhere but is retrieved in the msvs tool.
slnguid is still in all of the msvs test files.
For example, runfile.py
SConscript_contents = """\
env=Environment(tools=['msvs'], MSVS_VERSION = '8.0')
env.MSVSProject(target = 'Test.vcproj',
projectguid = '%(PROJECT_GUID)s', <=== ADDED IN THIS PR
slnguid = '{SLNGUID}',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
"""
Ultimately, the decision is above my paygrade.
The documentation for MSVS_PROJECT_GUID is pretty simple and there is no explicit reason to believe one would add it to MSVSProject instead of the Environment.
For single project solutions it would not matter:
env = env=Environment(tools=['msvs'], MSVS_VERSION = '8.0', MSVS_PROJECT_GUID="{MYGUID}")
env.MSVSProject(target = 'Test.vcproj',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 1)
For multiple projects, this would assign the same GUID to both projects:
env = env=Environment(tools=['msvs'], MSVS_VERSION = '8.0', MSVS_PROJECT_GUID="{MYGUID}")
p1 = env.MSVSProject(target = 'Test.vcproj',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
p2 = env.MSVSProject(target = 'Test.vcproj',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
env.MSVSSolution(
target = 'Test.sln',
projects = [p1, p2],
variant = 'Release',
)
Where this would allow projectguid to be documented with MSVSProject:
env = env=Environment(tools=['msvs'], MSVS_VERSION = '8.0')
p1 = env.MSVSProject(target = 'Test.vcproj',
projectguid = '{MYGUID1}',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
p2 = env.MSVSProject(target = 'Test.vcproj',
projectguid = '{MYGUID2}',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
env.MSVSSolution(
target = 'Test.sln',
projects = [p1, p2],
variant = 'Release',
)
Even for single projects, it would be more useful:
env = env=Environment(tools=['msvs'], MSVS_VERSION = '8.0')
env.MSVSProject(target = 'Test.vcproj',
projectguid = '{MYGUID}',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 1)
I like it better as lowercase argument like the other project arguments. Having both is probably asking for trouble.
There's no projectguid in the manpage, only MSVS_PROJECT_GUID, so that should be the variable name used.
I think you're still missing my point about OverrideEnvironment()
env = env=Environment(tools=['msvs'], MSVS_VERSION = '8.0', MSVS_PROJECT_GUID="{MYGUID}")
p1 = env.MSVSProject(target = 'Test.vcproj',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0)
# Or you can specify MSVS_PROJECT_GUID above and skip the declaration when you specify Environment() above.
p2 = env.MSVSProject(target = 'Test.vcproj',
srcs = ['test.cpp'],
buildtarget = 'Test.exe',
runfile = 'runfile.exe',
variant = 'Release',
auto_build_solution = 0,
MSVS_PROJECT_GUID="{MYGUID_for_proj_2}")
env.MSVSSolution(
target = 'Test.sln',
projects = [p1, p2],
variant = 'Release',
)
I think you're still missing my point about OverrideEnvironment()
I'm not missing the point.
There's nothing on the man page about projectguid because I added it in this PR and was planning on adding it to the argument list for MSVSProject where the documentation of the variable would be with its primary usage.
The documentation of MSVS_PROJECT_GUID is unclear and a search of all the files will reveal that the only python source file that contains it is in msvs.py. It does not appear in any of the tests. On the other hand slnguid appears in all of the tests files but is not documented.
I will remove projectguid and adjust all the tests accordingly.
But you were asserting For multiple projects, this would assign the same GUID to both projects: indeed that's true with the code, but you can specify any envvar as extra argument to any builder and that invocation of the builder would see that value and not that in the env it was called with.
But you were asserting For multiple projects, this would assign the same GUID to both projects: indeed that's true with the code, but you can specify any envvar as extra argument to any builder and that invocation of the builder would see that value and not that in the env it was called with.
Everything you said is correct.
How would a new user reading the man page know that MSVS_PROJECT_GUID should be used with MSVSProject and not the Environment?
This could be problematic if one started off with one project and then later added more projects.
Anyway, I've removed projectguid and updates all of the MSVS test files. Running WSL and Windows MSVS tests before committing and pushing.
@jcbrill - it's in the docs that you can do this. you can specify CPPPATH, LIBS, LIBPATH,etc to Program(), etc..
https://scons.org/doc/production/HTML/scons-user.html#builder_overrides
Sorry if you're taking offense here, it seemed like you weren't aware that this was standard practice (to override or specify env vars to builders in the builder call)..
Sorry if you're taking offense here, it seemed like you weren't aware that this was standard practice (to override or specify env vars to builders in the builder call)..
Apology not necessary. No offense taken.
<petulant rant> There are a number of features in the msvs tool around MSVSProject and MSVSSolution for which there were no tests nor were all combinations of features addressed and likely still aren't. Multiple projects with a single solution is one instance. Combinations of feature tests that were missing resulted in #4612, and #4613. Nor were there documentation examples.
When I added a multiple project single solution test using a variantdir from scratch, I couldn't understand why it wasn't working. A google search a user in the wild experiencing the same behavior: (https://stackoverflow.com/questions/41637580/how-to-correclty-use-msvssolution-and-msvsproject).
It is a bit annoying that particular use-case was not tested. I would have expected that to be somewhat popular. Multiple projects with a single solution does work as expected when not using a variantdir though. </petulant rant>
@jcbrill - I don't think MSVSSolution/Project are a widely used feature, especially with VariantDirs. And even less SCons developers use those.. so I'm to terribly suprised that the solution isn't as flexible as large real world usage would demand (as yet)..
Fixes #4608 [needs testing]
I can now confirm that it does. Got quite a bit slower, now those four tests are running all the combos, not just one :-)
Fixes #4608 [needs testing]
I can now confirm that it does. Got quite a bit slower, now those four tests are running all the combos, not just one :-)
Still think these tests could be skipped on non-Windows platforms... as noted elsewhere, these days we have adequate automated test coverage on Windows, and with the retirement of Visual Studio for Mac, we could limit these to the only platform they've of intetrest on.
I did run across a project recently that uses the generated vsxproj file... might have been Godot, or it might have been that automotive project, but can't remember at the moment. It does seem to be the case that people use it to inform the IDE about the project details but don't depend on using it to be able to build directly from Visual Studio using SCons.
Just checked Godot... they seem to create their own files as well (there's a directory of project file templates)
It is probably valuable for some as a code browser if nothing else.
Mongodb appears to generate their own project file (vcxproj).
From earlier testing, I'm reasonably confident platform="win32" can be removed from the Environment() calls in the MSVS tests and it still generates code as expected.
First pass: remove platform="win32"
Second pass: skip if not windows
Either way, the first pass can be done. I'm onboard with the second pass as well.
I have an almost-working example of generating the sln/vcxprof files in their own directory in user-space (sconstruct/sconscript files). It is not as straightforward as one might hope due to needing to having to generate relative file paths for the source files. I suspect that is why the files are generated in the source directory.
Known issue: the generated project files include the top-level "SConstruct" file as a source file without a relative path to the sconstruct file.
There are quite a view outstanding Issues for MSVSProject/MSVSSolution. At least one of them (not sure the issue number at the moment), I believe is "invalid usage" (i.e., SCons is correct to complain) rather than a bug.
Personal Internal Notes - Implementation vs Documentation:
-
MSVSProject
-
Documentation - MSVSProject:
srcs: "srcs must be a string or a list of strings"incs: "incs must be a string or a list of strings"localincs: "localincs must be a string or a list of strings"resources: "resources must be a string or a list of strings"misc: "misc must be a string or a list of strings"auto_build_solution: evaluates true (expected 0/1?)target: target project file namevariant: "variant must be a string or a list of strings"cmdargs: string or list of stringscppdefines: string or list of stringscppflags: string or list of stringscpppaths: string or list of stringsbuildtarget: "buildtarget can be a string, a node, a list of strings or nodes, or None"runfile: "runfile can be a string, a node, a list of strings or nodes, or None"
-
Documentation - MSVS_PROJECT_GUID:
MSVS_PROJECT_GUID: string-convertible? (not validated)
-
Implementation - Undocumented:
name: "name must be a string"outdir: "outdir can be a string, a node, a list of strings or nodes, or None"nokeep: 0, not 0
-
Implementation - Constraints:
cmdargs: "Sizes of 'cmdargs' and 'variant' lists must be the same."cppdefines: "Sizes of 'cppdefines' and 'variant' lists must be the same."cppflags: "Sizes of 'cppflags' and 'variant' lists must be the same."cpppaths: "Sizes of 'cpppaths' and 'variant' lists must be the same."buildtarget: "Sizes of 'buildtarget' and 'variant' lists must be the same."outdir: "Sizes of 'outdir' and 'variant' lists must be the same."runfile: "Sizes of 'runfile' and 'variant' lists must be the same."
-
Implementation - Notes:
name: Project name otherwise derived from target filename
-
-
MSVSolution
-
Documentation - MSVSSolution
-
target: target solution file name -
variant: "variant must be a string or a list of strings" -
projects: list of strings or Project nodesDocumentation:
A list of project file names, or Project nodes returned by calls to the MSVSProject Builder, to be placed into the solution file. Note that these filenames need to be specified as strings, NOT as SCons File Nodes.
Implementation details:
-
projectEmitter =
projects:if env.get('auto_build_solution', 1): env['projects'] = [env.File(t).srcnode() for t in targetlist] t, s = solutionEmitter(target, target, env) targetlist = targetlist + t -
solutionEmitter -
projects:Project Nodes Ignored (Important?)
if 'projects' in env: if SCons.Util.is_String(env['projects']): source = source + ' "%s"' % env['projects'] elif SCons.Util.is_List(env['projects']): for t in env['projects']: if SCons.Util.is_String(t): source = source + ' "%s"' % t -
_DSWGenerator -
projects:def _projectDSPNodes(env): if 'projects' not in env: raise SCons.Errors.UserError("You must specify a 'projects' argument to create an MSVSSolution.") projects = env['projects'] if not SCons.Util.is_List(projects): raise SCons.Errors.InternalError("The 'projects' argument must be a list of nodes.") projects = SCons.Util.flatten(projects) if len(projects) < 1: raise SCons.Errors.UserError("You must specify at least one project to create an MSVSSolution.") + sln_suffix = os.path.normcase(env.subst('$MSVSSOLUTIONSUFFIX')) dspnodes = [] for p in projects: node = env.File(p) + if os.path.normcase(str(node)).endswith(sln_suffix): + continue dspnodes.append(node) if len(dspnodes) < 1: raise SCons.Errors.UserError("You must specify at least one project node to create an MSVSSolution.") return dspnodes
-
-
-
Documentation - MSVS_SCC_*
MSVS_SCC_AUX_PATHMSVS_SCC_CONNECTION_ROOTMSVS_SCC_PROJECT_NAMEMSVS_SCC_PROVIDER
-
Implementation - Undocumented [deprecated]:
-
MSVS_SCC_LOCAL_PATH# MSVS_SCC_LOCAL_PATH is kept for backwards compatibility purpose and should be deprecated as soon as possible.
-
-
Implementation - Undocumented:
-
name: "name must be a string"MSVS 6.0 only?
-
slnguid: "slnguid must be a string"MSVS 6.0, 2002, 2003 (self.version_num < 8.0).
-
nokeep: 0, not 0 -
Implementation - Notes:
name: Solution name otherwise derived from target filename
-
-
I have nothing remaining planned for this PR.
Still think these tests could be skipped on non-Windows platforms... as noted elsewhere, these days we have adequate automated test coverage on Windows, and with the retirement of Visual Studio for Mac, we could limit these to the only platform they've of intetrest on.
Do we want to skip the "loopy" (i.e., iterates over all msvc versions and not a snarky commentary on the msvs tool itself) msvs tests on non-win32 platforms?
Personal Internal Notes - Implementation vs Documentation:
Was there someplace this could go, as it seems useful? I'm trying to gradually add annotations and docstring argument info where it makes sense - but these don't seem like they correspond to an actual function/method, do they?
Was there someplace this could go, as it seems useful? I'm trying to gradually add annotations and docstring argument info where it makes sense - but these don't seem like they correspond to an actual function/method, do they?
For the most part, the notes above don't correspond to an actual function/method but rather the emitter/builder for MSVSProject and MSVSSolution. Some, but not necessarily all, of the "arguments" (i.e., values retrieved from the environment) are documented for their respected builders.
There are three things at play: the implementation, the tests, and the documentation.
For example, "slnguid" is used in some of the tests and retrieved in the code but is not in the documentation. The comment in the code is:
if self.version_num < 8.0:
# When present, SolutionUniqueID is automatically removed by VS 2005
# TODO: check for Visual Studio versions newer than 2005
self.file.write('\t\tSolutionUniqueID = %s\n' % slnguid)
I'm not sure where the notes above would/should go. I guess if there is something important or useful to a user, the MSVSProject and MSVSSolution builder documentation should be updated.
At present, I'm not exactly sure what all of the undocumented "arguments" are doing.
Backstory: I had a "working" implementation that allowed changing the location of the generated project/solution files to a different folder which required generating relative source file locations in the project files. There is at least one outstanding issue for this. I was not convinced that applying relative paths to "all" sources would do the right thing in the wild and backed out the changes. The internal notes above were the result of trying to figure out all of the keys retrieved from the environment and which of those keys would need relative paths in the generated files.
Since another change is required...
Do we want to skip the "loopy" (i.e., iterates over all msvc versions and not a snarky commentary on the msvs tool itself) msvs tests on non-win32 platforms?
Since another change is required...
Do we want to skip the "loopy" (i.e., iterates over all msvc versions and not a snarky commentary on the msvs tool itself) msvs tests on non-win32 platforms?
Not at this time. I'd rather just sort the changes in flight in this PR, and leave any others to another PR.
@mwichmann Two of the workflows appear to have hung (i.e., making no forward progress for hours). Perhaps canceling is in order.
EDIT: Nevermind. New commit.