Skip to content

Commit

Permalink
fix regression about v12 vs v12.0.1
Browse files Browse the repository at this point in the history
  • Loading branch information
2bndy5 committed May 31, 2024
1 parent a23a644 commit 9a430ec
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 59 deletions.
41 changes: 19 additions & 22 deletions clang_tools/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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.
"""
Expand All @@ -241,21 +243,16 @@ 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.
:param version: The major version of the tools to install.
:param version: The version of the tools to install.
:param tools: The specify tool(s) to install.
:param directory: The installation directory.
:param overwrite: A flag to indicate if the creation of a symlink has
Expand All @@ -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
)
8 changes: 4 additions & 4 deletions clang_tools/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 22 additions & 14 deletions clang_tools/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,20 +101,28 @@ 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.
:param user_input: The version specification as a string.
"""
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):
#: The version input in string form
self.string = user_input
version_tuple = user_input.split(".")
self.info: Tuple[int, int, int]
"""
A tuple of integers that describes the major, minor, and patch versions.
If the version `string` is a path, then this tuple is just 3 zeros.
"""
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)
21 changes: 11 additions & 10 deletions tests/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}"
Expand All @@ -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"
Expand All @@ -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


Expand All @@ -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
Expand Down
13 changes: 4 additions & 9 deletions tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,7 @@
import pytest
from clang_tools import install_os
from clang_tools.install import clang_tools_binary_url
from clang_tools.util import (
check_install_os,
download_file,
get_sha_checksum,
parse_version,
)
from clang_tools.util import check_install_os, download_file, get_sha_checksum, Version
from clang_tools import release_tag


Expand All @@ -25,7 +20,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

Expand All @@ -37,11 +32,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)

0 comments on commit 9a430ec

Please sign in to comment.