Repository: canonical/charmcraft

Issue:
`python` and `poetry` plugins include `lib64` symlink as a second copy of the venv
### Bug Description

When using the `python` or `poetry` plugin, a second copy of the `lib` directory is created in `lib64` because it's a symlink. The charm works fine without that symlink, so we should just remove `lib64` if it's a symlink. (If it's a directory, leave it.)

### To Reproduce

Pack the charm below and view it in a zip file viewer

### Environment

irrelevant

### charmcraft.yaml

```yaml
# This file configures Charmcraft.
# See https://juju.is/docs/sdk/charmcraft-config for guidance.

# (Required)
# The charm package name, no spaces
# See https://juju.is/docs/sdk/naming#heading--naming-charms for guidance.
name: basic-charm


# (Required)
# The charm type, either 'charm' or 'bundle'.
type: charm


# (Recommended)
title: Charm Template


# (Required)
summary: A very short one-line summary of the charm.


# (Required)
description: |
A single sentence that says what the charm is, concisely and memorably.

A paragraph of one to three short sentences, that describe what the charm does.

A third paragraph that explains what need the charm meets.

Finally, a paragraph that describes whom the charm is useful for.


# (Required for 'charm' type)
# A list of environments (OS version and architecture) where charms must be
# built on and run on.
bases:
- build-on:
- name: ubuntu
channel: "22.04"
run-on:
- name: ubuntu
channel: "22.04"


# (Optional) Configuration options for the charm
# This config section defines charm config options, and populates the Configure
# tab on Charmhub.
# More information on this section at https://juju.is/docs/sdk/charmcraft-yaml#heading--config
# General configuration documentation: https://juju.is/docs/sdk/config
config:
options:
# An example config option to customise the log level of the workload
log-level:
description: |
Configures the log level of gunicorn.

Acceptable values are: "info", "debug", "warning", "error" and "critical"
default: "info"
type: string


# The containers and resources metadata apply to Kubernetes charms only.
# See https://juju.is/docs/sdk/metadata-reference for a checklist and guidance.
# Remove them if not required.


# Your workload’s containers.
containers:
httpbin:
resource: httpbin-image


# This field populates the Resources tab on Charmhub.
resources:
# An OCI image resource for each container listed above.
# You may remove this if your charm will run without a workload sidecar container.
httpbin-image:
type: oci-image
description: OCI image for httpbin
# The upstream-source field is ignored by Juju. It is included here as a
# reference so the integration testing suite knows which image to deploy
# during testing. This field is also used by the 'canonical/charming-actions'
# Github action for automated releasing.
upstream-source: kennethreitz/httpbin


parts:
my-charm:
plugin: python
source: .
python-requirements: [requirements.txt]
override-build: |
/usr/bin/python3 -m pip install -U pip@https://files.pythonhosted.org/packages/d4/55/90db48d85f7689ec6f81c0db0622d704306c5284850383c090e6c7195a5c/pip-24.2-py3-none-any.whl
craftctl default
```


### Relevant log output

```shell
n/a
```


Discussion:

Patch:
diff --git a/charmcraft/linters.py b/charmcraft/linters.py
index c8d50392..40e639ca 100644
--- a/charmcraft/linters.py
+++ b/charmcraft/linters.py
@@ -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.
@@ -22,6 +22,8 @@ import os
 import pathlib
 import re
 import shlex
+import subprocess
+import sys
 import typing
 from collections.abc import Generator
 from typing import final
@@ -689,6 +691,63 @@ class AdditionalFiles(Linter):
         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
+
+        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]] = [
@@ -701,4 +760,5 @@ CHECKERS: list[type[BaseChecker]] = [
     Entrypoint,
     OpsMainCall,
     AdditionalFiles,
+    PipCheck,
 ]
diff --git a/charmcraft/parts/plugins/_poetry.py b/charmcraft/parts/plugins/_poetry.py
index 21415dd6..5af1d1f2 100644
--- a/charmcraft/parts/plugins/_poetry.py
+++ b/charmcraft/parts/plugins/_poetry.py
@@ -91,7 +91,9 @@ class PoetryPlugin(poetry_plugin.PoetryPlugin):
     @override
     def get_build_commands(self) -> list[str]:
         """Get the build commands for the Python plugin."""
-        if self._options.poetry_keep_bins:
-            return super().get_build_commands()
-        venv_bin = self._get_venv_directory() / "bin"
-        return [*super().get_build_commands(), f"rm -rf {venv_bin}"]
+        return [
+            *super().get_build_commands(),
+            *utils.get_venv_cleanup_commands(
+                self._get_venv_directory(), keep_bins=self._options.poetry_keep_bins
+            ),
+        ]
diff --git a/charmcraft/parts/plugins/_python.py b/charmcraft/parts/plugins/_python.py
index f4dabd3f..99925008 100644
--- a/charmcraft/parts/plugins/_python.py
+++ b/charmcraft/parts/plugins/_python.py
@@ -96,7 +96,9 @@ class PythonPlugin(python_plugin.PythonPlugin):
     @override
     def get_build_commands(self) -> list[str]:
         """Get the build commands for the Python plugin."""
-        if self._options.python_keep_bins:
-            return super().get_build_commands()
-        venv_bin = self._get_venv_directory() / "bin"
-        return [*super().get_build_commands(), f"rm -rf {venv_bin}"]
+        return [
+            *super().get_build_commands(),
+            *utils.get_venv_cleanup_commands(
+                self._get_venv_directory(), keep_bins=self._options.python_keep_bins
+            ),
+        ]
diff --git a/charmcraft/templates/init-fastapi-framework/src/charm.py.j2 b/charmcraft/templates/init-fastapi-framework/src/charm.py.j2
index 8862cb8c..84f9fa77 100755
--- a/charmcraft/templates/init-fastapi-framework/src/charm.py.j2
+++ b/charmcraft/templates/init-fastapi-framework/src/charm.py.j2
@@ -9,7 +9,7 @@ import typing
 
 import ops
 
-import paas_charmer.fastapi
+import paas_charm.fastapi
 
 logger = logging.getLogger(__name__)
 
diff --git a/charmcraft/utils/__init__.py b/charmcraft/utils/__init__.py
index 48269b18..dad8da54 100644
--- a/charmcraft/utils/__init__.py
+++ b/charmcraft/utils/__init__.py
@@ -66,6 +66,7 @@ from charmcraft.utils.package import (
 from charmcraft.utils.parts import (
     extend_python_build_environment,
     get_charm_copy_commands,
+    get_venv_cleanup_commands,
 )
 from charmcraft.utils.project import (
     find_charm_sources,
@@ -116,6 +117,7 @@ __all__ = [
     "humanize_list",
     "extend_python_build_environment",
     "get_charm_copy_commands",
+    "get_venv_cleanup_commands",
     "find_charm_sources",
     "get_charm_name_from_path",
     "get_templates_environment",
diff --git a/charmcraft/utils/parts.py b/charmcraft/utils/parts.py
index 11583aa3..2c994621 100644
--- a/charmcraft/utils/parts.py
+++ b/charmcraft/utils/parts.py
@@ -17,6 +17,7 @@
 
 import pathlib
 import shlex
+import textwrap
 from collections.abc import Collection
 
 
@@ -54,3 +55,22 @@ def get_charm_copy_commands(
         )
 
     return commands
+
+
+def get_venv_cleanup_commands(venv_path: pathlib.Path, *, keep_bins: bool) -> list[str]:
+    """Get a script do Charmcraft-specific venv cleanup.
+
+    :param venv_path: The path to the venv.
+    :param keep_bins: Whether to keep the bin directory of the venv.
+    :returns: A shell script to do this, as a string.
+    """
+    venv_bin = venv_path / "bin"
+    venv_lib64 = venv_path / "lib64"
+    delete_bins = [] if keep_bins else [f"rm -rf {venv_bin}"]
+    delete_lib64 = textwrap.dedent(f"""
+        if [ -L '{venv_lib64}' ]; then
+          rm -f '{venv_lib64}'
+        fi
+    """)
+
+    return [*delete_bins, delete_lib64]
diff --git a/docs/reference/changelog.rst b/docs/reference/changelog.rst
index f031c4e6..20941fe6 100644
--- a/docs/reference/changelog.rst
+++ b/docs/reference/changelog.rst
@@ -86,6 +86,13 @@ Command line
 The pack command now updates charm the libs in the project directory if they don't meet
 the requirements in the ``charm-libs`` key of ``charmcraft.yaml``.
 
+3.2.2 (2024-10-16)
+------------------
+
+- The ``whoami`` command now works with charm-scoped credentials.
+
+For a complete list of commits, see the `3.2.2`_ release on GitHub.
+
 3.2.1 (2024-09-16)
 ------------------
 
diff --git a/pyproject.toml b/pyproject.toml
index 2355e36f..821beff3 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -24,6 +24,7 @@ dependencies = [
     "requests-toolbelt",
     "snap-helpers",
     "tabulate",
+    "pip>=24.2",
 ]
 classifiers = [
     "Development Status :: 5 - Production/Stable",
diff --git a/requirements-dev.txt b/requirements-dev.txt
index 275b8926..e199d392 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -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
diff --git a/requirements.txt b/requirements.txt
index 4823bfac..a13dd5aa 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -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
diff --git a/snap/hooks/configure b/snap/hooks/configure
index 3594428e..9683bbf5 100755
--- a/snap/hooks/configure
+++ b/snap/hooks/configure
@@ -73,6 +73,10 @@ def _delete_lxd_instance(instance: dict) -> None:
         print(f"Failed to remove LXD instance {instance['name']}.", file=sys.stderr)
 
 
+def _has_lxd() -> bool:
+    return subprocess.run(["snap", "list", "lxd"]).returncode == 0
+
+
 def configure_hook_main():
     # Unique valid base instances directory to prevent duplication.
     image_slots = {}
@@ -85,6 +89,10 @@ def configure_hook_main():
         print(f"Unsupported snap configuration: {reason}.", file=sys.stderr)
         sys.exit(1)
 
+    if not _has_lxd():
+        print("LXD is not installed.", file=sys.stderr)
+        return
+
     # Remove only base images in LXD related project
     try:
         lxd_images_json = subprocess.check_output(
diff --git a/snap/hooks/remove b/snap/hooks/remove
index 90789673..77332f2e 100755
--- a/snap/hooks/remove
+++ b/snap/hooks/remove
@@ -9,7 +9,15 @@ import sys
 PROJECT_NAME = "charmcraft"
 
 
+def _has_lxd() -> bool:
+    return subprocess.run(["snap", "list", "lxd"]).returncode == 0
+
+
 def remove_hook_main():
+    if not _has_lxd():
+        print("LXD is not installed.", file=sys.stderr)
+        return
+
     # Remove all images in LXD related project
     try:
         lxd_images_json = subprocess.check_output(


Test patch:
diff --git a/tests/integration/parts/plugins/test_poetry.py b/tests/integration/parts/plugins/test_poetry.py
index f210e2d6..a0493982 100644
--- a/tests/integration/parts/plugins/test_poetry.py
+++ b/tests/integration/parts/plugins/test_poetry.py
@@ -88,3 +88,4 @@ def test_poetry_plugin(
     # Check that the stage directory looks correct.
     assert (stage_path / "src" / "charm.py").read_text() == "# Charm file"
     assert (stage_path / "venv" / "lib").is_dir()
+    assert not (stage_path / "venv" / "lib64").is_symlink()
diff --git a/tests/integration/parts/plugins/test_python.py b/tests/integration/parts/plugins/test_python.py
index d03e7193..4d393333 100644
--- a/tests/integration/parts/plugins/test_python.py
+++ b/tests/integration/parts/plugins/test_python.py
@@ -87,3 +87,4 @@ def test_python_plugin(
     # Check that the stage directory looks correct.
     assert (stage_path / "src" / "charm.py").read_text() == "# Charm file"
     assert (stage_path / "venv" / "lib").is_dir()
+    assert not (stage_path / "venv" / "lib64").is_symlink()
diff --git a/tests/integration/test_linters.py b/tests/integration/test_linters.py
new file mode 100644
index 00000000..97645f3f
--- /dev/null
+++ b/tests/integration/test_linters.py
@@ -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
diff --git a/tests/unit/test_linters.py b/tests/unit/test_linters.py
new file mode 100644
index 00000000..ca1e7207
--- /dev/null
+++ b/tests/unit/test_linters.py
@@ -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.")
+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()