From a6466ab0a3531d5dbead313744c872f2a2b4823f Mon Sep 17 00:00:00 2001 From: "michael.hudson@canonical.com" Date: Thu, 19 Feb 2026 13:04:18 +1300 Subject: [PATCH] Make generate_grub_config return strings instead of writing files Separate config generation from file I/O by having generate_grub_config() and its helpers return strings. The base class make_bootable() now handles writing grub.cfg. Co-Authored-By: Claude Opus 4.6 --- live-build/isobuilder/boot/amd64.py | 115 +++++++++++++------------- live-build/isobuilder/boot/arm64.py | 20 ++--- live-build/isobuilder/boot/grub.py | 56 ++++++------- live-build/isobuilder/boot/ppc64el.py | 25 +++--- live-build/isobuilder/boot/riscv64.py | 27 +++--- live-build/isobuilder/boot/uefi.py | 10 +-- 6 files changed, 114 insertions(+), 139 deletions(-) diff --git a/live-build/isobuilder/boot/amd64.py b/live-build/isobuilder/boot/amd64.py index 588a8254..11c1ce16 100644 --- a/live-build/isobuilder/boot/amd64.py +++ b/live-build/isobuilder/boot/amd64.py @@ -87,102 +87,95 @@ class AMD64BootConfigurator(UEFIBootConfigurator): ["*.mod", "*.lst", "*.o"], ) - def generate_grub_config(self) -> None: - """Generate grub.cfg and loopback.cfg for the boot tree.""" - boot_grub_dir = self.iso_root.joinpath("boot", "grub") - boot_grub_dir.mkdir(parents=True, exist_ok=True) - - grub_cfg = boot_grub_dir.joinpath("grub.cfg") + def generate_grub_config(self) -> str: + """Generate grub.cfg content for AMD64.""" + result = self.grub_header() if self.project == "ubuntu-mini-iso": - self.write_grub_header(grub_cfg) - with grub_cfg.open("a") as f: - f.write( - """menuentry "Choose an Ubuntu version to install" { + result += """\ +menuentry "Choose an Ubuntu version to install" { set gfxpayload=keep linux /casper/vmlinuz iso-chooser-menu ip=dhcp --- initrd /casper/initrd } """ - ) - return + return result - # Generate grub.cfg kernel_params = default_kernel_params(self.project) - # Write common GRUB header - self.write_grub_header(grub_cfg) - # Main menu entry - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "Try or Install {self.humanproject}" {{ + result += f"""\ +menuentry "Try or Install {self.humanproject}" {{ set gfxpayload=keep linux /casper/vmlinuz {kernel_params} initrd /casper/initrd }} """ - ) # All but server get safe-graphics mode if self.project != "ubuntu-server": - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "{self.humanproject} (safe graphics)" {{ + result += f"""\ +menuentry "{self.humanproject} (safe graphics)" {{ set gfxpayload=keep linux /casper/vmlinuz nomodeset {kernel_params} initrd /casper/initrd }} """ - ) # ubiquity based projects get OEM mode if "maybe-ubiquity" in kernel_params: oem_kernel_params = kernel_params.replace( "maybe-ubiquity", "only-ubiquity oem-config/enable=true" ) - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "OEM install (for manufacturers)" {{ + result += f"""\ +menuentry "OEM install (for manufacturers)" {{ set gfxpayload=keep linux /casper/vmlinuz {oem_kernel_params} initrd /casper/initrd }} """ - ) # Calamares-based projects get OEM mode if self.project in CALAMARES_PROJECTS: - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "OEM install (for manufacturers)" {{ + result += f"""\ +menuentry "OEM install (for manufacturers)" {{ set gfxpayload=keep - linux /casper/vmlinuz {kernel_params} oem-config/enable=true - initrd /casper/initrd + linux /casper/vmlinuz {kernel_params} oem-config/enable=true + initrd /casper/initrd }} """ - ) # Currently only server is built with HWE, hence no safe-graphics/OEM if self.hwe: - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "{self.humanproject} with the HWE kernel" {{ + result += f"""\ +menuentry "{self.humanproject} with the HWE kernel" {{ set gfxpayload=keep - linux /casper/hwe-vmlinuz {kernel_params} - initrd /casper/hwe-initrd + linux /casper/hwe-vmlinuz {kernel_params} + initrd /casper/hwe-initrd }} """ - ) - # Create the loopback config, based on the main config - with grub_cfg.open("r") as f: - content = f.read() + # UEFI Entries (wrapped in grub_platform check for dual BIOS/UEFI support) + uefi_menu_entries = self.uefi_menu_entries() - # sed: delete from line 1 to menu_color_highlight, delete from - # grub_platform to end and replace '---' with - # 'iso-scan/filename=${iso_path} ---' in lines with 'linux' - lines = content.split("\n") + result += f"""\ +grub_platform +if [ "$grub_platform" = "efi" ]; then +{uefi_menu_entries} +fi +""" + + return result + + @staticmethod + def generate_loopback_config(grub_content: str) -> str: + """Derive loopback.cfg from grub.cfg content. + + Strips the header (up to menu_color_highlight) and the UEFI + trailer (from grub_platform to end), and adds iso-scan/filename + to linux lines. + """ + lines = grub_content.split("\n") start_idx = 0 for i, line in enumerate(lines): if "menu_color_highlight" in line: @@ -205,16 +198,20 @@ class AMD64BootConfigurator(UEFIBootConfigurator): for line in loopback_lines ] - loopback_cfg = boot_grub_dir.joinpath("loopback.cfg") - with loopback_cfg.open("w") as f: - f.write("\n".join(loopback_lines)) + return "\n".join(loopback_lines) - # UEFI Entries (wrapped in grub_platform check for dual BIOS/UEFI support) - with grub_cfg.open("a") as f: - f.write("grub_platform\n") - f.write('if [ "$grub_platform" = "efi" ]; then\n') - - self.write_uefi_menu_entries(grub_cfg) - - with grub_cfg.open("a") as f: - f.write("fi\n") + def make_bootable( + self, + workdir: pathlib.Path, + project: str, + capproject: str, + subarch: str, + hwe: bool, + ) -> None: + """Make the ISO bootable, including generating loopback.cfg.""" + super().make_bootable(workdir, project, capproject, subarch, hwe) + grub_cfg = self.iso_root.joinpath("boot", "grub", "grub.cfg") + grub_content = grub_cfg.read_text() + self.iso_root.joinpath("boot", "grub", "loopback.cfg").write_text( + self.generate_loopback_config(grub_content) + ) diff --git a/live-build/isobuilder/boot/arm64.py b/live-build/isobuilder/boot/arm64.py index 53e6ea19..effcaeda 100644 --- a/live-build/isobuilder/boot/arm64.py +++ b/live-build/isobuilder/boot/arm64.py @@ -33,19 +33,15 @@ class ARM64BootConfigurator(UEFIBootConfigurator): with self.logger.logged("extracting ARM64 boot files"): self.extract_uefi_files() - def generate_grub_config(self) -> None: + def generate_grub_config(self) -> str: """Generate grub.cfg for ARM64.""" kernel_params = default_kernel_params(self.project) - grub_cfg = self.iso_root.joinpath("boot", "grub", "grub.cfg") - - # Write common GRUB header - self.write_grub_header(grub_cfg) + result = self.grub_header() # ARM64-specific: Snapdragon workarounds - with grub_cfg.open("a") as f: - f.write( - """set cmdline= + result += f"""\ +set cmdline= smbios --type 4 --get-string 5 --set proc_version regexp "Snapdragon.*" "$proc_version" if [ $? = 0 ]; then @@ -63,11 +59,9 @@ menuentry "Try or Install {self.humanproject}" {{ \tinitrd\t/casper/initrd }} """ - ) # HWE kernel option if available - self.write_hwe_menu_entry( - grub_cfg, + result += self.hwe_menu_entry( "vmlinuz", f"{kernel_params} console=tty0", extra_params="$cmdline ", @@ -77,4 +71,6 @@ menuentry "Try or Install {self.humanproject}" {{ # but it's not actually set anywhere in the grub.cfg, so we omit it here # UEFI Entries (ARM64 is UEFI-only, no grub_platform check needed) - self.write_uefi_menu_entries(grub_cfg) + result += self.uefi_menu_entries() + + return result diff --git a/live-build/isobuilder/boot/grub.py b/live-build/isobuilder/boot/grub.py index d5a55845..878c9c4a 100644 --- a/live-build/isobuilder/boot/grub.py +++ b/live-build/isobuilder/boot/grub.py @@ -39,59 +39,52 @@ class GrubBootConfigurator(BaseBootConfigurator): Subclasses must implement generate_grub_config(). """ - def write_grub_header( - self, grub_cfg: pathlib.Path, include_loadfont: bool = True - ) -> None: - """Write common GRUB config header (timeout, colors). + def grub_header(self, include_loadfont: bool = True) -> str: + """Return common GRUB config header (timeout, colors). Args: - grub_cfg: Path to grub.cfg file include_loadfont: Whether to include 'loadfont unicode' (not needed for RISC-V) """ - with grub_cfg.open("a") as f: - f.write("set timeout=30\n\n") - if include_loadfont: - f.write("loadfont unicode\n\n") - f.write( - """set menu_color_normal=white/black + result = "set timeout=30\n\n" + if include_loadfont: + result += "loadfont unicode\n\n" + result += """\ +set menu_color_normal=white/black set menu_color_highlight=black/light-gray """ - ) + return result - def write_hwe_menu_entry( + def hwe_menu_entry( self, - grub_cfg: pathlib.Path, kernel_name: str, kernel_params: str, extra_params: str = "", - ) -> None: - """Write HWE kernel menu entry if HWE is enabled. + ) -> str: + """Return HWE kernel menu entry if HWE is enabled. Args: - grub_cfg: Path to grub.cfg file kernel_name: Kernel binary name (vmlinuz or vmlinux) kernel_params: Kernel parameters to append extra_params: Additional parameters (e.g., console=tty0, $cmdline) """ - if self.hwe: - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "{self.humanproject} with the HWE kernel" {{ -\tset gfxpayload=keep -\tlinux\t/casper/hwe-{kernel_name} {extra_params}{kernel_params} -\tinitrd\t/casper/hwe-initrd + if not self.hwe: + return "" + return f"""\ +menuentry "{self.humanproject} with the HWE kernel" {{ + set gfxpayload=keep + linux /casper/hwe-{kernel_name} {extra_params}{kernel_params} + initrd /casper/hwe-initrd }} """ - ) @abstractmethod - def generate_grub_config(self) -> None: - """Generate grub.cfg configuration file. + def generate_grub_config(self) -> str: + """Generate grub.cfg content. - Each GRUB-based architecture must implement this to create its - specific GRUB configuration. + Each GRUB-based architecture must implement this to return the + GRUB configuration. """ ... @@ -106,4 +99,7 @@ set menu_color_highlight=black/light-gray """Make the ISO bootable by extracting files and generating GRUB config.""" super().make_bootable(workdir, project, capproject, subarch, hwe) with self.logger.logged("generating grub config"): - self.generate_grub_config() + content = self.generate_grub_config() + grub_dir = self.iso_root.joinpath("boot", "grub") + grub_dir.mkdir(parents=True, exist_ok=True) + grub_dir.joinpath("grub.cfg").write_text(content) diff --git a/live-build/isobuilder/boot/ppc64el.py b/live-build/isobuilder/boot/ppc64el.py index ba3de9f4..94450d00 100644 --- a/live-build/isobuilder/boot/ppc64el.py +++ b/live-build/isobuilder/boot/ppc64el.py @@ -53,27 +53,22 @@ class PPC64ELBootConfigurator(GrubBootConfigurator): grub_pkg_dir, self.iso_root, "powerpc-ieee1275", ["*.mod", "*.lst"] ) - def generate_grub_config(self) -> None: + def generate_grub_config(self) -> str: """Generate grub.cfg for PPC64EL.""" kernel_params = default_kernel_params(self.project) - grub_cfg = self.iso_root.joinpath("boot", "grub", "grub.cfg") - - # Write common GRUB header - self.write_grub_header(grub_cfg) + result = self.grub_header() # Main menu entry - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "Try or Install {self.humanproject}" {{ -\tset gfxpayload=keep -\tlinux\t/casper/vmlinux quiet {kernel_params} -\tinitrd\t/casper/initrd + result += f"""\ +menuentry "Try or Install {self.humanproject}" {{ + set gfxpayload=keep + linux /casper/vmlinux quiet {kernel_params} + initrd /casper/initrd }} """ - ) # HWE kernel option if available - self.write_hwe_menu_entry( - grub_cfg, "vmlinux", kernel_params, extra_params="quiet " - ) + result += self.hwe_menu_entry("vmlinux", kernel_params, extra_params="quiet ") + + return result diff --git a/live-build/isobuilder/boot/riscv64.py b/live-build/isobuilder/boot/riscv64.py index beeaf770..7b41640d 100644 --- a/live-build/isobuilder/boot/riscv64.py +++ b/live-build/isobuilder/boot/riscv64.py @@ -126,35 +126,28 @@ class RISCV64BootConfigurator(GrubBootConfigurator): # Add DTBs to ESP self.logger.run(["mcopy", "-s", "-i", efi_img, dtb_dir, "::/."], check=True) - def generate_grub_config(self) -> None: + def generate_grub_config(self) -> str: """Generate grub.cfg for RISC-V64.""" - grub_dir = self.iso_root.joinpath("boot", "grub") - grub_dir.mkdir(parents=True, exist_ok=True) - - grub_cfg = grub_dir.joinpath("grub.cfg") - - # Write GRUB header (without loadfont for RISC-V) - self.write_grub_header(grub_cfg, include_loadfont=False) + result = self.grub_header(include_loadfont=False) # Main menu entry - with grub_cfg.open("a") as f: - f.write( - f"""menuentry "Try or Install {self.humanproject}" {{ -\tset gfxpayload=keep -\tlinux\t/casper/vmlinux efi=debug sysctl.kernel.watchdog_thresh=60 --- -\tinitrd\t/casper/initrd + result += f"""\ +menuentry "Try or Install {self.humanproject}" {{ + set gfxpayload=keep + linux /casper/vmlinux efi=debug sysctl.kernel.watchdog_thresh=60 --- + initrd /casper/initrd }} """ - ) # HWE kernel option if available - self.write_hwe_menu_entry( - grub_cfg, + result += self.hwe_menu_entry( "vmlinux", "---", extra_params="efi=debug sysctl.kernel.watchdog_thresh=60 ", ) + return result + def post_process_iso(self, iso_path: pathlib.Path) -> None: """Add GPT partitions with U-Boot for SiFive Unmatched board. diff --git a/live-build/isobuilder/boot/uefi.py b/live-build/isobuilder/boot/uefi.py index fae96687..8c321273 100644 --- a/live-build/isobuilder/boot/uefi.py +++ b/live-build/isobuilder/boot/uefi.py @@ -122,18 +122,16 @@ class UEFIBootConfigurator(GrubBootConfigurator): self.logger, self.iso_root, self.scratch.joinpath("cd-boot-efi.img") ) - def write_uefi_menu_entries(self, grub_cfg: pathlib.Path) -> None: - """Write UEFI firmware menu entries.""" - with grub_cfg.open("a") as f: - f.write( - """menuentry 'Boot from next volume' { + def uefi_menu_entries(self) -> str: + """Return UEFI firmware menu entries.""" + return """\ +menuentry 'Boot from next volume' { \texit 1 } menuentry 'UEFI Firmware Settings' { \tfwsetup } """ - ) def get_uefi_mkisofs_opts(self) -> list[str | pathlib.Path]: """Return common UEFI mkisofs options."""