Merge pull request systemd#3131 from NekkoDroid/no-more-split-uki

Allow more granular control on which artifacts are output

mkosi/__init__.py

@@ -50,6 +50,7 @@
 from mkosi.config import (
     PACKAGE_GLOBS,
     Args,
+    ArtifactOutput,
     Bootloader,
     Cacheonly,
     Compression,
@@ -2100,8 +2101,11 @@ def make_uki(
         output,
     )
 
-    extract_pe_section(context, output, ".linux", context.staging / context.config.output_split_kernel)
-    extract_pe_section(context, output, ".initrd", context.staging / context.config.output_split_initrd)
+    if ArtifactOutput.kernel in context.config.split_artifacts:
+        extract_pe_section(context, output, ".linux", context.staging / context.config.output_split_kernel)
+
+    if ArtifactOutput.initrd in context.config.split_artifacts:
+        extract_pe_section(context, output, ".initrd", context.staging / context.config.output_split_initrd)
 
 
 def compressor_command(context: Context, compression: Compression) -> list[PathString]:
@@ -2144,12 +2148,17 @@ def maybe_compress(
                 run(cmd, stdin=i, stdout=o, sandbox=context.sandbox(binary=cmd[0]))
 
 
-def copy_uki(context: Context) -> None:
-    if (context.staging / context.config.output_split_uki).exists():
-        return
+def copy_nspawn_settings(context: Context) -> None:
+    if context.config.nspawn_settings is None:
+        return None
+
+    with complete_step("Copying nspawn settings file…"):
+        shutil.copy2(context.config.nspawn_settings, context.staging / context.config.output_nspawn_settings)
+
 
+def get_uki_path(context: Context) -> Optional[Path]:
     if not want_efi(context.config) or context.config.unified_kernel_images == ConfigFeature.disabled:
-        return
+        return None
 
     ukis = sorted(
         (context.root / "boot/EFI/Linux").glob("*.efi"),
@@ -2168,42 +2177,55 @@ def copy_uki(context: Context) -> None:
     elif ukis:
         uki = ukis[0]
     else:
-        return
+        return None
 
-    shutil.copy(uki, context.staging / context.config.output_split_uki)
+    return uki
 
-    # Extract the combined initrds from the UKI so we can use it to direct kernel boot with qemu if needed.
-    extract_pe_section(context, uki, ".initrd", context.staging / context.config.output_split_initrd)
 
-    # ukify will have signed the kernel image as well. Let's make sure we put the signed kernel
-    # image in the output directory instead of the unsigned one by reading it from the UKI.
-    extract_pe_section(context, uki, ".linux", context.staging / context.config.output_split_kernel)
+def copy_uki(context: Context) -> None:
+    if ArtifactOutput.uki not in context.config.split_artifacts:
+        return
+
+    if (context.staging / context.config.output_split_uki).exists():
+        return
+
+    if uki := get_uki_path(context):
+        shutil.copy(uki, context.staging / context.config.output_split_uki)
 
 
 def copy_vmlinuz(context: Context) -> None:
+    if ArtifactOutput.kernel not in context.config.split_artifacts:
+        return
+
     if (context.staging / context.config.output_split_kernel).exists():
         return
 
+    # ukify will have signed the kernel image as well. Let's make sure we put the signed kernel
+    # image in the output directory instead of the unsigned one by reading it from the UKI.
+    if uki := get_uki_path(context):
+        extract_pe_section(context, uki, ".linux", context.staging / context.config.output_split_kernel)
+        return
+
     for _, kimg in gen_kernel_images(context):
         shutil.copy(context.root / kimg, context.staging / context.config.output_split_kernel)
         break
 
 
-def copy_nspawn_settings(context: Context) -> None:
-    if context.config.nspawn_settings is None:
-        return None
-
-    with complete_step("Copying nspawn settings file…"):
-        shutil.copy2(context.config.nspawn_settings, context.staging / context.config.output_nspawn_settings)
-
-
 def copy_initrd(context: Context) -> None:
+    if ArtifactOutput.initrd not in context.config.split_artifacts:
+        return
+
     if not want_initrd(context):
         return
 
     if (context.staging / context.config.output_split_initrd).exists():
         return
 
+    # Extract the combined initrds from the UKI so we can use it to direct kernel boot with qemu if needed.
+    if uki := get_uki_path(context):
+        extract_pe_section(context, uki, ".initrd", context.staging / context.config.output_split_initrd)
+        return
+
     for kver, _ in gen_kernel_images(context):
         initrds = finalize_initrds(context)
 
@@ -3370,7 +3392,7 @@ def make_extension_image(context: Context, output: Path) -> None:
         ]  # fmt: skip
     if context.config.sector_size:
         cmdline += ["--sector-size", str(context.config.sector_size)]
-    if context.config.split_artifacts:
+    if ArtifactOutput.partitions in context.config.split_artifacts:
         cmdline += ["--split=yes"]
 
     with complete_step(f"Building {context.config.output_format} extension image"):
@@ -3392,7 +3414,7 @@ def make_extension_image(context: Context, output: Path) -> None:
 
     logging.debug(json.dumps(j, indent=4))
 
-    if context.config.split_artifacts:
+    if ArtifactOutput.partitions in context.config.split_artifacts:
         for p in (Partition.from_dict(d) for d in j):
             if p.split_path:
                 maybe_compress(context, context.config.compress_output, p.split_path)
@@ -3637,7 +3659,7 @@ def build_image(context: Context) -> None:
     partitions = make_disk(context, msg="Formatting ESP/XBOOTLDR partitions")
     grub_bios_setup(context, partitions)
 
-    if context.config.split_artifacts:
+    if ArtifactOutput.partitions in context.config.split_artifacts:
         make_disk(context, split=True, msg="Extracting partitions")
 
     copy_nspawn_settings(context)

mkosi/config.py

@@ -499,7 +499,31 @@ def native(cls) -> "Architecture":
         return cls.from_uname(platform.machine())
 
 
-def parse_boolean(s: str) -> bool:
+class ArtifactOutput(StrEnum):
+    uki = enum.auto()
+    kernel = enum.auto()
+    initrd = enum.auto()
+    partitions = enum.auto()
+
+    @staticmethod
+    def compat_no() -> list["ArtifactOutput"]:
+        return [
+            ArtifactOutput.uki,
+            ArtifactOutput.kernel,
+            ArtifactOutput.initrd,
+        ]
+
+    @staticmethod
+    def compat_yes() -> list["ArtifactOutput"]:
+        return [
+            ArtifactOutput.uki,
+            ArtifactOutput.kernel,
+            ArtifactOutput.initrd,
+            ArtifactOutput.partitions,
+        ]
+
+
+def try_parse_boolean(s: str) -> Optional[bool]:
     "Parse 1/true/yes/y/t/on as true and 0/false/no/n/f/off/None as false"
 
     s_l = s.lower()
@@ -509,7 +533,16 @@ def parse_boolean(s: str) -> bool:
     if s_l in {"0", "false", "no", "n", "f", "off", "never"}:
         return False
 
-    die(f"Invalid boolean literal: {s!r}")
+    return None
+
+
+def parse_boolean(s: str) -> bool:
+    value = try_parse_boolean(s)
+
+    if value is None:
+        die(f"Invalid boolean literal: {s!r}")
+
+    return value
 
 
 def parse_path(
@@ -1291,6 +1324,21 @@ def config_parse_key_source(value: Optional[str], old: Optional[KeySource]) -> O
     return KeySource(type=type, source=source)
 
 
+def config_parse_artifact_output_list(
+    value: Optional[str], old: Optional[list[ArtifactOutput]]
+) -> Optional[list[ArtifactOutput]]:
+    if not value:
+        return None
+
+    # Keep for backwards compatibility
+    boolean_value = try_parse_boolean(value)
+    if boolean_value is not None:
+        return ArtifactOutput.compat_yes() if boolean_value else ArtifactOutput.compat_no()
+
+    list_value = config_make_list_parser(delimiter=",", parse=make_enum_parser(ArtifactOutput))(value, old)
+    return cast(list[ArtifactOutput], list_value)
+
+
 class SettingScope(StrEnum):
     # Not passed down to subimages
     local = enum.auto()
@@ -1616,7 +1664,7 @@ class Config:
     output_mode: Optional[int]
     image_id: Optional[str]
     image_version: Optional[str]
-    split_artifacts: bool
+    split_artifacts: list[ArtifactOutput]
     repart_dirs: list[Path]
     sysupdate_dir: Optional[Path]
     sector_size: Optional[int]
@@ -2312,11 +2360,11 @@ def parse_ini(path: Path, only_sections: Collection[str] = ()) -> Iterator[tuple
     ),
     ConfigSetting(
         dest="split_artifacts",
-        metavar="BOOL",
         nargs="?",
         section="Output",
-        parse=config_parse_boolean,
-        help="Generate split partitions",
+        parse=config_parse_artifact_output_list,
+        default=ArtifactOutput.compat_no(),
+        help="Split artifacts out of the final image",
     ),
     ConfigSetting(
         dest="repart_dirs",
@@ -4528,7 +4576,7 @@ def summary(config: Config) -> str:
                         Output Mode: {format_octal_or_default(config.output_mode)}
                            Image ID: {config.image_id}
                       Image Version: {config.image_version}
-                    Split Artifacts: {yes_no(config.split_artifacts)}
+                    Split Artifacts: {line_join_list(config.split_artifacts)}
                  Repart Directories: {line_join_list(config.repart_dirs)}
                         Sector Size: {none_to_default(config.sector_size)}
                             Overlay: {yes_no(config.overlay)}
@@ -4844,6 +4892,7 @@ def uki_profile_transformer(
         Vmm: enum_transformer,
         list[PEAddon]: pe_addon_transformer,
         list[UKIProfile]: uki_profile_transformer,
+        list[ArtifactOutput]: enum_list_transformer,
     }
 
     def json_transformer(key: str, val: Any) -> Any:

mkosi/resources/man/mkosi.1.md

@@ -596,13 +596,20 @@ boolean argument: either `1`, `yes`, or `true` to enable, or `0`, `no`,
     invoked. The image ID is automatically added to `/usr/lib/os-release`.
 
 `SplitArtifacts=`, `--split-artifacts`
-:   If specified and building a disk image, pass `--split=yes` to systemd-repart
-    to have it write out split partition files for each configured partition.
-    Read the [man](https://www.freedesktop.org/software/systemd/man/systemd-repart.html#--split=BOOL)
+:   The artifact types to split out of the final image. A comma-delimited
+    list consisting of `uki`, `kernel`, `initrd` and `partitions`. When
+    building a bootable image `kernel` and `initrd` correspond to their
+    artifact found in the image (or in the UKI), while `uki` copies out the
+    entire UKI.
+
+    When building a disk image and `partitions` is specified,
+    pass `--split=yes` to systemd-repart to have it write out split partition
+    files for each configured partition. Read the
+    [man](https://www.freedesktop.org/software/systemd/man/systemd-repart.html#--split=BOOL)
     page for more information. This is useful in A/B update scenarios where
     an existing disk image shall be augmented with a new version of a
     root or `/usr` partition along with its Verity partition and unified
-    kernel.
+    kernel. By default `uki`, `kernel` and `initrd` are split out.
 
 `RepartDirectories=`, `--repart-dir=`
 :   Paths to directories containing systemd-repart partition definition
@@ -2229,7 +2236,7 @@ current working directory. The following scripts are supported:
 * If **`mkosi.clean`** (`CleanScripts=`) exists, it is executed right
   after the outputs of a previous build have been cleaned up. A clean
   script can clean up any outputs that mkosi does not know about (e.g.
-  artifacts from `SplitArtifacts=yes` or RPMs built in a build script).
+  artifacts from `SplitArtifacts=partitions` or RPMs built in a build script).
   Note that this script does not use the tools tree even if one is configured.
 
 * If **`mkosi.version`** exists and is executable, it is run during

mkosi/sysupdate.py

@@ -4,15 +4,15 @@
 import sys
 from pathlib import Path
 
-from mkosi.config import Args, Config
+from mkosi.config import Args, ArtifactOutput, Config
 from mkosi.log import die
 from mkosi.run import run
 from mkosi.types import PathString
 
 
 def run_sysupdate(args: Args, config: Config) -> None:
-    if not config.split_artifacts:
-        die("SplitArtifacts= must be enabled to be able to use mkosi sysupdate")
+    if ArtifactOutput.partitions not in config.split_artifacts:
+        die("SplitArtifacts=partitions must be set to be able to use mkosi sysupdate")
 
     if not config.sysupdate_dir:
         die(

tests/test_config.py

@@ -12,6 +12,7 @@
 from mkosi import expand_kernel_specifiers
 from mkosi.config import (
     Architecture,
+    ArtifactOutput,
     Compression,
     Config,
     ConfigFeature,
@@ -235,19 +236,19 @@ def test_parse_config(tmp_path: Path) -> None:
     with chdir(d):
         _, [config] = parse_config()
         assert config.bootable == ConfigFeature.auto
-        assert config.split_artifacts is False
+        assert config.split_artifacts == ArtifactOutput.compat_no()
 
         # Passing the directory should include both the main config file and the dropin.
         _, [config] = parse_config(["--include", os.fspath(d / "abc")] * 2)
         assert config.bootable == ConfigFeature.enabled
-        assert config.split_artifacts is True
+        assert config.split_artifacts == ArtifactOutput.compat_yes()
         # The same extra config should not be parsed more than once.
         assert config.build_packages == ["abc"]
 
         # Passing the main config file should not include the dropin.
         _, [config] = parse_config(["--include", os.fspath(d / "abc/mkosi.conf")])
         assert config.bootable == ConfigFeature.enabled
-        assert config.split_artifacts is False
+        assert config.split_artifacts == ArtifactOutput.compat_no()
 
     (d / "mkosi.images").mkdir()
 
@@ -1277,3 +1278,54 @@ def test_mkosi_version_executable(tmp_path: Path) -> None:
     with chdir(d):
         _, [config] = parse_config()
         assert config.image_version == "1.2.3"
+
+
+def test_split_artifacts(tmp_path: Path) -> None:
+    d = tmp_path
+
+    (d / "mkosi.conf").write_text(
+        """
+        [Output]
+        SplitArtifacts=uki
+        """
+    )
+
+    with chdir(d):
+        _, [config] = parse_config()
+        assert config.split_artifacts == [ArtifactOutput.uki]
+
+    (d / "mkosi.conf").write_text(
+        """
+        [Output]
+        SplitArtifacts=uki
+        SplitArtifacts=kernel
+        SplitArtifacts=initrd
+        """
+    )
+
+    with chdir(d):
+        _, [config] = parse_config()
+        assert config.split_artifacts == [
+            ArtifactOutput.uki,
+            ArtifactOutput.kernel,
+            ArtifactOutput.initrd,
+        ]
+
+
+def test_split_artifacts_compat(tmp_path: Path) -> None:
+    d = tmp_path
+
+    with chdir(d):
+        _, [config] = parse_config()
+        assert config.split_artifacts == ArtifactOutput.compat_no()
+
+    (d / "mkosi.conf").write_text(
+        """
+        [Output]
+        SplitArtifacts=yes
+        """
+    )
+
+    with chdir(d):
+        _, [config] = parse_config()
+        assert config.split_artifacts == ArtifactOutput.compat_yes()