Portability Improvements (#262)

i80and · web-flow · commit 725cf86a19ec · 2020-12-02T11:11:34.000-05:00
* Support multiprocessing start methods other than fork()

* Support Windows

* Treat all paths within rst as POSIX-style paths
* Add workaround for primitive os.kill() on Windows
diff --git a/snooty/gizaparser/test_extracts.py b/snooty/gizaparser/test_extracts.py
@@ -44,7 +44,7 @@ def create_page(filename: Optional[str]) -> Tuple[Page, EmbeddedRstParser]:
         return (page, EmbeddedRstParser(project_config, page, all_diagnostics[path]))
 
     pages = category.to_pages(path, create_page, giza_node.data)
-    assert [str(page.fake_full_path()) for page in pages] == [
+    assert [page.fake_full_path().as_posix() for page in pages] == [
         "test_data/test_gizaparser/source/includes/extracts/installation-directory-rhel.rst",
         "test_data/test_gizaparser/source/includes/extracts/broken-inherit.rst",
         "test_data/test_gizaparser/source/includes/extracts/another-file.rst",
diff --git a/snooty/gizaparser/test_release.py b/snooty/gizaparser/test_release.py
@@ -43,7 +43,7 @@ def create_page(filename: Optional[str]) -> Tuple[Page, EmbeddedRstParser]:
         return (page, EmbeddedRstParser(project_config, page, all_diagnostics[path]))
 
     pages = category.to_pages(path, create_page, giza_node.data)
-    assert [str(page.fake_full_path()) for page in pages] == [
+    assert [page.fake_full_path().as_posix() for page in pages] == [
         "test_data/test_gizaparser/source/includes/release/untar-release-osx-x86_64.rst",
         "test_data/test_gizaparser/source/includes/release/install-ent-windows-default.rst",
     ]
diff --git a/snooty/gizaparser/test_steps.py b/snooty/gizaparser/test_steps.py
@@ -36,7 +36,7 @@ def create_page(filename: Optional[str]) -> Tuple[Page, EmbeddedRstParser]:
         return (page, EmbeddedRstParser(project_config, page, all_diagnostics[path]))
 
     pages = category.to_pages(path, create_page, giza_node.data)
-    assert [str(page.fake_full_path()) for page in pages] == [
+    assert [page.fake_full_path().as_posix() for page in pages] == [
         "test_data/test_gizaparser/source/includes/steps/test.rst"
     ]
     # Ensure that no diagnostics were raised
diff --git a/snooty/language_server.py b/snooty/language_server.py
@@ -151,13 +151,29 @@ class TextDocumentEdit:
     edits: List[TextEdit]
 
 
-def pid_exists(pid: int) -> bool:
-    try:
-        os.kill(pid, 0)
-    except OSError:
+if sys.platform == "win32":
+    import ctypes
+
+    kernel32 = ctypes.windll.kernel32
+    PROCESS_QUERY_INFROMATION = 0x1000
+
+    def pid_exists(pid: int) -> bool:
+        process = kernel32.OpenProcess(PROCESS_QUERY_INFROMATION, 0, pid)
+        if process != 0:
+            kernel32.CloseHandle(process)
+            return True
         return False
-    else:
-        return True
+
+
+else:
+
+    def pid_exists(pid: int) -> bool:
+        try:
+            os.kill(pid, 0)
+        except ProcessLookupError:
+            return False
+        else:
+            return True
 
 
 class Backend:
diff --git a/snooty/main.py b/snooty/main.py
@@ -19,6 +19,7 @@
 import getpass
 import json
 import logging
+import multiprocessing
 import os
 import sys
 from collections import defaultdict
@@ -269,6 +270,8 @@ def _generate_build_identifiers(args: Dict[str, Optional[str]]) -> BuildIdentifi
 
 
 def main() -> None:
+    multiprocessing.freeze_support()
+
     # docopt will terminate here and display usage instructions if snooty is run improperly
     args = docopt(__doc__)
 
diff --git a/snooty/n.py b/snooty/n.py
@@ -54,6 +54,17 @@ class FileId(PurePosixPath):
 
     PAT_FILE_EXTENSIONS = re.compile(r"\.((txt)|(rst)|(yaml))$")
 
+    def collapse_dots(self) -> "FileId":
+        result: List[str] = []
+        for part in self.parts:
+            if part == "..":
+                result.pop()
+            elif part == ".":
+                continue
+            else:
+                result.append(part)
+        return FileId(*result)
+
     @property
     def without_known_suffix(self) -> str:
         """Returns the fileid without any of its known file extensions (txt, rst, yaml)"""
diff --git a/snooty/parser.py b/snooty/parser.py
@@ -66,10 +66,6 @@
 )
 from .util import RST_EXTENSIONS
 
-# XXX: Work around to get snooty working with Python 3.8 until we can fix
-# our implicit data flow issues.
-multiprocessing.set_start_method("fork")
-
 NO_CHILDREN = (n.SubstitutionReference,)
 logger = logging.getLogger(__name__)
 
@@ -581,7 +577,7 @@ def handle_directive(
                 return doc
 
             openapi_fileid, filepath = util.reroot_path(
-                Path(argument_text), self.docpath, self.project_config.source_path
+                FileId(argument_text), self.docpath, self.project_config.source_path
             )
 
             try:
@@ -619,7 +615,7 @@ def create_page() -> Tuple[Page, EmbeddedRstParser]:
                 return doc
 
             _, filepath = util.reroot_path(
-                Path(argument_text), self.docpath, self.project_config.source_path
+                FileId(argument_text), self.docpath, self.project_config.source_path
             )
 
             # Attempt to read the literally included file
@@ -747,7 +743,7 @@ def _locate_text(text: str) -> int:
                 return doc
 
             fileid, path = util.reroot_path(
-                Path(argument_text), self.docpath, self.project_config.source_path
+                FileId(argument_text), self.docpath, self.project_config.source_path
             )
 
             # Validate if file exists
@@ -855,9 +851,8 @@ def validate_list_table(
             return
 
     def add_static_asset(self, raw_path: str, upload: bool) -> StaticAsset:
-        path = Path(raw_path)
         fileid, path = util.reroot_path(
-            path, self.docpath, self.project_config.source_path
+            FileId(raw_path), self.docpath, self.project_config.source_path
         )
         static_asset = StaticAsset.load(raw_path, fileid, path, upload)
         self.static_assets.add(static_asset)
@@ -1190,9 +1185,7 @@ def create_page(filename: str) -> Tuple[Page, EmbeddedRstParser]:
                         giza_node.path,
                         filename,
                         text,
-                        n.Root(
-                            (-1,), [], self.config.get_fileid(PurePath(filename)), {}
-                        ),
+                        n.Root((-1,), [], self.config.get_fileid(FileId(filename)), {}),
                     )
                     return (
                         page,
@@ -1272,9 +1265,7 @@ def create_page(filename: str) -> Tuple[Page, EmbeddedRstParser]:
                         giza_node.path,
                         filename,
                         giza_node.text,
-                        n.Root(
-                            (-1,), [], self.config.get_fileid(PurePath(filename)), {}
-                        ),
+                        n.Root((-1,), [], self.config.get_fileid(FileId(filename)), {}),
                     )
                     return (
                         page,
diff --git a/snooty/rstparser.py b/snooty/rstparser.py
@@ -1146,9 +1146,10 @@ class Parser(Generic[_V]):
     def __init__(self, project_config: ProjectConfig, visitor_class: Type[_V]) -> None:
         self.project_config = project_config
         self.visitor_class = visitor_class
-        Registry.get(project_config.default_domain).activate()
 
     def parse(self, path: Path, text: Optional[str]) -> Tuple[_V, str]:
+        Registry.get(self.project_config.default_domain).activate()
+
         diagnostics: List[Diagnostic] = []
         text, diagnostics = self.project_config.read(path, text)
 
diff --git a/snooty/test_language_server.py b/snooty/test_language_server.py
@@ -62,7 +62,7 @@ def inc() -> None:
 
 
 def test_pid_exists() -> None:
-    assert language_server.pid_exists(0)
+    assert language_server.pid_exists(os.getpid())
     # Test that an invalid PID returns False
     assert not language_server.pid_exists(537920)
 
@@ -263,7 +263,7 @@ def test_reporting_config_error() -> None:
     with language_server.LanguageServer(sys.stdin.buffer, sys.stdout.buffer) as server:
         server.m_initialize(None, CWD_URL + "/test_data/bad_project")
         doc = {
-            "uri": f"file://{CWD_URL}/test_data/bad_project/snooty.toml",
+            "uri": f"{CWD_URL}/test_data/bad_project/snooty.toml",
             "languageId": "",
             "version": 0,
             "text": "",
diff --git a/snooty/test_main.py b/snooty/test_main.py
@@ -2,6 +2,7 @@
 import json
 import os
 import subprocess
+import sys
 from typing import Any, List
 
 from . import main
@@ -77,5 +78,7 @@ def test_print(*values: Any, **kwargs: Any) -> None:
 
 
 def test_parser_failure() -> None:
-    return_code = subprocess.call(["snooty", "build", "test_data/test_parser_failure"])
+    return_code = subprocess.call(
+        [sys.executable, "-m", "snooty", "build", "test_data/test_parser_failure"]
+    )
     assert return_code == 1
diff --git a/snooty/test_util.py b/snooty/test_util.py
@@ -1,7 +1,7 @@
 import os
 import sys
 import time
-from pathlib import Path, PurePath
+from pathlib import Path, PurePath, PurePosixPath
 from typing import List, Tuple
 
 import pytest
@@ -12,13 +12,19 @@
 
 def test_reroot_path() -> None:
     relative, absolute = util.reroot_path(
-        PurePath("/foo/bar/baz.rst"), PurePath("/foo/dir/test.txt"), Path("foo")
+        PurePosixPath("/test_data/bar/baz.rst"),
+        PurePath("/test_data/dir/test.txt"),
+        Path("test_data"),
     )
     assert absolute.is_absolute()
-    assert relative == PurePath("foo/bar/baz.rst")
-    assert util.reroot_path(
-        PurePath("../bar/baz.rst"), PurePath("foo/dir/test.txt"), Path("foo")
-    )[0] == PurePath("foo/bar/baz.rst")
+    assert relative.parts == ("test_data", "bar", "baz.rst")
+
+    relative, absolute = util.reroot_path(
+        PurePosixPath("../bar/baz.rst"),
+        PurePath("test_data/dir/test.txt"),
+        Path("test_data"),
+    )
+    assert relative.parts == ("test_data", "bar", "baz.rst")
 
 
 def test_option_string() -> None:
diff --git a/snooty/types.py b/snooty/types.py
@@ -123,7 +123,9 @@ def config_path(self) -> Path:
         return self.root.joinpath("snooty.toml")
 
     def get_fileid(self, path: PurePath) -> FileId:
-        return FileId(os.path.relpath(path, self.source_path))
+        result = PurePath(os.path.relpath(path, self.source_path))
+        # Ensure that we transform any Windows-style paths into a Posix-style FileId
+        return FileId(*result.parts)
 
     @classmethod
     def open(cls, root: Path) -> Tuple["ProjectConfig", List[Diagnostic]]:
diff --git a/snooty/util.py b/snooty/util.py
@@ -7,7 +7,7 @@
 from collections import defaultdict
 from contextlib import contextmanager
 from dataclasses import dataclass
-from pathlib import Path, PurePath
+from pathlib import Path, PurePath, PurePosixPath
 from typing import (
     Callable,
     Container,
@@ -42,14 +42,14 @@
 
 
 def reroot_path(
-    filename: PurePath, docpath: PurePath, project_root: Path
+    filename: PurePosixPath, docpath: PurePath, project_root: Path
 ) -> Tuple[FileId, Path]:
     """Files within a project may refer to other files. Return a canonical path
        relative to the project root."""
     if filename.is_absolute():
         rel_fn = FileId(*filename.parts[1:])
     else:
-        rel_fn = FileId(os.path.normpath(docpath.parent.joinpath(filename)))
+        rel_fn = FileId(*docpath.parent.joinpath(filename).parts).collapse_dots()
     return rel_fn, project_root.joinpath(rel_fn).resolve()
 
 
@@ -99,7 +99,7 @@ def ast_dive(ast: n.Node) -> Iterator[n.Node]:
 def add_doc_target_ext(target: str, docpath: PurePath, project_root: Path) -> Path:
     """Given the target file of a doc role, add the appropriate extension and return full file path"""
     # Add .txt to end of doc role target path
-    target_path = PurePath(target)
+    target_path = PurePosixPath(target)
     # Adding the current suffix first takes into account dotted targets
     new_suffix = target_path.suffix + ".txt"
     target_path = target_path.with_suffix(new_suffix)

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ def create_page(filename: Optional[str]) -> Tuple[Page, EmbeddedRstParser]:`
`43`	`43`	`return (page, EmbeddedRstParser(project_config, page, all_diagnostics[path]))`
`44`	`44`
`45`	`45`	`pages = category.to_pages(path, create_page, giza_node.data)`
`46`		`- assert [str(page.fake_full_path()) for page in pages] == [`
	`46`	`+ assert [page.fake_full_path().as_posix() for page in pages] == [`
`47`	`47`	`"test_data/test_gizaparser/source/includes/release/untar-release-osx-x86_64.rst",`
`48`	`48`	`"test_data/test_gizaparser/source/includes/release/install-ent-windows-default.rst",`
`49`	`49`	`]`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ def create_page(filename: Optional[str]) -> Tuple[Page, EmbeddedRstParser]:`
`36`	`36`	`return (page, EmbeddedRstParser(project_config, page, all_diagnostics[path]))`
`37`	`37`
`38`	`38`	`pages = category.to_pages(path, create_page, giza_node.data)`
`39`		`- assert [str(page.fake_full_path()) for page in pages] == [`
	`39`	`+ assert [page.fake_full_path().as_posix() for page in pages] == [`
`40`	`40`	`"test_data/test_gizaparser/source/includes/steps/test.rst"`
`41`	`41`	`]`
`42`	`42`	`# Ensure that no diagnostics were raised`