MSVS: update generation of the vcxproj embedded python script

[jcbrill]

Alternative fix for #4809 for discussion purposes.

Fixes some issues with the msvs tests.

Changes:

• Replace the existing two code paths that generate the msvs tool vcxproj file with a single code path that generates the one-line python script string compatible with all cases. Prior to this change, the default test runs would not test the alternate path as SCONS_LIB_DIR is set in the test os environment.
• The msvs test case SCons version string was changed from self.scons_version to SCons.__version__. Depending on the tests run, the test framework version may not match the SCons library version being tested (e.g., retaining packaging version). The test now matches what the msvs tool uses.
• Modify the embedded vcxproj script and generated python executable string for consistency with each other with respect to SCons env and os.environ usage.
• Modify the embedded vcxproj script to retrieve SCONS_HOME and SCONS_LIB_DIR from the environment if needed.
• Priority order for msvs tool SCons library specification:
  1. env['SCONS_HOME'] literal path when generated if True
  2. os.environ.get('SCONS_HOME', <os.environ['SCONS_HOME'] path when generated>) if True
  3. os.environ.get('SCONS_LIB_DIR', <os.environ['SCONS_LIB_DIR'] path when generated>) if True
  4. if exists and is valid SCons library
  5. first in list of generated locations that exists and is valid SCons library
• Modify generation of python executable path order:
  1. env['PYTHON_ROOT'] literal path if True
  2. $(PYTHON_ROOT) if os.environ['PYTHON_ROOT'] is True
  3. sys.executable literal path
• Add bare asterisk after second argument in msvs_substitute function in testing/framework/TestSConsMSVS.py requiring keyword argument specifications for all optional arguments. The first two arguments are by position the remaining seven arguments must be keyword specified.
• The existing tests will fail if SCONS_HOME is defined in the environment in which the tests are launched and the SCONS_HOME specification does not match exactly the SCONS_LIB_DIR added to the default test environment.
• The environment that generates the msvs vcxproj file is not necessarily equivalent to the environment uses by Visual Studio when the sln file is opened (i.e., double clicked) from Windows File Explorer. This PR should minimize the effects.

Locally, passes msvc/msvs tests for all supported versions of VS using python 3.9. Passes scoop tests with SCons installed as an application and SCons installed via python.

Contributor Checklist:

• I have created a new test or updated the unit tests to cover the new/changed functionality.
• I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
• I have updated the appropriate documentation

[jcbrill]

One (hopefully unrelated) AppVeyor test failed: test\Interactive\cache-force.py.

Image: Visual Studio 2022; Environment: WINPYTHON=Python313:

571/1314 ( 43.46%) C:\Python313\python.exe test\Interactive\cache-force.py
Traceback (most recent call last):
  File "C:\projects\scons\test\Interactive\cache-force.py", line 80, in <module>
    shutil.rmtree(test.workpath('cache'))
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Python313\Lib\shutil.py", line 790, in rmtree
    return _rmtree_unsafe(path, onexc)
  File "C:\Python313\Lib\shutil.py", line 629, in _rmtree_unsafe
    onexc(os.unlink, fullname, err)
    ~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Python313\Lib\shutil.py", line 625, in _rmtree_unsafe
    os.unlink(fullname)
    ~~~~~~~~~^^^^^^^^^^
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\scons\\testcmd.6716.2gdxfl31\\cache\\D9\\d9b75eba1c0a5e85a44cebd150545ffa.tmpd349df6486fb465393f0ad36bd83dff0'
Test execution time: 2.6 seconds

...

Failed the following test:
	test\Interactive\cache-force.py

NO RESULT from the following 207 tests:
...

[mwichmann]

One (hopefully unrelated) AppVeyor test failed: test\Interactive\cache-force.py.

since it only failed on one of the four appveyor runs, it seems likely to be unrelated. Let's keep an eye on it.

[bdbaddog]

Did you miss #4817 ? ;)

[jcbrill]

Did you miss #4817 ? ;)

No.

[bdbaddog]

So if I understand the impact of your changes, rather than fix the location of SCons when generating the project file, you've changed the logic so it can be impacted by user's shell env variables when run via msvs which could be different than when the project file was generated?

[jcbrill]

So if I understand the impact of your changes, rather than fix the location of SCons when generating the project file, you've changed the logic so it can be impacted by user's shell env variables when run via msvs which could be different than when the project file was generated?

Your understanding is correct.

In a nutshell, the generated vcxproj SCons library location logic is equivalent to that of the msvs tool.

Background:

• For the generating the string of the python executable which executes the one-line python script, the value of the os environment variable PYTHON_ROOT is queried. If PYTHON_ROOT is defined, the generated string is effectively $(PYTHON_ROOT) meaning the vcxproj file will use the PYTHON_ROOT environment variable.

• When generating the one-line python script, the literal value of the os environment variable values SCONS_HOME and SCONS_LIB_DIR are generated rather than generating os environment queries in the vcxproj file. Technically, the SCons environment is queried for SCONS_HOME; if undefined, the os environment is queried for SCONS_HOME; if undefined, the os environment is queried for SCONS_LIB_DIR.

  If a value was defined, the user-defined literal string is generated. If a value was not defined, a list of default values was generated. In Fixed GH Issue 4809. scoop.sh installed scons (and likely scons-local )created msvs project files would fail. #4817, only the current SCons library path is generated.

• The SCons msvs.py tool takes into account the SCons environment SCONS_HOME, os environment SCONS_HOME, and os environment SCONS_LIB_DIR. If any of these three are defined the literal value is generated; otherwise, the default list is generated.

  The testing framework script TestSConsMSVS.py only takes into account the os environment SCONS_LIB_DIR. If this is defined the literal value is generated; otherwise, the default list is generated.

  Unless I'm mistaken, SCONS_LIB_DIR is defined in the "default" test environment. This means that the "else" path is never actually tested.

  Additionally, if SCONS_HOME is defined and is not identical to the string used for SCONS_LIB_DIR (e.g., case-differences for the same path), some of the tests will fail as the SCons library uses the SCONS_HOME value and the test suite expects the SCONS_LIB_DIR value.

• There may be differences between the os environment in which the vcxproj file is generated for the first time and the os environment in which Visual Studio is launched and the solution opened.

This PR attempts to achieve:

• Unify the behavior of generating the python executable and the one-line python script.

  For the python executable:

  • If PYTHON_ROOT is defined in the SCons environment: generate the literal string path value. [new]
  • If PYTHON_ROOT is defined in the os environment: generate a reference to the environment variable. [unchanged]
  • Otherwise: generate the path for the currently running python executable. [unchanged]

  For the one-line python script:

  • If SCONS_HOME is defined in the SCons environment: generate the literal string path value. [unchanged]

  • If SCONS_HOME is defined in the os environment: generate an os environment query of SCONS_HOME with a default value of the current os environment value. If SCONS_HOME is not defined in the os environment, generate an os environment query of SCONS_HOME with a default empty string. [new]

  • If SCONS_LIB_DIR is defined in the os environment: generate an environment query of SCONS_LIB_DIR with a default value of the current os environment value. If SCONS_LIB_DIR is not defined in the os environment, generate an os environment query of SCONS_LIB_DIR with a default empty string. [new]

  • Generate checked values for the currently running SCons library path and the existing default library list with a new trailing path appended. There is a quick check to make sure that the library path exists and contains SCons (i.e., it won't add a directory to the python system path unless it contains SCons). [new]

  • The first non-empty path specification of the three "user" path definitions is used. The generated library path is used as long as it still exists when the solution is run. Otherwise, the first path specification for the default library list that contains an SCons library instance is used. [new]

    Non-empty user strings are not validated. Default library locations are validated so not to add any directories to the python system path that exist but do not contain SCons.

• There is a single path that generates the python script. The SCons msvs tool and test framework generate strings that are equal.

Here's an example used during testing (the diff format is used to show the source of the library path definition):

1. SCONS_HOME defined only in the windows terminal command-line.

   set "SCONS_HOME=C:\Work\jbrill-msvs-4809"
   python "%SCONS_HOME%\scripts\scons.py"

   With the print statement uncommented:

   exec_script_main:
     from os.path import isdir, isfile, join
     import os
     import sys
     sconslibs = lambda l: [p for p in l if p and isdir(p) and isfile(join(p, 'SCons', '__init__.py'))]
     libspec = r''
   + libspec = libspec if libspec else os.environ.get('SCONS_HOME', r'C:\Work\jbrill-msvs-4809')
     libspec = libspec if libspec else os.environ.get('SCONS_LIB_DIR', r'')
     libs = [libspec] if libspec else sconslibs([r'C:\Work\jbrill-msvs-4809'])
     libs = libs if libs else sconslibs([join(sys.prefix, 'Lib', 'site-packages', 'scons-4.10.2'), join(sys.prefix, 'scons-4.10.2'), join(sys.prefix, 'Lib', 'site-packages', 'scons'), join(sys.prefix, 'scons'), join(sys.prefix, 'Lib', 'site-packages')])
     sys.path = libs[:1] + sys.path if libs else sys.path
     print(f'libs = {libs}')
     print(f'sys.path = {sys.path}')
     import SCons.Script
     SCons.Script.main()

2. Visual Studio opened and solution loaded. Clean:

     Clean started at 9:22 AM...
     1>------ Clean started: Project: Hello, Configuration: Release Win32 ------
     1>  Starting SCons
     1>  libs = ['C:\\Work\\jbrill-msvs-4809']
     1>  sys.path = ['C:\\Work\\jbrill-msvs-4809', '', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\python314.zip', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\DLLs', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\Lib', 'C:\\Users\\jbrill\\scoop\\apps\\python\\3.14.2', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\Lib\\site-packages']
     1>  scons: Reading SConscript files ...
     1>  exec_script_main:
     1>    from os.path import isdir, isfile, join
     1>    import os
     1>    import sys
     1>    sconslibs = lambda l: [p for p in l if p and isdir(p) and isfile(join(p, 'SCons', '__init__.py'))]
     1>    libspec = r''
     1>    libspec = libspec if libspec else os.environ.get('SCONS_HOME', r'')
     1>    libspec = libspec if libspec else os.environ.get('SCONS_LIB_DIR', r'')
   + 1>    libs = [libspec] if libspec else sconslibs([r'C:\Work\jbrill-msvs-4809'])
     1>    libs = libs if libs else sconslibs([join(sys.prefix, 'Lib', 'site-packages', 'scons-4.10.2'), join(sys.prefix, 'scons-4.10.2'), join(sys.prefix, 'Lib', 'site-packages', 'scons'), join(sys.prefix, 'scons'), join(sys.prefix, 'Lib', 'site-packages')])
     1>    sys.path = libs[:1] + sys.path if libs else sys.path
     1>    print(f'libs = {libs}')
     1>    print(f'sys.path = {sys.path}')
     1>    import SCons.Script
     1>    SCons.Script.main()
     1>  scons: done reading SConscript files.
     ...
     ========== Clean: 1 succeeded, 0 failed, 0 skipped ==========
     ========== Clean completed at 9:22 AM and took 04.470 seconds ==========

3. Visual Studio opened and solution loaded. Build:

     1>------ Build started: Project: Hello, Configuration: Release Win32 ------
     1>  Starting SCons
     1>  libs = ['C:\\Work\\jbrill-msvs-4809']
     1>  sys.path = ['C:\\Work\\jbrill-msvs-4809', '', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\python314.zip', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\DLLs', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\Lib', 'C:\\Users\\jbrill\\scoop\\apps\\python\\3.14.2', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current', 'C:\\Users\\jbrill\\scoop\\apps\\python\\current\\Lib\\site-packages']
     1>  scons: Reading SConscript files ...
     1>  exec_script_main:
     1>    from os.path import isdir, isfile, join
     1>    import os
     1>    import sys
     1>    sconslibs = lambda l: [p for p in l if p and isdir(p) and isfile(join(p, 'SCons', '__init__.py'))]
     1>    libspec = r''
     1>    libspec = libspec if libspec else os.environ.get('SCONS_HOME', r'')
     1>    libspec = libspec if libspec else os.environ.get('SCONS_LIB_DIR', r'')
   + 1>    libs = [libspec] if libspec else sconslibs([r'C:\Work\jbrill-msvs-4809'])
     1>    libs = libs if libs else sconslibs([join(sys.prefix, 'Lib', 'site-packages', 'scons-4.10.2'), join(sys.prefix, 'scons-4.10.2'), join(sys.prefix, 'Lib', 'site-packages', 'scons'), join(sys.prefix, 'scons'), join(sys.prefix, 'Lib', 'site-packages')])
     1>    sys.path = libs[:1] + sys.path if libs else sys.path
     1>    print(f'libs = {libs}')
     1>    print(f'sys.path = {sys.path}')
     1>    import SCons.Script
     1>    SCons.Script.main()
     1>  scons: done reading SConscript files.
     1>  scons: Building targets ...
     ...
     ========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========
     ========== Build completed at 9:22 AM and took 04.327 seconds ==========

If one wants to ALWAYS generate a path that is used, the easiest way is to define SCONS_HOME in the SCons environment as opposed to the os environment (behavior is slightly different):

import os.path
import SCons

env = Environment(
    SCONS_HOME=os.path.abspath(os.path.join(os.path.dirname(SCons.__file__), "..")),
)

sources = ['hello.cpp']
program = env.Program(
    target='hello.exe',
    source=sources,
    CPPFLAGS=['/EHsc'],
)

env.MSVSProject(
    target='Hello' + env['MSVSPROJECTSUFFIX'],
    srcs=sources,
    buildtarget=program,
    variant='Release',
)

The proposed PR is more adaptive than it was before and there is only "one" generated script instead of two.

Is it foolproof? No.

Even if this PR is not accepted, there are plenty of reasons to adapt this PR to the equivalent code in #4817 (i.e., unify both code generation paths in a single script).

I hope that answers the question and provides some background and rationale.

<whining> Comments like these take a long time to write </whining>

[bdbaddog]

@jcbrill they only take a long time when there as thorough as you usually are! :)

A few thoughts/notes:

1. I'm not sure we should use the shell environment msvs is run in to affect which scons is used by msvs to build. That's a non-trivial change from previous behavior, in theory it could require a deprecation cycle. Seems an uncommon occurrence unless the generated project file is committed and then used by another user, which seems even more likely to cause issues?
2. Currently we don't document that PYTHON_ROOT can affect SCons' behavior (see end of manpage : https://scons.org/doc/production/HTML/scons-man.html#ENVIRONMENT
3. Same is true for SCONS_HOME, it is mentioned as a Environment() variable though
4. Unifying the code which generates the snippet and modifying the test logic to allow testing when SCONS_LIB_DIR is not defined is a good update/fix.
5. I think in general we can avoid the long sys.path addition as really we only need one path to find SCons, in fact having the other paths could lead to picking up another install than the one which was used to create the msvs project file

[jcbrill]

@bdbaddog The latest commit should be functionally equivalent to #4817.

I need to do some more local testing.

There was an earlier version of the generated script that detected when the generated SCons path would not be used and printed warnings:

import os.path
import sys
scons_home = r'{scons_home}'
scons_path = r'{scons_path}'
scons_spec = scons_home if scons_home else scons_path
have_scons = bool(scons_spec and os.path.isdir(scons_spec) and os.path.isfile(os.path.join(scons_spec, 'SCons', '__init__.py')))
_ = print(f'*** SCONS WARNING: SCons was not found at the generated library path ({{scons_spec!s}}). ***') if not have_scons else None
sys.path = [scons_spec] + sys.path if have_scons else sys.path
import SCons.Script
_ = print(f'*** SCONS WARNING: SCons was found on the default python system path ({{os.path.dirname(os.path.dirname(SCons.__file__))!s}}). ***') if not have_scons else None
SCons.Script.main()

While it "appears" to work, the VS output console looked a little odd. It may have been interpreting the print statement as a command. I can't be sure.

It was simple enough to force both warnings to be printed. I'm not wild about an SCons being picked up which is not the SCons at the generated path without some kind of feedback to the user.

[jcbrill]

It appears that printing a message with the word "warning" from the embedded python script triggers different behavior.