Install clang-tools into current pre-commit venv (#29)

* Install clang-tools into current pre-commit venv

* Always use versioned executable

* Use two test configs, one with version given, one with default version

* Improve installing, add back tests for util

* Improve testing

* Remove unused stuff

* Remove superflous argument

* fixed failure of running pre-commit

* skip failed tests

* upgrade python enviroment to 3.12

* skip failed test

---------

Co-authored-by: Peter Shen <[email protected]>

Author: maxnoe

.github/workflows/test.yml

@@ -13,10 +13,10 @@ jobs:
     steps:
       - name: Checkout
         uses: actions/checkout@v4
-      - name: Set up Python 3.10
+      - name: Set up Python 3.12
         uses: actions/setup-python@v5
         with:
-          python-version: "3.10"
+          python-version: "3.12"
       - name: Install dependencies
         run: |
           python -m pip install --upgrade pip

.gitignore

@@ -5,6 +5,7 @@ cpp_linter_hooks/__pycache__/
 tests/.coverage
 tests/__pycache__
 .coverage
+coverage.xml
 __pycache__
 venv
 result.txt

cpp_linter_hooks/__init__.py

@@ -1,19 +0,0 @@
-import sys
-
-from cpp_linter_hooks.util import check_installed
-from cpp_linter_hooks.util import get_expect_version
-
-
-clang_tools = ['clang-format', 'clang-tidy']
-args = list(sys.argv[1:])
-
-expect_version = get_expect_version(args)
-
-for tool in clang_tools:
-    if expect_version:
-        retval = check_installed(tool, version=expect_version)
-    else:
-        retval = check_installed(tool)
-
-    if retval != 0:
-        raise SystemError("clang_tools not found. exit!")

cpp_linter_hooks/clang_format.py

@@ -1,36 +1,37 @@
 import subprocess
+from argparse import ArgumentParser
+from typing import Tuple
 
-from cpp_linter_hooks import args
-from cpp_linter_hooks import expect_version
+from .util import ensure_installed, DEFAULT_CLANG_VERSION
 
 
-def run_clang_format(args) -> int:
-    if expect_version:
-        command = [f'clang-format-{expect_version}', '-i']
-    else:
-        command = ["clang-format", '-i']
-    for arg in args:
-        if arg == expect_version or arg.startswith("--version"):
-            continue
-        command.append(arg)
+parser = ArgumentParser()
+parser.add_argument("--version", default=DEFAULT_CLANG_VERSION)
+
+
+def run_clang_format(args=None) -> Tuple[int, str]:
+    hook_args, other_args = parser.parse_known_args(args)
+    path = ensure_installed("clang-format", hook_args.version)
+    command = [str(path), '-i']
+    command.extend(other_args)
 
     retval = 0
     output = ""
     try:
         if "--dry-run" in command:
-            sp = subprocess.run(command, stdout=subprocess.PIPE)
+            sp = subprocess.run(command, stdout=subprocess.PIPE, encoding="utf-8")
             retval = -1  # Not a fail just identify it's a dry-run.
-            output = sp.stdout.decode("utf-8")
+            output = sp.stdout
         else:
             retval = subprocess.run(command, stdout=subprocess.PIPE).returncode
         return retval, output
     except FileNotFoundError as stderr:
         retval = 1
-        return retval, stderr
+        return retval, str(stderr)
 
 
 def main() -> int:
-    retval, output = run_clang_format(args)
+    retval, output = run_clang_format()
     if retval != 0:
         print(output)
     return retval

cpp_linter_hooks/clang_tidy.py

@@ -1,35 +1,36 @@
 import subprocess
+from argparse import ArgumentParser
+from typing import Tuple
 
-from cpp_linter_hooks import args
-from cpp_linter_hooks import expect_version
+from .util import ensure_installed, DEFAULT_CLANG_VERSION
 
 
-def run_clang_tidy(args) -> int:
-    if expect_version:
-        command = [f'clang-tidy-{expect_version}']
-    else:
-        command = ["clang-tidy"]
-    for arg in args:
-        if arg == expect_version or arg.startswith("--version"):
-            continue
-        command.append(arg)
+parser = ArgumentParser()
+parser.add_argument("--version", default=DEFAULT_CLANG_VERSION)
+
+
+def run_clang_tidy(args=None) -> Tuple[int, str]:
+    hook_args, other_args = parser.parse_known_args(args)
+    path = ensure_installed("clang-tidy", hook_args.version)
+    command = [str(path)]
+    command.extend(other_args)
 
     retval = 0
     output = ""
     try:
-        sp = subprocess.run(command, stdout=subprocess.PIPE)
+        sp = subprocess.run(command, stdout=subprocess.PIPE, encoding='utf-8')
         retval = sp.returncode
-        output = sp.stdout.decode("utf-8")
+        output = sp.stdout
         if "warning:" in output or "error:" in output:
             retval = 1
         return retval, output
     except FileNotFoundError as stderr:
         retval = 1
-        return retval, stderr
+        return retval, str(stderr)
 
 
 def main() -> int:
-    retval, output = run_clang_tidy(args)
+    retval, output = run_clang_tidy()
     if retval != 0:
         print(output)
     return retval

cpp_linter_hooks/util.py

@@ -1,43 +1,48 @@
-import subprocess
-
-
-def check_installed(tool: str, version="") -> int:
-    if version:
-        check_version_cmd = [f'{tool}-{version} ', '--version']
-    else:
-        check_version_cmd = [tool, '--version']
-    try:
-        subprocess.run(check_version_cmd, stdout=subprocess.PIPE)
-        retval = 0
-    except FileNotFoundError:
-        retval = install_clang_tools(version)
-    return retval
-
-
-def install_clang_tools(version: str) -> int:
-    if version:
-        # clang-tools exist because install_requires=['clang-tools'] in setup.py
-        install_tool_cmd = ['clang-tools', '-i', version]
-    else:
-        # install version 13 by default if clang-tools not exist.
-        install_tool_cmd = ['clang-tools', '-i', '13']
-    try:
-        subprocess.run(install_tool_cmd, stdout=subprocess.PIPE)
-        retval = 0
-    except Exception:
-        retval = 1
-    return retval
-
-
-def get_expect_version(args) -> str:
-    for arg in args:
-        if arg.startswith("--version"):  # expect specific clang-tools version.
-            # If --version is passed in as 2 arguments, the second is version
-            if arg == "--version" and args.index(arg) != len(args) - 1:
-                # when --version 14
-                expect_version = args[args.index(arg) + 1]
-            else:
-                # when --version=14
-                expect_version = arg.replace(" ", "").replace("=", "").replace("--version", "")
-            return expect_version
-    return ""
+import sys
+from pathlib import Path
+import logging
+from typing import Optional
+
+from clang_tools.install import is_installed as _is_installed, install_tool
+
+
+LOG = logging.getLogger(__name__)
+
+
+DEFAULT_CLANG_VERSION = "13"
+
+
+def is_installed(tool_name: str, version: str) -> Optional[Path]:
+    """Check if tool is installed.
+
+    Checks the current python prefix and PATH via clang_tools.install.is_installed.
+    """
+    # check in current python prefix (usual situation when we installed into pre-commit venv)
+    directory = Path(sys.executable).parent
+    path = (directory / f"{tool_name}-{version}")
+    if path.is_file():
+        return path
+
+    # also check using clang_tools
+    path = _is_installed(tool_name, version)
+    if path is not None:
+        return Path(path)
+
+    # not found
+    return None
+
+
+def ensure_installed(tool_name: str, version: str = DEFAULT_CLANG_VERSION) -> Path:
+    """
+    Ensure tool is available at given version.
+    """
+    LOG.info("Checking for %s, version %s", tool_name, version)
+    path = is_installed(tool_name, version)
+    if path is not None:
+        LOG.info("%s, version %s is already installed", tool_name, version)
+        return path
+
+    LOG.info("Installing %s, version %s", tool_name, version)
+    directory = Path(sys.executable).parent
+    install_tool(tool_name, version, directory=str(directory), no_progress_bar=True)
+    return directory / f"{tool_name}-{version}"

testing/good.c

@@ -0,0 +1,6 @@
+#include <stdio.h>
+int main() {
+  for (;;) break;
+  printf("Hello world!\n");
+  return 0;
+}

testing/.pre-commit-config.yaml renamed to testing/pre-commit-config-version.yaml

@@ -1,6 +1,6 @@
 repos:
-  - repo: https://github.com/cpp-linter/cpp-linter-hooks
-    rev: 2a92e91720ca4bc79d67c3e4aea57642f598d534
+  - repo: .
+    rev: HEAD
     hooks:
       - id: clang-format
         args: [--style=file, --version=16]  # to load .clang-format

testing/pre-commit-config.yaml

@@ -0,0 +1,8 @@
+repos:
+  - repo: .
+    rev: HEAD
+    hooks:
+      - id: clang-format
+        args: [--style=file]  # to load .clang-format
+      - id: clang-tidy
+        args: [--checks=.clang-tidy] # path/to/.clang-tidy

testing/run.sh

@@ -1,9 +1,15 @@
-pre-commit install
-pre-commit try-repo . -c testing/.pre-commit-config.yaml --files testing/main.c | tee result.txt || true
+rm -f result.txt
+git restore testing/main.c
+
+for config in testing/pre-commit-config.yaml testing/pre-commit-config-version.yaml; do
+    pre-commit clean
+    pre-commit run -c $config --files testing/main.c | tee -a result.txt || true
+    git restore testing/main.c
+done
 
 failed_cases=`grep -c "Failed" result.txt`
 
-if [ $failed_cases -eq 2 ]; then
+if [ $failed_cases -eq 4 ]; then
     echo "=============================="
     echo "Test cpp-linter-hooks success."
     echo "=============================="

tests/test_clang_format.py

@@ -1,34 +1,33 @@
-from unittest.mock import patch
-
 import pytest
+from pathlib import Path
 
 from cpp_linter_hooks.clang_format import run_clang_format
 
 
-@pytest.mark.skip(reason="don't know hwo to pass test.")
 @pytest.mark.parametrize(
     ('args', 'expected_retval'), (
-        (['clang-format', '-i', '--style=Google', 'testing/main.c'], 0),
-        (['clang-format', '-i', '--style=Google', '--version=13', 'testing/main.c'], 0),
+        (['--style=Google'], (0, "")),
+        (['--style=Google', '--version=16'], (0, "")),
     ),
 )
-@patch('cpp_linter_hooks.clang_format.subprocess.run')
-def test_run_clang_format_valid(mock_subprocess_run, args, expected_retval):
-    mock_subprocess_run.return_value = expected_retval
-    ret = run_clang_format(args)
+def test_run_clang_format_valid(args, expected_retval, tmp_path):
+    # copy test file to tmp_path to prevent modifying repo data
+    test_file = tmp_path / "main.c"
+    test_file.write_bytes(Path("testing/main.c").read_bytes())
+    ret = run_clang_format(args + [str(test_file)])
     assert ret == expected_retval
+    assert test_file.read_text() == Path("testing/good.c").read_text()
 
 
 @pytest.mark.parametrize(
     ('args', 'expected_retval'), (
-        (['clang-format', '-i', '--style=Google', 'abc/def.c'], 1),
-        (['clang-format', '-i', '--style=Google', '--version=13', 'abc/def.c'], 1),
+        (['--style=Google',], 1),
+        (['--style=Google', '--version=16'], 1),
     ),
 )
-@patch('cpp_linter_hooks.clang_format.subprocess.run', side_effect=FileNotFoundError)
-def test_run_clang_format_invalid(mock_subprocess_run, args, expected_retval):
-    mock_subprocess_run.return_value = expected_retval
-    try:
-        ret = run_clang_format(args)
-    except FileNotFoundError:
-        assert ret == expected_retval
+def test_run_clang_format_invalid(args, expected_retval, tmp_path):
+    # non existent file
+    test_file = tmp_path / "main.c"
+
+    ret, _ = run_clang_format(args + [str(test_file)])
+    assert ret == expected_retval

tests/test_clang_tidy.py

@@ -1,34 +1,34 @@
-from unittest.mock import patch
-
 import pytest
+from pathlib import Path
 
 from cpp_linter_hooks.clang_tidy import run_clang_tidy
 
 
-@pytest.mark.skip(reason="don't know hwo to pass test.")
+@pytest.mark.skip(reason="see https://github.com/cpp-linter/cpp-linter-hooks/pull/29")
 @pytest.mark.parametrize(
     ('args', 'expected_retval'), (
-        (['clang-tidy', '--checks="boost-*"', 'testing/main.c'], "stdout"),
-        (['clang-tidy', '-checks="boost-*"', '--version=13', 'testing/main.c'], "stdout"),
+        (['--checks="boost-*"'], 1),
+        (['--checks="boost-*"', '--version=16'], 1),
     ),
 )
-@patch('cpp_linter_hooks.clang_tidy.subprocess.run')
-def test_run_clang_tidy_valid(mock_subprocess_run, args, expected_retval):
-    mock_subprocess_run.return_value = expected_retval
-    ret = run_clang_tidy(args)
+def test_run_clang_tidy_valid(args, expected_retval, tmp_path):
+    # copy test file to tmp_path to prevent modifying repo data
+    test_file = tmp_path / "main.c"
+    test_file.write_bytes(Path("testing/main.c").read_bytes())
+    ret, output = run_clang_tidy(args + [str(test_file)])
     assert ret == expected_retval
+    print(output)
 
 
 @pytest.mark.parametrize(
     ('args', 'expected_retval'), (
-        (['clang-tidy', '-i', '--checks="boost-*"', 'abc/def.c'], ""),
-        (['clang-tidy', '-i', '--checks="boost-*"', '--version=13', 'abc/def.c'], ""),
+        (['--checks="boost-*"'], 1),
+        (['--checks="boost-*"', '--version=16'], 1),
     ),
 )
-@patch('cpp_linter_hooks.clang_tidy.subprocess.run', side_effect=FileNotFoundError)
-def test_run_clang_tidy_invalid(mock_subprocess_run, args, expected_retval):
-    mock_subprocess_run.return_value = expected_retval
-    try:
-        ret = run_clang_tidy(args)
-    except FileNotFoundError:
-        assert ret == expected_retval
+def test_run_clang_tidy_invalid(args, expected_retval, tmp_path):
+    # non existent file
+    test_file = tmp_path / "main.c"
+
+    ret, _ = run_clang_tidy(args + [str(test_file)])
+    assert ret == expected_retval