From ca29b9243a3b28da0921dc920c3b908dec3a0863 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 24 Jul 2024 14:03:48 +0200 Subject: [PATCH 1/4] Refactor the step splitting out to a single source of truth Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/build.py | 4 ++-- cfbs/utils.py | 6 ++++++ cfbs/validate.py | 5 ++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index 37ee32f8..fc55ae9d 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -10,6 +10,7 @@ read_json, rm, sh, + split_command, strip_left, touch, user_error, @@ -71,8 +72,7 @@ def _generate_augment(module_name, input_data): def _perform_build_step(module, step, max_length): - step = step.split(" ") - operation, args = step[0], step[1:] + operation, args = split_command(step) source = module["_directory"] counter = module["_counter"] destination = "out/masterfiles" diff --git a/cfbs/utils.py b/cfbs/utils.py index 43afb4f3..72e3b5f9 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -86,6 +86,12 @@ def pad_right(s, n) -> int: return s if len(s) >= n else s + " " * (n - len(s)) +def split_command(command) -> tuple[str, list[str]]: + terms = command.split(" ") + operation, args = terms[0], terms[1:] + return operation, args + + def user_error(msg: str): sys.exit("Error: " + msg) diff --git a/cfbs/validate.py b/cfbs/validate.py index 4fdcc3ca..9730200b 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -4,7 +4,7 @@ import re from collections import OrderedDict -from cfbs.utils import is_a_commit_hash, user_error +from cfbs.utils import is_a_commit_hash, split_command, user_error from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig from cfbs.build import AVAILABLE_BUILD_STEPS @@ -266,8 +266,7 @@ def validate_steps(name, module): raise CFBSValidationError( name, '"steps" must be a list of non-empty / non-whitespace strings' ) - step_array = step.split(" ") - operation, args = step_array[0], step_array[1:] + operation, args = split_command(step) if not operation in AVAILABLE_BUILD_STEPS: x = ", ".join(AVAILABLE_BUILD_STEPS) raise CFBSValidationError( From b43266fabcc6105679ab95e0d02442ab0a477966 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 25 Jul 2024 14:56:55 +0200 Subject: [PATCH 2/4] Refactor out the arg count validation Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/utils.py | 17 +++++++++++++++++ cfbs/validate.py | 13 +++++-------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/cfbs/utils.py b/cfbs/utils.py index 72e3b5f9..dc2087f5 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -92,6 +92,23 @@ def split_command(command) -> tuple[str, list[str]]: return operation, args +def is_valid_arg_count(args, expected): + actual = len(args) + + if type(expected) is int: + if actual != expected: + return False + + else: + # Only other option is a string of 1+, 2+ or similar: + assert type(expected) is str and expected.endswith("+") + expected = int(expected[0:-1]) + if actual < expected: + return False + + return True + + def user_error(msg: str): sys.exit("Error: " + msg) diff --git a/cfbs/validate.py b/cfbs/validate.py index 9730200b..5bc7751e 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -4,7 +4,7 @@ import re from collections import OrderedDict -from cfbs.utils import is_a_commit_hash, split_command, user_error +from cfbs.utils import is_valid_arg_count, is_a_commit_hash, split_command, user_error from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig from cfbs.build import AVAILABLE_BUILD_STEPS @@ -276,18 +276,15 @@ def validate_steps(name, module): ) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) - if type(expected) is int: - if expected != actual: + if not is_valid_arg_count(args, expected): + if type(expected) is int: raise CFBSValidationError( name, "The %s build step expects %d arguments, %d were given" % (operation, expected, actual), ) - else: - # Only other option is a string of 1+, 2+ or similar: - assert type(expected) is str and expected.endswith("+") - expected = int(expected[0:-1]) - if actual < expected: + else: + expected = int(expected[0:-1]) raise CFBSValidationError( name, "The %s build step expects %d or more arguments, %d were given" From cbb303986abd7ed38cd2a80816f9193626ceef03 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 25 Jul 2024 15:27:26 +0200 Subject: [PATCH 3/4] Building with invalid build steps now fails before executing any build steps, with an error message instead of a traceback Ticket: ENT-11207 Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/build.py | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/cfbs/build.py b/cfbs/build.py index fc55ae9d..3c6ba6e2 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -4,6 +4,7 @@ canonify, cp, find, + is_valid_arg_count, merge_json, mkdir, pad_right, @@ -222,14 +223,41 @@ def _perform_build_step(module, step, max_length): merged = merge_json(original, augment) if original else augment log.debug("Merged def.json: %s", pretty(merged)) write_json(path, merged) - else: - user_error("Unknown build step operation: %s" % operation) def perform_build_steps(config) -> int: if not config.get("build"): user_error("No 'build' key found in the configuration") - return 1 + + # mini-validation + for module in config.get("build", []): + for step in module["steps"]: + operation, args = split_command(step) + + if step.split() != [operation] + args: + user_error( + "Incorrect whitespace in the `%s` build step - singular spaces are required" + % step + ) + + if operation not in AVAILABLE_BUILD_STEPS: + user_error("Unknown build step operation: %s" % operation) + + expected = AVAILABLE_BUILD_STEPS[operation] + actual = len(args) + if not is_valid_arg_count(args, expected): + if type(expected) is int: + user_error( + "The `%s` build step expects %d arguments, %d were given" + % (step, expected, actual) + ) + else: + expected = int(expected[0:-1]) + user_error( + "The `%s` build step expects %d or more arguments, %d were given" + % (step, expected, actual) + ) + print("\nSteps:") module_name_length = config.longest_module_name() for module in config.get("build", []): From 4df07a7bfa7956cc14bdad245d69a7b8d6f975e6 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 25 Jul 2024 15:39:25 +0200 Subject: [PATCH 4/4] changes to support older Python versions Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cfbs/utils.py b/cfbs/utils.py index dc2087f5..652097f1 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -6,6 +6,7 @@ import subprocess import hashlib import logging as log +from typing import List, Tuple import urllib import urllib.request # needed on some platforms from collections import OrderedDict @@ -86,7 +87,7 @@ def pad_right(s, n) -> int: return s if len(s) >= n else s + " " * (n - len(s)) -def split_command(command) -> tuple[str, list[str]]: +def split_command(command) -> Tuple[str, List[str]]: terms = command.split(" ") operation, args = terms[0], terms[1:] return operation, args