diff --git a/SCons/Tool/MSCommon/common.py b/SCons/Tool/MSCommon/common.py index 27a8460393..d857c9099d 100644 --- a/SCons/Tool/MSCommon/common.py +++ b/SCons/Tool/MSCommon/common.py @@ -31,6 +31,7 @@ import re import sys import time +from collections import namedtuple from contextlib import suppress from subprocess import DEVNULL, PIPE from pathlib import Path @@ -51,7 +52,7 @@ # control execution in interesting ways. # Note these really should be unified - either controlled by vs.py, # or synced with the the common_tools_var # settings in vs.py. -VS_VC_VARS = [ +MSVC_ENVIRON_SHELLVARS = ( 'COMSPEC', # path to "shell" 'OS', # name of OS family: Windows_NT or undefined (95/98/ME) 'VS170COMNTOOLS', # path to common tools for given version @@ -74,7 +75,23 @@ 'POWERSHELL_TELEMETRY_OPTOUT', 'PSDisableModuleAnalysisCacheCleanup', 'PSModuleAnalysisCachePath', -] +) + +# Hard-coded list of the variables that are imported from the msvc +# environment and added to the calling environment. Currently, this +# list does not contain any variables that are in the shell variables +# list above. Due to the existing implementation, variables in both +# lists may not work as expected (e.g., overwites, adding empty values, +# etc.). +MSVC_ENVIRON_KEEPVARS = ( + "INCLUDE", + "LIB", + "LIBPATH", + "PATH", + "VSCMD_ARG_app_plat", + "VCINSTALLDIR", # needed by clang -VS 2017 and newer + "VCToolsInstallDir", # needed by clang - VS 2015 and older +) class MSVCCacheInvalidWarning(SCons.Warnings.WarningOnByDefault): pass @@ -223,12 +240,30 @@ def debug_extra(*args, **kwargs): if os.environ.get('SCONS_CACHE_MSVC_FORCE_DEFAULTS') in ('1', 'true', 'True'): CONFIG_CACHE_FORCE_DEFAULT_ARGUMENTS = True +# Cache file version number: +# * bump the file version up every time the structure of the file changes +# so that exising cache files are reconstructed. +_CACHE_FILE_VERSION = 1 +_CACHE_RECORD_VERSION = 0 + +def register_cache_record_version(record_version) -> None: + global _CACHE_RECORD_VERSION + _CACHE_RECORD_VERSION=record_version + +_CacheHeader = namedtuple('_CacheHeader', [ + 'file_version', + 'record_version', +]) + + def read_script_env_cache() -> dict: """ fetch cached msvc env vars if requested, else return empty dict """ envcache = {} p = Path(CONFIG_CACHE) if not CONFIG_CACHE or not p.is_file(): return envcache + + envcache_dict = {} with SCons.Util.FileLock(CONFIG_CACHE, timeout=5, writer=False), p.open('r') as f: # Convert the list of cache entry dictionaries read from # json to the cache dictionary. Reconstruct the cache key @@ -236,7 +271,7 @@ def read_script_env_cache() -> dict: # Note we need to take a write lock on the cachefile, as if there's # an error and we try to remove it, that's "writing" on Windows. try: - envcache_list = json.load(f) + envcache_dict = json.load(f) except json.JSONDecodeError: # If we couldn't decode it, it could be corrupt. Toss. with suppress(FileNotFoundError): @@ -244,14 +279,38 @@ def read_script_env_cache() -> dict: warn_msg = "Could not decode msvc cache file %s: dropping." SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg % CONFIG_CACHE) debug(warn_msg, CONFIG_CACHE) - else: - if isinstance(envcache_list, list): - envcache = {tuple(d['key']): d['data'] for d in envcache_list} - else: - # don't fail if incompatible format, just proceed without it - warn_msg = "Incompatible format for msvc cache file %s: file may be overwritten." - SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg % CONFIG_CACHE) - debug(warn_msg, CONFIG_CACHE) + return envcache + + is_valid = False + do_once = True + while do_once: + do_once = False + if not isinstance(envcache_dict, dict): + break + envcache_header = envcache_dict.get("header") + if not isinstance(envcache_header, dict): + break + try: + envcache_header_t = _CacheHeader(**envcache_header) + except TypeError: + break + if envcache_header_t.file_version != _CACHE_FILE_VERSION: + break + if envcache_header_t.record_version != _CACHE_RECORD_VERSION: + break + envcache_records = envcache_dict.get("records") + if not isinstance(envcache_records, list): + break + is_valid = True + envcache = { + tuple(d['key']): d['val'] for d in envcache_records + } + + if not is_valid: + # don't fail if incompatible format, just proceed without it + warn_msg = "Incompatible format for msvc cache file %s: file may be overwritten." + SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg % CONFIG_CACHE) + debug(warn_msg, CONFIG_CACHE) return envcache @@ -261,16 +320,25 @@ def write_script_env_cache(cache) -> None: if not CONFIG_CACHE: return + cache_header = _CacheHeader( + file_version=_CACHE_FILE_VERSION, + record_version=_CACHE_RECORD_VERSION, + ) + p = Path(CONFIG_CACHE) try: with SCons.Util.FileLock(CONFIG_CACHE, timeout=5, writer=True), 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) + envcache_dict = { + 'header': cache_header._asdict(), + 'records': [ + {'key': list(key), 'val': val._asdict()} + for key, val in cache.items() + ], + } + json.dump(envcache_dict, f, indent=2) except TypeError: # data can't serialize to json, don't leave partial file with suppress(FileNotFoundError): @@ -329,6 +397,64 @@ def has_reg(value) -> bool: ret = False return ret +# Functions for shell variables and keep variables cache key. + +class _EnvVarsUtil: + + _cache_varlist_unique = {} + + @classmethod + def varlist_unique_t(cls, varlist): + key = tuple(varlist) + unique = cls._cache_varlist_unique.get(key) + if unique is None: + seen = set() + unique = [] + for var in varlist: + var_key = var.lower() + if var_key in seen: + continue + seen.add(var_key) + unique.append(var) + unique = tuple(sorted(unique, key=str.lower)) + cls._cache_varlist_unique[key] = unique + return unique + + _cache_varlist_norm = {} + + @classmethod + def varlist_norm_t(cls, varlist): + unique_t = cls.varlist_unique_t(varlist) + norm_t = cls._cache_varlist_norm.get(unique_t) + if norm_t is None: + norm_t = tuple(var.lower() for var in unique_t) + cls._cache_varlist_norm[unique_t] = norm_t + return norm_t + + _cache_varlist_keystr = {} + + @classmethod + def varlist_keystr(cls, varlist): + norm_t = cls.varlist_norm_t(varlist) + keystr = cls._cache_varlist_keystr.get(norm_t) + if keystr is None: + keystr = ":".join(norm_t) + cls._cache_varlist_keystr[norm_t] = keystr + return keystr + +def msvc_environ_cache_keys(): + keys = ( + _EnvVarsUtil.varlist_keystr(MSVC_ENVIRON_SHELLVARS), + _EnvVarsUtil.varlist_keystr(MSVC_ENVIRON_KEEPVARS), + ) + debug("keys=%r", keys) + return keys + +# Populate cache with default variable lists. +# Useful when logging to detect user modifications of default variable lists. +_ = msvc_environ_cache_keys() + + # Functions for fetching environment variable settings from batch files. @@ -522,7 +648,9 @@ def get_output(vcbat, args=None, env=None, skip_sendtelemetry=False): # Create a blank environment, for use in launching the tools env = SCons.Environment.Environment(tools=[]) - env['ENV'] = normalize_env(env['ENV'], VS_VC_VARS, force=False) + shellvars_t = _EnvVarsUtil.varlist_unique_t(MSVC_ENVIRON_SHELLVARS) + + env['ENV'] = normalize_env(env['ENV'], shellvars_t, force=False) if skip_sendtelemetry: _force_vscmd_skip_sendtelemetry(env) @@ -563,23 +691,17 @@ def get_output(vcbat, args=None, env=None, skip_sendtelemetry=False): return cp.stdout.decode(OEM) -KEEPLIST = ( - "INCLUDE", - "LIB", - "LIBPATH", - "PATH", - "VSCMD_ARG_app_plat", - "VCINSTALLDIR", # needed by clang -VS 2017 and newer - "VCToolsInstallDir", # needed by clang - VS 2015 and older -) - - -def parse_output(output, keep=KEEPLIST): +def parse_output(output, keep=None): """ Parse output from running visual c++/studios vcvarsall.bat and running set To capture the values listed in keep """ + if keep is None: + keep = MSVC_ENVIRON_KEEPVARS + + keep = _EnvVarsUtil.varlist_unique_t(keep) + # dkeep is a dict associating key: path_list, where key is one item from # keep, and path_list the associated list of paths dkeep = {i: [] for i in keep} diff --git a/SCons/Tool/MSCommon/vc.py b/SCons/Tool/MSCommon/vc.py index 032dbf202e..d94f819626 100644 --- a/SCons/Tool/MSCommon/vc.py +++ b/SCons/Tool/MSCommon/vc.py @@ -61,6 +61,8 @@ from .common import ( CONFIG_CACHE, debug, + msvc_environ_cache_keys, + register_cache_record_version, ) from .sdk import get_installed_sdks @@ -1392,7 +1394,7 @@ def vswhere_update_msvc_map(cls, vswhere_exe): for instance in vswhere_json: - #print(json.dumps(instance, indent=4, sort_keys=True)) + # print(json.dumps(instance, indent=4, sort_keys=True)) installation_path = instance.get('installationPath') if not installation_path or not os.path.exists(installation_path): @@ -2105,33 +2107,84 @@ def _check_cl_exists_in_script_env(data): # not the entire output of running the batch file - saves a bit # of time not parsing every time. +# Cache record version number: +# * bump the record version up every time the contents of payload changes +# so that exising cache files are reconstructed. +_CACHE_RECORD_VERSION = 1 + +register_cache_record_version(_CACHE_RECORD_VERSION) + +_CachePayload = namedtuple('_CachePayload', [ + 'data', + 'verify', +]) + +def _cache_payload_verify(payload_dict): + + if not isinstance(payload_dict, dict): + return None + + try: + cache_payload = _CachePayload(**payload_dict) + except TypeError: + return None + + for p in cache_payload.verify: + if not os.path.exists(p): + return None + + return cache_payload + +def _cache_data_verify_paths(cache_data, add_paths, cache_data_pathvars): + + check_paths = [] + for val in add_paths: + if not val: + continue + check_paths.append(val) + for var in cache_data_pathvars: + val = cache_data.get(var) + if not val: + continue + if isinstance(val, list): + check_paths.extend(val) + continue + check_paths.append(val) + + seen = set() + verify_paths = [] + for p in check_paths: + if not p or not os.path.exists(p): + continue + norm_path = os.path.normcase(os.path.abspath(p)) + if norm_path in seen: + continue + seen.add(norm_path) + verify_paths.append(norm_path) + + return verify_paths + +def _cache_verify(file_cache): + cache = {} + for key, payload_dict in file_cache.items(): + cache_payload = _cache_payload_verify(payload_dict) + if not cache_payload: + continue + cache[key] = cache_payload + return cache + script_env_cache = None def script_env(env, script, args=None): global script_env_cache if script_env_cache is None: - script_env_cache = common.read_script_env_cache() - 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 + script_env_cache = _cache_verify(common.read_script_env_cache()) + + cache_key = (script, args if args else None, *msvc_environ_cache_keys()) + cache_payload = script_env_cache.get(cache_key, None) - if cache_data is None: + if not cache_payload: skip_sendtelemetry = _skip_sendtelemetry(env) stdout = common.get_output(script, args, skip_sendtelemetry=skip_sendtelemetry) cache_data = common.parse_output(stdout) @@ -2147,11 +2200,11 @@ def script_env(env, script, args=None): script_errlog.append('vc script errors detected:') script_errlog.append(line) + have_cl, cl_path = _check_cl_exists_in_script_env(cache_data) + if script_errlog: script_errmsg = '\n'.join(script_errlog) - have_cl, _ = _check_cl_exists_in_script_env(cache_data) - debug( 'script=%s args=%s have_cl=%s, errors=%s', repr(script), repr(args), repr(have_cl), script_errmsg @@ -2162,11 +2215,18 @@ def script_env(env, script, args=None): # detected errors, cl.exe not on path raise BatchFileExecutionError(script_errmsg) + cache_payload = _CachePayload( + data=cache_data, + verify=_cache_data_verify_paths( + cache_data, [cl_path], ["VCToolsInstallDir", "LIB", "LIBPATH"], + ), + ) + # once we updated cache, give a chance to write out if user wanted - script_env_cache[cache_key] = cache_data + script_env_cache[cache_key] = cache_payload common.write_script_env_cache(script_env_cache) - return cache_data + return cache_payload.data def get_default_version(env): msvc_version = env.get('MSVC_VERSION') diff --git a/test/MSVC/msvc_cache_force_defaults.py b/test/MSVC/msvc_cache_force_defaults.py index ad67304d56..e8717f05a5 100644 --- a/test/MSVC/msvc_cache_force_defaults.py +++ b/test/MSVC/msvc_cache_force_defaults.py @@ -61,12 +61,13 @@ envcache_keys = [] with open(cache_file, 'r') as file: - envcache_list = json.load(file) + envcache_dict = json.load(file) + envcache_list = envcache_dict['records'] envcache_keys = [tuple(d['key']) for d in envcache_list] if envcache_keys: - # key = (script, arguments) - print("SCRIPT_ARGS: {}".format(envcache_keys[0][-1])) + # key = (script, arguments, envkeystr, envkeystr) + print("SCRIPT_ARGS: {}".format(envcache_keys[0][1])) """ )) test.run(arguments="-Q -s", status=0, stdout=None)