I have tried to make a purge command, please give me feedback. Thank you! by Vova956 · Pull Request #2566 · canonical/charmcraft

Vova956 · 2026-02-10T21:31:34Z

I've followed the contribution guidelines.
I've signed the CLA.
I've successfully run make lint && make test.
I've added or updated any relevant documentation.
I've updated the relevant release notes.

Copilot

Pull request overview

This pull request introduces a new purge command to Charmcraft that allows users to clean up cached data and build containers. The command provides options to selectively remove the charmcraft cache directory and/or LXD build containers (with filtering for stopped vs. running containers).

Changes:

Adds a new PurgeCommand class that removes charmcraft cache and LXD containers prefixed with "craft-"
Provides command-line flags for selective purging (--only-cache, --only-builders, --include-running, --all)
Integrates the command into the charmcraft command registry

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.

File	Description
charmcraft/application/commands/purge.py	New command implementation with cache and container cleanup logic
charmcraft/application/commands/init.py	Registers the PurgeCommand in the command groups

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-20T21:51:46Z

charmcraft/application/commands/purge.py

+from __future__ import annotations
+
+import json
+import shutil
+import subprocess
+import textwrap
+from pathlib import Path
+
+from charmcraft.application.commands.base import CharmcraftCommand
+from craft_cli import emit
+


Missing docstring for the module. Other command modules in this codebase include a docstring describing the purpose of the module (e.g., "Infrastructure for the 'init' command." in init.py or "Version command." in version.py).

Copilot · 2026-02-20T21:51:47Z

charmcraft/application/commands/purge.py

+
+    # Public entry point
+
+
+    def run(self, parsed_args) -> None:


These comment lines appear to be placeholder comments for code organization but are inconsistent with the codebase style. Consider removing them or replacing with proper docstrings if these sections need documentation.

Suggested change

# Public entry point

def run(self, parsed_args) -> None:

def run(self, parsed_args) -> None:

"""Execute the purge command based on the provided arguments."""

Copilot · 2026-02-20T21:51:47Z

charmcraft/application/commands/purge.py

+            return
+
+        emit.progress(f"Removing cache directory: {cache_dir}")
+        shutil.rmtree(cache_dir)


Missing error handling for shutil.rmtree. If the removal fails due to permission errors or file system issues, the exception will propagate uncaught. Consider wrapping this in a try-except block and providing a user-friendly error message using emit.warning or similar, as done in _list_lxd_containers at lines 126-131.

Suggested change

shutil.rmtree(cache_dir)

try:

shutil.rmtree(cache_dir)

except OSError as exc:

emit.warning(f"Failed to remove cache directory {cache_dir}: {exc}")

Copilot · 2026-02-20T21:51:47Z

charmcraft/application/commands/purge.py

+                check=True,
+            )
+
+    def _list_lxd_containers(self) -> list[dict]:


The dict type annotation should specify the expected structure. Based on the usage in _filter_builder_containers, containers have at least "name" and "status" keys. Consider using a more specific type like list[dict[str, Any]] or defining a TypedDict for the container structure to improve type safety and documentation.

Copilot · 2026-02-20T21:51:47Z

charmcraft/application/commands/purge.py

+class PurgeCommand(CharmcraftCommand):
+    name = "purge"
+    help_msg = "Remove cached data and build containers"
+    overview = textwrap.dedent(
+        """
+        Remove cached data and build containers created by Charmcraft.
+
+        By default, this command removes the pip cache and stopped build
+        containers. Running containers are preserved unless explicitly
+        requested.
+
+        Use this command to recover disk space or clean up broken build
+        environments.
+        """
+    )
+
+    def fill_parser(self, parser) -> None:
+        super().fill_parser(parser)
+
+        parser.add_argument(
+            "--only-cache",
+            action="store_true",
+            help="Clear only the pip cache",
+        )
+        parser.add_argument(
+            "--only-builders",
+            action="store_true",
+            help="Remove only stopped build containers",
+        )
+        parser.add_argument(
+            "--include-running",
+            action="store_true",
+            help="Include running build containers",
+        )
+        parser.add_argument(
+            "--all",
+            action="store_true",
+            help="Remove all caches and build containers, including bases",
+        )
+
+
+    # Public entry point
+
+
+    def run(self, parsed_args) -> None:
+        # Resolve behavior
+        purge_cache = True
+        purge_builders = True
+        include_running = parsed_args.include_running
+
+        if parsed_args.only_cache:
+            purge_builders = False
+
+        if parsed_args.only_builders:
+            purge_cache = False
+
+        if parsed_args.all:
+            purge_cache = True
+            purge_builders = True
+            include_running = True
+
+        # Execute
+        if purge_cache:
+            self._purge_cache()
+
+        if purge_builders:
+            self._purge_build_containers(include_running)
+
+        emit.message("Purge complete.")
+
+
+    # Cache handling
+
+
+    def _purge_cache(self) -> None:
+        cache_dir = Path.home() / ".cache" / "charmcraft"
+
+        if not cache_dir.exists():
+            emit.message("No Charmcraft cache found.")
+            return
+
+        emit.progress(f"Removing cache directory: {cache_dir}")
+        shutil.rmtree(cache_dir)
+
+
+    # LXD handling
+
+
+    def _purge_build_containers(self, include_running: bool) -> None:
+        containers = self._list_lxd_containers()
+        builders = self._filter_builder_containers(
+            containers, include_running=include_running
+        )
+
+        if not builders:
+            emit.message("No build containers to remove.")
+            return
+
+        for name in builders:
+            emit.progress(f"Removing build container: {name}")
+            subprocess.run(
+                ["lxc", "delete", "--force", name],
+                check=True,
+            )
+
+    def _list_lxd_containers(self) -> list[dict]:
+        try:
+            result = subprocess.run(
+                ["lxc", "list", "--format=json"],
+                capture_output=True,
+                text=True,
+                check=True,
+            )
+        except FileNotFoundError:
+            emit.warning("LXD is not installed; skipping container cleanup.")
+            return []
+        except subprocess.CalledProcessError as exc:
+            emit.warning(f"Failed to list LXD containers: {exc}")
+            return []
+
+        return json.loads(result.stdout)
+
+    def _filter_builder_containers(
+        self, containers: list[dict], *, include_running: bool
+    ) -> list[str]:
+        builders: list[str] = []
+
+        for container in containers:
+            name = container.get("name", "")
+            status = container.get("status", "").lower()
+
+            # Charmcraft / craft-application builders use this prefix
+            if not name.startswith("craft-"):
+                continue
+
+            if status == "running" and not include_running:
+                continue
+
+            builders.append(name)
+
+        return builders


Missing test coverage for the new PurgeCommand. Other commands in the codebase have corresponding test files (e.g., tests/unit/commands/test_lifecycle.py for lifecycle commands). The command has several behaviors that should be tested: cache removal, container filtering, error handling for missing LXD, different flag combinations (--only-cache, --only-builders, --include-running, --all), and edge cases like permission errors or malformed JSON from LXD.

Copilot · 2026-02-20T21:51:48Z

charmcraft/application/commands/purge.py

+
+    # LXD handling
+


Similar placeholder comment style as above. Consider removing or replacing with proper docstrings.

Copilot · 2026-02-20T21:51:49Z

charmcraft/application/commands/purge.py

+            subprocess.run(
+                ["lxc", "delete", "--force", name],
+                check=True,
+            )


Missing error handling for subprocess.run when deleting containers. If the deletion fails (e.g., due to LXD daemon issues, permission problems, or container state issues), the CalledProcessError will propagate uncaught and crash the command. This could leave the command in an inconsistent state where some containers are deleted but others are not. Consider wrapping this in a try-except block to handle failures gracefully and continue attempting to delete other containers, similar to the error handling pattern used in _list_lxd_containers.

Suggested change

subprocess.run(

["lxc", "delete", "--force", name],

check=True,

)

try:

subprocess.run(

["lxc", "delete", "--force", name],

check=True,

)

except FileNotFoundError:

emit.warning("LXD is not installed; stopping container cleanup.")

break

except subprocess.CalledProcessError as exc:

emit.warning(f"Failed to delete LXD container {name!r}: {exc}")

Copilot · 2026-02-20T21:51:49Z

charmcraft/application/commands/purge.py

+            emit.warning(f"Failed to list LXD containers: {exc}")
+            return []
+
+        return json.loads(result.stdout)


Missing error handling for json.loads. If the LXD output is malformed or not valid JSON, this will raise a JSONDecodeError and crash the command. Consider wrapping this in a try-except block and returning an empty list with a warning message if JSON parsing fails.

Suggested change

return json.loads(result.stdout)

try:

return json.loads(result.stdout)

except json.JSONDecodeError as exc:

emit.warning(f"Failed to parse LXD JSON output: {exc}; skipping container cleanup.")

return []

Copilot · 2026-02-20T21:51:49Z

charmcraft/application/commands/__init__.py

            Analyze,
            InitCommand,
-            Version,
+            Version,  


Trailing whitespace on this line. Most linters flag this as a style issue.

Suggested change

Version,

Version,

Copilot · 2026-02-20T21:51:49Z

charmcraft/application/commands/purge.py

@@ -0,0 +1,153 @@
+from __future__ import annotations


Missing copyright header. All files in this codebase should start with a copyright header following the pattern seen in other command files (Copyright year(s) Canonical Ltd., Apache License 2.0, and "For further info..." comment). See charmcraft/application/commands/base.py:1-15 or charmcraft/application/commands/version.py:1-15 for examples.

mr-cal · 2026-02-23T14:28:16Z

Fixes #1042

mr-cal

Hi @Vova956, thanks for the PR. While this command would work, this isn't the approach we want to take. Additionally, it doesn't follow the cli args outlined in #1042 nor does it handle Multipass.

Most of the work needs to be done in the upstream repositories craft-application and craft-providers.

craft-providers should be responsible for interfacing with lxd and multipass (see canonical/craft-providers#903)
craft-application should implement the command using the new calls from craft-providers
charmcraft should update craft-application, to inherit the new command

Vova956 added 2 commits February 6, 2026 16:23

Adding lxd purge

81d0aef

Finished and polished Purge Logic

7000e41

Vova956 requested a review from lengau as a code owner February 10, 2026 21:31

Merge branch 'main' into main

fe247bf

steinbro requested a review from Copilot February 20, 2026 21:46

Copilot started reviewing on behalf of steinbro February 20, 2026 21:47 View session

steinbro requested a review from mr-cal February 20, 2026 21:47

Copilot AI reviewed Feb 20, 2026

View reviewed changes

mr-cal reviewed Feb 23, 2026

View reviewed changes

steinbro requested a review from bepri February 23, 2026 15:23

Conversation

Vova956 commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

Uh oh!

mr-cal commented Feb 23, 2026

Uh oh!

mr-cal left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Vova956 commented Feb 10, 2026 •

edited

Loading