From b3578a516ded0506ddc902fb290b938763e9d1b0 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Tue, 4 Apr 2017 16:00:20 -0700 Subject: [PATCH 1/5] Always write the cache (unless cache_dir is /dev/null) --- mypy/build.py | 47 ++++++++++++++++++++++++++--------------------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 51e82e6a8543..a0f2ae10ea5b 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -891,7 +891,12 @@ def write_cache(id: str, path: str, tree: MypyFile, manager.trace("Interface for {} has changed".format(id)) # Obtain and set up metadata - st = manager.get_stat(path) # TODO: Handle errors + try: + st = manager.get_stat(path) + except OSError: + manager.log("Cannot write {}".format(path)) + return interface_hash + mtime = st.st_mtime size = st.st_size options = manager.options.clone_for_module(id) @@ -1523,26 +1528,26 @@ def valid_references(self) -> Set[str]: return valid_refs def write_cache(self) -> None: - ok = self.path and self.options.incremental - if ok: - if self.manager.options.quick_and_dirty: - is_errors = self.manager.errors.is_errors_for_file(self.path) - else: - is_errors = self.manager.errors.is_errors() - ok = not is_errors - if ok: - dep_prios = [self.priorities.get(dep, PRI_HIGH) for dep in self.dependencies] - new_interface_hash = write_cache( - self.id, self.path, self.tree, - list(self.dependencies), list(self.suppressed), list(self.child_modules), - dep_prios, self.interface_hash, - self.manager) - if new_interface_hash == self.interface_hash: - self.manager.log("Cached module {} has same interface".format(self.id)) - else: - self.manager.log("Cached module {} has changed interface".format(self.id)) - self.mark_interface_stale() - self.interface_hash = new_interface_hash + if not self.path or self.options.cache_dir == os.devnull: + return + if self.manager.options.quick_and_dirty: + is_errors = self.manager.errors.is_errors_for_file(self.path) + else: + is_errors = self.manager.errors.is_errors() + if is_errors: + return + dep_prios = [self.priorities.get(dep, PRI_HIGH) for dep in self.dependencies] + new_interface_hash = write_cache( + self.id, self.path, self.tree, + list(self.dependencies), list(self.suppressed), list(self.child_modules), + dep_prios, self.interface_hash, + self.manager) + if new_interface_hash == self.interface_hash: + self.manager.log("Cached module {} has same interface".format(self.id)) + else: + self.manager.log("Cached module {} has changed interface".format(self.id)) + self.mark_interface_stale() + self.interface_hash = new_interface_hash def dispatch(sources: List[BuildSource], manager: BuildManager) -> None: From 3f4706cfee494bb212fb83a32ede2ab3b41e5463 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 5 Apr 2017 08:42:05 -0700 Subject: [PATCH 2/5] Respond to review; move get_stat(path) up and be more robust --- mypy/build.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index a0f2ae10ea5b..018b72ff6022 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -876,6 +876,20 @@ def write_cache(id: str, path: str, tree: MypyFile, data_str = json.dumps(data, sort_keys=True) interface_hash = compute_hash(data_str) + # Obtain and set up metadata + try: + st = manager.get_stat(path) + except OSError as err: + manager.log("Cannot get stat for {}: %s".format(path, err)) + # Remove apparently-invalid cache files. + for filename in [data_json, meta_json]: + try: + os.remove(filename) + except OSError: + pass + # Still return the interface hash we computed. + return interface_hash + # Write data cache file, if applicable if old_interface_hash == interface_hash: # If the interface is unchanged, the cached data is guaranteed @@ -890,13 +904,6 @@ def write_cache(id: str, path: str, tree: MypyFile, os.replace(data_json_tmp, data_json) manager.trace("Interface for {} has changed".format(id)) - # Obtain and set up metadata - try: - st = manager.get_stat(path) - except OSError: - manager.log("Cannot write {}".format(path)) - return interface_hash - mtime = st.st_mtime size = st.st_size options = manager.options.clone_for_module(id) From 1338bca6d85df63582ef15ecc32a77e04aec0844 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 19 Apr 2017 13:24:24 -0700 Subject: [PATCH 3/5] Fix format string --- mypy/build.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index 018b72ff6022..3313ce1374da 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -880,7 +880,7 @@ def write_cache(id: str, path: str, tree: MypyFile, try: st = manager.get_stat(path) except OSError as err: - manager.log("Cannot get stat for {}: %s".format(path, err)) + manager.log("Cannot get stat for {}: {}".format(path, err)) # Remove apparently-invalid cache files. for filename in [data_json, meta_json]: try: From 114be687871722590df29a992194f5d50e4bd779 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 19 Apr 2017 13:58:03 -0700 Subject: [PATCH 4/5] Handle a rare race condition with makedirs(). There was a Travis-CI test failure, I presume it was due to runtests.py parallelizing jobs that share the cache? --- mypy/build.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/build.py b/mypy/build.py index 36450dd927d3..0f88d5614b52 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -860,8 +860,10 @@ def write_cache(id: str, path: str, tree: MypyFile, # Make sure directory for cache files exists parent = os.path.dirname(data_json) - if not os.path.isdir(parent): + try: os.makedirs(parent) + except os.error: + pass # Assume it already exists. assert os.path.dirname(meta_json) == parent # Construct temp file names From c836125800f311a49c553f19a871d4eb417e0c74 Mon Sep 17 00:00:00 2001 From: Guido van Rossum Date: Wed, 19 Apr 2017 14:17:50 -0700 Subject: [PATCH 5/5] Better fix for makedirs() race --- mypy/build.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 0f88d5614b52..5ab87ab323f0 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -860,10 +860,7 @@ def write_cache(id: str, path: str, tree: MypyFile, # Make sure directory for cache files exists parent = os.path.dirname(data_json) - try: - os.makedirs(parent) - except os.error: - pass # Assume it already exists. + os.makedirs(parent, exist_ok=True) assert os.path.dirname(meta_json) == parent # Construct temp file names