mkosi: verify scripts after parsing config, show status in summary

We would refuse a script if not found or not executable immediately when
parsing the argument. But only the last specified script matters, so it's
better to delay the check: in initial parsing store the path, and later check
that it exists and matches the relevant criteria, at the time when we also
check the outputs.

Similarly for tree inputs: they are checked at this time too.
This fixes #1167.

Instead of trying to generate error messages, just reuse normal Python
exceptions: i.e. os.access() is replaced by open(). This way we don't need to
come up with error messages for various conditions and possibly get them wrong.

Rework status command to show status of inputs and scripts:

$ bin/mkosi --extra-tree={./missing,/root/inaccessible} --{prepare,build,finalize,postinst}-script=/etc/fstab summary
COMMANDS:
                      verb: summary
                   cmdline:
...
CONTENT:
               Extra Trees: /home/zbyszek/src/mkosi/missing (No such file or directory)
                            /root/inaccessible (Permission denied)
            Skeleton Trees: none
...
              Build Script: /etc/fstab (Not executable)
...
        Postinstall Script: /etc/fstab (Not executable)
            Prepare Script: /etc/fstab (Not executable)
           Finalize Script: /etc/fstab (Not executable)
...

Colors are used to draw the eye to the problematic lines.

diff --git a/mkosi/__init__.py b/mkosi/__init__.py
index d6da25a0e46322148438d74050068d1bdd33d578..b52268111a2e1c7da925cb9598e4f6177d65b099 100644 (file)
--- a/mkosi/__init__.py
+++ b/mkosi/__init__.py
@@ -105,6 +105,8 @@ from .manifest import Manifest
 from .syscall import blkpg_add_partition, blkpg_del_partition, reflink
 
 complete_step = MkosiPrinter.complete_step
+color_error = MkosiPrinter.color_error
+
 
 __version__ = "13"
 
@@ -6404,14 +6406,7 @@ def script_path(value: Optional[str]) -> Optional[Path]:
 def normalize_script(path: Optional[Path]) -> Optional[Path]:
     if not path or path is DISABLED:
         return None
-    path = Path(path).absolute()
-    if not path.exists():
-        die(f"{path} does not exist")
-    if not path.is_file():
-        die(f"{path} is not a file")
-    if not os.access(path, os.X_OK):
-        die(f"{path} is not executable")
-    return path
+    return Path(path).absolute()
 
 
 def load_args(args: argparse.Namespace) -> MkosiConfig:
@@ -6796,7 +6791,46 @@ def cache_image_path(config: MkosiConfig, is_final_image: bool) -> Optional[Path
         return Path(f"{config.output}.{suffix}")
 
 
-def check_output(config: MkosiConfig) -> None:
+def check_tree_input(path: Optional[Path]) -> None:
+    # Each path may be a directory or a tarball.
+    # Open the file or directory to simulate an access check.
+    # If that fails, an exception will be thrown.
+    if not path:
+        return
+
+    os.open(path, os.R_OK)
+
+
+def check_script_input(path: Optional[Path]) -> None:
+    if not path:
+        return
+
+    os.open(path, os.R_OK)
+    if not path.is_file():
+        raise OSError(errno.ENOENT, 'Not a normal file')
+    if not os.access(path, os.X_OK):
+        raise OSError(errno.ENOENT, 'Not executable')
+    return None
+
+
+def check_inputs(config: MkosiConfig) -> None:
+    try:
+        for tree in (config.skeleton_trees,
+                     config.extra_trees):
+            for item in tree:
+                check_tree_input(item)
+
+        for path in (config.build_script,
+                     config.prepare_script,
+                     config.postinst_script,
+                     config.finalize_script,
+                     config.base_image):
+            check_script_input(path)
+    except OSError as e:
+        die(f'{e.filename} {e.strerror}')
+
+
+def check_outputs(config: MkosiConfig) -> None:
     if config.skip_final_phase:
         return
 
@@ -6813,7 +6847,6 @@ def check_output(config: MkosiConfig) -> None:
         config.output_split_verity_sig if config.split_artifacts else None,
         config.output_split_kernel if config.split_artifacts else None,
     ):
-
         if f and f.exists():
             die(f"Output path {f} exists already. (Consider invocation with --force.)")
 
@@ -6843,19 +6876,34 @@ def format_bytes_or_auto(sz: int) -> str:
 def none_to_na(s: Optional[T]) -> Union[T, str]:
     return "n/a" if s is None else s
 
-
 def none_to_no(s: Optional[T]) -> Union[T, str]:
     return "no" if s is None else s
 
+def none_to_none(s: Optional[T]) -> Union[T, str]:
+    return "none" if s is None else s
 
-def none_to_none(o: Optional[object]) -> str:
-    return "none" if o is None else str(o)
 
+def path_or_none(
+        path: Optional[Path],
+        checker: Optional[Callable[[Optional[Path]], None]] = None,
+) -> Union[Optional[Path], str]:
+    try:
+        if checker:
+            checker(path)
+    except OSError as e:
+        return f'{color_error(path)} ({e.strerror})'
+    else:
+        return path
 
-def line_join_list(array: Sequence[PathString]) -> str:
+def line_join_list(
+        array: Sequence[PathString],
+        checker: Optional[Callable[[Optional[Path]], None]] = None,
+) -> str:
     if not array:
         return "none"
-    return "\n                            ".join(str(item) for item in array)
+
+    items = (str(path_or_none(cast(Path, item), checker=checker)) for item in array)
+    return "\n                            ".join(items)
 
 
 def print_summary(config: MkosiConfig) -> None:
@@ -6969,8 +7017,8 @@ def print_summary(config: MkosiConfig) -> None:
         print("        With Documentation:", yes_no(config.with_docs))
 
     print("             Package Cache:", none_to_none(config.cache_path))
-    print("               Extra Trees:", line_join_list(config.extra_trees))
-    print("            Skeleton Trees:", line_join_list(config.skeleton_trees))
+    print("               Extra Trees:", line_join_list(config.extra_trees, check_tree_input))
+    print("            Skeleton Trees:", line_join_list(config.skeleton_trees, check_tree_input))
     print("      CleanPackageMetadata:", yes_no_or(config.clean_package_metadata))
 
     if config.remove_files:
@@ -6978,7 +7026,7 @@ def print_summary(config: MkosiConfig) -> None:
     if config.remove_packages:
         print("           Remove Packages:", line_join_list(config.remove_packages))
 
-    print("              Build Script:", none_to_none(config.build_script))
+    print("              Build Script:", path_or_none(config.build_script, check_script_input))
 
     env = [f"{k}={v}" for k, v in config.environment.items()]
     print("        Script Environment:", line_join_list(env))
@@ -6997,9 +7045,9 @@ def print_summary(config: MkosiConfig) -> None:
     print("         Install Directory:", none_to_none(config.install_dir))
     print("            Build Packages:", line_join_list(config.build_packages))
     print("          Skip final phase:", yes_no(config.skip_final_phase))
-    print("        Postinstall Script:", none_to_none(config.postinst_script))
-    print("            Prepare Script:", none_to_none(config.prepare_script))
-    print("           Finalize Script:", none_to_none(config.finalize_script))
+    print("        Postinstall Script:", path_or_none(config.postinst_script, check_script_input))
+    print("            Prepare Script:", path_or_none(config.prepare_script, check_script_input))
+    print("           Finalize Script:", path_or_none(config.finalize_script, check_script_input))
     print("      Scripts with network:", yes_no_or(config.with_network))
     print("           nspawn Settings:", none_to_none(config.nspawn_settings))
 
@@ -8247,8 +8295,11 @@ def run_verb(raw: argparse.Namespace) -> None:
         if config.verb in MKOSI_COMMANDS_SUDO:
             check_root()
 
-        if config.verb == Verb.build and not config.force:
-            check_output(config)
+        if config.verb == Verb.build:
+            check_inputs(config)
+
+            if not config.force:
+                check_outputs(config)
 
         if needs_build(config) or config.verb == Verb.clean:
             check_root()

diff --git a/mkosi/backend.py b/mkosi/backend.py
index 25c841057897f31f565d10008ee1bccdafe82b7d..4950a2c1109fe64a2db58e9a45af744e73ec5db0 100644 (file)
--- a/mkosi/backend.py
+++ b/mkosi/backend.py
@@ -914,6 +914,10 @@ class MkosiPrinter:
     def _print(cls, text: str) -> None:
         cls.out_file.write(text)
 
+    @classmethod
+    def color_error(cls, text: Any) -> str:
+        return f"{cls.red}{text}{cls.reset}"
+
     @classmethod
     def print_step(cls, text: str) -> None:
         prefix = cls.prefix + " " * cls.level
@@ -932,7 +936,7 @@ class MkosiPrinter:
 
     @classmethod
     def warn(cls, text: str) -> None:
-        cls._print(f"{cls.prefix}{cls.red}{text}{cls.reset}\n")
+        cls._print(f"{cls.prefix}{cls.color_error(text)}\n")
 
     @classmethod
     @contextlib.contextmanager

diff --git a/mkosi/machine.py b/mkosi/machine.py
index 51171d03c3870337d84fe8a326c12ca79b5c89a3..57b37513d2bfeef3cea87cbae4c301273035f540 100644 (file)
--- a/mkosi/machine.py
+++ b/mkosi/machine.py
@@ -18,8 +18,9 @@ from . import (
     MKOSI_COMMANDS_NEED_BUILD,
     CompletedProcess,
     build_stuff,
+    check_inputs,
     check_native,
-    check_output,
+    check_outputs,
     check_root,
     init_namespace,
     load_args,
@@ -113,7 +114,8 @@ class Machine:
             unlink_output(self.config)
 
         if self.config.verb == Verb.build:
-            check_output(self.config)
+            check_inputs(self.config)
+            check_outputs(self.config)
 
         if needs_build(self.config):
             check_root()