Merge remote-tracking branch 'upstream/master' into feature/minor_java_improvements

Author: bdbaddog

CHANGES.txt

@@ -31,6 +31,23 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       determination when configuring the build environment.  This could lead to build failures when
       only an MSVC Express instance is installed and the MSVC version is not explicitly specified
       (issue #2668 and issue #2697).
+    - Added MSVC_USE_SETTINGS variable to pass a dictionary to configure the msvc compiler
+      system environment as an alternative to bypassing Visual Studio autodetection entirely.
+    - The imported system environment variable names for MSVC 7.0 and 6.0 have been changed to the
+      names set by their respective installers.  Prior to this change, bypassing MSVC detection by
+      specifying the MSVC 7.0 batch file directly would fail due to using an erroneous environment
+      variable name.  Arguments are no longer passed to the MSVC 6.0 to 7.1 batch files as no
+      arguments are required and could improve the effectiveness of the internal MSVC cache.
+    - Propagate the OS and windir environment variables from the system environment to the msvc
+      environment.  The OS and windir environment variables are used in the MSVC 6.0 batch file
+      and the SDK 6.0-7.1 SetEnv.cmd batch files.  Inclusion of the OS and windir environment
+      variables eliminates some partial paths and warnings generated by the MSVC 6.0 and SDK
+      6.0-7.1 batch files when the variables are not defined.
+      Note: Attempting to run the SDK 6.0-7.1 batch files directly via MSVC_USE_SCRIPT can lead to
+            build failures and/or incomplete build environments.  The SDK 6.0-7.1 batch files
+            require delayed expansion to be enabled which is currently not supported and is
+            typically not enabled by default on the host system. The batch files may also require
+            environment variables that are not included by default in the msvc environment.
 
   From William Deegan:
     - Fix check for unsupported Python version. It was broken. Also now the error message
@@ -111,6 +128,15 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       processors for testing threads.
     - Fixed crash in C scanner's dictify_CPPDEFINES() function which happens if
       AppendUnique is called on CPPPATH. (Issue #4108).
+    - The MSVC script_env_cache now contains a sanity check: if the retrieved
+      tools path does not exist, the entry is invalidated so it will
+      be recomputed, in an attempt to avoid scons failing when certain
+      compiler version bumps have taken place.  The dictionary key (uses
+      the name of a batch file and any arguments which may have been
+      passes), is now computed a bit differently: the dashes are left
+      off if there are no arguments.  The default cachefile is changed
+      to have a .json suffix, for better recognition on Windows since
+      the contents are json.
     - As "code modernization" all of SCons now uses the current super()
       zero-argument syntax instead of direct calls to a parent class method
       or the super() two-argument syntax.

RELEASE.txt

@@ -19,6 +19,8 @@ NEW FUNCTIONALITY
 - Added SHELL_ENV_GENERATOR construction variables. This variable allows the user to Define
   a function which will be called to generate or alter the execution environment which will
   be used in the shell command of some Action.
+- Added MSVC_USE_SETTINGS variable to pass a dictionary to configure the msvc compiler
+  system environment as an alternative to bypassing Visual Studio autodetection entirely.
 
 
 DEPRECATED FUNCTIONALITY
@@ -30,9 +32,10 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 ---------------------------------------
 
 - On Windows, %AllUsersProfile%\scons\site_scons is now the default "system"
-  location for a site_scons. %AllUsersProfile%\Application Data\scons\site_scons
-  will continue to work. There does not seem to be any convention to use
-  an "Application Data" subdirectory here.
+  location for a site_scons directory.
+  %AllUsersProfile%\Application Data\scons\site_scons will continue to work.
+  There does not seem to be any existing convention to use an
+  "Application Data" subdirectory here.
 - Action._subproc() can now be used as a python context manager to ensure that the
   POpen object is properly closed.
 - SCons help (-H) no longer prints the "ignored for compatibility" options,
@@ -45,6 +48,16 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 - The change to "content" and "content-timestamp" Decider names is reflected
   in the User Guide as well, since the hash function may be other than md5
   (tidying up from earlier change)
+- If the (experimental) SCONS_CACHE_MSVC_CONFIG feature is used, it will now
+  attempt a sanity check for the cached compiler information, and regenerate
+  it if needed. Previously, this cache would fail if a compiler upgrade caused
+  a change to internal paths (e.g. upgrading from 17.1 to 17.2 causes
+  a necessary path component in some of the cached vars to need to 14.32.31326
+  instead of 14.31.31103), and the cache file needed to be manually removed.
+  The default cachefile name is now "scons_msvc_cache.json" rather than
+  ".scons_msvc_cache" so there should be no transition problem if using the
+  default; if using a custom cache file name, the cache should still be
+  manually removed if there are problems to transition to the new style.
 - Update ninja file generation to only create response files for build commands
   which exceed MAXLINELENGTH
 - Update the debug output written to stdout for MSVC initialization which is enabled
@@ -53,6 +66,20 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
   message of MSVC initialization debugging output.  A single space was added before the
   message for all debugging output records written to stdout and to files.
 - Add JavaScanner to include JAVACLASSPATH as a dependency when using the Java tool.
+- The build argument (i.e., x86) is no longer passed to the MSVC 6.0 to 7.1 batch
+  files.  This may improve the effectiveness of the internal msvc cache when using 
+  MSVC detection and when bypassing MSVC detection as the MSVC 6.0 to 7.1 batch files 
+  do not expect any arguments.
+- Propagate the OS and windir environment variables from the system environment to the msvc
+  environment.  The OS and windir environment variables are used in the MSVC 6.0 batch file
+  and the SDK 6.0-7.1 SetEnv.cmd batch files.  Inclusion of the OS and windir environment
+  variables eliminates some partial paths and warnings generated by the MSVC 6.0 and SDK
+  6.0-7.1 batch files when the variables are not defined.
+  Note: Attempting to run the SDK 6.0-7.1 batch files directly via MSVC_USE_SCRIPT can lead to
+        build failures and/or incomplete build environments.  The SDK 6.0-7.1 batch files
+        require delayed expansion to be enabled which is currently not supported and is
+        typically not enabled by default on the host system. The batch files may also require
+        environment variables that are not included by default in the msvc environment.
 
 FIXES
 -----
@@ -96,6 +123,9 @@ FIXES
 - Fix typo in ninja scons daemon startup which causes ConnectionRefusedError to not retry 
   to connect to the server during start up.
 - Fix incorrect Java classpath generation when a NodeList is used as part of any JAVA*PATH variables.
+- The system environment variable names imported for MSVC 7.0 and 6.0 were updated to be
+  consistent with the variables names defined by their respective installers.  This fixes an
+  error caused when bypassing MSVC detection by specifying the MSVC 7.0 batch file directly.
 
 IMPROVEMENTS
 ------------

SCons/Tool/MSCommon/common.py

@@ -31,6 +31,8 @@
 import re
 import subprocess
 import sys
+from contextlib import suppress
+from pathlib import Path
 
 import SCons.Util
 
@@ -94,16 +96,21 @@ def debug(x, *args):
 # SCONS_CACHE_MSVC_CONFIG is public, and is documented.
 CONFIG_CACHE = os.environ.get('SCONS_CACHE_MSVC_CONFIG')
 if CONFIG_CACHE in ('1', 'true', 'True'):
-    CONFIG_CACHE = os.path.join(os.path.expanduser('~'), '.scons_msvc_cache')
+    CONFIG_CACHE = os.path.join(os.path.expanduser('~'), 'scons_msvc_cache.json')
 
 
 def read_script_env_cache():
     """ fetch cached msvc env vars if requested, else return empty dict """
     envcache = {}
     if CONFIG_CACHE:
         try:
-            with open(CONFIG_CACHE, 'r') as f:
-                envcache = json.load(f)
+            p = Path(CONFIG_CACHE)
+            with p.open('r') as f:
+                # Convert the list of cache entry dictionaries read from
+                # json to the cache dictionary. Reconstruct the cache key
+                # tuple from the key list written to json.
+                envcache_list = json.load(f)
+                envcache = {tuple(d['key']): d['data'] for d in envcache_list}
         except FileNotFoundError:
             # don't fail if no cache file, just proceed without it
             pass
@@ -114,11 +121,17 @@ def write_script_env_cache(cache):
     """ write out cache of msvc env vars if requested """
     if CONFIG_CACHE:
         try:
-            with open(CONFIG_CACHE, 'w') as f:
-                json.dump(cache, f, indent=2)
+            p = Path(CONFIG_CACHE)
+            with p.open('w') as f:
+                # Convert the cache dictionary to a list of cache entry
+                # dictionaries. The cache key is converted from a tuple to
+                # a list for compatibility with json.
+                envcache_list = [{'key': list(key), 'data': data} for key, data in cache.items()]
+                json.dump(envcache_list, f, indent=2)
         except TypeError:
             # data can't serialize to json, don't leave partial file
-            os.remove(CONFIG_CACHE)
+            with suppress(FileNotFoundError):
+                p.unlink()
         except IOError:
             # can't write the file, just skip
             pass
@@ -243,6 +256,7 @@ def get_output(vcbat, args=None, env=None):
     # or synced with the the common_tools_var # settings in vs.py.
     vs_vc_vars = [
         'COMSPEC',  # path to "shell"
+        'OS', # name of OS family: Windows_NT or undefined (95/98/ME)
         'VS170COMNTOOLS',  # path to common tools for given version
         'VS160COMNTOOLS',
         'VS150COMNTOOLS',
@@ -253,10 +267,11 @@ def get_output(vcbat, args=None, env=None):
         'VS90COMNTOOLS',
         'VS80COMNTOOLS',
         'VS71COMNTOOLS',
-        'VS70COMNTOOLS',
-        'VS60COMNTOOLS',
+        'VSCOMNTOOLS',
+        'MSDevDir',
         'VSCMD_DEBUG',   # enable logging and other debug aids
         'VSCMD_SKIP_SENDTELEMETRY',
+        'windir', # windows directory (SystemRoot not available in 95/98/ME)
     ]
     env['ENV'] = normalize_env(env['ENV'], vs_vc_vars, force=False)
 

SCons/Tool/MSCommon/vc.py

@@ -43,6 +43,7 @@
 import subprocess
 import os
 import platform
+from pathlib import Path
 from string import digits as string_digits
 from subprocess import PIPE
 import re
@@ -84,6 +85,9 @@ class BatchFileExecutionError(VisualCException):
 class MSVCScriptNotFound(VisualCException):
     pass
 
+class MSVCUseSettingsError(VisualCException):
+    pass
+
 # Dict to 'canonalize' the arch
 _ARCH_TO_CANONICAL = {
     "amd64"     : "amd64",
@@ -777,12 +781,8 @@ def find_batch_file(env, msvc_version, host_arch, target_arch):
         # 14.0 (VS2015) to 8.0 (VS2005)
         arg, _ = _LE2015_HOST_TARGET_BATCHARG_CLPATHCOMPS[(host_arch, target_arch)]
         batfilename = os.path.join(pdir, "vcvarsall.bat")
-    elif 8 > vernum >= 7:
-        # 7.1 (VS2003) to 7.0 (VS2003)
-        pdir = os.path.join(pdir, os.pardir, "Common7", "Tools")
-        batfilename = os.path.join(pdir, "vsvars32.bat")
     else:
-        # 6.0 (VS6) and earlier
+        # 7.1 (VS2003) and earlier
         pdir = os.path.join(pdir, "Bin")
         batfilename = os.path.join(pdir, "vcvars32.bat")
 
@@ -985,8 +985,25 @@ def script_env(script, args=None):
 
     if script_env_cache is None:
         script_env_cache = common.read_script_env_cache()
-    cache_key = "{}--{}".format(script, args)
+    cache_key = (script, args if args else None)
     cache_data = script_env_cache.get(cache_key, None)
+
+    # Brief sanity check: if we got a value for the key,
+    # see if it has a VCToolsInstallDir entry that is not empty.
+    # If so, and that path does not exist, invalidate the entry.
+    # If empty, this is an old compiler, just leave it alone.
+    if cache_data is not None:
+        try:
+            toolsdir = cache_data["VCToolsInstallDir"]
+        except KeyError:
+            # we write this value, so should not happen
+            pass
+        else:
+            if toolsdir:
+                toolpath = Path(toolsdir[0])
+                if not toolpath.exists():
+                    cache_data = None
+
     if cache_data is None:
         stdout = common.get_output(script, args)
 
@@ -1125,6 +1142,37 @@ def msvc_find_valid_batch_script(env, version):
 
     return d
 
+_undefined = None
+
+def get_use_script_use_settings(env):
+    global _undefined
+
+    if _undefined is None:
+        _undefined = object()
+
+    #   use_script  use_settings   return values   action
+    #     value       ignored      (value, None)   use script or bypass detection
+    #   undefined  value not None  (False, value)  use dictionary
+    #   undefined  undefined/None  (True,  None)   msvc detection
+
+    # None (documentation) or evaluates False (code): bypass detection
+    # need to distinguish between undefined and None
+    use_script = env.get('MSVC_USE_SCRIPT', _undefined)
+
+    if use_script != _undefined:
+        # use_script defined, use_settings ignored (not type checked)
+        return (use_script, None)
+
+    # undefined or None: use_settings ignored
+    use_settings = env.get('MSVC_USE_SETTINGS', None)
+
+    if use_settings is not None:
+        # use script undefined, use_settings defined and not None (type checked)
+        return (False, use_settings)
+
+    # use script undefined, use_settings undefined or None
+    return (True, None)
+
 
 def msvc_setup_env(env):
     debug('called')
@@ -1142,7 +1190,7 @@ def msvc_setup_env(env):
     env['MSVS'] = {}
 
 
-    use_script = env.get('MSVC_USE_SCRIPT', True)
+    use_script, use_settings = get_use_script_use_settings(env)
     if SCons.Util.is_String(use_script):
         use_script = use_script.strip()
         if not os.path.exists(use_script):
@@ -1155,6 +1203,12 @@ def msvc_setup_env(env):
         debug('use_script 2 %s', d)
         if not d:
             return d
+    elif use_settings is not None:
+        if not SCons.Util.is_Dict(use_settings):
+            error_msg = 'MSVC_USE_SETTINGS type error: expected a dictionary, found {}'.format(type(use_settings).__name__)
+            raise MSVCUseSettingsError(error_msg)
+        d = use_settings
+        debug('use_settings %s', d)
     else:
         debug('MSVC_USE_SCRIPT set to False')
         warn_msg = "MSVC_USE_SCRIPT set to False, assuming environment " \

SCons/Tool/MSCommon/vs.py

@@ -391,7 +391,7 @@ def reset(self):
     VisualStudio('7.0',
                  sdk_version='2003R2',
                  hkeys=[r'Microsoft\VisualStudio\7.0\Setup\VS\ProductDir'],
-                 common_tools_var='VS70COMNTOOLS',
+                 common_tools_var='VSCOMNTOOLS',
                  executable_path=r'Common7\IDE\devenv.com',
                  batch_file_path=r'Common7\Tools\vsvars32.bat',
                  default_dirname='Microsoft Visual Studio .NET',
@@ -403,7 +403,7 @@ def reset(self):
                  sdk_version='2003R1',
                  hkeys=[r'Microsoft\VisualStudio\6.0\Setup\Microsoft Visual Studio\ProductDir',
                         'use_dir'],
-                 common_tools_var='VS60COMNTOOLS',
+                 common_tools_var='MSDevDir',
                  executable_path=r'Common\MSDev98\Bin\MSDEV.COM',
                  batch_file_path=r'Common7\Tools\vsvars32.bat',
                  default_dirname='Microsoft Visual Studio',

SCons/Tool/msvc.xml

@@ -422,6 +422,72 @@ Provides arguments passed to the script &cv-link-MSVC_USE_SCRIPT;.
 </summary>
 </cvar>
 
+<cvar name="MSVC_USE_SETTINGS">
+<summary>
+<para>
+Use a dictionary to set up the Microsoft Visual C++ compiler.
+</para>
+
+<para>
+&cv-MSVC_USE_SETTINGS; is ignored when &cv-link-MSVC_USE_SCRIPT; is defined
+and/or when &cv-MSVC_USE_SETTINGS; is set to <constant>None</constant>.
+</para>
+
+<para>
+The dictionary is used to populate the environment with the relevant variables
+(typically <envar>%INCLUDE%</envar>, <envar>%LIB%</envar>, and <envar>%PATH%</envar>)
+for supplying to the build. This can be useful to force the use of a compiler environment
+that &SCons; does not configure correctly. This is an alternative to manually configuring
+the environment when bypassing Visual Studio autodetection entirely by setting
+&cv-link-MSVC_USE_SCRIPT; to <constant>None</constant>.
+</para>
+
+<para>
+Here is an example of configuring a build environment using the Microsoft Visual C/C++ compiler
+included in the Microsoft SDK on a 64-bit host and building for a 64-bit architecture:
+<programlisting>
+# Microsoft SDK 6.0 (MSVC 8.0): 64-bit host and 64-bit target
+msvc_use_settings = {
+    "PATH": [
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\VC\\Bin\\x64",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\Bin\\x64",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\Bin",
+        "C:\\Windows\\Microsoft.NET\\Framework\\v2.0.50727",
+        "C:\\Windows\\system32",
+        "C:\\Windows",
+        "C:\\Windows\\System32\\Wbem",
+        "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\"
+    ],
+    "INCLUDE": [
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\VC\\Include",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\VC\\Include\\Sys",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\Include",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\Include\\gl",
+    ],
+    "LIB": [
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\VC\\Lib\\x64",
+        "C:\\Program Files\\Microsoft SDKs\\Windows\\v6.0\\Lib\\x64",
+    ],
+    "LIBPATH": [],
+    "VSCMD_ARG_app_plat": [],
+    "VCINSTALLDIR": [],
+    "VCToolsInstallDir": []
+}
+
+# Specifying MSVC_VERSION is recommended
+env = Environment(MSVC_VERSION='8.0', MSVC_USE_SETTINGS=msvc_use_settings)
+</programlisting>
+</para>
+
+<para>
+Note: the dictionary content requirements are based on the internal msvc implementation and
+therefore may change at any time. The burden is on the user to ensure the dictionary contents
+are minimally sufficient to ensure successful builds.
+</para>
+
+</summary>
+</cvar>
+
 <!-- This has moved to Platform/Platform.xml to avoid duplication
 <cvar name="HOST_ARCH">
 <summary>