MSVC enhancement to add all remaining msvc batch file command-line options as SCons variables #4174

jcbrill · 2022-06-12T18:16:20Z

Add all remaining msvc batch file command-line options as SCons environment variables.

The intent of this PR is to enable all existing msvc batch arguments to be utilized with SCons auto-detection.

This PR modifies and/or adds the following SCons construction variables

MSVC_UWP_APP [modifies]
MSVC_SDK_VERSION [adds]
MSVC_TOOLSET_VERSION [adds]
MSVC_SPECTRE_LIBS [adds]
MSVC_SCRIPT_ARGS [adds]

With the exception of MSVC_SCRIPT_ARGS, all of the SCons variables have a one-to-one mapping with msvc batch file arguments.

Note: the implementation of MSVC_TOOLSET_VERSION does not affect the selection of the msvc instances (i.e., "auto-detection") in any way. Similar to the msvc batch files: given an msvc instance, a toolset version may be specified. This distinction is important as it makes the implementation reasonably straightforward.

All of the environment variables listed above are applied after the msvc instance is selected. This could be the default version of msvc if MSVC_VERSION is not specified.

The MSVC_SCRIPTERROR_POLICY construction variable was added as a mechanism to report msvc batch file errors which could prove useful when attempting to diagnose mysterious build failures.

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 read the README.rst)
I have updated the appropriate documentation

Source Code Tasks

Move proposed code into appropriate locations
Rename and adjust some code added in recent PRs for consistency
Finalize handling of batch file errors

`MSVC_UWP_APP`

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

Known changes from the current main branch:

Boolean True is accepted in addition to string '1'
Attempting to use MSVC_UWP_APP in VS2013 and earlier results in an exception. Prior behavior was to silently ignore the UWP request.

`MSVC_SDK_VERSION`

Base functionality complete
Argument validation complete
Extended functionality complete
I have created a new test or updated the unit tests to cover the new/changed functionality.
I have updated CHANGES.txt (and read the README.rst)
I have updated the appropriate documentation

As currently implemented, the SDK version is passed to the msvc batch files without complete validation (i.e., that the SDK version is actually installed). This is planned.

`MSVC_TOOLSET_VERSION`

Addresses: #3265, #3664, #4149

Base functionality complete
Argument validation complete
Extended functionality complete
I have created a new test or updated the unit tests to cover the new/changed functionality.
I have updated CHANGES.txt (and read the README.rst)
I have updated the appropriate documentation

Known differences between the implementation and the msvc batch files:

The msvc batch files accept toolset versions specified in the form XX.Y, XX.YY, XX.YY.ZZZZZ.
The implementation accepts toolset versions in the form XX.Y, XX.YY, XX.YY.Z, XX.YY.ZZ, XX.YY.ZZZ, XX.YY.ZZZZ, XX.YY.ZZZZZ.

The extended functionality consists of forcing the toolset version to be passed to the msvc batch files when not specified. Currently, there is an internal hard-coded value disabling this feature. By passing the toolset version to the batch files, the SCons cache would be impervious to toolset updates and would execute the batch files appropriately. This could be enabled via an internal-use only windows environment variable when running the test suite(s).

`MSVC_SPECTRE_LIBS`

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

`MSVC_SCRIPT_ARGS`

Closes: #4106

Base functionality complete
Argument validation complete (first-class arguments above and multiple declarations only)
I have created a new test or updated the unit tests to cover the new/changed functionality.
I have updated CHANGES.txt (and read the README.rst)
I have updated the appropriate documentation

`MSVC_SCRIPTERROR_POLICY`

The msvc batch file error detection was modified to process all of the lines of stdout instead of first line. VS2017 and later, display a banner which means that no msvc batch file errors are detected if only the first line is inspected.

If msvc batch file errors are detected, the msvc script error handler is called. The scons behavior is based on the construction variable value or the global policy setting if undefined or None.

The policy values are:

Ignore or Suppress: ignore script error messages when detected.
Warn or Warning: issue a warning when script error messages are detected.
Error or Exception: raise and exception when script error messages are detected.

The default policy is to ignore script error messages.

If script error messages are detected and cl.exe is not detected on the path, an internal exception is raised.

Prior behavior was when an error was detected to raise an internal exception. This was typically the case when querying host/target combinations.

There are errors messages possible that are non-fatal and solely due to the difference between the "command-line environment" and the "scons shell environment" in which the batch file is invoked. These messages would be annoying and there would be no way to easily rectify the situation.

For example, with VS2017 with Typescript installed and MSVC_SCRIPTERROR_POLICY='Warn', the following warning is produced:

    scons: warning: vc script errors detected:
    [ERROR:typescript.bat] TypeScript was not added to PATH since a valid installation was not found
    [ERROR:VsDevCmd.bat] *** VsDevCmd.bat encountered errors. Environment may be incomplete and/or incorrect. ***
    [ERROR:VsDevCmd.bat] In an uninitialized command prompt, please 'set VSCMD_DEBUG=[value]' and then re-run 
    [ERROR:VsDevCmd.bat] vsdevcmd.bat [args] for additional details.
    [ERROR:VsDevCmd.bat] Where [value] is:
    [ERROR:VsDevCmd.bat]    1 : basic debug logging
    [ERROR:VsDevCmd.bat]    2 : detailed debug logging
    [ERROR:VsDevCmd.bat]    3 : trace level logging. Redirection of output to a file when using this level is recommended.
    [ERROR:VsDevCmd.bat] Example: set VSCMD_DEBUG=3
    [ERROR:VsDevCmd.bat]          vsdevcmd.bat > vsdevcmd.trace.txt 2>&1

jcbrill · 2022-06-12T18:17:57Z

WIP PR for discussion purposes.

Test SConstruct for MSVC_TOOLSET_VERSION:

from SCons.Tool.MSCommon.vc import get_sdk_versions

env_list = []
def environment(**kwargs):
    global env_list
    build_n = len(env_list) + 1
    build = '_build{:03d}'.format(build_n)
    print('\nBuild:', build, kwargs)
    VariantDir(build, '.', duplicate=0)
    env=Environment(**kwargs)
    #print("CL.EXE", env.WhereIs('cl.exe'))
    #print(env.Dump())
    env.Program(build + '/hello', [build + '/hello.c'])
    env_list.append(env)
    return env

do_once = True
while do_once:
    do_once = False

    # SDK 10/8.1 Folders 
    # 10.0.22621.0 [Default]
    # 10.0.22000.0
    # 10.0.20348.0
    # 10.0.19041.0
    # 10.0.18362.0
    # 10.0.17763.0
    # 10.0.17134.0
    # 10.0.16299.0
    # 10.0.14393.0
    # 10.0.10586.0
    # 10.0.10240.0
    # 8.1
       
    # Default MSVC [VS2022 Community] toolsets installed

    # SxS toolsets
    # 14.32.17.2
    # 14.31.17.1
    # 14.30.17.0
    # 14.29.16.11

    # toolsets
    # 14.32.31326
    # 14.31.31103
    # 14.30.30705
    # 14.29.30133
    # 14.16.27023

    sdk_versions = [None] + get_sdk_versions(MSVC_UWP_APP=False)
    for sdk_version in sdk_versions:
        for spectre_libs in (True, False):

            environment(MSVC_TOOLSET_VERSION='14.32.17.2',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.31.17.1',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.30.17.0',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.29.16.11', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.16',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)

            environment(MSVC_TOOLSET_VERSION=None,          MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.3',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.32',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.32.31326', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.31',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.31.31103', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.30',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.30.30705', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.2',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.29',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.29.30133', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.1',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.16',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.16.27023', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_TOOLSET_VERSION='14.0',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)

    # VS2022 Community toolsets installed

    # SxS toolsets
    # 14.32.17.2
    # 14.31.17.1
    # 14.30.17.0
    # 14.29.16.11

    # toolsets
    # 14.32.31326
    # 14.31.31103
    # 14.30.30705
    # 14.29.30133
    # 14.16.27023

    sdk_versions = [None] + get_sdk_versions(MSVC_VERSION='14.3')
    for sdk_version in sdk_versions:
        for spectre_libs in (True, False):

            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.32.17.2',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.31.17.1',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.30.17.0',  MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.29.16.11', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.16',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)

            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION=None,          MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.3',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.32',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.32.31326', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.31',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.31.31103', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.30',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.30.30705', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.2',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.29',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.29.30133', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.1',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.16',       MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.16.27023', MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)
            environment(MSVC_VERSION='14.3', MSVC_TOOLSET_VERSION='14.0',        MSVC_SDK_VERSION=sdk_version, MSVC_SPECTRE_LIBS=spectre_libs)

    # VS2019 Community toolsets installed

    # SxS toolsets
    # 14.29.16.10
    # 14.28.16.9
    # 14.28.16.8

    # toolsets
    # 14.29.30133
    # 14.29.30037
    # 14.28.29910
    # 14.28.29333
    # 14.27.29110
    # 14.16.27023

    sdk_versions = [None] + get_sdk_versions(MSVC_VERSION='14.2')
    for sdk_version in sdk_versions:

        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.29.16.10',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28.16.9',   MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28.16.8',   MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.27',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.16',        MSVC_SDK_VERSION=sdk_version)

        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION=None,           MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.2',         MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.29',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.29.30133',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.29.30037',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28.29910',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.28.29333',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.27.29110',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.1',         MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.16',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.16.27023',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.2', MSVC_TOOLSET_VERSION='14.0',         MSVC_SDK_VERSION=sdk_version)

    # VS2017 Community toolsets installed

    # toolsets
    # 14.16.27023
    # 14.15.26726

    sdk_versions = [None] + get_sdk_versions(MSVC_VERSION='14.1')
    for sdk_version in sdk_versions:

        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.15',        MSVC_SDK_VERSION=sdk_version)

        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION=None,           MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.1',         MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.16',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.16.27023',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.15',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.15.26726',  MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1', MSVC_TOOLSET_VERSION='14.0',         MSVC_SDK_VERSION=sdk_version)

    # VS2017 Express toolsets installed

    # toolsets
    # 14.16.27023

    sdk_versions = [None] + get_sdk_versions(MSVC_VERSION='14.1Exp')
    #sdk_versions = []
    for sdk_version in sdk_versions:

        environment(MSVC_VERSION='14.1Exp', MSVC_TOOLSET_VERSION=None,          MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1Exp', MSVC_TOOLSET_VERSION='14.1',        MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1Exp', MSVC_TOOLSET_VERSION='14.16',       MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1Exp', MSVC_TOOLSET_VERSION='14.16.27023', MSVC_SDK_VERSION=sdk_version)
        environment(MSVC_VERSION='14.1Exp', MSVC_TOOLSET_VERSION='14.0',        MSVC_SDK_VERSION=sdk_version)

    # VS2015 Community [No toolset support]

    sdk_versions = [None] + get_sdk_versions(MSVC_VERSION='14.0')
    for sdk_version in sdk_versions:

        environment(MSVC_VERSION='14.0', MSVC_SDK_VERSION=sdk_version)

…es. Sort arguments in preferred order.

…e consistency for msvc sdk versions.

…anup.

jcbrill · 2022-06-20T17:41:47Z

@bdbaddog and @mwichmann Guidance request at your conveniences.

The "code portion" is effectively complete for this PR. However, I need some guidance on the preferred code organization moving forward.

Some of the additions that I authored recently (e.g., setting up the default environment warning suppression primarily) used a large internal class with class attributes and classmethods. In the implementation of this PR, both the amount of code and the number of new internal classes expanded. While I am fond of this style, there is a drawback with the debug log as the class names are not written. I'm guessing that unit testing is a bit harder as well.

As an experiment, the current branch was re-worked to use a sub-module (SCons.Tool.MSCommon.MSVC) organization with the "class namespaces" moved to proper module files. There were a few quirks regarding a clean separation of the existing vc.py code and the new submodules. For instance, the base exception type was moved to the submodule and imported by vc.py rather than defined in vc.py. The recent msvc not found policy and default environment setup were moved into their own standalone modules. The default environment setup required a callback function to avoid circular dependencies/references.

This PR: https://github.com/jcbrill/scons/tree/jbrill-msvc-batchargs
This PR using modules: https://github.com/jcbrill/scons/tree/jbrill-msvc-batchargs-modules

When comparing vc.py in both branches, once the big chunks that moved to submodules are ignored there aren't that many changes. As of this morning, both branches should be functionally equivalent. I still need to do some of my own testing for the default environment warning suppression to make sure I didn't break anything.

The outstanding question is how to proceed? monolithic vc.py or smaller pieces via sub-modules?

There are advantages and disadvantages of each way forward.

Any and all feedback is appreciated.

mwichmann · 2022-06-20T17:48:41Z

Sadly, I agree with your comment that there are advantages either way. If I'm not dealing with a transition from earlier code, I strongly prefer dividing things out into smaller pieces, and implementing separation of responsibility. We're of course dealing with transition from a long-standing code arrangement, so the "If" part clearly applies...

There's nothing magical about the debug printing, if we need to add a class name or other information to how the logging setup is initialized, we can do so - but on the other hand, the msvc debug output is pretty "busy" already, at least for me it's already hard to pick out meaningful bits from lots of noise.

bdbaddog · 2022-06-20T18:58:03Z

Would it work to name the module vc and then just have a __init__.py which would have the functionality/variables currently in vc.py?
That way anyone importing MSCommon/vc.py would not see any difference?

I like the submodule for when a file becomes way too large.. I did similar for ninja (if you noticed it moved from ninja.py to ninja/*).
Especially anything that refactors a nearly 3000 line source file.. ;)
(So thumbs up on that reorg)

Also are your complete checkboxes in the initial description at the top up to date?
Lastly, should probably update the README in MSCommon.

jcbrill · 2022-06-20T19:43:01Z

Also are your complete checkboxes in the initial description at the top up to date?
Lastly, should probably update the README in MSCommon.

Unfortunately, the checkboxes are up to date.
The two biggest remaining tasks are:

msvc.xml documentation of all new variables, and
unit tests

README and CHANGES should not be very difficult once the other two are complete.

Would it work to name the module vc and then just have a init.py which would have the functionality/variables currently in vc.py?
That way anyone importing MSCommon/vc.py would not see any difference?

Probably. I'm pretty sure that importing MSCommon/vc.py as it stands now would not be different.

@bdbaddog Would it be useful create a possible throwaway [WIP] PR with the modules branch so that the two of you can review the layout as it stands? If anyone likes the new layout, I can add it to this PR in one commit. If no one likes the other layout, then no harm done.

bdbaddog · 2022-06-20T19:51:48Z

@jcbrill - Just thinking that since MSCommon/MSVC is effectively what was previously in vc.py, it might make sense to remove vc.py and make that the module name and move it's logic into vc/init.py ?

I'm ok with that refactor being in this PR.

jcbrill · 2022-06-20T20:32:40Z

Just thinking that since MSCommon/MSVC is effectively what was previously in vc.py, it might make sense to remove vc.py and make that the module name and move it's logic into vc/init.py ?

vc.py is still pretty large after the refactor: ~1350 loc.

At present, MSCommon/MSVC, only contains "recent/new components from vc.py":

msvc_notfound_policy support (parallel functions moved from vc.py to new module),
Suppress warning for default tools (_MSVCSetupEnvDefault class moved from vc.py to new module),
and this PR

None of the MSVS/MSVC tests have been affected yet.

I'm not arguing ... just feeling a little uneasy.

bdbaddog · 2022-06-20T20:45:19Z

Ok. Let's not tie the vc.py -> module with this functionality.
I do like the moving logic to MSVC, so if you can push that into this PR.
I think that would be an improvement.

mwichmann · 2022-07-18T16:28:37Z

SCons/Tool/MSCommon/README.rst

+        +-> MSCommon/vc.py:_check_cl_exists_in_vc_dir:
+          | Figure out host/target pair
+          | if version > 14.0 get specific version by looking in
+          |    pdir + Auxiliary/Build/Microsoft/VCToolsVersion/default.txt


[Secret Scan] It may be dangerous to commit the AWS secret access key. (view)

AWS secret access key is an important credential that allows you to use AWS programmatically.
Once this leaks, an attacker can access and operate your AWS resources.

See also:

https://docs.aws.amazon.com/general/latest/gr/aws-access-keys-best-practices.html

Rule Severity

sider.secrets.aws.secret_access_key warning

You can close this issue if no need to fix it. Learn more.

Sider got fooled, it thinks this might contain an AWS secret key.

The line it's flagging sure doesn't look like that to me:

| pdir + Auxiliary/Build/Microsoft/VCToolsVersion/default.txt

closed that one.. Weird.

bdbaddog · 2022-07-18T17:43:36Z

I'll leave a "ruling" on that to @bdbaddog, but don't see where that should be a problem - it's a little riskier for code, where you might want to trace history backwards and have to remember git flags to track back across the rename (--follow), but that should be a non-factor for in-directory README files.

Yes. Rename is fine. use git mv old new should do the trick I think.

…skip]" This reverts commit 4822e9b.

jcbrill · 2022-07-18T19:12:35Z

Mini-update: The only outstanding item is the request above to add content to MSCommon/README.rst.

Currently, there are no other planned code or documentation changes.

While there are a lot files, when broken down into logical groups it seems more manageable as the source files are really about 1/3 of the files and some are very small.

Note: due to moving the recent code to suppress warnings for msvc missing when there are no installations into MSVC/SetupEnvDefault.py, there appears to be a large difference with vc.py. Comparing the current PR with a state prior to that particular merge may be more illuminating.

Source Files [Total=15, New=11, Modified=4]:

SCons/Tool
- MSCommon
  - MSVC
    - Config.py [New]
    - Dispatcher.py [New]
    - Exceptions.py [New]
    - Policy.py [New]
    - Registry.py [New]
    - ScriptArguments.py [New]
    - SetupEnvDefault.py [New]
    - Util.py [New]
    - Warnings.py [New]
    - WinSDK.py [New]
    - __init__.py [New]
  - __init__.py [Modified]
  - common.py [Bug Fix & Modified]
  - vc.py [Modified]
- docbook
  - __init__.py [Bug Fix]

Documentation Files [Total=5, New=0, Modified=5]:

CHANGES.txt [Modified]
RELEASE.txt [Modified]
SCons/Tool
- MSCommon
  - README [Renamed/Removed]
  - README.rst [Modified]
- msvc.xml [Modified]

Tests [Total=21, New=16, Modified=5]:

SCons/Tool
- MSCommon
  - MSVC
    - ConfigTests.py [New]
    - DispatcherTests.py [New]
    - PolicyTests.py [New]
    - RegistryTests.py [New]
    - ScriptArgumentsTests.py [New]
    - UtilTests.py [New]
    - WinSDKTests.py [New]
  - vcTests [Modified]
test
- MSVC
  - MSVC_NOTFOUND_POLICY.py [New]
  - MSVC_SCRIPTERROR_POLICY.py [New]
  - MSVC_SDK_VERSION.py [New]
  - MSVC_SPECTRE_LIBS.py [New]
  - MSVC_TOOLSET_VERSION.py [New]
  - MSVC_USE_SCRIPT_ARGS-fixture
    - SConstruct [Modified]
  - MSVC_UWP_APP.py [Modified]
  - msvc_cache_force_defaults.py [New]
  - no_msvc.py [Modified]
fixture
- no_msvc
  - no_msvcs_sconstruct_msvc_query_toolset_version.py[New]
  - no_msvcs_sconstruct_msvc_sdk_versions.py [New]
  - no_msvcs_sconstruct_msvc_toolset_versions.py [New]
  - no_msvcs_sconstruct_version.py [Modified]

Generated Documentation Files [Total=4, New=0, Modified=4]:

doc/genenerated
- functions.gen [Modified]
- tools.gen [Modified]
- variabled.gen [Modified]
- variables.mod [Modifed]

bdbaddog · 2022-07-19T18:30:58Z

@jcbrill - is this done now?

jcbrill · 2022-07-19T18:42:18Z

@bdbaddog PR is complete!

The wherefore and the why of the PR implementation is likely easiest to comprehend by starting the review in the following sequence:

MSCommon/README.rst: For rendered version see https://github.com/jcbrill/scons/blob/jbrill-msvc-batchargs/SCons/Tool/MSCommon/README.rst [edit: link updated]
The generated html documentation for the man page: All of the new construction variables have been fully documented which may provide valuable context when reviewing the implementation.
MSCommon/MSVC/ScriptArguments.py: The primary source code implementing this PR. Lengthy due to validation considerations.
MSCommon/vc.py: large differences due to deletions for moved code (e.g., MSCommon/MSVC/SetupEnvDefault.py and MSCommon/MSVC/Policy.py), otherwise should be fairly straightforward.
MSCommon/common.py: largely uneventful.

Everything else is to support the above in one fashion or another.

bdbaddog · 2022-07-25T03:52:12Z

Merged.
Note. I'll likely have some follow on PR's to "clean up" a few things and perhaps some requests to @jcbrill to add some more inline code comment's documentation.
The new code is not simple, and so some more comments inline would probably be helpful for longer term maintanence.

Specifically.

How do we add a new version of MSVC or the toolset to the existing code. I'm sure this is fairly simple, but an explicit HOWTO would prevent errors in doing so.
I noticed there are some duplicates of utility functions already in SCons.Util and/or functions/code which would better live there than as a utility function inside MSVC.

None of these need hold up the release, but ideally would be addressed in the near term while the code is fresh in @jcbrill's wildly productive mind. :)

Initial support for msvc batch file arguments

3a760ae

jcbrill added 2 commits June 12, 2022 14:26

Fix sider complaints

49d6e9a

Convert VS constants to namedtuples. Update debug message content.

ef1cf08

bdbaddog added this to In progress in msvc bug scrub 04-2022 via automation Jun 13, 2022

bdbaddog added this to the 4.4 milestone Jun 13, 2022

bdbaddog added the MSVC Microsoft Visual C++ Support label Jun 13, 2022

jcbrill added 15 commits June 13, 2022 11:56

Expand data structures and rework version comparisons and messages.

b9c8f0c

Detect multiple declarations between MSVC_SCRIPT_ARGS and new variabl…

74740d2

…es. Sort arguments in preferred order.

Convert SortOrder class to enum

7d195b6

Disable MSVC_TOOLSET_DEFAULT_VCVARSVER

7b8a915

Change IOError to OSError

5b8d1c4

Add SDK version support and validate all arguments.

5187917

Rename msvc batch platform (uwp/store/desktop) to platform_type.

33bef70

Update comments

716f93a

Add dispatch class for reset and verify methods. Verify data structur…

ac6045d

…e consistency for msvc sdk versions.

Merge branch 'master' into jbrill-msvc-batchargs

126a34e

Rename Manager to Dispatcher

736c7e5

Remove unnecessary variable initialization

a5cb849

Update boolean symbols

27cd6c4

Rename _Const to _Config. Rework msvc sdk version function. Minor cle…

94b23f7

…anup.

Fix windows SDK reference

dbd3018

Refactor recent portions of vc.py into MSVC module

010c2ad

mwichmann reviewed Jul 18, 2022

View reviewed changes

Merge branch 'master' into jbrill-msvc-batchargs

da2f846

jcbrill added 5 commits July 18, 2022 14:24

Rename README to README.rst and update contents for RST. [ci skip]

4822e9b

Revert "Rename README to README.rst and update contents for RST. [ci …

587d55d

…skip]" This reverts commit 4822e9b.

Remove README.rst.

d9f89fe

Rename README to README.rst.

1eefde2

Convert format from plain text to RST. [ci skip]

89fc869

jcbrill added 2 commits July 18, 2022 15:37

Remove sentence that predates file rename. [ci skip]

e274cf1

Update MSCommon/README.rst.

aca539b

bdbaddog mentioned this pull request Jul 20, 2022

Add support for multiple MSVC platform toolsets for a single Visual Studio installation #4149

Closed

3 tasks

bdbaddog added 8 commits July 23, 2022 17:06

refactore to avoid shadowing 'bool' changed variable name to bool_val

65204c9

Format code via black

69bb077

Format code via black

6625bc4

Format code via black

3b79d43

fix variable names which were shadowing global/module symbols

eebf3b6

Updated copyright block, minor code fixes

bf659c3

minor formatting updates

2cf871f

Fix sphinx version < 5.1.0. For now 5.1.0 breaks the sphinx api build

94133d4

bdbaddog merged commit 41b0831 into SCons:master Jul 25, 2022

msvc bug scrub 04-2022 automation moved this from In progress to Done Jul 25, 2022

jcbrill deleted the jbrill-msvc-batchargs branch July 26, 2022 13:44

This was referenced Jul 26, 2022

MSVC tool supports choosing specific msvc version down to minor version #3265

Closed

MSVC add support for preview/prerelease versions #4048

Open

jcbrill mentioned this pull request Jul 27, 2022

MSVC Version 14.1 not found with 14.2 installed #3664

Open

jcbrill mentioned this pull request Sep 27, 2022

[DOC] improve description of CPPDEFINES #4235

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MSVC enhancement to add all remaining msvc batch file command-line options as SCons variables #4174

MSVC enhancement to add all remaining msvc batch file command-line options as SCons variables #4174

jcbrill commented Jun 12, 2022 •

edited

Loading

jcbrill commented Jun 12, 2022 •

edited

Loading

jcbrill commented Jun 20, 2022

mwichmann commented Jun 20, 2022

bdbaddog commented Jun 20, 2022

jcbrill commented Jun 20, 2022 •

edited

Loading

bdbaddog commented Jun 20, 2022

jcbrill commented Jun 20, 2022

bdbaddog commented Jun 20, 2022

mwichmann Jul 18, 2022

bdbaddog Jul 18, 2022

bdbaddog commented Jul 18, 2022

jcbrill commented Jul 18, 2022

bdbaddog commented Jul 19, 2022

jcbrill commented Jul 19, 2022 •

edited

Loading

bdbaddog commented Jul 25, 2022

MSVC enhancement to add all remaining msvc batch file command-line options as SCons variables #4174

MSVC enhancement to add all remaining msvc batch file command-line options as SCons variables #4174

Conversation

jcbrill commented Jun 12, 2022 • edited Loading

Contributor Checklist:

Source Code Tasks

MSVC_UWP_APP

MSVC_SDK_VERSION

MSVC_TOOLSET_VERSION

MSVC_SPECTRE_LIBS

MSVC_SCRIPT_ARGS

MSVC_SCRIPTERROR_POLICY

jcbrill commented Jun 12, 2022 • edited Loading

jcbrill commented Jun 20, 2022

mwichmann commented Jun 20, 2022

bdbaddog commented Jun 20, 2022

jcbrill commented Jun 20, 2022 • edited Loading

bdbaddog commented Jun 20, 2022

jcbrill commented Jun 20, 2022

bdbaddog commented Jun 20, 2022

mwichmann Jul 18, 2022

Choose a reason for hiding this comment

bdbaddog Jul 18, 2022

Choose a reason for hiding this comment

bdbaddog commented Jul 18, 2022

jcbrill commented Jul 18, 2022

bdbaddog commented Jul 19, 2022

jcbrill commented Jul 19, 2022 • edited Loading

bdbaddog commented Jul 25, 2022

jcbrill commented Jun 12, 2022 •

edited

Loading

`MSVC_UWP_APP`

`MSVC_SDK_VERSION`

`MSVC_TOOLSET_VERSION`

`MSVC_SPECTRE_LIBS`

`MSVC_SCRIPT_ARGS`

`MSVC_SCRIPTERROR_POLICY`

jcbrill commented Jun 12, 2022 •

edited

Loading

jcbrill commented Jun 20, 2022 •

edited

Loading

jcbrill commented Jul 19, 2022 •

edited

Loading