MSVC detection: 2019, 2017, 2017 Express, and 6.0

[jcbrill]

Describe the bug

During the investigation of possible to solutions to issue #3664, 3 problems with msvc detection and another problem with clearing a local rather than global variable were discovered:

1. The function reset_installed_vcs is creating and setting a local variable of __INSTALLED_VCS_RUN rather than resetting the global variable. Ramifications unkown.

2. Initial detection of MSVC 6.0 fails due to a case-sensitive search for "cl.exe". On the local machine, the file name is "CL.EXE". Note that while 6.0 is not in the reported installed versions list, actually using 6.0 is successful (i.e., the initial detection reports a false negative).

3. Detection of "14.1Exp" fails with an UnsupportedVersion exception because the _VCVER_TO_VSWHERE_VER dictionary does not contain a key for "14.Exp".

4. The current vswhere queries are unreliable when multiple versions of msvc for each product are installed. This is due in part to the limitations of the vswhere tool itself. (Note: "14.1Exp" was added to the _VCVER_TO_VSWHERE_VER dictionary mentioned above.

   • 14.2 [2019] with Community and BuildTools installed: find_vc_pdir_vswhere returns the BuildTools instance (i.e., the first entry in the products list).

   • 14.1 [2017] with Community and Express installed: find_vc_pdir_vswhere returns the WDExpress instance for "14.1" (i.e., the first entry in the products list).

   • 14.1Exp [2017] with Community and Express installed: find_vc_pdir_vswhere returns the WDExpress instance for "14.1Exp" (i.e., the first entry in the products list).

Required information

• Link to SCons Users thread discussing your issue: this has not gone through the mailing list. Some discussions took place in MSVC Version 14.1 not found with 14.2 installed #3664.

• Version of SCons: current master

• Version of Python: 3.7.7

• Which python distribution if applicable (python.org, cygwin, anaconda, macports, brew,etc): WinPython 32-bit

• How you installed SCons: setup from scons_master.zip download

• What Platform are you on? (Linux/Windows and which version): Windows 7 32-bit

• How to reproduce your issue? Please include a small self contained reproducer. Likely a SConstruct should do for most issues.

  Install: MSVC 6.0, 2017 Community, 2017 Express, 2019 Community, 2019 Build Tools

  set SCONS_MSCOMMON_DEBUG=-

  env = Environment()
  env.Program("hello", "hello.c")
  

Further information:

No tests where performed concerning the global variable issue mentioned above.

Possible solutions to all of the msvc detection issues were implemented. If desired, a pull request can be generated.

[mwichmann]

Detection of "14.1Exp" fails with an UnsupportedVersion exception because the _VCVER_TO_VSWHERE_VER dictionary does not contain a key for "14.Exp".

There's a routine to trim the version to a numeric-only version, perhaps use that rather than add another entry to the dictionary? Not that it's a big deal either way.

The function reset_installed_vcs is creating and setting a local variable of __INSTALLED_VCS_RUN rather than resetting the global variable. Ramifications unkown.

It's used by a testcase in msvsTests.py, should not affect any normal usage, but easy enough to fix.

[jcbrill]

Unless I'm missing something, the larger problem is I don't think you want the same vswhere query for both 14.1 and 14.1Exp: the same list is going to be returned for both 14.1 and 14.1Exp. In the end, one or both may be wrong depending on the number of installed instances.

Proposed implementation notes and rationale:

• vswhere detection for 14.2, 14.1, and 14.Exp:

  Change the _VCVER_TO_VSWHERE_VER dictionary from having a string for the version range to a list of arguments for the vswhere command construction.

  _VCVER_TO_VSWHERE_VER = {
      '14.2': [
          ["-version", "[16.0, 17.0)", ], # default: Enterprise, Professional, Community  (order unpredictable?)
          ["-version", "[16.0, 17.0)", "-products", "Microsoft.VisualStudio.Product.BuildTools"], # BuildTools
          ],
      '14.1': [
          ["-version", "[15.0, 16.0)", ], # default: Enterprise, Professional, Community (order unpredictable?)
          ["-version", "[15.0, 16.0)", "-products", "Microsoft.VisualStudio.Product.BuildTools"], # BuildTools
          ],
      '14.1Exp': [
          ["-version", "[15.0, 16.0)", "-products", "Microsoft.VisualStudio.Product.WDExpress"], # Express
          ],
  }
  

  Move the vswhere command construction and query inside a loop based on the number of argument lists required.

  The first query for 14.2 and 14.1 does not have a "-products" argument which defaults to Enterprise, Professional, and Community. The second query is solely for the Build Tools.

  The query for 14.1Exp is solely for 2017 Express (i.e, WDExpress).

  A limitation of vswhere is that only one property can be returned (i.e., its either one property or all of the properties).

  Searching the list of returned installation paths can be unreliable if trying to determine which product is installed based on the path name due to the ability of a user to customize the installation location.

  In the long run, a single call to vswhere for all products returning json is going to be a better solution.

• msvc 6.0, 7.0, and 7.1 detection:

  Existing code ("cl.exe" in files list of walk):

      # not sure about these versions so if a walk the VC dir (could be slow)
      for root, _, files in os.walk(vc_dir):
          if _CL_EXE_NAME in files:
              debug(_CL_EXE_NAME + ' found %s' % os.path.join(root, _CL_EXE_NAME))
              return True
      return False
  

  Local installations:

  • MSVC 6.0: 1904 candidate files without a match ("CL.EXE")
  • MSVC 7.0: 669 candidate files before a match
  • MSVC 7.1: 664 candidate files before a match

  Proposed code:

      # quick check for vc_dir/bin and vc_dir/ before walk
      # need to check root as the walk only considers subdirectories
      for cl_dir in ('bin', ''):
          cl_path = os.path.join(vc_dir, cl_dir, _CL_EXE_NAME)
          if os.path.exists(cl_path):
              debug(_CL_EXE_NAME + ' found %s' % cl_path)
              return True
      # not in bin or root: must be in a subdirectory
      for cl_root, cl_dirs, _ in os.walk(vc_dir):
          for cl_dir in cl_dirs:
              cl_path = os.path.join(cl_root, cl_dir, _CL_EXE_NAME)
              if os.path.exists(cl_path):
                  debug(_CL_EXE_NAME + ' found %s' % cl_path)
                  return True
      return False
  

  Testing for existence with os.path.exists is case-insensitive which solves the "cl.exe" versus "CL.EXE" file name problem with the original code.

  For local installations, cl.exe is in the vc_dir/bin folder for 6.0, 7.0, and 7.1. Might as well check there first (as well as the installation root). Should those fail, only sub-directories need to be evaluated rather than individual files when performing the walk.

  Local installations:

  • MSVC 6.0: first call to os.path.exists (vc_dir/bin/cl.exe)
  • MSVC 7.0: first call to os.path.exists (vc_dir/bin/cl.exe)
  • MSVC 7.1: first call to os.path.exists (vc_dir/bin/cl.exe)

  Should the quick test fail, the os.walk finds cl.exe:

  • MSVC 6.0: 2nd call to os.path.exists (Atl, Bin)
  • MSVC 7.0: 2nd call to os.path.exists (atlmfc, bin)
  • MSVC 7.1: 3rd call to os.path.exists (1033, atlmfc, bin)

[mwichmann]

different products string, right. I was just thinking of the old/existing dict which has only the version part of the query.

In the long run, a single call to vswhere for all products returning json is going to be a better solution.

yes, we've talked about this elsewhere before, "the maintainer" is in favor, I know. an issue was whether to bother with what vswhere calls "legacy" versions - I feel those don't return enough information to be worth bothering with, but that's just one opinion.

A limitation of vswhere is that only one property can be returned (i.e., its either one property or all of the properties).

if the json output is used, it is easy enough to exttract the properties you want (which I think you were saying)

[jcbrill]

In the prototypes to support specific version numbers, vswhere was moved to a single call with json output as the product types and installation paths for all installations where easily identified. This also allowed specific product type to be selected by a user via the environment.

Moving forward, I'm not sure there is a compelling reason to have "14.1Exp" as a user-facing option. In the prototype work having a distinction between "14.1" and "14.1Exp" made the implementation of specific version/product selection harder than it needs to be.

[bdbaddog]

Does #3701 resolve all these issues?

[jcbrill]

Yes it resolves all of these issues.

[bdbaddog]

Closing