From 40ff0dec080dcddbf603e69cc16508c28604f612 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 3 Feb 2025 10:34:50 +0100 Subject: [PATCH 1/3] test/ukify: deduplicate path specification --- src/ukify/test/test_ukify.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ukify/test/test_ukify.py b/src/ukify/test/test_ukify.py index 8ae4d04e8d..f75ef0c891 100755 --- a/src/ukify/test/test_ukify.py +++ b/src/ukify/test/test_ukify.py @@ -59,8 +59,10 @@ except ValueError: slow_tests = True arg_tools = ['--tools', build_root] if build_root else [] -if build_root and pathlib.Path(f"{build_root}/linux{ukify.guess_efi_arch()}.efi.stub").exists(): - arg_tools += ['--stub', f"{build_root}/linux{ukify.guess_efi_arch()}.efi.stub"] +if build_root and ( + p := pathlib.Path(f"{build_root}/linux{ukify.guess_efi_arch()}.efi.stub") +).exists(): + arg_tools += ['--stub', p] def systemd_measure(): opts = ukify.create_parser().parse_args(arg_tools) From bd208a5421c10f6f523864553941f281d931fefc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 3 Feb 2025 10:38:01 +0100 Subject: [PATCH 2/3] ukify: drop dots from exception messages In Python, exception messages are often embedded in surrounding text, so in general they should not contain punctuation. Also, no need to instantiate the exception object if no arguments are used. --- src/ukify/ukify.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index 097a7ee0c6..fb3912f902 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -489,11 +489,11 @@ class UKI: class SignTool: @staticmethod def sign(input_f: str, output_f: str, opts: UkifyConfig) -> None: - raise NotImplementedError() + raise NotImplementedError @staticmethod def verify(opts: UkifyConfig) -> bool: - raise NotImplementedError() + raise NotImplementedError @staticmethod def from_string(name: str) -> type['SignTool']: @@ -1028,7 +1028,7 @@ def pe_add_sections(opts: UkifyConfig, uki: UKI, output: str) -> None: for i, s in enumerate(pe.sections[:n_original_sections]): if pe_strip_section_name(s.Name) == section.name and section.name != '.dtbauto': if new_section.Misc_VirtualSize > s.SizeOfRawData: - raise PEError(f'Not enough space in existing section {section.name} to append new data.') + raise PEError(f'Not enough space in existing section {section.name} to append new data') padding = bytes(new_section.SizeOfRawData - new_section.Misc_VirtualSize) pe.__data__ = ( @@ -1081,9 +1081,8 @@ def pe_add_sections(opts: UkifyConfig, uki: UKI, output: str) -> None: encoded = json.dumps(j).encode() if len(encoded) > section.SizeOfRawData: raise PEError( - f'Not enough space in existing section .pcrsig of size {section.SizeOfRawData} to append new data of size {len(encoded)}.' # noqa: E501 + f'Not enough space in existing section .pcrsig of size {section.SizeOfRawData} to append new data of size {len(encoded)}' # noqa: E501 ) - section.Misc_VirtualSize = len(encoded) # bytes(n) results in an array of n zeroes padding = bytes(section.SizeOfRawData - len(encoded)) @@ -1242,12 +1241,12 @@ def parse_hwid_dir(path: Path) -> bytes: def parse_efifw_dir(path: Path) -> bytes: if not path.is_dir(): - raise ValueError(f'{path} is not a directory or it does not exist.') + raise ValueError(f'{path} is not a directory or it does not exist') # only one firmware image must be present in the directory # to uniquely identify that firmware with its ID. if len(list(path.glob('*'))) != 1: - raise ValueError(f'{path} must contain exactly one firmware image file.') + raise ValueError(f'{path} must contain exactly one firmware image file') payload_blob = b'' for fw in path.iterdir(): From 6446c7d99bcc08534c31cead59a3b0028583a984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 Feb 2025 11:16:03 +0100 Subject: [PATCH 3/3] ukify: avoid treating invalid option as an argument For some reason, argparse treats undefined options as positional args in certain scenarios: $ src/ukify/ukify.py --badopt='11' ukify.py: error: unrecognized arguments: --badopt=11 $ src/ukify/ukify.py --badopt '11' ukify.py: error: unrecognized arguments: --badopt $ src/ukify/ukify.py --badopt '11 12' Assuming obsolete command line syntax with no verb. Please use 'build'. Traceback (most recent call last): File "/home/zbyszek/src/systemd/src/ukify/ukify.py", line 2497, in main() ~~~~^^ File "/home/zbyszek/src/systemd/src/ukify/ukify.py", line 2485, in main check_inputs(opts) ~~~~~~~~~~~~^^^^^^ File "/home/zbyszek/src/systemd/src/ukify/ukify.py", line 671, in check_inputs value.open().close() ~~~~~~~~~~^^ File "/usr/lib64/python3.13/pathlib/_local.py", line 537, in open return io.open(self, mode, buffering, encoding, errors, newline) ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ FileNotFoundError: [Errno 2] No such file or directory: '--badopt=11 12' I suspect that this is some crap compat for Windows, where option parsing is an even bigger mess than here. Being told about positional args, when no positional args were specified is confusing, so add a check for this. --- src/ukify/ukify.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ukify/ukify.py b/src/ukify/ukify.py index fb3912f902..87af8d7864 100755 --- a/src/ukify/ukify.py +++ b/src/ukify/ukify.py @@ -2485,6 +2485,12 @@ def finalize_options(opts: argparse.Namespace) -> None: def parse_args(args: Optional[list[str]] = None) -> argparse.Namespace: opts = create_parser().parse_args(args) + + # argparse puts some unknown options in opts.positional. Make sure we don't + # try to interpret something that is an option as a positional argument. + if any((bad_opt := o).startswith('-') for o in opts.positional): + raise ValueError(f'Unknown option: {bad_opt.partition("=")[0]}') + apply_config(opts) finalize_options(opts) return opts