cpp_linter_hooks/util.py (4)

214-221: Bug: unconditional warning + breaking return type; restore return value and fix logging.

LOG.warning executes even when the tool is found, and changing the return type to None breaks callers/tests.

-def ensure_installed(tool: str, version: Optional[str] = None) -> None:
-    """Ensure a tool is installed, resolving its version if necessary."""
-    LOG.info("Ensuring %s is installed", tool)
-    tool_path = _resolve_install(tool, version)
-    if tool_path:
-        LOG.info("%s available at %s", tool, tool_path)
-    LOG.warning("%s not found and could not be installed", tool)
+def ensure_installed(tool: str, version: Optional[str] = None) -> Optional[Path]:
+    """Ensure a tool is installed, resolving its version if necessary. Returns the resolved executable path or None."""
+    LOG.info("Ensuring %s is installed (requested=%s)", tool, version or "auto")
+    tool_path = _resolve_install(tool, version)
+    if tool_path:
+        LOG.info("%s available at %s", tool, tool_path)
+        return tool_path
+    LOG.error("%s not found and could not be installed", tool)
+    return None

---

177-204: Avoid repeat installs, handle None versions safely, and prefer existing tools unless explicitly pinned.

When version is not provided, we should not reinstall; also choose a sane install target when the tool is missing. This addresses the repeat‑install issue and the pipeline failures with version=None or '20'.

 def _resolve_install(tool: str, version: Optional[str]) -> Optional[Path]:
-    """Resolve the installation of a tool, checking for version and installing if necessary."""
-    user_version = _resolve_version(
-        CLANG_FORMAT_VERSIONS if tool == "clang-format" else CLANG_TIDY_VERSIONS,
-        version,
-    )
-    if user_version is None:
-        user_version = (
-            DEFAULT_CLANG_FORMAT_VERSION
-            if tool == "clang-format"
-            else DEFAULT_CLANG_TIDY_VERSION
-        )
-
-    path = shutil.which(tool)
-    if path:
-        runtime_version = _get_runtime_version(tool)
-        if runtime_version and user_version not in runtime_version:
-            LOG.info(
-                "%s version mismatch (%s != %s), reinstalling...",
-                tool,
-                runtime_version,
-                user_version,
-            )
-            return _install_tool(tool, user_version)
-        return Path(path)
-
-    return _install_tool(tool, user_version)
+    """Resolve installation, only reinstalling when an explicit version is requested."""
+    versions = CLANG_FORMAT_VERSIONS if tool == "clang-format" else CLANG_TIDY_VERSIONS
+    requested_version = _resolve_version(versions, version) if version is not None else None
+    default_version = (
+        (DEFAULT_CLANG_FORMAT_VERSION if tool == "clang-format" else DEFAULT_CLANG_TIDY_VERSION)
+        or (versions[-1] if versions else None)
+    )
+
+    path = shutil.which(tool)
+    if path:
+        # No explicit version requested: trust the existing tool to avoid repeated installs.
+        if requested_version is None:
+            return Path(path)
+        runtime_version = _get_runtime_version(tool)
+        if runtime_version and requested_version not in runtime_version:
+            LOG.info("%s version mismatch (%s != %s), reinstalling...", tool, runtime_version, requested_version)
+            return _install_tool(tool, requested_version)
+        # If we can't detect the runtime version, assume OK and avoid reinstall.
+        return Path(path)
+
+    # Tool not found: choose a version to install.
+    install_version = requested_version or default_version
+    if not install_version:
+        LOG.error("No candidate version available to install for %s", tool)
+        return None
+    return _install_tool(tool, install_version)

---

165-175: After pip install, resolve the binary robustly; PATH lookup can fail immediately.

shutil.which(tool) can return None right after install in some CI shells. Add a fallback to search next to sys.executable.

 def _install_tool(tool: str, version: str) -> Optional[Path]:
     """Install a tool using pip."""
     try:
-        subprocess.check_call(
-            [sys.executable, "-m", "pip", "install", f"{tool}=={version}"]
-        )
-        return shutil.which(tool)
+        subprocess.check_call(
+            [sys.executable, "-m", "pip", "install", "--disable-pip-version-check", f"{tool}=={version}"]
+        )
+        path = shutil.which(tool)
+        if path:
+            return Path(path)
+        # Fallback: look next to the current interpreter (handles PATH quirks in CI)
+        bin_dir = Path(sys.executable).parent
+        candidates = [bin_dir / tool]
+        if sys.platform.startswith("win"):
+            candidates.insert(0, bin_dir / f"{tool}.exe")
+        for p in candidates:
+            if p.exists():
+                return p
+        return None
     except subprocess.CalledProcessError:
         LOG.error("Failed to install %s==%s", tool, version)
         return None

---

151-161: Runtime version parsing is brittle; parse semver with a regex for both tools.

Current tidy parsing assumes a second line; format parsing uses the last token. Use a single regex to extract \d+(\.\d+)+.

Example helper (apply outside this hunk):

import re
_VERSION_RE = re.compile(r"\b(\d+(?:\.\d+)+)\b")
def _get_runtime_version(tool: str) -> Optional[str]:
    try:
        out = subprocess.check_output([tool, "--version"], text=True)
        m = _VERSION_RE.search(out)
        return m.group(1) if m else None
    except Exception:
        return None

README.md (1)

177-187: Use --verbose (remove -v) — clang-format has no -v short alias.
Update README.md (lines 177–187): remove the -v suggestion and recommend --verbose only.

cpp_linter_hooks/util.py (2)

145-154: Type mismatch: returning str where Optional[Path] is annotated.

shutil.which returns Optional[str], not Path. This causes typing drift and confuses callers.

Apply this diff:

-def _install_tool(tool: str, version: str) -> Optional[Path]:
+def _install_tool(tool: str, version: str) -> Optional[Path]:
@@
-        subprocess.check_call(
-            [sys.executable, "-m", "pip", "install", f"{tool}=={version}"]
-        )
-        return shutil.which(tool)
+        subprocess.check_call(
+            [sys.executable, "-m", "pip", "install", f"{tool}=={version}"]
+        )
+        which = shutil.which(tool)
+        return Path(which) if which else None

---

157-170: None version can reach pip; add idempotency and clear failure.

If DEFAULT_* is None, _install_tool is called with None (TypeError). Also, every call triggers a pip invocation, which risks the repeat-install loop described in #112.

Apply this diff to (a) guard against None, (b) skip re-install when already installed and unchanged within the same process, and (c) fail fast with a clear error:

+_LAST_INSTALLED: dict[str, Optional[str]] = {}
+
 def _resolve_install(tool: str, version: Optional[str]) -> Optional[Path]:
@@
-    if user_version is None:
-        user_version = (
-            DEFAULT_CLANG_FORMAT_VERSION
-            if tool == "clang-format"
-            else DEFAULT_CLANG_TIDY_VERSION
-        )
+    if user_version is None:
+        user_version = (
+            DEFAULT_CLANG_FORMAT_VERSION
+            if tool == "clang-format"
+            else DEFAULT_CLANG_TIDY_VERSION
+        )
+    if user_version is None:
+        raise RuntimeError(
+            f"No version available for {tool}. "
+            "Set --version or pin it in build-system.requires."
+        )
+
+    # Idempotency within a single process to avoid repeat installs
+    if _LAST_INSTALLED.get(tool) == user_version:
+        which = shutil.which(tool)
+        return Path(which) if which else None
@@
-    return _install_tool(tool, user_version)
+    installed = _install_tool(tool, user_version)
+    if installed:
+        _LAST_INSTALLED[tool] = user_version
+    return installed

cpp_linter_hooks/clang_tidy.py (1)

21-29: Capture stderr too; clearer error reporting.

clang-tidy may emit diagnostics on stderr. Combine streams for consistent detection and messaging.

Apply this diff:

-    try:
-        sp = subprocess.run(command, stdout=subprocess.PIPE, encoding="utf-8")
+    try:
+        sp = subprocess.run(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8")
         retval = sp.returncode
-        output = sp.stdout
+        output = (sp.stdout or "") + (sp.stderr or "")
         if "warning:" in output or "error:" in output:
             retval = 1
         return retval, output
     except FileNotFoundError as stderr:
         retval = 1
         return retval, str(stderr)

cpp_linter_hooks/util.py (1)

145-156: Invalid requirement when version is None + return type mismatch.

• When version is None, f"{tool}=={version}" becomes "tool==None" (pip error).
• Function advertises Optional[Path] but returns Optional[str] from shutil.which.

Apply this diff to handle None safely and align types:

-def _install_tool(tool: str, version: str) -> Optional[Path]:
-    """Install a tool using pip, suppressing output."""
+def _install_tool(tool: str, version: Optional[str]) -> Optional[str]:
+    """Install a tool using pip, suppressing output."""
     try:
-        subprocess.check_call(
-            [sys.executable, "-m", "pip", "install", f"{tool}=={version}"],
-            stdout=subprocess.DEVNULL,
-            stderr=subprocess.DEVNULL,
-        )
-        return shutil.which(tool)
+        req = f"{tool}=={version}" if version else tool
+        subprocess.check_call(
+            [sys.executable, "-m", "pip", "install", req, "--disable-pip-version-check", "--no-input"],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )
+        return shutil.which(tool)
     except subprocess.CalledProcessError:
         LOG.error("Failed to install %s==%s", tool, version)
         return None

tests/test_util.py (1)

135-137: Fix failing assertion to match suppressed pip output.

The implementation redirects stdout/stderr to DEVNULL; assert accordingly (this is the current CI failure).

Apply this diff:

-        mock_check_call.assert_called_once_with(
-            [sys.executable, "-m", "pip", "install", "clang-format==20.1.7"]
-        )
+        mock_check_call.assert_called_once_with(
+            [sys.executable, "-m", "pip", "install", "clang-format==20.1.7"],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+        )