From d5e91e5c8a7c629852f936ac13722789bfdcba28 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Mon, 9 Mar 2020 21:27:10 +0100 Subject: [PATCH 1/8] Keep recent caches on startup The caches are *very* expensive to compute so we should not trash them on every start of Sublime. We of course trash everything if the plugin gets uninstalled, otherwise we clean if the last access time (`st_atime`) is older than 14 days. A couple of tricks here: - The `except NameError` trick keeps the cache warm if SublimeLinter (or this plugin) reloads. Otherwise Python would clean up the directories. - If you exit Sublime, it is always a fast exit. (Sublime wants to be fast!) And so the temporary directories are kept. However, on a cold load `tmpdirs` is obviously empty, we thus need to repopulate it. Unfortunately we cannot just pass strings around because Python would trash every `TemporaryDirectory` immediateley on garbage collection. So we still store these objects, and resurrected directories become `FakeTemporaryDirectory` just for the interface of having a `.name`. --- linter.py | 97 ++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 81 insertions(+), 16 deletions(-) diff --git a/linter.py b/linter.py index 2edde4b..dc4dda9 100644 --- a/linter.py +++ b/linter.py @@ -11,14 +11,26 @@ """This module exports the Mypy plugin class.""" +import hashlib import logging import os import shutil import tempfile +import time import getpass from SublimeLinter.lint import const from SublimeLinter.lint import PythonLinter +from SublimeLinter.lint.linter import PermanentError + + +MYPY = False +if MYPY: + from typing import Dict, Protocol + + class TemporaryDirectory(Protocol): + name = None # type: str + USER = getpass.getuser() TMPDIR_PREFIX = "SublimeLinter-contrib-mypy-%s" % USER @@ -28,7 +40,10 @@ # Mapping for our created temporary directories. # For smarter caching purposes, # we index different cache folders based on the working dir. -tmpdirs = {} +try: + tmpdirs +except NameError: + tmpdirs = {} # type: Dict[str, TemporaryDirectory] class Mypy(PythonLinter): @@ -80,37 +95,87 @@ def cmd(self): # by not littering everything with `.mypy_cache` folders. if not self.settings.get('cache-dir'): cwd = self.get_working_dir() - if cwd in tmpdirs: - cache_dir = tmpdirs[cwd].name + if not cwd: # abort silently + self.notify_unassign() + raise PermanentError() + + if os.path.exists(os.path.join(cwd, '.mypy_cache')): + self.settings.set('cache-dir', False) # do not set it as arg else: - tmp_dir = tempfile.TemporaryDirectory(prefix=TMPDIR_PREFIX) - tmpdirs[cwd] = tmp_dir - cache_dir = tmp_dir.name - logger.info("Created temporary cache dir at: %s", cache_dir) - cmd[1:1] = ["--cache-dir", cache_dir] + # Add a temporary cache dir to the command if none was specified. + # Helps keep the environment clean by not littering everything + # with `.mypy_cache` folders. + try: + cache_dir = tmpdirs[cwd].name + except KeyError: + tmpdirs[cwd] = tmp_dir = _get_tmpdir(cwd) + cache_dir = tmp_dir.name + + self.settings.set('cache-dir', cache_dir) return cmd -def _cleanup_tmpdirs(): +class FakeTemporaryDirectory: + def __init__(self, name): + # type: (str) -> None + self.name = name + + +def _get_tmpdir(folder): + # type: (str) -> TemporaryDirectory + folder_hash = hashlib.sha256(folder.encode('utf-8')).hexdigest()[:7] + tmpdir = tempfile.gettempdir() + for dirname in os.listdir(tmpdir): + if dirname.startswith(TMPDIR_PREFIX) and dirname.endswith(folder_hash): + path = os.path.join(tmpdir, dirname) + tmp_dir = FakeTemporaryDirectory(path) # type: TemporaryDirectory + try: # touch it so `_cleanup_tmpdirs` doesn't catch it + os.utime(path) + except OSError: + pass + logger.info("Reuse temporary cache dir at: %s", path) + return tmp_dir + else: + tmp_dir = tempfile.TemporaryDirectory(prefix=TMPDIR_PREFIX, suffix=folder_hash) + logger.info("Created temporary cache dir at: %s", tmp_dir.name) + return tmp_dir + + +def _cleanup_tmpdirs(keep_recent=False): + def _onerror(function, path, exc_info): logger.exception("Unable to delete '%s' while cleaning up temporary directory", path, exc_info=exc_info) + tmpdir = tempfile.gettempdir() for dirname in os.listdir(tmpdir): if dirname.startswith(TMPDIR_PREFIX): - shutil.rmtree(os.path.join(tmpdir, dirname), onerror=_onerror) + full_path = os.path.join(tmpdir, dirname) + if keep_recent: + try: + atime = os.stat(full_path).st_atime + except OSError: + pass + else: + if (time.time() - atime) / 60 / 60 / 24 < 14: + continue + + shutil.rmtree(full_path, onerror=_onerror) def plugin_loaded(): """Attempt to clean up temporary directories from previous runs.""" - _cleanup_tmpdirs() + _cleanup_tmpdirs(keep_recent=True) def plugin_unloaded(): - """Clear references to TemporaryDirectory instances. + try: + from package_control import events + + if events.remove('SublimeLinter-contrib-mypy'): + logger.info("Cleanup temporary directories.") + _cleanup_tmpdirs() - They should then be removed automatically. - """ - # (Actually, do we even need to do this?) - tmpdirs.clear() + except ImportError: + pass From 72f651dc6d6f4b8fd232d8648e896e2bdb95bc76 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Mon, 9 Mar 2020 21:31:43 +0100 Subject: [PATCH 2/8] Capture filename and error code --- linter.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/linter.py b/linter.py index dc4dda9..f7869e4 100644 --- a/linter.py +++ b/linter.py @@ -49,7 +49,10 @@ class TemporaryDirectory(Protocol): class Mypy(PythonLinter): """Provides an interface to mypy.""" - regex = r'^(\w:)?[^:]+:(?P\d+):((?P\d+):)?\s*(?P[^:]+):\s*(?P.+)' + regex = ( + r'^(?P.+?):(?P\d+):((?P\d+):)?\s*' + r'(?P[^:]+):\s*(?P.+?)(\s\s\[(?P.+)\])?$' + ) line_col_base = (1, 1) tempfile_suffix = 'py' default_type = const.WARNING @@ -63,6 +66,7 @@ class Mypy(PythonLinter): "--cache-dir:": "", # Allow users to disable this "--incremental": True, + "--show-error-codes": True, # Need this to silent lints for other files. Alternatively: 'skip' "--follow-imports:": "silent", } From 5b691687c19d94e884be4ea718e403b4bbe7f39d Mon Sep 17 00:00:00 2001 From: herr kaste Date: Mon, 9 Mar 2020 20:39:06 +0100 Subject: [PATCH 3/8] Remove unused `default_type` --- linter.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/linter.py b/linter.py index f7869e4..595be53 100644 --- a/linter.py +++ b/linter.py @@ -19,7 +19,6 @@ import time import getpass -from SublimeLinter.lint import const from SublimeLinter.lint import PythonLinter from SublimeLinter.lint.linter import PermanentError @@ -55,7 +54,6 @@ class Mypy(PythonLinter): ) line_col_base = (1, 1) tempfile_suffix = 'py' - default_type = const.WARNING # Pretty much all interesting options don't expect a value, # so you'll have to specify those in "args" anyway. From 48ae00b1169e76623e1946e144058a2a8f6ed915 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Mon, 9 Mar 2020 21:33:27 +0100 Subject: [PATCH 4/8] Modernize `defaults` Remove the trailing `:`, these are probably typos although SublimeLinter is permissive here. Remove `--incremental` which is outdated. Users can still provide this option using `args`. --- linter.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/linter.py b/linter.py index 595be53..97e8a84 100644 --- a/linter.py +++ b/linter.py @@ -61,12 +61,10 @@ class Mypy(PythonLinter): defaults = { 'selector': "source.python", # Will default to tempfile.TemporaryDirectory if empty. - "--cache-dir:": "", - # Allow users to disable this - "--incremental": True, + "--cache-dir": "", "--show-error-codes": True, # Need this to silent lints for other files. Alternatively: 'skip' - "--follow-imports:": "silent", + "--follow-imports": "silent", } def cmd(self): @@ -76,7 +74,6 @@ def cmd(self): '${args}', '--show-column-numbers', '--hide-error-context', - # '--incremental', ] if self.filename: cmd.extend([ From 65e6a558727829d5ae4adc01c2029d457ecf86ef Mon Sep 17 00:00:00 2001 From: herr kaste Date: Sat, 21 Mar 2020 19:56:29 +0100 Subject: [PATCH 5/8] Allow only one process per directory mypy actually operates with caches (probably even using sqlite) and these aren't locked while running. This leads to various errors when we try to run it concurrently. SublimeLinter itself has no flag which forbids concurrent processing, so we just lock it ourself. Creating and acquiring a lock using a `defaultdict` is in itself not thread aware but it is simple enough here. --- linter.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/linter.py b/linter.py index 97e8a84..7089563 100644 --- a/linter.py +++ b/linter.py @@ -11,12 +11,14 @@ """This module exports the Mypy plugin class.""" +from collections import defaultdict import hashlib import logging import os import shutil import tempfile import time +import threading import getpass from SublimeLinter.lint import PythonLinter @@ -25,7 +27,7 @@ MYPY = False if MYPY: - from typing import Dict, Protocol + from typing import Dict, DefaultDict, Optional, Protocol class TemporaryDirectory(Protocol): name = None # type: str @@ -43,6 +45,7 @@ class TemporaryDirectory(Protocol): tmpdirs except NameError: tmpdirs = {} # type: Dict[str, TemporaryDirectory] +locks = defaultdict(lambda: threading.Lock()) # type: DefaultDict[Optional[str], threading.Lock] class Mypy(PythonLinter): @@ -114,6 +117,10 @@ def cmd(self): return cmd + def run(self, cmd, code): + with locks[self.get_working_dir()]: + return super().run(cmd, code) + class FakeTemporaryDirectory: def __init__(self, name): From 869d9bb04ccc3906b74a6bec11c54b398388947b Mon Sep 17 00:00:00 2001 From: herr kaste Date: Sat, 21 Mar 2020 19:57:51 +0100 Subject: [PATCH 6/8] Allow setting `cache-dir` to `False` to omit the arg The user can set `cache-dir` to `False` to omit the arg from the command completely. This is especially useful if the actual cache dir is configured in the `mypy.ini` file. Omit automatically in case the default location `.mypy_cache` exists. Likely it is populated and the right thing. In all cases do not manually edit `cmd`. Just set the setting and let SublimeLinter do the rest. --- linter.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/linter.py b/linter.py index 7089563..0b10729 100644 --- a/linter.py +++ b/linter.py @@ -92,10 +92,9 @@ def cmd(self): else: cmd.append('${temp_file}') - # Add a temporary cache dir to the command if none was specified. - # Helps keep the environment clean - # by not littering everything with `.mypy_cache` folders. - if not self.settings.get('cache-dir'): + # Compare against `''` so the user can set just `False`, + # for example if the cache is configured in "mypy.ini". + if self.settings.get('cache-dir') == '': cwd = self.get_working_dir() if not cwd: # abort silently self.notify_unassign() From 18379ccd92856cfa893e9f44928a02233352ba92 Mon Sep 17 00:00:00 2001 From: herr kaste Date: Mon, 9 Mar 2020 21:23:04 +0100 Subject: [PATCH 7/8] Add mypy.ini config --- mypy.ini | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 mypy.ini diff --git a/mypy.ini b/mypy.ini new file mode 100644 index 0000000..fb90e81 --- /dev/null +++ b/mypy.ini @@ -0,0 +1,10 @@ +[mypy] +check_untyped_defs = True +warn_redundant_casts = True +warn_unused_ignores = True +mypy_path = + ../ +sqlite_cache = True + +[mypy-package_control] +ignore_missing_imports = True From ec8e1b1025cb1be301c25aef2b3fe646b0f0e3dd Mon Sep 17 00:00:00 2001 From: herr kaste Date: Sun, 22 Mar 2020 20:47:54 +0100 Subject: [PATCH 8/8] Update README --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 450dffe..0ca8763 100644 --- a/README.md +++ b/README.md @@ -42,9 +42,9 @@ Following is a list of *additional* settings specific to this linter: |Setting|Description| |:------|:----------| -|cache-dir|The directory to store the cache in. Creates a sub-folder in your temporary directory if not specified.| -|follow-imports|Whether imports should be followed and linted. The default is `"silent"`, but `"skip"` may also be used. The other options are not interesting.| -|incremental|By default, we use incremental mode to speed up lint passes. Set this to `false` to disable.| +|cache-dir|The directory to store the cache in. Creates a sub-folder in your temporary directory if not specified. Set it to `false` to disable this automatic behavior, for example if the cache location is set in your mypy.ini file.| +|follow-imports|Whether imports should be followed and linted. The default is `"silent"` for speed, but `"normal"` or `"skip"` may also be used.| +|show-error-codes|Set to `false` for older mypy versions, or better yet update mypy.| All other args to mypy should be specified in the `args` list directly.