Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] MSVC support for selecting specific toolset versions and product types #3717

Closed
wants to merge 65 commits into from

Conversation

jcbrill
Copy link
Contributor

@jcbrill jcbrill commented Jun 26, 2020

This PR expands MSVC support for end-user selection of specific MSVC toolset versions (e.g., 14.26, 14.26.27, 14.26.27023) and/or specific MSVS product type installations (e.g., Enterprise, Community, Build Tools).

This PR unifies #3265 and #3664:

The definition of MSVC_VERSION was extended to allow specification of an msvc toolset down to the minor version and/or the specification of an msvs product type.

The implementation is backwards compatible with current definitions of MSVC_VERSION. There are subtle, but important, changes in semantics due to the change from a "product" orientation to a "toolset" orientation.

The proposed implementation is "wired-in" to the existing code base in a few select locations. The existing code base continues to operate on an MSVC_VERSION that is compatible with the _VCVER version list (e.g., "14.2"). A single internal environment variable and dictionary was added to maintain the specific version and product state.

There is a single hard-coded variable at the top of the source file that selectively enables/disables the specific version support. When disabled, the functionality is the same as the current master. The hooks into the existing code were intentionally minimized to ease updating/merging from the master. The bulk of the new code was added at the bottom of the file. This might allow an optional command-line option to selectively enable the version specific support by an end-user.

There are four (4) new exception types that are propagated outside of the vc.py module that cause SCons to exit with an error code.

Known issues:

  • Remove developer initials (JCB) from comments in locations of special interest.
  • Additional hooks into the existing code base.
  • Update existing code comments.
  • Add new code implementation notes.
  • Remove innocuous one-line comments from new code.
  • Review/document exception handling.
  • Add function docstrings as necessary.
  • Add debug message invocations in select locations.
  • Additional testing in other environments and msvs combinations.
  • Documentation.

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

@jcbrill
Copy link
Contributor Author

jcbrill commented Jun 26, 2020

Version Specific Code

The implementation contains a variable to enable/disable the specific version support. When disabled, the implementation is the same as the current master.

# Experimental version - specific version and product types:
#    True:  specific msvc toolset and product selection
#    False: _VCVER msvc version (master compatible)
#
_MSVC_EXPERIMENTAL_ACTIVE = True

The implementation contains code to print the module call tree (stack trace) to stderr based on a hard-coded internal variable. The number of frames outside the current module is configurable but currently set to one.

The call trees are identical for the current master and the proposed implementation with the experimental code disabled:

A call tree when the experimental code is active with file and line locations is:

The call tree files can also be used to compare/update the existing README for the msvc tools.

Extended MSVC_VERSION Format

The MSVC_VERSION format was extended to allow:

  • specifying an individual toolset at varying degrees of precision:

    • "14.1"
    • "14.16"
    • "14.16.27"
    • "14.16.27023"
  • specifying individual product types (attached to a specific toolset or a product):

    • Enterprise: "14.26Ent"
    • Professional: "14.2Pro"
    • Community: "14.26.27Com"
    • Build Tools: "14.2BT"
    • Express: "14.1Exp"
  • a "right assignment operator" ("->") that maps a specific toolset to a defined product version with an optional product type:

    • "14.0->14.2" use the the 14.0 toolset via a 2019 installation
    • "14.16.27->14.2BT" use the 14.16.27 toolset via a 2019 Build Tools installation
    • "14.1->14.2Com" use the 14.1 toolset via a 2019 Community installation

Without a product qualifier (e.g., "BT"), there is a subtle but significant change semantically: the product selected will be the newest/latest toolset that matches the user specification within installed toolsets for a given (host,target) combination.

Without a product qualifier, any product type could be returned based on the installed toolsets. This only affects multiple installations of the same product version (e.g., 14.1 and 14.Exp and/or 14.2Com and 14.BT).

This means that a user does not need to specify "14.1Exp" explicitly if there is a single 14.1 product installation. On the flip side, it also means that "14.1Exp" may be returned for a "14.1" request. This might make SConstruct files a little more portable as "14.1" could be used any computer that has a 14.1 product installed regardless of product type.

In local testing, "14.1Exp" was returned for an "arm" target. In the local installations of 14.1 Community and 14.1 Express, the "newest" toolset for an arm target was in the 14.1 Express installation as it was installed recently.

With a product qualifier, a specific product version and type is used.

The regular expression to validate/parse the extended MSVC_VERSION format is:

# Regular Expression to validate/parse MSVC_VERSION
#
#    named groups:
#       version:  _VCVER version or specific version
#       vcver:    _VCVER version or None
#       specific: specific version or None
#       product:  _VCVER version or None
#       suffix:   product type suffix or None

_MSVC_EXTENDED_VERSION_RE = re.compile("""
    # anchor at start of string
    ^
    # version number
    (?P<version>
        # _VCVER version: 14.2
        (?P<vcver> \d{1,2} \. \d{1} )
        |
        # toolset version: 14.16, 14.16.27023
        (?P<specific> \d{1,2} \. \d{2} (?: \. \d{1,5} )* )
    )
    # optional product: '->' 14.2
    (?:
        # optional whitespace
        \s*
        # right assignment literal '->'
        [-][>]
        # optional whitespace
        \s*
        # _VCVER version: 14.2
        (?P<product> \d{1,2} \. \d{1} )
    )*
    # optional whitespace
    \s*
    # optional product type: Ent, Pro, Com, Exp, BT, etc.
    (?P<suffix> [A-Z][A-Za-z]+)*
    # anchor at end of string
    $
""", re.VERBOSE)

Internal Data Structures

There are a number of new data structures that are constructed to support specific toolset versions and product types. The data structures can be "pretty-printed" to stderr based on an internal variable.

Some examples of the internal structures are:

  • 14.0 Installed, 14.1 Not Installed, 14.2 Installed [BuildTools]:

    env = Environment(MSVC_VERSION="14.1")
    

    Output: output-141-142.txt

  • 14.0 Installed, 14.1 Installed [Community, Express], 14.2 Installed [Community, BuildTools]

    env = Environment(MSVC_VERSION="14.2")
    

    Output: output-142.txt

MSVC_TOOLSET and Example

The following environment variable was added to support specific toolset versions and product types:

env['MSVC_TOOLSET'] = {

	'USERVER': None,  # save user version specification
	'VERSION': None,  # specific version without suffix
	'PRODUCT': None,  # product version (suffix optional)
	'MSVCVER': None,  # _VCVER compatible version
	'VARSVER': None,  # version used for vcvars batch file

	# cache values to allow state rollback if lookup fails
	_MSVC_CONFIG.MSVC_TOOLSET_CACHE: {},

}

The example below is based on #3664.

There are no 14.1 products are installed. The 14.2 Build Tools product is installed with the 14.16 toolset.

env = Environment(MSVC_VERSION="14.1")

The example shows the MSVC_TOOLSET state evolution at four locations:

  • msvc_version_from_toolset: called in msvc_setup_env after get_default_version and before MSVC_VERSION is recorded
  • msvc_setup_env: called in msvc_setup_env after MSVC_VERSION is recorded.
  • msvc_find_specific_version: called in msvc_find_valid_batch_script.
  • msvc_finalize_specific_version: called in msvc_find_valid_batch_script prior to exit.
msvc_version_from_toolset:
  MSVC_VERSION: 14.1
  MSVC_TOOLSET:
    USERVER: 14.1
    VERSION: 14.1
    PRODUCT: None
    MSVCVER: 14.1
    VARSVER: None
    _CACHE_: {}

msvc_setup_env:
  MSVC_VERSION: 14.1
  MSVC_TOOLSET:
    USERVER: 14.1
    VERSION: 14.1
    PRODUCT: None
    MSVCVER: 14.1
    VARSVER: None
    _CACHE_: {}

msvc_find_specific_version:
  MSVC_VERSION: 14.1
  MSVC_TOOLSET:
    USERVER: 14.1
    VERSION: 14.1
    PRODUCT: 14.2BT
    MSVCVER: 14.2
    VARSVER: 14.16.27023
    _CACHE_: {'PRODUCT': None, 'MSVCVER': '14.1', 'VARSVER': None}

msvc_finalize_specific_version:
  MSVC_VERSION: 14.2
  MSVC_TOOLSET:
    USERVER: 14.1
    VERSION: 14.1
    PRODUCT: 14.2BT
    MSVCVER: 14.2
    VARSVER: 14.16.27023
    _CACHE_: {}

Sample SConstruct File

The SConstruct file used for testing is attached below as a text file. The SConstuct file and c files are attached in the zip file.

Combinations in the SConstruct file have been tested on both x86 and amd64 computers with 14.0 Community, 14.1 Community, 14.1 Express, 14.2 Community, and 14.2 Build Tools installed and Python 3.7.

The implementation has an internal variable and list that allows a 2017+ product to be ignore for testing.

The comments indicating which tool is selected is obviously dependent on the toolsets installed.

File: SConstruct.txt
Archive: test_specific.zip

jcbrill added 28 commits June 26, 2020 14:19
…sion. Ignore componentId that is not in the selection ranking dictionary (safety measure: indicates module variables are not consistent).
… it replaces find_vc_pdir_vswhere (share common prefix).
…n of the vswhere function (find_vc_pdir_vswhere_instance) to DummyVsWhere: necessary as long as both functions co-exist.
…stalled_vcs (the test suite was bypassing the cached function unnecessarily setting up the installed vcs multiple times). Rename the existing find_vc_pdir_* with a "_classic" suffix. Set function names for find_vc_pdir and find_vc_pdir_vswhere based on experimental flag (all tests work without changes).
… files ends in a "#" character, replace the "#' with a serial number from 001 to 999 and 000 based file existence.
…rk serial number processing for rewriting debug/trace file names.
… been set when Sync is False: debug and trace both using serial numbers.
…onstructing the installed visual studios list.
jcbrill added 4 commits August 3, 2020 06:00
…symbolic constants rather than a binary literal. Add comment for each binary bit field constant that contains the corresponding base 10 equivalent.
@mwichmann mwichmann added the MSVC Microsoft Visual C++ Support label Aug 3, 2020
@bdbaddog
Copy link
Contributor

bdbaddog commented Aug 7, 2020

I'd rather add MSVC_TYPE to MSVC_VERSION instead of embedding two types of info in MSVC_VERSION.. and needed more complicated decoding logic..

@bdbaddog
Copy link
Contributor

bdbaddog commented Aug 7, 2020

@jcbrill - are you subscribed to scons-devel and/or our discord server?
I'd rather move there for a more complete discussion.

I'm not going to merge the new tracing logic. I think it's a dead end. we'd like to do something more holistic, and potential re-implement MSVC altogether.

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

The PR is roughly the third implementation of the specific version support. There are a few details concerning the progression in #3664. The initial versions used a separate variable. Using a separate variable makes it harder to "upcast/promote" a requested version to a newer toolset (which is the point of #3664). Expanding the definition of MSVC_VERSION was suggested in #3265. A toolset is not product specific (i.e., it is ambiguous when installed in more than one product).

I have not spent much time in the specific version code in the last 5-6 weeks. There are a number of very subtle issues. There are also issues that didn't exist before: the product is installed but the desired toolset may not be installed for all host/target pairs which can change the product used. This happened during testing where an older version of MSVS had a newer toolset installed for ARM targets. This could be a support challenge in that toolset installations/combinations are specific to an end-users installation and may not be easily diagnosed.

The current implementation is backwards compatible with all existing code. When the experimental code is disabled it works exactly like the current master. Possibly adding a command-line option to enable the experimental code would allow the current logic by default and the specific version by user-specified option. At the time, I was trying to minimize the number of files modified and to minimize the number of places that it "hooks" into the current logic.

I am currently not subscribed to scons-devel or the discord server. I can subscribe to scons-devel. I am using Outlook 2007 on a desktop which does not play nice with mailing lists.

With respect to the tracing logic, I figured as much. That is why the last update moves almost all of the code into a sub-folder: it is trivial for me to merge into the master.

@bdbaddog
Copy link
Contributor

bdbaddog commented Aug 8, 2020

Hmm. not sure I understand what you mean by

Using a separate variable makes it harder to "upcast/promote" a requested version to a newer toolset (which is the point of #3664).

If a user requests 14.1, then only 14.1 should satisfy that request.
Not 14.2 or above
And if they're more explicit nn.mm.xx.yy then only nn.mm.xx.yy.* should match.

Otherwise they should not request a version and get the newest version.

Though I guess there are situations where version between N and M could be also usable. But we don't currently support that.

From the manpage:

If $MSVC_VERSION is not set, SCons will (by default) select the latest version of Visual C/C++ installed on your system. If the specified version isn't installed, tool initialization will fail. This variable must be passed as an argument to the Environment() constructor; setting it later has no effect.

I think it's reasonable to say, I only want to use an Express (or enterprise) version, for example, and not specify the version number.

Thoughts?

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

In the toolset specific versions:

  • If a user requests 14.1, 14.1 is installed, and 14.2 is installed with the 14.1 toolset: the "newest" 14.1 toolset in the 14.1 product for the host/target combination is selected.
  • If a user requests 14.1, 14.1 is not installed, and 14.2 is installed with the 14.1 toolsets, the "newest" 14.1 toolset in the 14.2 product for the host/target is selected. MSVC_VERSION=14.2 and the batch file argument --vcvars_ver=14.16.XX.YYYYY

The MSVC_TOOLSET and Example above shows this exact case: 14.1 is requested and the 14.2 Build Tools are installed with the 14.1 toolset. This is the issue in #3664.

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

Expanded example with third option:

  1. MSVC_VERSION="14.1" [MSVC 14.1 installed, MSVC 14.2 installed w/14.1 toolset]:
    MSVC_VERSION="14.1" --vcvars_ver=14.16.XX.YYYYY

  2. MSVC_VERSION="14.1" [MSVC 14.1 not installed, MSVC 14.2 installed w/14.1 toolset]:
    MSVC_VERSION="14.2" --vcvars_ver=14.16.XX.YYYYY

  3. MSVC_VERSION="14.1->14.2" [MSVC 14.1 installed, MSVC 14.2 installed w/14.1 toolset]:
    MSVC_VERSION="14.2" --vcvars_ver=14.16.XX.YYYY

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

The issue that I see with the specific version numbers is that it introduces an end-user maintenance problem as there are frequent toolset releases as compared to product releases. As an SCons user, I'm not sure I want to specify the specific version because I don't want to have to keep changing my build files. The product releases are not that big a deal as they are every couple of years.

I see the utility of #3664 (list item 2 above). The builds were configured to use 14.1 and who know what other requirements tied them to the 14.1 tools. It would be nice in a new installation of 14.2 with the 14.1 toolset that the existing build system "just works".

There are a number of "edge/corner cases". I tried to adhere to the "Principle of Least Astonishment" in the implementation. When something goes wrong with the specific versions, it is almost imperative that an exception is raised as the toolset support is via an argument to the vcvars batch file. Silently doing the wrong thing is worse than raising an exception.

@bdbaddog
Copy link
Contributor

bdbaddog commented Aug 8, 2020

One of the first principles of SCons..

The build should be repeatable regardless of user's environment.

So, if a user requests version X.1, then get version X.1, even if they have version X.2.
If X.1 is not available, it should bail out.

We're not going to change a fundamental principle of SCons (reproducible builds) because you don't want to update your build scripts when there's new releases of the tools.

Now, if you were suggesting that we introduced a way for the build system to EXPLICITLY introduce flexibility, that would be acceptable.

But not without explicitly requesting it.

So MSVC_CHOOSE_VERSION(or even MSVC_VERSION if it's a callable) as a python function which gets passed the list of available versions and selects, that'd be fine.

But asking for 14.1 and accepting 14.2... nope.

BTW. I'm totally not saying that what you need SCons to do is unreasonable, just that it changes a fundamental SCons principle, and so wouldn't be merge-able without some explicit way to tell SCons to be flexible.

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

It is helpful to stop thinking in terms of products and start thinking in terms of toolsets and from an end user's perspective.

We're not going to change a fundamental principle of SCons (reproducible builds) because you don't want to update your build scripts when there's new releases of the tools.

In #3664, the desire is to use the 14.1 toolset not the 14.1 product.

I suggest you post the (somewhat offensive) comment above in #3664 and then close the issue as "not going to happen".

#3265 is nineteen (19) months old.
#3664 is eleven (11) weeks old.

My first post with a prototype solution that solved both issues was approximately 3 weeks after #3664 was posted. #3265 was so old I was unaware of its existence.

When necessary, I have been able to modify the msvc code locally since 2.X. I have not had the luxury to wait around for msvc enhancements or fixes. My personal build scripts rely on approximately 3500 lines of python code and configuration data. When there is a build problem with SCons itself, by the time I have diagnosed the problem in most cases the solution is also apparent.

Yesterday, I offered a number of bite-size PRs that address all outstanding issues. It has been my experience that
Windows/msvc patches/PRs are not really welcome unless they are the coding equivalent of duct tape, baling wire, and bubble gum. As always, I could be wrong.

There was an open window starting in June for active contributions and participation that is now rapidly closing. Furthermore, my interest has waned considerably. It seems that my work here will be complete once #3723 and #3764 are either accepted, explicitly rejected, or die a death of neglect.

@jcbrill
Copy link
Contributor Author

jcbrill commented Aug 8, 2020

To be clear, it is not my need.

The trace code (jbrill-mscommon-trace) branch and msvc specific version code (jbrill-msvc-specific-version) branch are available in my fork of the SCons repository.

Good luck and godspeed.

@jcbrill jcbrill closed this Aug 8, 2020
@jcbrill jcbrill deleted the msvc-specific-version branch August 8, 2020 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MSVC Microsoft Visual C++ Support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants