LSP will use a temporary root dir when needed

Author: suoto

.ci/scripts/run_tests.py

@@ -155,7 +155,9 @@ def _getNoseCommandLineArgs(args):
     if args.debugger:
         argv += ["--debugger"]
     if args.fail_fast:
-        argv += ["--fail-fast", "--verbose"]
+        argv += ["--fail-fast", ]
+        # Need at least 2 verbose levels for this to work!
+        argv += ["--verbose"] * (2 - len(args.verbose))
     if not args.log_to_stdout:
         argv += ["--log-capture"]
 
@@ -199,6 +201,9 @@ def main():
     logging.getLogger("nose2").setLevel(logging.FATAL)
     logging.getLogger("vunit").setLevel(logging.ERROR)
     logging.getLogger("requests").setLevel(logging.WARNING)
+    logging.getLogger("pyls").setLevel(logging.INFO)
+    logging.getLogger("pyls.config.config").setLevel(logging.WARNING)
+    logging.getLogger("pyls.python_ls").setLevel(logging.INFO)
     file_handler = logging.FileHandler(args.log_file)
     log_format = (
         "%(levelname)-7s | %(asctime)s | "

hdl_checker/base_server.py

@@ -333,6 +333,17 @@ def _setupIfNeeded(self):
         _logger.warning("Not all directories exist, forcing setup")
         self.clean()
 
+        os.makedirs(str(self.work_dir))
+
+        del self._builder
+        del self._database
+
+        database = Database()
+        self._database = database
+        self._builder = Fallback(self.work_dir, database)
+
+        self._builder.setup()
+
         if self.config_file is None:
             return
 
@@ -346,16 +357,6 @@ def clean(self):
         """
         _logger.debug("Cleaning up project")
         removeDirIfExists(str(self.work_dir))
-        os.makedirs(str(self.work_dir))
-
-        del self._builder
-        del self._database
-
-        database = Database()
-        self._database = database
-        self._builder = Fallback(self.work_dir, database)
-
-        self._builder.setup()
 
     @abc.abstractmethod
     def _handleUiInfo(self, message):  # type: (AnyStr) -> None

hdl_checker/exceptions.py

@@ -66,12 +66,3 @@ def __init__(self, path):
 
     def __str__(self):
         return "Couldn't determine file type for path '%s'" % self._path
-
-
-class HdlCheckerNotInitializedError(HdlCheckerBaseException):
-    """
-    Exception thrown by the LSP server when the client asks for info that
-    depends on having BaseServer properly setup.
-    """
-    def __str__(self):
-        return "HDL Checker server needs a root URI to work correctly"

hdl_checker/lsp.py

@@ -18,7 +18,9 @@
 
 import json
 import logging
-import os.path as p
+from os import getpid
+from os import path as p
+from tempfile import mkdtemp
 from typing import Any, Callable, Dict, Iterable, List, Optional, Set, Tuple, Union
 
 import six
@@ -34,16 +36,21 @@
 from hdl_checker.config_generators.simple_finder import SimpleFinder
 from hdl_checker.database import _DEFAULT_LIBRARY_NAME
 from hdl_checker.diagnostics import CheckerDiagnostic, DiagType
-from hdl_checker.exceptions import HdlCheckerNotInitializedError, UnknownParameterError
+from hdl_checker.exceptions import UnknownParameterError
 from hdl_checker.parsers.elements.dependency_spec import DependencySpec
 from hdl_checker.parsers.elements.design_unit import (
     VerilogDesignUnit,
     VhdlDesignUnit,
     tAnyDesignUnit,
 )
-from hdl_checker.path import Path
+from hdl_checker.path import Path, TemporaryPath
 from hdl_checker.types import Location, MarkupKind
-from hdl_checker.utils import getTemporaryFilename, logCalls, onNewReleaseFound
+from hdl_checker.utils import (
+    getTemporaryFilename,
+    logCalls,
+    onNewReleaseFound,
+    removeIfExists,
+)
 
 _logger = logging.getLogger(__name__)
 
@@ -150,15 +157,16 @@ def __init__(self, *args, **kwargs):
 
     @property
     def checker(self):
-        # type: () -> HdlCodeCheckerServer
+        # type: () -> Server
         """
-        Returns a valid checker, otherwise raise HdlCheckerNotInitializedError.
-        In theory this shouldn't happen since root URI is mandatory when
-        initializing the server.
+        Returns a valid checker, either the one configured during
+        HdlCheckerLanguageServer._onConfigUpdate or a new one using a temporary
+        directory.
         """
         if self._checker is None:
-            _logger.error("Checker not initialized")
-            raise HdlCheckerNotInitializedError()
+            _logger.info("Server was not initialized, using a temporary one")
+            root_dir = mkdtemp(prefix="temp_hdl_checker_pid{}_".format(getpid()))
+            self._checker = Server(self.workspace, root_dir=TemporaryPath(root_dir))
         return self._checker
 
     def showInfo(self, msg):

hdl_checker/tests/test_lsp.py

@@ -46,7 +46,6 @@
 from nose2.tools import such  # type: ignore
 
 from hdl_checker.base_server import WatchedFile
-from hdl_checker.exceptions import HdlCheckerNotInitializedError
 from hdl_checker.parsers.elements.dependency_spec import DependencySpec
 from hdl_checker.parsers.elements.design_unit import (
     DesignUnitType,
@@ -158,7 +157,7 @@ def test_workspace_notify(self):
         workspace = MagicMock(spec=Workspace)
 
         server = lsp.Server(
-            workspace, root_path=TemporaryPath(mkdtemp(prefix="hdl_checker_"))
+            workspace, root_dir=TemporaryPath(mkdtemp(prefix="hdl_checker_"))
         )
 
         server._handleUiInfo("some info")  # pylint: disable=protected-access
@@ -197,7 +196,8 @@ def _initializeServer(server, params=None):
             },
         )
 
-        it.assertEqual(server.m_initialized(), None)
+        with patch("hdl_checker.lsp.onNewReleaseFound"):
+            it.assertEqual(server.m_initialized(), None)
 
     def startLspServer():
         _logger.debug("Creating server")
@@ -421,81 +421,37 @@ def test():
                 ],
             )
 
-    # as per LSP spec, this is invalid. rootUri is a mandatory field
-    with it.having("neither root URI nor project file set"):
-
-        @it.has_setup
-        def setup():
-            startLspServer()
-
-        @it.has_teardown
-        def teardown():
-            stopLspServer()
-
-        @it.should("respond capabilities upon initialization")  # type: ignore
-        def test():
-            _initializeServer(
-                it.server, params={"initializationOptions": {"project_file": None}}
-            )
-
-        # Linting might happen on a different thread, so we might not actually
-        # get the exception. In any case, its constructor must be called
-        @it.should("fail to lint files")  # type: ignore
-        @patch.object(HdlCheckerNotInitializedError, "__init__", return_value=None)
-        def test(exc):
-            source = p.join(TEST_PROJECT, "basic_library", "clock_divider.vhd")
-            try:
-                checkLintFileOnOpen(source)
-                it.fail(
-                    "Should have raised one of %s",
-                    HdlCheckerNotInitializedError,
-                    MockWaitTimeout,
-                )
-            except (HdlCheckerNotInitializedError, MockWaitTimeout):
-                pass
+    @it.should("lint file with neither root URI nor project file set")  # type: ignore
+    def test():
+        startLspServer()
 
-            exc.assert_called_once()
+        _initializeServer(
+            it.server, params={"initializationOptions": {"project_file": None}}
+        )
 
-    with it.having("no root URI but project file set"):
+        source = p.join(TEST_PROJECT, "basic_library", "clock_divider.vhd")
+        checkLintFileOnOpen(source)
 
-        @it.has_setup
-        def setup():
-            startLspServer()
+        stopLspServer()
 
-        @it.has_teardown
-        def teardown():
-            stopLspServer()
+    @it.should("lint file with no root URI but project file set")  # type: ignore
+    def test():
+        startLspServer()
 
-        @it.should("respond capabilities upon initialization")  # type: ignore
-        def test():
-            # In this case, project file is an absolute path, since there's no
-            # root URI
-            _initializeServer(
-                it.server,
-                params={
-                    "initializationOptions": {
-                        "project_file": p.join(TEST_PROJECT, "vimhdl.prj")
-                    }
+        _initializeServer(
+            it.server,
+            params={
+                "rootUri": None,
+                "initializationOptions": {
+                    "project_file": p.join(TEST_PROJECT, "vimhdl.prj")
                 },
-            )
+            },
+        )
 
-        # Linting might happen on a different thread, so we might not actually
-        # get the exception. In any case, its constructor must be called
-        @it.should("fail to lint files")  # type: ignore
-        @patch.object(HdlCheckerNotInitializedError, "__init__", return_value=None)
-        def test(exc):
-            source = p.join(TEST_PROJECT, "basic_library", "clk_en_generator.vhd")
-            try:
-                checkLintFileOnOpen(source)
-                it.fail(
-                    "Should have raised one of %s",
-                    HdlCheckerNotInitializedError,
-                    MockWaitTimeout,
-                )
-            except (HdlCheckerNotInitializedError, MockWaitTimeout):
-                pass
+        source = p.join(TEST_PROJECT, "basic_library", "clock_divider.vhd")
+        checkLintFileOnOpen(source)
 
-            exc.assert_called_once()
+        stopLspServer()
 
 
 class TestValidProject(TestCase):
@@ -531,7 +487,8 @@ def setUp(self):
         )
 
         _logger.info("Calling m_initialized")
-        self.assertIsNone(self.server.m_initialized())
+        with patch("hdl_checker.lsp.onNewReleaseFound"):
+            self.assertIsNone(self.server.m_initialized())
 
     def teardown(self):
         _logger.debug("Shutting down server")

hdl_checker/tests/test_misc.py

@@ -28,7 +28,7 @@
 
 import parameterized  # type: ignore
 import unittest2  # type: ignore
-from mock import MagicMock, patch
+from mock import MagicMock, Mock, patch
 
 from hdl_checker.builder_utils import BuilderName, getBuilderByName
 from hdl_checker.builders.fallback import Fallback
@@ -100,29 +100,34 @@ def test_getBuilderByName(self):
 
 
 class TestReportingRelease(unittest2.TestCase):
-    @patch(
-        "hdl_checker.utils.subp.check_output",
-        return_value=b"""\
-ee3443bee4ba4ef91a7f8282d6af25c9c75e85d5        refs/tags/v0.1.0
-db561d5c26f8371650a60e9c3f1d3f6c2cb564d5        refs/tags/v0.2
-b23cce9938ea6723ed9969473f1c84c668f86bab        refs/tags/v0.3
-3b1b105060ab8dd91e73619191bf44d7a3e70a95        refs/tags/v0.4
-23ce39aaaa494ebc33f9aefec317feaea4200222        refs/tags/v0.4.1
-8bca9a3b5764da502d22d9a4c3563bbcdbc6aaa6        refs/tags/v0.4.2
-c455f5ca764d17365d77c23d14f2fe3a6234960b        refs/tags/v0.4.3
-09a1ee298f30a7412d766fe61974ae7de429d00c        refs/tags/v0.5
-ab8d07a45195cc70403cbe31b571f593dfc18c56        refs/tags/v0.5.1
-3deeffb3c3e75c385782db66613babfdab60c019        refs/tags/v0.5.2
-95444ca3051c0174dab747d4b4d5792ced5f87b6        refs/tags/v0.6
-108016f86c3e36ceb7f54bcd12227fb335cd9e25        refs/tags/v0.6.1
+    @patch("hdl_checker.utils.subp.Popen")
+    def test_GetCorrectVersion(self, popen):
+        process_mock = Mock()
+        stdout = b"""\
 7e5264355da66f71e8d1b80887ac6df55b9829ef        refs/tags/v0.6.2
-a65602477ef860b48bacfa90a96d8518eb51f030        refs/tags/v0.6.3""",
-    )
-    def test_GetCorrectVersion(self, *_):
+a65602477ef860b48bacfa90a96d8518eb51f030        refs/tags/v0.6.3"""
+
+        stderr = ""
+
+        attrs = {"communicate.return_value": (stdout, stderr)}
+        process_mock.configure_mock(**attrs)
+        popen.return_value = process_mock
+
         self.assertEqual(_getLatestReleaseVersion(), "0.6.3")
 
-    @patch("hdl_checker.utils.subp.check_output", return_value=b"0.6.3")
-    def test_RejectsInvalidFormats(self, *_):
+
+    @patch("hdl_checker.utils.subp.Popen")
+    def test_RejectsInvalidFormats(self, popen):
+        process_mock = Mock()
+        stdout = b"""\
+7e5264355da66f71e8d1b80887ac6df55b9829ef        refs/tags/v0.6.2
+a65602477ef860b48bacfa90a96d8518eb51f030        refs/tags/0.6.3"""
+
+        stderr = ""
+
+        attrs = {"communicate.return_value": (stdout, stderr)}
+        process_mock.configure_mock(**attrs)
+        popen.return_value = process_mock
         self.assertIsNone(_getLatestReleaseVersion())
 
     @patch("hdl_checker.utils.REPO_URL", "localhost")

hdl_checker/utils.py

@@ -28,6 +28,7 @@
 import sys
 from collections import Counter
 from tempfile import NamedTemporaryFile
+from threading import Timer
 from typing import Callable, Dict, Iterable, List, Optional, Tuple, TypeVar, Union
 
 import six
@@ -61,10 +62,10 @@ def setupLogging(stream, level):  # pragma: no cover
 
     logging.getLogger("urllib3").setLevel(logging.WARNING)
     logging.getLogger("pynvim").setLevel(logging.WARNING)
+    logging.getLogger("matplotlib").setLevel(logging.INFO)
     logging.getLogger("pyls").setLevel(logging.INFO)
-    logging.getLogger("pyls.python_ls").setLevel(logging.INFO)
     logging.getLogger("pyls.config.config").setLevel(logging.WARNING)
-    logging.getLogger("matplotlib").setLevel(logging.INFO)
+    logging.getLogger("pyls.python_ls").setLevel(logging.INFO)
     logging.getLogger("pyls_jsonrpc.endpoint").setLevel(logging.INFO)
 
 
@@ -400,19 +401,27 @@ def _getLatestReleaseVersion():
     the leading 'v' (so that v1.0.0 becomes simply 1.0.0). If the connection to
     the URL fails, return None
     """
+    proc = subp.Popen(
+        ["git", "ls-remote", "--sort=v:refname", "--tags", REPO_URL],
+        env={"GIT_TERMINAL_PROMPT": "0"},
+        stdout=subp.PIPE,
+        stderr=subp.PIPE,
+    )
+
+    timer = Timer(5, proc.kill)
+    timer.start()
+
     try:
-        tags = [
-            x.decode()
-            for x in subp.check_output(
-                ["git", "ls-remote", "--sort=v:refname", "--tags", REPO_URL],
-                env={"GIT_TERMINAL_PROMPT": "0"},
-                stderr=subp.PIPE,
-            ).splitlines()
-        ]
-    except subp.CalledProcessError:
-        _logger.warning("Couldn't fetch latest tag from %s", REPO_URL)
+        stdout, stderr = proc.communicate()
+    finally:
+        timer.cancel()
+
+    if not stdout or stderr:
+        _logger.info("Couldn't fetch latest tag from %s", REPO_URL)
         return None
 
+    tags = [x.decode() for x in stdout.splitlines()]
+
     latest = tags[-1].split("/")[-1]
 
     if not re.match(r"v\d+\.\d+\.\d+", latest):