fix regression about v12 vs v12.0.1

Author: 2bndy5

clang_tools/install.py

@@ -14,14 +14,14 @@
 from typing import Optional, cast
 
 from . import release_tag, install_os, RESET_COLOR, suffix, YELLOW
-from .util import download_file, verify_sha512, get_sha_checksum, parse_version
+from .util import download_file, verify_sha512, get_sha_checksum, Version
 
 
 #: This pattern is designed to match only the major version number.
 RE_PARSE_VERSION = re.compile(rb"version\s([\d\.]+)", re.MULTILINE)
 
 
-def is_installed(tool_name: str, version: int) -> Optional[Path]:
+def is_installed(tool_name: str, version: Version) -> Optional[Path]:
     """Detect if the specified tool is installed.
 
     :param tool_name: The name of the specified tool.
@@ -30,7 +30,9 @@ def is_installed(tool_name: str, version: int) -> Optional[Path]:
     :returns: The path to the detected tool (if found), otherwise `None`.
     """
     exe_name = (
-        f"{tool_name}" + (f"-{version}" if install_os != "windows" else "") + suffix
+        f"{tool_name}"
+        + (f"-{version.info[0]}" if install_os != "windows" else "")
+        + suffix
     )
     try:
         result = subprocess.run(
@@ -56,16 +58,16 @@ def is_installed(tool_name: str, version: int) -> Optional[Path]:
     path = Path(exe_path).resolve()
     print("at", str(path))
     ver_tuple = ver_match.split(".")
-    if ver_tuple is None or ver_tuple[0] != str(version):
+    if ver_tuple is None or ver_tuple[0] != str(version.info[0]):
         return None  # version is unknown or not the desired major release
     return path
 
 
-def clang_tools_binary_url(tool: str, version: int, tag: str = release_tag) -> str:
+def clang_tools_binary_url(tool: str, version: str, tag: str = release_tag) -> str:
     """Assemble the URL to the binary.
 
     :param tool: The name of the tool to download.
-    :param version: The major version of the tool to download.
+    :param version: The version of the tool to download.
     :param tag: The release tag used in the base URL.
 
     :returns: The URL used to download the specified tool.
@@ -79,12 +81,12 @@ def clang_tools_binary_url(tool: str, version: int, tag: str = release_tag) -> s
 
 
 def install_tool(
-    tool_name: str, version: int, directory: str, no_progress_bar: bool
+    tool_name: str, version: str, directory: str, no_progress_bar: bool
 ) -> bool:
     """An abstract function that can install either clang-tidy or clang-format.
 
     :param tool_name: The name of the clang-tool to install.
-    :param version: The major version of the tools to install.
+    :param version: The version of the tools to install.
     :param directory: The installation directory.
     :param no_progress_bar: A flag used to disable the downloads' progress bar.
 
@@ -154,7 +156,7 @@ def move_and_chmod_bin(old_bin_name: str, new_bin_name: str, install_dir: str) -
 
 def create_sym_link(
     tool_name: str,
-    version: int,
+    version: str,
     install_dir: str,
     overwrite: bool = False,
     target: Optional[Path] = None,
@@ -163,7 +165,7 @@ def create_sym_link(
     doesn't have the version number appended.
 
     :param tool_name: The name of the clang-tool to symlink.
-    :param version: The major version of the clang-tool to symlink.
+    :param version: The version of the clang-tool to symlink.
     :param install_dir: The installation directory to create the symlink in.
     :param overwrite: A flag to indicate if an existing symlink should be overwritten.
     :param target: The target executable's path and name for which to create a symlink
@@ -212,11 +214,11 @@ def create_sym_link(
         return False
 
 
-def uninstall_tool(tool_name: str, version: int, directory: str):
+def uninstall_tool(tool_name: str, version: str, directory: str):
     """Remove a specified tool of a given version.
 
     :param tool_name: The name of the clang tool to uninstall.
-    :param version: The major version of the clang-tools to remove.
+    :param version: The version of the clang-tools to remove.
     :param directory: The directory from which to remove the
         installed clang-tools.
     """
@@ -241,17 +243,12 @@ def uninstall_clang_tools(version: str, directory: str):
     """
     install_dir = install_dir_name(directory)
     print(f"Uninstalling version {version} from {str(install_dir)}")
-    version_tuple = parse_version(version)
     for tool in ("clang-format", "clang-tidy"):
-        uninstall_tool(tool, version_tuple[0], install_dir)
+        uninstall_tool(tool, version, install_dir)
 
 
 def install_clang_tools(
-    version: int,
-    tools: str,
-    directory: str,
-    overwrite: bool,
-    no_progress_bar: bool,
+    version: Version, tools: str, directory: str, overwrite: bool, no_progress_bar: bool
 ) -> None:
     """Wraps functions used to individually install tools.
 
@@ -272,7 +269,7 @@ def install_clang_tools(
         native_bin = is_installed(tool_name, version)
         if native_bin is None:  # (not already installed)
             # `install_tool()` guarantees that the binary exists now
-            install_tool(tool_name, version, install_dir, no_progress_bar)
+            install_tool(tool_name, version.string, install_dir, no_progress_bar)
         create_sym_link(  # pragma: no cover
-            tool_name, version, install_dir, overwrite, native_bin
+            tool_name, version.string, install_dir, overwrite, native_bin
         )

clang_tools/main.py

@@ -9,7 +9,7 @@
 
 from .install import install_clang_tools, uninstall_clang_tools
 from . import RESET_COLOR, YELLOW
-from .util import parse_version
+from .util import Version
 
 
 def get_parser() -> argparse.ArgumentParser:
@@ -70,10 +70,10 @@ def main():
     if args.uninstall:
         uninstall_clang_tools(args.uninstall, args.directory)
     elif args.install:
-        version = parse_version(args.install)
-        if version != (0, 0, 0):
+        version = Version(args.install)
+        if version.info != (0, 0, 0):
             install_clang_tools(
-                version[0],
+                version,
                 args.tool,
                 args.directory,
                 args.overwrite,

clang_tools/util.py

@@ -101,20 +101,26 @@ def verify_sha512(checksum: str, exe: bytes) -> bool:
     return checksum == hashlib.sha512(exe).hexdigest()
 
 
-def parse_version(version: str) -> Tuple[int, int, int]:
+class Version:
     """Parse the given version string into a semantic specification.
 
     :param version: The version specification as a string.
 
     :returns: A tuple of ints that describes the major, minor, and patch versions.
         If the version is a path, then the tuple is just 3 zeros.
     """
-    version_tuple = version.split(".")
-    if len(version_tuple) < 3:
-        # append minor and patch version numbers if not specified
-        version_tuple += ["0"] * (3 - len(version_tuple))
-    try:
-        return tuple([int(x) for x in version_tuple])  # type: ignore[return-value]
-    except ValueError:
-        assert Path(version).exists(), "specified version is not a semantic or a path"
-        return (0, 0, 0)
+
+    def __init__(self, user_input: str):
+        self.string = user_input
+        version_tuple = user_input.split(".")
+        self.info: Tuple[int, int, int]
+        if len(version_tuple) < 3:
+            # append minor and patch version numbers if not specified
+            version_tuple += ["0"] * (3 - len(version_tuple))
+        try:
+            self.info = tuple([int(x) for x in version_tuple])  # type: ignore[assignment]
+        except ValueError:
+            assert Path(
+                user_input
+            ).exists(), "specified version is not a semantic or a path"
+            self.info = (0, 0, 0)

tests/test_install.py

@@ -12,14 +12,15 @@
     is_installed,
     uninstall_clang_tools,
 )
+from clang_tools.util import Version
 
 
-@pytest.mark.parametrize("version", list(range(7, 17)))
+@pytest.mark.parametrize("version", [str(v) for v in range(7, 17)] + ["12.0.1"])
 @pytest.mark.parametrize(
     "tool_name",
     ["clang-format", "clang-tidy", "clang-query", "clang-apply-replacements"],
 )
-def test_clang_tools_binary_url(tool_name: str, version: int):
+def test_clang_tools_binary_url(tool_name: str, version: str):
     """Test `clang_tools_binary_url()`"""
     url = clang_tools_binary_url(tool_name, version)
     assert f"{tool_name}-{version}_{install_os}-amd64" in url
@@ -35,7 +36,7 @@ def test_dir_name(monkeypatch: pytest.MonkeyPatch, directory: str):
 
 def test_create_symlink(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
     """Test creation of symlink."""
-    tool_name, version = ("clang-tool", 1)
+    tool_name, version = ("clang-tool", "1")
     monkeypatch.chdir(str(tmp_path))
     # use a test tar file and rename it to "clang-tool-1" (+ OS suffix)
     test_target = tmp_path / f"{tool_name}-{version}{suffix}"
@@ -54,8 +55,8 @@ def test_create_symlink(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
     assert not create_sym_link(tool_name, version, str(tmp_path), True)
 
 
-@pytest.mark.parametrize("version", [12])
-def test_install_tools(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, version: int):
+@pytest.mark.parametrize("version", ["12"])
+def test_install_tools(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, version: str):
     """Test install tools to a temp directory."""
     monkeypatch.chdir(tmp_path)
     tool_name = "clang-format"
@@ -64,16 +65,16 @@ def test_install_tools(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, version:
     # invoking again should return False
     assert not install_tool(tool_name, version, str(tmp_path), False)
     # uninstall the tool deliberately
-    uninstall_clang_tools(f"{version}.0.0", str(tmp_path))
+    uninstall_clang_tools(version, str(tmp_path))
     assert f"{tool_name}-{version}{suffix}" not in [
         fd.name for fd in tmp_path.iterdir()
     ]
 
 
-@pytest.mark.parametrize("version", [0])
-def test_is_installed(version: int):
+@pytest.mark.parametrize("version", ["0"])
+def test_is_installed(version: str):
     """Test if installed version matches specified ``version``"""
-    tool_path = is_installed("clang-format", version=version)
+    tool_path = is_installed("clang-format", version=Version(version))
     assert tool_path is None
 
 
@@ -84,7 +85,7 @@ def test_path_warning(capsys: pytest.CaptureFixture):
     2. indicates a failure to download a tool
     """
     try:
-        install_clang_tools(0, "x", ".", False, False)
+        install_clang_tools(Version("0"), "x", ".", False, False)
     except OSError as exc:
         if install_dir_name(".") not in os.environ.get("PATH", ""):  # pragma: no cover
             # this warning does not happen in an activated venv

tests/test_util.py

@@ -8,7 +8,7 @@
     check_install_os,
     download_file,
     get_sha_checksum,
-    parse_version,
+    Version,
 )
 from clang_tools import release_tag
 
@@ -25,7 +25,7 @@ def test_check_install_os():
 def test_download_file(monkeypatch: pytest.MonkeyPatch, tmp_path: Path, tag: str):
     """Test that deliberately fails to download a file."""
     monkeypatch.chdir(str(tmp_path))
-    url = clang_tools_binary_url("clang-format", 12, tag=tag)
+    url = clang_tools_binary_url("clang-format", "12", tag=tag)
     file_name = download_file(url, "file.tar.gz", True)
     assert file_name is not None
 
@@ -37,11 +37,11 @@ def test_get_sha(monkeypatch: pytest.MonkeyPatch):
     expected = Path(f"clang-format-12_{install_os}-amd64.sha512sum").read_text(
         encoding="utf-8"
     )
-    url = clang_tools_binary_url("clang-format", 12, tag=release_tag)
+    url = clang_tools_binary_url("clang-format", "12", tag=release_tag)
     assert get_sha_checksum(url) == expected
 
 
 def test_version_path():
     """Tests version parsing when given specification is a path."""
     version = str(Path(__file__).parent)
-    assert parse_version(version) == (0, 0, 0)
+    assert Version(version).info == (0, 0, 0)