From 5b4a081f26d7db4ca35d7bde2fff9f9be5ae609e Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sat, 26 Aug 2023 15:43:52 -0600 Subject: Handle json decode error in mavc cache A bit of a revamp of read_script_env_cache and write_script_env_cache. The significant change is that on read, a JSDONDecodeError is detected, and the cache file removed if so - we're guessing a write race corrupted the file. If this works, will update with more "PR polish", else will withdraw. Signed-off-by: Mats Wichmann --- SCons/Tool/MSCommon/common.py | 85 ++++++++++++++++++++++++------------------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/SCons/Tool/MSCommon/common.py b/SCons/Tool/MSCommon/common.py index f497f1a..2be7ad7 100644 --- a/SCons/Tool/MSCommon/common.py +++ b/SCons/Tool/MSCommon/common.py @@ -103,7 +103,7 @@ else: # SCONS_CACHE_MSVC_CONFIG is public, and is documented. -CONFIG_CACHE = os.environ.get('SCONS_CACHE_MSVC_CONFIG') +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.json') @@ -113,50 +113,59 @@ if CONFIG_CACHE: if os.environ.get('SCONS_CACHE_MSVC_FORCE_DEFAULTS') in ('1', 'true', 'True'): CONFIG_CACHE_FORCE_DEFAULT_ARGUMENTS = True -def read_script_env_cache(): +def read_script_env_cache() -> dict: """ fetch cached msvc env vars if requested, else return empty dict """ envcache = {} - if CONFIG_CACHE: + p = Path(CONFIG_CACHE) + if not CONFIG_CACHE or not p.is_file(): + return envcache + 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. try: - 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) - 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 {}: file may be overwritten.".format( - repr(CONFIG_CACHE) - ) - SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg) - debug(warn_msg) - except FileNotFoundError: - # don't fail if no cache file, just proceed without it - pass + envcache_list = json.load(f) + except json.JSONDecodeError: + # If we couldn't decode it, it could be corrupt. Toss. + with suppress(FileNotFoundError): + p.unlink() + warn_msg = f"Could not decode msvc cache file %s: dropping." + SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg) + debug(warn_msg, repr(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 = f"Incompatible format for msvc cache file %s: file may be overwritten." + SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg) + debug(warn_msg, repr(CONFIG_CACHE)) + return envcache def write_script_env_cache(cache) -> None: """ write out cache of msvc env vars if requested """ - if CONFIG_CACHE: - try: - 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 - with suppress(FileNotFoundError): - p.unlink() - except OSError: - # can't write the file, just skip - pass + if not CONFIG_CACHE: + return + + p = Path(CONFIG_CACHE) + try: + 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 + with suppress(FileNotFoundError): + p.unlink() + except OSError: + # can't write the file, just skip + pass + + return _is_win64 = None @@ -381,7 +390,7 @@ def parse_output(output, keep=KEEPLIST): # rdk will keep the regex to match the .bat file output line starts rdk = {} for i in keep: - rdk[i] = re.compile('%s=(.*)' % i, re.I) + rdk[i] = re.compile(r'%s=(.*)' % i, re.I) def add_env(rmatch, key, dkeep=dkeep) -> None: path_list = rmatch.group(1).split(os.pathsep) -- cgit v0.12 From 9681d6cafeb41d5adf7ff030796fa621ad533b48 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sun, 27 Aug 2023 08:03:10 -0600 Subject: Add CHANGES for msvc cache change Signed-off-by: Mats Wichmann --- CHANGES.txt | 3 +++ RELEASE.txt | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 0757ee7..06f0269 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -145,6 +145,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER file an error. It has issued a warning since 3.0, with "warn instead of fail" deprecated since 3.1. Fixes #3958. - Minor (non-functional) cleanup of some tests, particuarly test/MSVC. + - msvc "config cache" made more resilient - now throws away the + cache if it took a decode error reading it in. Suspect a possible + race condition creating it in certain CI builds. From Jonathon Reinhart: - Fix another instance of `int main()` in CheckLib() causing failures diff --git a/RELEASE.txt b/RELEASE.txt index 069ca1d..73222fd 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -98,8 +98,12 @@ FIXES - MSCommon: Test SConfTests.py would fail when mscommon debugging was enabled via the MSVC_MSCOMMON_DEBUG environment variable. The mscommon logging filter class registered with the python logging module was refactored to prevent test failure. -- MSVS: Add arm64 to the MSVS supported architectures list for VS2017 and later to be - consistent with the current documentation of MSVS_ARCH. +- MSVS: Add arm64 to the MSVS supported architectures list for VS2017 and later + to be consistent with the current documentation of MSVS_ARCH. +- MSCommon: "config cache" made more resilient - now throws away the cache if + it took a decode error reading it in. Suspect a possible race condition + creating it in certain CI builds, which always start from a "no cache file + exists" state. IMPROVEMENTS ------------ -- cgit v0.12 From b8e7451c43ac01e80484c1324f6fab50da29e916 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 28 Aug 2023 08:06:08 -0600 Subject: Fix up incomplete change to mscommon error msg Signed-off-by: Mats Wichmann --- SCons/Tool/MSCommon/common.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/SCons/Tool/MSCommon/common.py b/SCons/Tool/MSCommon/common.py index 2be7ad7..9ab7179 100644 --- a/SCons/Tool/MSCommon/common.py +++ b/SCons/Tool/MSCommon/common.py @@ -129,17 +129,17 @@ def read_script_env_cache() -> dict: # If we couldn't decode it, it could be corrupt. Toss. with suppress(FileNotFoundError): p.unlink() - warn_msg = f"Could not decode msvc cache file %s: dropping." - SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg) - debug(warn_msg, repr(CONFIG_CACHE)) + 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 = f"Incompatible format for msvc cache file %s: file may be overwritten." - SCons.Warnings.warn(MSVCCacheInvalidWarning, warn_msg) - debug(warn_msg, repr(CONFIG_CACHE)) + 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 -- cgit v0.12 From 9c5c4a863600fd99db1b64c9a00b04e618d91d70 Mon Sep 17 00:00:00 2001 From: William Deegan Date: Sun, 3 Sep 2023 16:54:04 -0700 Subject: [ci skip] minor updates to CHANGE/RELEASE --- CHANGES.txt | 7 ++++--- RELEASE.txt | 8 ++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 06f0269..e63255b 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -145,9 +145,10 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER file an error. It has issued a warning since 3.0, with "warn instead of fail" deprecated since 3.1. Fixes #3958. - Minor (non-functional) cleanup of some tests, particuarly test/MSVC. - - msvc "config cache" made more resilient - now throws away the - cache if it took a decode error reading it in. Suspect a possible - race condition creating it in certain CI builds. + - Added more error handling while reading msvc config cache. + (Enabled/specified by SCONS_CACHE_MSVC_CONFIG). + The existing cache will be discarded if there's a decode error reading it. + It's possible there's a race condition creating this issue it in certain CI builds. From Jonathon Reinhart: - Fix another instance of `int main()` in CheckLib() causing failures diff --git a/RELEASE.txt b/RELEASE.txt index 73222fd..7f706bf 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -100,10 +100,10 @@ FIXES with the python logging module was refactored to prevent test failure. - MSVS: Add arm64 to the MSVS supported architectures list for VS2017 and later to be consistent with the current documentation of MSVS_ARCH. -- MSCommon: "config cache" made more resilient - now throws away the cache if - it took a decode error reading it in. Suspect a possible race condition - creating it in certain CI builds, which always start from a "no cache file - exists" state. +- MSCommon: Added more error handling while reading msvc config cache. + (Enabled/specified by SCONS_CACHE_MSVC_CONFIG). + The existing cache will be discarded if there's a decode error reading it. + It's possible there's a race condition creating this issue it in certain CI builds. IMPROVEMENTS ------------ -- cgit v0.12