From 842bca2f1488521ccf987684219e5a5c5af29394 Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 02:49:50 -0300 Subject: [PATCH 1/6] feat(poly delete): delete bricks and clean project config --- bases/polylith/cli/core.py | 9 +- bases/polylith/cli/delete.py | 56 ++++++ components/polylith/commands/__init__.py | 14 +- components/polylith/commands/delete.py | 184 ++++++++++++++++++ components/polylith/toml/__init__.py | 2 + components/polylith/toml/core.py | 93 +++++++++ .../polylith/commands/test_delete.py | 57 ++++++ .../polylith/toml/test_remove_bricks.py | 75 +++++++ 8 files changed, 487 insertions(+), 3 deletions(-) create mode 100644 bases/polylith/cli/delete.py create mode 100644 components/polylith/commands/delete.py create mode 100644 test/components/polylith/commands/test_delete.py create mode 100644 test/components/polylith/toml/test_remove_bricks.py diff --git a/bases/polylith/cli/core.py b/bases/polylith/cli/core.py index 6d39109f..c0814d09 100644 --- a/bases/polylith/cli/core.py +++ b/bases/polylith/cli/core.py @@ -2,7 +2,7 @@ from typing import List, Union from polylith import commands, configuration, info, repo -from polylith.cli import build, create, env, options, test +from polylith.cli import build, create, delete, env, options, test from typer import Exit, Option, Typer from typing_extensions import Annotated @@ -15,6 +15,13 @@ no_args_is_help=True, ) +app.add_typer( + delete.app, + name="delete", + help="Commands for deleting bases and components.", + no_args_is_help=True, +) + app.add_typer( test.app, name="test", diff --git a/bases/polylith/cli/delete.py b/bases/polylith/cli/delete.py new file mode 100644 index 00000000..8a535b04 --- /dev/null +++ b/bases/polylith/cli/delete.py @@ -0,0 +1,56 @@ +from pathlib import Path + +from polylith import commands, configuration, repo +from typer import Exit, Option, Typer +from typing_extensions import Annotated + +app = Typer(no_args_is_help=True) + + +def _delete_brick(brick_type: str, name: str, dry_run: bool, force: bool) -> None: + root = repo.get_workspace_root(Path.cwd()) + namespace = configuration.get_namespace_from_config(root) + + cli_options = { + "brick_type": brick_type, + "name": name, + "dry_run": dry_run, + "force": force, + } + + result = commands.delete.run(root, namespace, cli_options) + + if not result: + raise Exit(code=1) + + +@app.command("component") +def component_command( + name: Annotated[str, Option(help="Name of the component to delete.")], + dry_run: Annotated[ + bool, Option(help="Print what would be deleted, but do nothing.") + ] = False, + force: Annotated[ + bool, + Option(help="Delete even if other bricks/projects still use it."), + ] = False, +): + """Deletes a Polylith component (including generated tests).""" + + _delete_brick("component", name, dry_run, force) + + +@app.command("base") +def base_command( + name: Annotated[str, Option(help="Name of the base to delete.")], + dry_run: Annotated[ + bool, Option(help="Print what would be deleted, but do nothing.") + ] = False, + force: Annotated[ + bool, + Option(help="Delete even if other bricks/projects still use it."), + ] = False, +): + """Deletes a Polylith base (including generated tests).""" + + _delete_brick("base", name, dry_run, force) diff --git a/components/polylith/commands/__init__.py b/components/polylith/commands/__init__.py index 1e3b148a..9ff8d6f2 100644 --- a/components/polylith/commands/__init__.py +++ b/components/polylith/commands/__init__.py @@ -1,3 +1,13 @@ -from polylith.commands import check, create, deps, diff, info, libs, sync, test +from polylith.commands import check, create, delete, deps, diff, info, libs, sync, test -__all__ = ["check", "create", "deps", "diff", "info", "libs", "sync", "test"] +__all__ = [ + "check", + "create", + "delete", + "deps", + "diff", + "info", + "libs", + "sync", + "test", +] diff --git a/components/polylith/commands/delete.py b/components/polylith/commands/delete.py new file mode 100644 index 00000000..da6d8c09 --- /dev/null +++ b/components/polylith/commands/delete.py @@ -0,0 +1,184 @@ +import shutil +from pathlib import Path +from typing import List, Set, Tuple + +import tomlkit +from polylith import configuration, deps, info, project, repo, toml + + +def _brick_dir(theme: str, namespace: str, name: str, brick_dir_name: str) -> Path: + if theme == "loose": + return Path(brick_dir_name, namespace, name) + + return Path(brick_dir_name, name) + + +def _test_dir(theme: str, namespace: str, name: str, brick_dir_name: str) -> Path: + if theme == "loose": + return Path("test", brick_dir_name, namespace, name) + + return Path(brick_dir_name, name, "test", namespace, name) + + +def _collect_projects_using_brick( + projects_data: List[dict], brick_type: str, name: str +) -> List[str]: + key = "components" if brick_type == "component" else "bases" + + return sorted([p["name"] for p in projects_data if name in (p.get(key) or [])]) + + +def _collect_bricks_using_brick(import_data: dict, brick: str) -> List[str]: + used_by = {k for k, v in import_data.items() if brick in (v or set())} + + return sorted(used_by.difference({brick})) + + +def _remove_brick_from_project_pyproject( + root: Path, project_path: Path, namespace: str, brick_dir_name: str, brick: str +) -> bool: + fullpath = project_path / repo.default_toml + + if not fullpath.exists(): + return False + + data = project.get_toml(fullpath) + + brick_include = f"{namespace}/{brick}" + changed = toml.remove_brick_from_project_packages( + data, brick_include, brick_dir_name + ) + + if not changed: + return False + + with fullpath.open("w", encoding="utf-8") as f: + f.write(tomlkit.dumps(data)) + + return True + + +def _project_paths(root: Path) -> List[Path]: + paths = [root] + paths.extend([p.parent for p in root.glob(f"projects/*/{repo.default_toml}")]) + + return paths + + +def _get_all_bricks(root: Path, namespace: str) -> Tuple[Set[str], Set[str]]: + bases = set(info.get_bases(root, namespace)) + components = set(info.get_components(root, namespace)) + + return bases, components + + +def _get_import_data(root: Path, namespace: str) -> dict: + bases, components = _get_all_bricks(root, namespace) + brick_imports = deps.get_brick_imports(root, namespace, bases, components) + + return {**brick_imports.get("bases", {}), **brick_imports.get("components", {})} + + +def _validate_can_delete( + root: Path, + namespace: str, + brick_type: str, + name: str, +) -> Tuple[List[str], List[str]]: + import_data = _get_import_data(root, namespace) + projects_data = info.get_projects_data(root, namespace) + + used_by_bricks = _collect_bricks_using_brick(import_data, name) + used_by_projects = _collect_projects_using_brick(projects_data, brick_type, name) + + return used_by_bricks, used_by_projects + + +def _delete_paths(paths: List[Path]) -> None: + for p in paths: + if p.exists(): + shutil.rmtree(p) + + +def run(root: Path, namespace: str, options: dict) -> bool: + brick_type = options["brick_type"] + name = options["name"] + dry_run = bool(options.get("dry_run")) + force = bool(options.get("force")) + + if brick_type not in {"component", "base"}: + raise ValueError(f"Unknown brick type: {brick_type}") + + brick_dir_name = ( + repo.components_dir if brick_type == "component" else repo.bases_dir + ) + + theme = configuration.get_theme_from_config(root) + + bases, components = _get_all_bricks(root, namespace) + + if brick_type == "component" and name not in components: + print(f"Component not found: {name}") + return False + + if brick_type == "base" and name not in bases: + print(f"Base not found: {name}") + return False + + brick_dir = root / _brick_dir(theme, namespace, name, brick_dir_name) + test_dir = root / _test_dir(theme, namespace, name, brick_dir_name) + + paths_to_delete = [brick_dir] + + # In loose theme, tests live outside the brick folder. + if theme == "loose" and test_dir != brick_dir: + paths_to_delete.append(test_dir) + + used_by_bricks, used_by_projects = _validate_can_delete( + root, namespace, brick_type, name + ) + + if (used_by_bricks or used_by_projects) and not force: + if used_by_bricks: + print(f"Cannot delete '{name}': used by bricks: {', '.join(used_by_bricks)}") + if used_by_projects: + print( + f"Cannot delete '{name}': used by projects: {', '.join(used_by_projects)}" + ) + print("Re-run with --force to delete anyway.") + return False + + project_paths = _project_paths(root) + + if dry_run: + existing = [p for p in paths_to_delete if p.exists()] + missing = [p for p in paths_to_delete if not p.exists()] + + for p in existing: + print(f"Would delete: {p}") + for p in missing: + print(f"Missing (skip): {p}") + + for p in project_paths: + fullpath = p / repo.default_toml + if fullpath.exists(): + print(f"Would update: {fullpath}") + + return True + + updated_projects = [] + for p in project_paths: + if _remove_brick_from_project_pyproject( + root, p, namespace, brick_dir_name, name + ): + updated_projects.append(p) + + _delete_paths(paths_to_delete) + + if updated_projects: + for p in updated_projects: + print(f"Updated: {p / repo.default_toml}") + + print(f"Deleted {brick_type}: {name}") + + return True diff --git a/components/polylith/toml/__init__.py b/components/polylith/toml/__init__.py index 1132fde9..42788b3a 100644 --- a/components/polylith/toml/__init__.py +++ b/components/polylith/toml/__init__.py @@ -7,6 +7,7 @@ load_toml, parse_project_dependencies, read_toml_document, + remove_brick_from_project_packages, ) __all__ = [ @@ -18,4 +19,5 @@ "load_toml", "parse_project_dependencies", "read_toml_document", + "remove_brick_from_project_packages", ] diff --git a/components/polylith/toml/core.py b/components/polylith/toml/core.py index 11a3ef9c..0f777397 100644 --- a/components/polylith/toml/core.py +++ b/components/polylith/toml/core.py @@ -124,6 +124,99 @@ def get_project_package_includes(namespace: str, data) -> List[dict]: return [transform_to_package(namespace, key) for key in includes.keys()] +def _remove_brick_from_mapping(mapping, brick_include: str, brick_dir_name: str) -> bool: + try: + items = list(mapping.items()) + except AttributeError: + return False + + keys_to_remove = [ + k + for k, v in items + if v == brick_include and f"{brick_dir_name}/" in str(k).replace("\\", "/") + ] + + for k in keys_to_remove: + del mapping[k] + + return bool(keys_to_remove) + + +def _remove_brick_from_poetry_packages( + data, brick_include: str, brick_dir_name: str +) -> bool: + poetry = data.get("tool", {}).get("poetry") + + if not isinstance(poetry, dict): + return False + + packages = poetry.get("packages") + + if not packages: + return False + + indexes = [ + i + for i, p in enumerate(list(packages)) + if p.get("include") == brick_include and brick_dir_name in str(p.get("from", "")) + ] + + for i in reversed(indexes): + packages.pop(i) + + if indexes and hasattr(packages, "multiline"): + packages.multiline(True) + + return bool(indexes) + + +def remove_brick_from_project_packages(data, brick_include: str, brick_dir_name: str) -> bool: + """Remove a brick from a project's packaging include configuration. + + This targets the configuration that `poly sync` updates: + - Poetry: `[tool.poetry.packages]` + - Hatch: `[tool.polylith.bricks]` (or `[tool.hatch.build.force-include]`) + - PDM/PEP 621: `[tool.polylith.bricks]` + + Args: + data: TOML document to update. + brick_include: The include value, e.g. `my_ns/my_brick`. + brick_dir_name: `components` or `bases`. + + Returns: + bool: True if anything was removed. + """ + + if repo.is_poetry(data): + return _remove_brick_from_poetry_packages(data, brick_include, brick_dir_name) + + polylith_bricks = data.get("tool", {}).get("polylith", {}).get("bricks") + + if repo.is_hatch(data): + if polylith_bricks: + return _remove_brick_from_mapping( + polylith_bricks, brick_include, brick_dir_name + ) + + force_include = ( + data.get("tool", {}) + .get("hatch", {}) + .get("build", {}) + .get("force-include") + ) + + return _remove_brick_from_mapping( + force_include, brick_include, brick_dir_name + ) + + if polylith_bricks: + return _remove_brick_from_mapping( + polylith_bricks, brick_include, brick_dir_name + ) + + return False + + def parse_pep_621_dependency(dep: str) -> dict: parts = re.split(r"[\^~=!<>]", dep) diff --git a/test/components/polylith/commands/test_delete.py b/test/components/polylith/commands/test_delete.py new file mode 100644 index 00000000..d2d31d07 --- /dev/null +++ b/test/components/polylith/commands/test_delete.py @@ -0,0 +1,57 @@ +import tomlkit +from polylith import repo +from polylith.bricks.component import create_component +from polylith.commands import delete + + +def test_delete_component_removes_generated_files_and_updates_pyproject(handle_workspace_files): + root = handle_workspace_files + + options = { + "brick": repo.components_dir, + "namespace": "test_space", + "package": "greet", + "description": "test desc", + "modulename": "core", + } + + create_component(path=root, options=options) + + component_dir = root / "components/test_space/greet" + test_dir = root / "test/components/test_space/greet" + + assert component_dir.exists() + assert test_dir.exists() + + # Create a minimal hatch-style pyproject that includes the brick. + pyproject = tomlkit.parse( + """\ +[build-system] +requires = [\"hatchling\"] +build-backend = \"hatchling.build\" + +[project] +name = "development" +version = "0.0.0" + +[tool.polylith.bricks] +\"components/test_space/greet\" = \"test_space/greet\" +""" + ) + + (root / repo.default_toml).write_text(tomlkit.dumps(pyproject), encoding="utf-8") + + res = delete.run( + root, + "test_space", + {"brick_type": "component", "name": "greet", "dry_run": False, "force": True}, + ) + + assert res is True + assert not component_dir.exists() + assert not test_dir.exists() + + updated = tomlkit.parse((root / repo.default_toml).read_text(encoding="utf-8")) + bricks = updated.get("tool", {}).get("polylith", {}).get("bricks", {}) + + assert "components/test_space/greet" not in bricks diff --git a/test/components/polylith/toml/test_remove_bricks.py b/test/components/polylith/toml/test_remove_bricks.py new file mode 100644 index 00000000..4392a71f --- /dev/null +++ b/test/components/polylith/toml/test_remove_bricks.py @@ -0,0 +1,75 @@ +import tomlkit + +from polylith import toml + + +def test_remove_brick_from_poetry_packages(): + data = tomlkit.parse( + """\ +[tool.poetry] +packages = [ + {include = "test_space/hello", from = "components"}, + {include = "test_space/world", from = "components"}, +] + +[build-system] +requires = ["poetry-core"] +build-backend = "poetry.core.masonry.api" +""" + ) + + changed = toml.remove_brick_from_project_packages( + data, "test_space/world", "components" + ) + + assert changed is True + res = data["tool"]["poetry"]["packages"] + assert len(res) == 1 + assert res[0]["include"] == "test_space/hello" + + +def test_remove_brick_from_polylith_bricks_section(): + data = tomlkit.parse( + """\ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.polylith.bricks] +"components/test_space/world" = "test_space/world" +"components/test_space/hello" = "test_space/hello" +"bases/test_space/world" = "test_space/world" +""" + ) + + changed = toml.remove_brick_from_project_packages( + data, "test_space/world", "components" + ) + + assert changed is True + bricks = data["tool"]["polylith"]["bricks"] + assert "components/test_space/world" not in bricks + assert "bases/test_space/world" in bricks + + +def test_remove_brick_from_force_include_section(): + data = tomlkit.parse( + """\ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[tool.hatch.build.force-include] +"components/test_space/world" = "test_space/world" +"components/test_space/hello" = "test_space/hello" +""" + ) + + changed = toml.remove_brick_from_project_packages( + data, "test_space/world", "components" + ) + + assert changed is True + include = data["tool"]["hatch"]["build"]["force-include"] + assert "components/test_space/world" not in include + assert "components/test_space/hello" in include From 6c17ba13752e84d5b388dac5309cac685a24077a Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 11:52:48 -0300 Subject: [PATCH 2/6] fix(polylith-cli): correct poly entrypoint module --- projects/polylith_cli/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/polylith_cli/pyproject.toml b/projects/polylith_cli/pyproject.toml index 2f30f150..68d038f5 100644 --- a/projects/polylith_cli/pyproject.toml +++ b/projects/polylith_cli/pyproject.toml @@ -49,7 +49,7 @@ typer = "0.*" pyyaml = "*" [tool.poetry.scripts] -poly = "polylith_cli.polylith.cli.core:app" +poly = "polylith.cli.core:app" [build-system] requires = ["poetry-core>=1.0.0"] From 154ec2893e0b1ac8f23191e99387cf3617a1f3e0 Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 12:47:24 -0300 Subject: [PATCH 3/6] Revert "fix(polylith-cli): correct poly entrypoint module" This reverts commit 6c17ba13752e84d5b388dac5309cac685a24077a. --- projects/polylith_cli/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/polylith_cli/pyproject.toml b/projects/polylith_cli/pyproject.toml index 68d038f5..2f30f150 100644 --- a/projects/polylith_cli/pyproject.toml +++ b/projects/polylith_cli/pyproject.toml @@ -49,7 +49,7 @@ typer = "0.*" pyyaml = "*" [tool.poetry.scripts] -poly = "polylith.cli.core:app" +poly = "polylith_cli.polylith.cli.core:app" [build-system] requires = ["poetry-core>=1.0.0"] From 858556e4a99e707e0e7868d4cef7fcda6bc01b35 Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 13:09:26 -0300 Subject: [PATCH 4/6] refactor(delete): reduce complexity in poly delete --- components/polylith/commands/delete.py | 249 +++++++++++++++++-------- 1 file changed, 171 insertions(+), 78 deletions(-) diff --git a/components/polylith/commands/delete.py b/components/polylith/commands/delete.py index da6d8c09..917e2b98 100644 --- a/components/polylith/commands/delete.py +++ b/components/polylith/commands/delete.py @@ -1,9 +1,30 @@ import shutil +from dataclasses import dataclass from pathlib import Path -from typing import List, Set, Tuple +from typing import Iterable, List, Set, Tuple import tomlkit -from polylith import configuration, deps, info, project, repo, toml +from polylith import configuration, deps, info, repo, toml + + +@dataclass(frozen=True) +class DeleteRequest: + brick_type: str + name: str + dry_run: bool + force: bool + + +@dataclass(frozen=True) +class BrickRef: + include: str + dir_name: str + + +@dataclass(frozen=True) +class BrickUsage: + used_by_bricks: List[str] + used_by_projects: List[str] def _brick_dir(theme: str, namespace: str, name: str, brick_dir_name: str) -> Path: @@ -34,30 +55,6 @@ def _collect_bricks_using_brick(import_data: dict, brick: str) -> List[str]: return sorted(used_by.difference({brick})) -def _remove_brick_from_project_pyproject( - root: Path, project_path: Path, namespace: str, brick_dir_name: str, brick: str -) -> bool: - fullpath = project_path / repo.default_toml - - if not fullpath.exists(): - return False - - data = project.get_toml(fullpath) - - brick_include = f"{namespace}/{brick}" - changed = toml.remove_brick_from_project_packages( - data, brick_include, brick_dir_name - ) - - if not changed: - return False - - with fullpath.open("w", encoding="utf-8") as f: - f.write(tomlkit.dumps(data)) - - return True - - def _project_paths(root: Path) -> List[Path]: paths = [root] paths.extend([p.parent for p in root.glob(f"projects/*/{repo.default_toml}")]) @@ -79,106 +76,202 @@ def _get_import_data(root: Path, namespace: str) -> dict: return {**brick_imports.get("bases", {}), **brick_imports.get("components", {})} +def _get_brick_dir_name(brick_type: str) -> str: + if brick_type == "component": + return repo.components_dir + if brick_type == "base": + return repo.bases_dir + + raise ValueError(f"Unknown brick type: {brick_type}") + + +def _brick_exists( + brick_type: str, + name: str, + bases: Set[str], + components: Set[str], +) -> bool: + if brick_type == "component": + return name in components + + return name in bases + + +def _missing_brick_message(brick_type: str, name: str) -> str: + if brick_type == "component": + return f"Component not found: {name}" + + return f"Base not found: {name}" + + +def _paths_to_delete( + root: Path, + theme: str, + namespace: str, + brick_dir_name: str, + name: str, +) -> List[Path]: + brick_dir = root / _brick_dir(theme, namespace, name, brick_dir_name) + test_dir = root / _test_dir(theme, namespace, name, brick_dir_name) + + paths = [brick_dir] + + # In loose theme, tests live outside the brick folder. + if theme == "loose" and test_dir != brick_dir: + paths.append(test_dir) + + return paths + + def _validate_can_delete( root: Path, namespace: str, brick_type: str, name: str, -) -> Tuple[List[str], List[str]]: +) -> BrickUsage: import_data = _get_import_data(root, namespace) projects_data = info.get_projects_data(root, namespace) used_by_bricks = _collect_bricks_using_brick(import_data, name) used_by_projects = _collect_projects_using_brick(projects_data, brick_type, name) - return used_by_bricks, used_by_projects + return BrickUsage(used_by_bricks=used_by_bricks, used_by_projects=used_by_projects) -def _delete_paths(paths: List[Path]) -> None: +def _delete_paths(paths: Iterable[Path]) -> None: for p in paths: if p.exists(): shutil.rmtree(p) -def run(root: Path, namespace: str, options: dict) -> bool: +def _project_pyproject_path(project_path: Path) -> Path: + return project_path / repo.default_toml + + +def _would_update_pyproject(project_path: Path, brick_ref: BrickRef) -> bool: + fullpath = _project_pyproject_path(project_path) + + if not fullpath.exists(): + return False + + data = toml.read_toml_document(fullpath) + + return bool( + toml.remove_brick_from_project_packages(data, brick_ref.include, brick_ref.dir_name) + ) + + +def _update_pyproject(project_path: Path, brick_ref: BrickRef) -> bool: + fullpath = _project_pyproject_path(project_path) + + if not fullpath.exists(): + return False + + data = toml.read_toml_document(fullpath) + + changed = toml.remove_brick_from_project_packages( + data, brick_ref.include, brick_ref.dir_name + ) + + if not changed: + return False + + with fullpath.open("w", encoding="utf-8") as f: + f.write(tomlkit.dumps(data)) + + return True + + +def _parse_request(options: dict) -> DeleteRequest: brick_type = options["brick_type"] name = options["name"] dry_run = bool(options.get("dry_run")) force = bool(options.get("force")) - if brick_type not in {"component", "base"}: - raise ValueError(f"Unknown brick type: {brick_type}") + return DeleteRequest(brick_type=brick_type, name=name, dry_run=dry_run, force=force) - brick_dir_name = ( - repo.components_dir if brick_type == "component" else repo.bases_dir - ) - theme = configuration.get_theme_from_config(root) +def _print_usage_block(usage: BrickUsage, name: str) -> None: + if usage.used_by_bricks: + print(f"Used by bricks: {', '.join(usage.used_by_bricks)}") + if usage.used_by_projects: + print(f"Used by projects: {', '.join(usage.used_by_projects)}") - bases, components = _get_all_bricks(root, namespace) + if usage.used_by_bricks or usage.used_by_projects: + print(f"Cannot delete '{name}' without --force") - if brick_type == "component" and name not in components: - print(f"Component not found: {name}") - return False - if brick_type == "base" and name not in bases: - print(f"Base not found: {name}") - return False +def _print_dry_run( + request: DeleteRequest, + paths_to_delete: List[Path], + project_paths: List[Path], + brick_ref: BrickRef, + usage: BrickUsage, +) -> None: + print(f"Dry run: delete {request.brick_type} '{request.name}'") - brick_dir = root / _brick_dir(theme, namespace, name, brick_dir_name) - test_dir = root / _test_dir(theme, namespace, name, brick_dir_name) + _print_usage_block(usage, request.name) - paths_to_delete = [brick_dir] + existing = [p for p in paths_to_delete if p.exists()] + missing = [p for p in paths_to_delete if not p.exists()] - # In loose theme, tests live outside the brick folder. - if theme == "loose" and test_dir != brick_dir: - paths_to_delete.append(test_dir) + for p in existing: + print(f"Would delete: {p}") + for p in missing: + print(f"Skip missing: {p}") - used_by_bricks, used_by_projects = _validate_can_delete( - root, namespace, brick_type, name - ) + pyprojects_to_update = [] + for p in project_paths: + if _would_update_pyproject(p, brick_ref): + pyprojects_to_update.append(_project_pyproject_path(p)) + + for fullpath in pyprojects_to_update: + print(f"Would update: {fullpath}") - if (used_by_bricks or used_by_projects) and not force: - if used_by_bricks: - print(f"Cannot delete '{name}': used by bricks: {', '.join(used_by_bricks)}") - if used_by_projects: - print( - f"Cannot delete '{name}': used by projects: {', '.join(used_by_projects)}" - ) - print("Re-run with --force to delete anyway.") + +def _print_updated_pyprojects(updated_projects: List[Path]) -> None: + for p in updated_projects: + print(f"Updated: {_project_pyproject_path(p)}") + + +def run(root: Path, namespace: str, options: dict) -> bool: + request = _parse_request(options) + brick_dir_name = _get_brick_dir_name(request.brick_type) + + theme = configuration.get_theme_from_config(root) + bases, components = _get_all_bricks(root, namespace) + + if not _brick_exists(request.brick_type, request.name, bases, components): + print(_missing_brick_message(request.brick_type, request.name)) return False - project_paths = _project_paths(root) + paths_to_delete = _paths_to_delete( + root, theme, namespace, brick_dir_name, request.name + ) - if dry_run: - existing = [p for p in paths_to_delete if p.exists()] - missing = [p for p in paths_to_delete if not p.exists()] + usage = _validate_can_delete(root, namespace, request.brick_type, request.name) - for p in existing: - print(f"Would delete: {p}") - for p in missing: - print(f"Missing (skip): {p}") + if not request.force and (usage.used_by_bricks or usage.used_by_projects): + _print_usage_block(usage, request.name) + return False - for p in project_paths: - fullpath = p / repo.default_toml - if fullpath.exists(): - print(f"Would update: {fullpath}") + brick_ref = BrickRef(include=f"{namespace}/{request.name}", dir_name=brick_dir_name) + project_paths = _project_paths(root) + if request.dry_run: + _print_dry_run(request, paths_to_delete, project_paths, brick_ref, usage) return True updated_projects = [] for p in project_paths: - if _remove_brick_from_project_pyproject( - root, p, namespace, brick_dir_name, name - ): + if _update_pyproject(p, brick_ref): updated_projects.append(p) _delete_paths(paths_to_delete) if updated_projects: - for p in updated_projects: - print(f"Updated: {p / repo.default_toml}") + _print_updated_pyprojects(updated_projects) - print(f"Deleted {brick_type}: {name}") + print(f"Deleted {request.brick_type}: {request.name}") return True From b66e47ca2271d250d212ad2dcfb3d9043a8347c3 Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 13:23:15 -0300 Subject: [PATCH 5/6] refactor(delete): simplify delete control flow --- components/polylith/commands/delete.py | 139 ++++++++++++++++--------- 1 file changed, 92 insertions(+), 47 deletions(-) diff --git a/components/polylith/commands/delete.py b/components/polylith/commands/delete.py index 917e2b98..c10c957f 100644 --- a/components/polylith/commands/delete.py +++ b/components/polylith/commands/delete.py @@ -1,7 +1,7 @@ import shutil from dataclasses import dataclass from pathlib import Path -from typing import Iterable, List, Set, Tuple +from typing import Iterable, List, Optional, Set, Tuple import tomlkit from polylith import configuration, deps, info, repo, toml @@ -27,6 +27,18 @@ class BrickUsage: used_by_projects: List[str] +@dataclass(frozen=True) +class DeleteContext: + root: Path + namespace: str + request: DeleteRequest + brick_ref: BrickRef + theme: str + paths_to_delete: List[Path] + project_paths: List[Path] + usage: BrickUsage + + def _brick_dir(theme: str, namespace: str, name: str, brick_dir_name: str) -> Path: if theme == "loose": return Path(brick_dir_name, namespace, name) @@ -104,20 +116,15 @@ def _missing_brick_message(brick_type: str, name: str) -> str: return f"Base not found: {name}" -def _paths_to_delete( - root: Path, - theme: str, - namespace: str, - brick_dir_name: str, - name: str, -) -> List[Path]: - brick_dir = root / _brick_dir(theme, namespace, name, brick_dir_name) - test_dir = root / _test_dir(theme, namespace, name, brick_dir_name) +def _paths_to_delete(ctx: DeleteContext) -> List[Path]: + request = ctx.request + brick_dir = ctx.root / _brick_dir(ctx.theme, ctx.namespace, request.name, ctx.brick_ref.dir_name) + test_dir = ctx.root / _test_dir(ctx.theme, ctx.namespace, request.name, ctx.brick_ref.dir_name) paths = [brick_dir] # In loose theme, tests live outside the brick folder. - if theme == "loose" and test_dir != brick_dir: + if ctx.theme == "loose" and test_dir != brick_dir: paths.append(test_dir) return paths @@ -138,6 +145,10 @@ def _validate_can_delete( return BrickUsage(used_by_bricks=used_by_bricks, used_by_projects=used_by_projects) +def _has_dependents(usage: BrickUsage) -> bool: + return bool(usage.used_by_bricks or usage.used_by_projects) + + def _delete_paths(paths: Iterable[Path]) -> None: for p in paths: if p.exists(): @@ -157,7 +168,9 @@ def _would_update_pyproject(project_path: Path, brick_ref: BrickRef) -> bool: data = toml.read_toml_document(fullpath) return bool( - toml.remove_brick_from_project_packages(data, brick_ref.include, brick_ref.dir_name) + toml.remove_brick_from_project_packages( + data, brick_ref.include, brick_ref.dir_name + ) ) @@ -191,29 +204,28 @@ def _parse_request(options: dict) -> DeleteRequest: return DeleteRequest(brick_type=brick_type, name=name, dry_run=dry_run, force=force) -def _print_usage_block(usage: BrickUsage, name: str) -> None: +def _print_usage_block(ctx: DeleteContext) -> None: + usage = ctx.usage + name = ctx.request.name + if usage.used_by_bricks: print(f"Used by bricks: {', '.join(usage.used_by_bricks)}") if usage.used_by_projects: print(f"Used by projects: {', '.join(usage.used_by_projects)}") - if usage.used_by_bricks or usage.used_by_projects: + if _has_dependents(usage): print(f"Cannot delete '{name}' without --force") -def _print_dry_run( - request: DeleteRequest, - paths_to_delete: List[Path], - project_paths: List[Path], - brick_ref: BrickRef, - usage: BrickUsage, -) -> None: +def _print_dry_run(ctx: DeleteContext) -> None: + request = ctx.request print(f"Dry run: delete {request.brick_type} '{request.name}'") - _print_usage_block(usage, request.name) + if _has_dependents(ctx.usage): + _print_usage_block(ctx) - existing = [p for p in paths_to_delete if p.exists()] - missing = [p for p in paths_to_delete if not p.exists()] + existing = [p for p in ctx.paths_to_delete if p.exists()] + missing = [p for p in ctx.paths_to_delete if not p.exists()] for p in existing: print(f"Would delete: {p}") @@ -221,8 +233,8 @@ def _print_dry_run( print(f"Skip missing: {p}") pyprojects_to_update = [] - for p in project_paths: - if _would_update_pyproject(p, brick_ref): + for p in ctx.project_paths: + if _would_update_pyproject(p, ctx.brick_ref): pyprojects_to_update.append(_project_pyproject_path(p)) for fullpath in pyprojects_to_update: @@ -234,44 +246,77 @@ def _print_updated_pyprojects(updated_projects: List[Path]) -> None: print(f"Updated: {_project_pyproject_path(p)}") -def run(root: Path, namespace: str, options: dict) -> bool: - request = _parse_request(options) +def _create_context( + root: Path, namespace: str, request: DeleteRequest +) -> Optional[DeleteContext]: brick_dir_name = _get_brick_dir_name(request.brick_type) - theme = configuration.get_theme_from_config(root) - bases, components = _get_all_bricks(root, namespace) + bases, components = _get_all_bricks(root, namespace) if not _brick_exists(request.brick_type, request.name, bases, components): print(_missing_brick_message(request.brick_type, request.name)) - return False - - paths_to_delete = _paths_to_delete( - root, theme, namespace, brick_dir_name, request.name - ) + return None + brick_ref = BrickRef(include=f"{namespace}/{request.name}", dir_name=brick_dir_name) usage = _validate_can_delete(root, namespace, request.brick_type, request.name) + project_paths = _project_paths(root) - if not request.force and (usage.used_by_bricks or usage.used_by_projects): - _print_usage_block(usage, request.name) - return False + # Build once so dry-run and apply are consistent. + seed = DeleteContext( + root=root, + namespace=namespace, + request=request, + brick_ref=brick_ref, + theme=theme, + paths_to_delete=[], + project_paths=project_paths, + usage=usage, + ) - brick_ref = BrickRef(include=f"{namespace}/{request.name}", dir_name=brick_dir_name) - project_paths = _project_paths(root) + paths_to_delete = _paths_to_delete(seed) + + return DeleteContext( + root=root, + namespace=namespace, + request=request, + brick_ref=brick_ref, + theme=theme, + paths_to_delete=paths_to_delete, + project_paths=project_paths, + usage=usage, + ) - if request.dry_run: - _print_dry_run(request, paths_to_delete, project_paths, brick_ref, usage) - return True +def _apply_delete(ctx: DeleteContext) -> None: updated_projects = [] - for p in project_paths: - if _update_pyproject(p, brick_ref): + for p in ctx.project_paths: + if _update_pyproject(p, ctx.brick_ref): updated_projects.append(p) - _delete_paths(paths_to_delete) + _delete_paths(ctx.paths_to_delete) if updated_projects: _print_updated_pyprojects(updated_projects) - print(f"Deleted {request.brick_type}: {request.name}") + print(f"Deleted {ctx.request.brick_type}: {ctx.request.name}") + + +def run(root: Path, namespace: str, options: dict) -> bool: + request = _parse_request(options) + ctx = _create_context(root, namespace, request) + + if ctx is None: + return False + + if not request.force: + if _has_dependents(ctx.usage): + _print_usage_block(ctx) + return False + + if request.dry_run: + _print_dry_run(ctx) + return True + + _apply_delete(ctx) return True From ae3d68e4f8f0dee409fe2692bbdfb984934a4925 Mon Sep 17 00:00:00 2001 From: mnsosa Date: Wed, 17 Dec 2025 13:28:43 -0300 Subject: [PATCH 6/6] refactor(delete): reduce dry-run print complexity --- components/polylith/commands/delete.py | 45 ++++++++++++++++++-------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/components/polylith/commands/delete.py b/components/polylith/commands/delete.py index c10c957f..321c5c3f 100644 --- a/components/polylith/commands/delete.py +++ b/components/polylith/commands/delete.py @@ -217,6 +217,34 @@ def _print_usage_block(ctx: DeleteContext) -> None: print(f"Cannot delete '{name}' without --force") +def _partition_existing_paths(paths: Iterable[Path]) -> Tuple[List[Path], List[Path]]: + existing: List[Path] = [] + missing: List[Path] = [] + + for p in paths: + if p.exists(): + existing.append(p) + else: + missing.append(p) + + return existing, missing + + +def _collect_pyprojects_to_update(ctx: DeleteContext) -> List[Path]: + pyprojects: List[Path] = [] + + for p in ctx.project_paths: + if _would_update_pyproject(p, ctx.brick_ref): + pyprojects.append(_project_pyproject_path(p)) + + return pyprojects + + +def _print_prefixed_paths(prefix: str, paths: Iterable[Path]) -> None: + for p in paths: + print(f"{prefix}: {p}") + + def _print_dry_run(ctx: DeleteContext) -> None: request = ctx.request print(f"Dry run: delete {request.brick_type} '{request.name}'") @@ -224,21 +252,12 @@ def _print_dry_run(ctx: DeleteContext) -> None: if _has_dependents(ctx.usage): _print_usage_block(ctx) - existing = [p for p in ctx.paths_to_delete if p.exists()] - missing = [p for p in ctx.paths_to_delete if not p.exists()] - - for p in existing: - print(f"Would delete: {p}") - for p in missing: - print(f"Skip missing: {p}") + existing, missing = _partition_existing_paths(ctx.paths_to_delete) - pyprojects_to_update = [] - for p in ctx.project_paths: - if _would_update_pyproject(p, ctx.brick_ref): - pyprojects_to_update.append(_project_pyproject_path(p)) + _print_prefixed_paths("Would delete", existing) + _print_prefixed_paths("Skip missing", missing) - for fullpath in pyprojects_to_update: - print(f"Would update: {fullpath}") + _print_prefixed_paths("Would update", _collect_pyprojects_to_update(ctx)) def _print_updated_pyprojects(updated_projects: List[Path]) -> None: