Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(linter): add 'pip check' linter #1951

Merged
merged 18 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion charmcraft/linters.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2021-2022 Canonical Ltd.
# Copyright 2021-2024 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -22,6 +22,8 @@
import pathlib
import re
import shlex
import subprocess
import sys
import typing
from collections.abc import Generator
from typing import final
Expand Down Expand Up @@ -689,6 +691,63 @@ def run(self, basedir: pathlib.Path) -> str:
return self._check_additional_files(stage_dir, basedir)


class PipCheck(Linter):
"""Check that the pip virtual environment is valid."""

name = "pip-check"
text = "Virtual environment is valid."
url = "https://pip.pypa.io/en/stable/cli/pip_check/"

def run(self, basedir: pathlib.Path) -> str:
"""Run pip check."""
venv_dir = basedir / "venv"
if not venv_dir.is_dir():
self.text = "Charm does not contain a Python venv."
return self.Result.NONAPPLICABLE
if not (venv_dir / "lib").is_dir():
self.text = "Python venv is not valid."
return self.Result.NONAPPLICABLE
if sys.platform == "win32":
self.text = "Linter does not work on Windows."
return self.Result.NONAPPLICABLE
python_exe = venv_dir / "bin" / "python"
delete_parent = False
if not python_exe.parent.exists():
delete_parent = True
python_exe.parent.mkdir()
if not python_exe.exists():
delete_python_exe = True
python_exe.symlink_to(sys.executable)
else:
delete_python_exe = False

mr-cal marked this conversation as resolved.
Show resolved Hide resolved
pip_cmd = [sys.executable, "-m", "pip", "--python", str(python_exe), "check"]
try:
check = subprocess.run(
pip_cmd,
text=True,
capture_output=True,
check=False,
)
if check.returncode == os.EX_OK:
result = self.Result.OK
else:
self.text = check.stdout
result = self.Result.WARNING
except (FileNotFoundError, PermissionError) as e:
self.text = (
f"{e.strerror}: Could not run Python executable at {sys.executable}."
)
result = self.Result.NONAPPLICABLE
finally:
if delete_python_exe:
python_exe.unlink()
if delete_parent:
python_exe.parent.rmdir()

return result


# all checkers to run; the order here is important, as some checkers depend on the
# results from others
CHECKERS: list[type[BaseChecker]] = [
Expand All @@ -701,4 +760,5 @@ def run(self, basedir: pathlib.Path) -> str:
Entrypoint,
OpsMainCall,
AdditionalFiles,
PipCheck,
]
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ dependencies = [
"requests-toolbelt",
"snap-helpers",
"tabulate",
"pip>=24.2",
]
classifiers = [
"Development Status :: 5 - Production/Stable",
Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ more-itertools==10.5.0
oauthlib==3.2.2
overrides==7.7.0
packaging==24.1
pip==24.2
platformdirs==4.3.6
pluggy==1.5.0
protobuf==5.28.2
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ more-itertools==10.5.0
oauthlib==3.2.2
overrides==7.7.0
packaging==24.1
pip==24.2
platformdirs==4.3.6
protobuf==5.28.2
pycparser==2.22
Expand Down
61 changes: 61 additions & 0 deletions tests/integration/test_linters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Copyright 2024 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# For further info, check https://github.com/canonical/charmcraft
"""Unit tests for linters."""

import pathlib
import subprocess
import sys

import pytest

from charmcraft import linters
from charmcraft.models.lint import LintResult

pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="Windows not supported")


@pytest.mark.parametrize(
"pip_cmd",
[
["--version"],
["install", "pytest", "hypothesis"],
],
)
@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported")
def test_pip_check_success(tmp_path: pathlib.Path, pip_cmd: list[str]):
venv_path = tmp_path / "venv"
subprocess.run([sys.executable, "-m", "venv", venv_path], check=True)
subprocess.run([venv_path / "bin" / "python", "-m", "pip", *pip_cmd], check=True)

lint = linters.PipCheck()
assert lint.run(tmp_path) == LintResult.OK
assert lint.text == linters.PipCheck.text


@pytest.mark.parametrize(
"pip_cmd",
[
["install", "--no-deps", "pydantic==2.9.2"],
],
)
def test_pip_check_failure(tmp_path: pathlib.Path, pip_cmd: list[str]):
venv_path = tmp_path / "venv"
subprocess.run([sys.executable, "-m", "venv", venv_path], check=True)
subprocess.run([venv_path / "bin" / "python", "-m", "pip", *pip_cmd], check=True)

lint = linters.PipCheck()
assert lint.run(tmp_path) == LintResult.WARNING
assert "pydantic 2.9.2 requires pydantic-core" in lint.text
138 changes: 138 additions & 0 deletions tests/unit/test_linters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
# Copyright 2024 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
# For further info, check https://github.com/canonical/charmcraft
"""Unit tests for linters."""

import pathlib
import subprocess
import sys

import pytest

from charmcraft import linters
from charmcraft.models.lint import LintResult


@pytest.fixture
def valid_venv_path(fake_path) -> pathlib.Path:
"""Create and return a fakefs path that contains a valid venv structure"""
(fake_path / "venv" / "lib").mkdir(parents=True)
return fake_path


def test_pip_check_not_venv(fake_path: pathlib.Path):
lint = linters.PipCheck()
assert lint.run(fake_path) == LintResult.NONAPPLICABLE
assert lint.text == "Charm does not contain a Python venv."


def test_pip_invalid_venv(fake_path: pathlib.Path):
(fake_path / "venv").mkdir()
lint = linters.PipCheck()
assert lint.run(fake_path) == LintResult.NONAPPLICABLE
assert lint.text == "Python venv is not valid."


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
mr-cal marked this conversation as resolved.
Show resolved Hide resolved
def test_pip_check_success(valid_venv_path: pathlib.Path, fp):
fp.register(
[sys.executable, "-m", "pip", "--python", fp.any(), "check"],
returncode=0,
stdout="Loo loo loo, doing pip stuff. Pip stuff is my favourite stuff.",
)

lint = linters.PipCheck()
assert lint.run(valid_venv_path) == LintResult.OK
assert lint.text == linters.PipCheck.text


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
def test_pip_check_warning(valid_venv_path: pathlib.Path, fp):
fp.register(
[sys.executable, "-m", "pip", "--python", fp.any(), "check"],
returncode=1,
stdout="This error was sponsored by Raytheon Knife Missiles™",
)

lint = linters.PipCheck()
assert lint.run(valid_venv_path) == LintResult.WARNING
assert lint.text == "This error was sponsored by Raytheon Knife Missiles™"


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
def test_pip_check_exception(valid_venv_path: pathlib.Path, monkeypatch):
def _raises_eperm(*args, **kwargs) -> None:
raise PermissionError(13, "Permission denied")

monkeypatch.setattr(subprocess, "run", _raises_eperm)

lint = linters.PipCheck()
assert lint.run(valid_venv_path) == LintResult.NONAPPLICABLE
assert (
lint.text
== f"Permission denied: Could not run Python executable at {sys.executable}."
)


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
def test_pip_check_repair_no_bin(valid_venv_path: pathlib.Path, fp):
"""Check that the bin directory is deleted if it was missing before"""
fp.register(
[sys.executable, "-m", "pip", "--python", fp.any(), "check"],
returncode=0,
stdout="Gosh, I sure hope I remember where everything went.",
)
lint = linters.PipCheck()

# Make sure it doesn't leave behind "bin" if it didn't exist
assert lint.run(valid_venv_path) == LintResult.OK
assert lint.text == "Virtual environment is valid."
assert not (valid_venv_path / "venv" / "bin").exists()


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
def test_pip_check_repair_no_py(valid_venv_path: pathlib.Path, fp):
"""Check that the python symlink is deleted if it was missing before"""
fp.register(
[sys.executable, "-m", "pip", "--python", fp.any(), "check"],
returncode=0,
stdout="Gosh, I sure hope I remember where everything went.",
)
lint = linters.PipCheck()

# Make sure it keeps "bin" if only the Python binary didn't exist
(valid_venv_path / "venv" / "bin").mkdir()
assert lint.run(valid_venv_path) == LintResult.OK
assert lint.text == "Virtual environment is valid."
assert (valid_venv_path / "venv" / "bin").exists()
assert not (valid_venv_path / "venv" / "bin" / "python").exists()


@pytest.mark.skipif(sys.platform == "win32", reason="Windows not [yet] supported.")
def test_pip_check_repair_all(valid_venv_path: pathlib.Path, fp):
"""Check that nothing is changed if all components are present"""
fp.register(
[sys.executable, "-m", "pip", "--python", fp.any(), "check"],
returncode=0,
stdout="Gosh, I sure hope I remember where everything went.",
)
lint = linters.PipCheck()

(valid_venv_path / "venv" / "bin").mkdir()
(valid_venv_path / "venv" / "bin" / "python").touch()

assert lint.run(valid_venv_path) == LintResult.OK
assert lint.text == "Virtual environment is valid."
assert (valid_venv_path / "venv" / "bin" / "python").is_file()
Loading