From a12432dc94b54b6ff9e0b53607859ed6bd7141ca Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sat, 29 Apr 2017 21:13:19 -0700 Subject: [PATCH 1/3] Catch and ignore errors writing JSON files. This is an alternative attempt at fixing issue #3215, given the complexity of PR #3239. --- mypy/build.py | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 5ab87ab323f0..6f3471581173 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -897,12 +897,17 @@ def write_cache(id: str, path: str, tree: MypyFile, data_mtime = os.path.getmtime(data_json) manager.trace("Interface for {} is unchanged".format(id)) else: - with open(data_json_tmp, 'w') as f: - f.write(data_str) - f.write('\n') - data_mtime = os.path.getmtime(data_json_tmp) - os.replace(data_json_tmp, data_json) manager.trace("Interface for {} has changed".format(id)) + try: + with open(data_json_tmp, 'w') as f: + f.write(data_str) + f.write('\n') + os.replace(data_json_tmp, data_json) + data_mtime = os.path.getmtime(data_json) + except os.error as err: + # Most likely the error is the replace() call + # (see https://github.com/python/mypy/issues/3215). + manager.log("Error writing data JSON file {}".format(data_json_tmp)) mtime = st.st_mtime size = st.st_size @@ -922,12 +927,17 @@ def write_cache(id: str, path: str, tree: MypyFile, } # Write meta cache file - with open(meta_json_tmp, 'w') as f: - if manager.options.debug_cache: - json.dump(meta, f, indent=2, sort_keys=True) - else: - json.dump(meta, f) - os.replace(meta_json_tmp, meta_json) + try: + with open(meta_json_tmp, 'w') as f: + if manager.options.debug_cache: + json.dump(meta, f, indent=2, sort_keys=True) + else: + json.dump(meta, f) + os.replace(meta_json_tmp, meta_json) + except os.error as err: + # Most likely the error is the replace() call + # (see https://github.com/python/mypy/issues/3215). + manager.log("Error writing meta JSON file {}".format(meta_json_tmp)) return interface_hash From be30f8b473721c4b0aae07c49be4e3b6c6382af2 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Sat, 29 Apr 2017 21:23:25 -0700 Subject: [PATCH 2/3] Bail early when writing the data file failes --- mypy/build.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/mypy/build.py b/mypy/build.py index 6f3471581173..5380409fa7a9 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -882,6 +882,7 @@ def write_cache(id: str, path: str, tree: MypyFile, except OSError as err: manager.log("Cannot get stat for {}: {}".format(path, err)) # Remove apparently-invalid cache files. + # (This is purely an optimization.) for filename in [data_json, meta_json]: try: os.remove(filename) @@ -908,6 +909,15 @@ def write_cache(id: str, path: str, tree: MypyFile, # Most likely the error is the replace() call # (see https://github.com/python/mypy/issues/3215). manager.log("Error writing data JSON file {}".format(data_json_tmp)) + # Let's continue without writing the meta file. Analysis: + # If the replace failed, we've changed nothing except left + # behind an extraneous temporary file; if the replace + # worked but the getmtime() call failed, the meta file + # will be considered invalid on the next run because the + # data_mtime field won't match the data file's mtime. + # Both have the effect of slowing down the next run a + # little bit due to an out-of-date cache file. + return interface_hash mtime = st.st_mtime size = st.st_size @@ -937,6 +947,7 @@ def write_cache(id: str, path: str, tree: MypyFile, except os.error as err: # Most likely the error is the replace() call # (see https://github.com/python/mypy/issues/3215). + # The next run will simply find the cache entry out of date. manager.log("Error writing meta JSON file {}".format(meta_json_tmp)) return interface_hash From 743e012d7111f529f39748baa332e8d7c8dde7a3 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 2 May 2017 09:56:13 -0700 Subject: [PATCH 3/3] Add a clarification to the docstring. --- mypy/build.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mypy/build.py b/mypy/build.py index 5380409fa7a9..af1c6b474ca0 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -839,6 +839,10 @@ def write_cache(id: str, path: str, tree: MypyFile, old_interface_hash: str, manager: BuildManager) -> str: """Write cache files for a module. + Note that this mypy's behavior is still correct when any given + write_cache() call is replaced with a no-op, so error handling + code that bails without writing anything is okay. + Args: id: module ID path: module path