Make sure we don't build the same tools tree more than once

We can do this by simply checking if the output path already exists
instead of relying on needs_build(). This allows us to refactor
needs_build() to needs_clean(). We also move some prechecks into
run_build() and run_clean() so as to not duplicate them and improve
the logging messages in run_clean().

Fixes #2280

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index 6912fb7a8678b4603c568b65e9d5d11bd77abb08..9b4eeb25815ae13fbfb36390375367764a5d0c4d 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -3195,20 +3195,6 @@ def expand_specifier(s: str) -> str:
     return s.replace("%u", INVOKING_USER.name())
 
 
-def needs_build(args: Args, config: Config) -> bool:
-    return (
-        args.verb.needs_build() and
-        (
-            args.force > 0 or
-            not (config.output_dir_or_cwd() / config.output_with_compression).exists() or
-            # When the output is a directory, its name is the same as the symlink we create that points to the actual
-            # output when not building a directory. So if the full output path exists, we have to check that it's not
-            # a symlink as well.
-            (config.output_dir_or_cwd() / config.output_with_compression).is_symlink()
-        )
-    )
-
-
 @contextlib.contextmanager
 def prepend_to_environ_path(config: Config) -> Iterator[None]:
     if config.tools_tree or not config.extra_search_paths:
@@ -3285,7 +3271,21 @@ def check_workspace_directory(config: Config) -> None:
                 hint="Use WorkspaceDirectory= to configure a different workspace directory")
 
 
+def needs_clean(args: Args, config: Config) -> bool:
+    return (
+        args.force > 0 or
+        not (config.output_dir_or_cwd() / config.output_with_compression).exists() or
+        # When the output is a directory, its name is the same as the symlink we create that points to the actual
+        # output when not building a directory. So if the full output path exists, we have to check that it's not
+        # a symlink as well.
+        (config.output_dir_or_cwd() / config.output_with_compression).is_symlink()
+    )
+
+
 def run_clean(args: Args, config: Config) -> None:
+    if not needs_clean(args, config):
+        return
+
     become_root()
 
     # We remove any cached images if either the user used --force twice, or he/she called "clean" with it
@@ -3299,32 +3299,33 @@ def run_clean(args: Args, config: Config) -> None:
         remove_build_cache = args.force > 1
         remove_package_cache = args.force > 2
 
-    with complete_step("Removing output files…"):
-        if config.output_dir_or_cwd().exists():
-            rmtree(*(p for p in config.output_dir_or_cwd().iterdir() if p.name.startswith(config.output)))
+    if (outputs := list(config.output_dir_or_cwd().glob(f"{config.output}*"))):
+        with complete_step(f"Removing output files of {config.name()} image…"):
+            rmtree(*outputs)
 
     if remove_build_cache:
         if config.cache_dir:
-            with complete_step("Removing cache entries…"):
+            with complete_step(f"Removing cache entries of {config.name()} image…"):
                 rmtree(*(p for p in cache_tree_paths(config) if p.exists()))
 
         if config.build_dir and config.build_dir.exists() and any(config.build_dir.iterdir()):
-            with complete_step("Clearing out build directory…"):
+            with complete_step(f"Clearing out build directory of {config.name()} image…"):
                 rmtree(*config.build_dir.iterdir())
 
-    if remove_package_cache:
-        if config.cache_dir and config.cache_dir.exists() and any(config.cache_dir.iterdir()):
-            with complete_step("Clearing out package cache…"):
-                rmtree(
-                    *(
-                        config.cache_dir / p / d
-                        for p in ("cache", "lib")
-                        for d in ("apt", "dnf", "libdnf5", "pacman", "zypp")
-                    ),
-                )
+    if remove_package_cache and config.cache_dir and config.cache_dir.exists() and any(config.cache_dir.iterdir()):
+        with complete_step(f"Clearing out package cache of {config.name()} image…"):
+            rmtree(
+                *(
+                    config.cache_dir / p / d
+                    for p in ("cache", "lib")
+                    for d in ("apt", "dnf", "libdnf5", "pacman", "zypp")
+                ),
+            )
 
 
 def run_build(args: Args, config: Config) -> None:
+    check_inputs(config)
+
     if (uid := os.getuid()) != 0:
         become_root()
     unshare(CLONE_NEWNS)
@@ -3411,7 +3412,7 @@ def run_verb(args: Args, images: Sequence[Config]) -> None:
     # image build could end up deleting the output generated by an earlier image build.
 
     for config in images:
-        if not needs_build(args, config) and args.verb != Verb.clean:
+        if not args.verb.needs_build() and args.verb != Verb.clean:
             continue
 
         if config.tools_tree and config.tools_tree.name == "default":
@@ -3426,6 +3427,9 @@ def run_verb(args: Args, images: Sequence[Config]) -> None:
     build = False
 
     for i, config in enumerate(images):
+        if not args.verb.needs_build():
+            continue
+
         with (
             finalize_default_tools(args, config)
             if config.tools_tree and config.tools_tree.name == "default"
@@ -3437,14 +3441,12 @@ def run_verb(args: Args, images: Sequence[Config]) -> None:
                 tools_tree=tools.output_dir_or_cwd() / tools.output if tools else config.tools_tree,
             )
 
-            if tools and needs_build(args, tools):
-                check_inputs(tools)
+            if tools and not (tools.output_dir_or_cwd() / tools.output_with_compression).exists():
                 fork_and_wait(lambda: run_build(args, tools)) # pyright: ignore
 
-        if not needs_build(args, config):
+        if (config.output_dir_or_cwd() / config.output_with_compression).exists():
             continue
 
-        check_inputs(config)
         fork_and_wait(lambda: run_build(args, config))
 
         build = True