Merge pull request #199 from jakub-nt/ENT-11207

ENT-11207: Building with invalid build steps now fails before executing any build steps, with an error message instead of a traceback

cfbs/build.py

@@ -4,12 +4,14 @@
     canonify,
     cp,
     find,
+    is_valid_arg_count,
     merge_json,
     mkdir,
     pad_right,
     read_json,
     rm,
     sh,
+    split_command,
     strip_left,
     touch,
     user_error,
@@ -71,8 +73,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"
@@ -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_key_length("name")
     for module in config.get("build", []):

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,6 +87,29 @@ 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 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)
 

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_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
@@ -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(
@@ -277,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"