From: Zbigniew Jędrzejewski-Szmek Date: Thu, 20 Apr 2023 08:27:05 +0000 (+0200) Subject: Reformat run commands to have "--option", "value" pairs on the same line X-Git-Tag: v15~226^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=79c0b441d8f8c255de5087a0de0ddfa705450416;p=thirdparty%2Fmkosi.git Reformat run commands to have "--option", "value" pairs on the same line We were already doing this for the majority of invocations, but there were some exceptions. Since this makes it much easier to mentally split the command into logical parts, let's do this everywhere. To make the command stand out a bit, add one space of seperation between "[" and the first argument and between the last argument and "]". --- diff --git a/mkosi/__init__.py b/mkosi/__init__.py index 7c040bd96..1b45f2186 100644 --- a/mkosi/__init__.py +++ b/mkosi/__init__.py @@ -483,18 +483,11 @@ def install_boot_loader(state: MkosiState) -> None: with complete_step("Signing systemd-boot binaries…"): for f in itertools.chain(directory.glob('*.efi'), directory.glob('*.EFI')): - run( - [ - "sbsign", - "--key", - state.config.secure_boot_key, - "--cert", - state.config.secure_boot_certificate, - "--output", - f"{f}.signed", - f, - ], - ) + run(["sbsign", + "--key", state.config.secure_boot_key, + "--cert", state.config.secure_boot_certificate, + "--output", f"{f}.signed", + f]) with complete_step("Installing boot loader…"): run(["bootctl", "install", "--root", state.root], env={"SYSTEMD_ESP_PATH": "/boot"}) @@ -508,48 +501,27 @@ def install_boot_loader(state: MkosiState) -> None: keys.mkdir(parents=True, exist_ok=True) # sbsiglist expects a DER certificate. - run( - [ - "openssl", - "x509", - "-outform", - "DER", - "-in", - state.config.secure_boot_certificate, - "-out", - state.workspace / "mkosi.der", - ], - ) - run( - [ - "sbsiglist", - "--owner", - str(uuid.uuid4()), - "--type", - "x509", - "--output", - state.workspace / "mkosi.esl", - state.workspace / "mkosi.der", - ], - ) + run(["openssl", + "x509", + "-outform", "DER", + "-in", state.config.secure_boot_certificate, + "-out", state.workspace / "mkosi.der"]) + run(["sbsiglist", + "--owner", str(uuid.uuid4()), + "--type", "x509", + "--output", state.workspace / "mkosi.esl", + state.workspace / "mkosi.der"]) # We reuse the key for all secure boot databases to keep things simple. for db in ["PK", "KEK", "db"]: - run( - [ - "sbvarsign", - "--attr", - "NON_VOLATILE,BOOTSERVICE_ACCESS,RUNTIME_ACCESS,TIME_BASED_AUTHENTICATED_WRITE_ACCESS", - "--key", - state.config.secure_boot_key, - "--cert", - state.config.secure_boot_certificate, - "--output", - keys / f"{db}.auth", - db, - state.workspace / "mkosi.esl", - ], - ) + run(["sbvarsign", + "--attr", + "NON_VOLATILE,BOOTSERVICE_ACCESS,RUNTIME_ACCESS,TIME_BASED_AUTHENTICATED_WRITE_ACCESS", + "--key", state.config.secure_boot_key, + "--cert", state.config.secure_boot_certificate, + "--output", keys / f"{db}.auth", + db, + state.workspace / "mkosi.esl"]) def install_skeleton_trees(state: MkosiState, cached: bool) -> None: @@ -794,7 +766,7 @@ def install_unified_kernel(state: MkosiState, roothash: Optional[str]) -> None: if state.config.sign_expected_pcr: cmd += [ "--pcr-private-key", state.config.secure_boot_key, - "--pcr-banks", "sha1,sha256" + "--pcr-banks", "sha1,sha256", ] if state.config.initrds: @@ -886,18 +858,16 @@ def acl_toggle_remove(config: MkosiConfig, root: Path, uid: int, *, allow: bool) if not config.acl: return - ret = run( - [ - "setfacl", - "--physical", - "--modify" if allow else "--remove", - f"user:{uid}:rwx" if allow else f"user:{uid}", - "-", - ], - check=False, - text=True, - # Supply files via stdin so we don't clutter --debug run output too much - input="\n".join([str(root), *(e.path for e in cast(Iterator[os.DirEntry[str]], scandir_recursive(root)) if e.is_dir())]) + ret = run(["setfacl", + "--physical", + "--modify" if allow else "--remove", + f"user:{uid}:rwx" if allow else f"user:{uid}", + "-"], + check=False, + text=True, + # Supply files via stdin so we don't clutter --debug run output too much + input="\n".join([str(root), + *(e.path for e in cast(Iterator[os.DirEntry[str]], scandir_recursive(root)) if e.is_dir())]) ) if ret.returncode != 0: warn("Failed to set ACLs, you'll need root privileges to remove some generated files/directories") @@ -1955,10 +1925,14 @@ def machine_cid(config: MkosiConfig) -> int: def nspawn_knows_arg(arg: str) -> bool: # Specify some extra incompatible options so nspawn doesn't try to boot a container in the current # directory if it has a compatible layout. - return "unrecognized option" not in run(["systemd-nspawn", arg, - "--directory", "/dev/null", "--image", "/dev/null"], - stdout=subprocess.DEVNULL, stderr=subprocess.PIPE, check=False, - text=True).stderr + c = run(["systemd-nspawn", arg, + "--directory", "/dev/null", + "--image", "/dev/null"], + stdout=subprocess.DEVNULL, + stderr=subprocess.PIPE, + check=False, + text=True) + return "unrecognized option" not in c.stderr def run_shell(config: MkosiConfig) -> None: @@ -2167,18 +2141,12 @@ def run_qemu(config: MkosiConfig) -> None: cmdline: list[PathString] = [ find_qemu_binary(config), - "-machine", - machine, - "-smp", - config.qemu_smp, - "-m", - config.qemu_mem, - "-object", - "rng-random,filename=/dev/urandom,id=rng0", - "-device", - "virtio-rng-pci,rng=rng0,id=rng-device0", - "-nic", - "user,model=virtio-net-pci", + "-machine", machine, + "-smp", config.qemu_smp, + "-m", config.qemu_mem, + "-object", "rng-random,filename=/dev/urandom,id=rng0", + "-device", "virtio-rng-pci,rng=rng0,id=rng-device0", + "-nic", "user,model=virtio-net-pci", ] try: @@ -2222,12 +2190,9 @@ def run_qemu(config: MkosiConfig) -> None: ovmf_vars = stack.enter_context(tempfile.NamedTemporaryFile(prefix=".mkosi-", dir=tmp_dir())) copy_path(find_ovmf_vars(config), Path(ovmf_vars.name)) cmdline += [ - "-global", - "ICH9-LPC.disable_s3=1", - "-global", - "driver=cfi.pflash01,property=secure,value=on", - "-drive", - f"file={ovmf_vars.name},if=pflash,format=raw", + "-global", "ICH9-LPC.disable_s3=1", + "-global", "driver=cfi.pflash01,property=secure,value=on", + "-drive", f"file={ovmf_vars.name},if=pflash,format=raw", ] if config.ephemeral: @@ -2252,32 +2217,20 @@ def run_qemu(config: MkosiConfig) -> None: kernel = (config.output_dir or Path.cwd()) / config.output_split_kernel if not kernel.exists() and "-kernel" not in config.cmdline: die("No kernel found, please install a kernel in the cpio or provide a -kernel argument to mkosi qemu") - cmdline += [ - "-kernel", kernel, - "-initrd", fname, - "-append", " ".join(config.kernel_command_line + config.kernel_command_line_extra), - ] + cmdline += ["-kernel", kernel, + "-initrd", fname, + "-append", " ".join(config.kernel_command_line + config.kernel_command_line_extra)] if config.distribution == Distribution.debian: - cmdline += [ - "-drive", - f"if=virtio,id=hd,file={fname},format=raw", - ] + cmdline += ["-drive", f"if=virtio,id=hd,file={fname},format=raw"] else: - cmdline += [ - "-drive", - f"if=none,id=hd,file={fname},format=raw", - "-device", - "virtio-scsi-pci,id=scsi", - "-device", - "scsi-hd,drive=hd,bootindex=1", - ] + cmdline += ["-drive", f"if=none,id=hd,file={fname},format=raw", + "-device", "virtio-scsi-pci,id=scsi", + "-device", "scsi-hd,drive=hd,bootindex=1"] swtpm_socket = stack.enter_context(start_swtpm()) if swtpm_socket is not None: - cmdline += [ - "-chardev", f"socket,id=chrtpm,path={swtpm_socket}", - "-tpmdev", "emulator,id=tpm0,chardev=chrtpm", - ] + cmdline += ["-chardev", f"socket,id=chrtpm,path={swtpm_socket}", + "-tpmdev", "emulator,id=tpm0,chardev=chrtpm"] if config.architecture == "x86_64": cmdline += ["-device", "tpm-tis,tpmdev=tpm0"] @@ -2346,20 +2299,14 @@ def generate_secure_boot_key(config: MkosiConfig) -> None: crt = config.secure_boot_certificate or "mkosi.secure-boot.crt" cmd: list[PathString] = [ - "openssl", - "req", + "openssl", "req", "-new", "-x509", - "-newkey", - f"rsa:{keylength}", - "-keyout", - key, - "-out", - crt, - "-days", - str(config.secure_boot_valid_days), - "-subj", - f"/CN={cn}/", + "-newkey", f"rsa:{keylength}", + "-keyout", key, + "-out", crt, + "-days", str(config.secure_boot_valid_days), + "-subj", f"/CN={cn}/", "-nodes", ] run(cmd) diff --git a/mkosi/manifest.py b/mkosi/manifest.py index cd8fb937d..3b412235f 100644 --- a/mkosi/manifest.py +++ b/mkosi/manifest.py @@ -105,12 +105,13 @@ class Manifest: if not (root / dbpath).exists(): dbpath = "/var/lib/rpm" - c = run( - ["rpm", f"--root={root}", f"--dbpath={dbpath}", "-qa", "--qf", - r"%{NEVRA}\t%{SOURCERPM}\t%{NAME}\t%{ARCH}\t%{LONGSIZE}\t%{INSTALLTIME}\n"], - stdout=PIPE, - text=True, - ) + c = run(["rpm", + f"--root={root}", + f"--dbpath={dbpath}", + "-qa", + "--qf", r"%{NEVRA}\t%{SOURCERPM}\t%{NAME}\t%{ARCH}\t%{LONGSIZE}\t%{INSTALLTIME}\n"], + stdout=PIPE, + text=True) packages = sorted(c.stdout.splitlines()) @@ -145,7 +146,12 @@ class Manifest: source = self.source_packages.get(srpm) if source is None: - c = run(["rpm", f"--root={root}", f"--dbpath={dbpath}", "-q", "--changelog", nevra], + c = run(["rpm", + f"--root={root}", + f"--dbpath={dbpath}", + "-q", + "--changelog", + nevra], stdout=PIPE, stderr=DEVNULL, text=True) @@ -156,9 +162,11 @@ class Manifest: source.add(package) def record_deb_packages(self, root: Path) -> None: - c = run( - ["dpkg-query", f"--admindir={root}/var/lib/dpkg", "--show", "--showformat", - r'${Package}\t${source:Package}\t${Version}\t${Architecture}\t${Installed-Size}\t${db-fsys:Last-Modified}\n'], + c = run(["dpkg-query", + f"--admindir={root}/var/lib/dpkg", + "--show", + "--showformat", + r'${Package}\t${source:Package}\t${Version}\t${Architecture}\t${Installed-Size}\t${db-fsys:Last-Modified}\n'], stdout=PIPE, text=True, )