diff --git a/cfbs/commands.py b/cfbs/commands.py index 1053e2e5..e2f45643 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -19,6 +19,7 @@ from cfbs.utils import ( CFBSNetworkError, CFBSUserError, + CFBSValidationError, cfbs_filename, is_cfbs_repo, read_json, @@ -45,7 +46,11 @@ perform_build, ) from cfbs.cfbs_config import CFBSConfig, CFBSReturnWithoutCommit -from cfbs.validate import validate_config, validate_config_raise_exceptions +from cfbs.validate import ( + validate_config, + validate_config_raise_exceptions, + validate_single_module, +) from cfbs.internal_file_management import ( clone_url_repo, SUPPORTED_URI_SCHEMES, @@ -568,7 +573,15 @@ def _someone_needs_me(this) -> bool: @commit_after_command("Updated module%s", [PLURAL_S]) def update_command(to_update) -> Result: config = CFBSConfig.get_instance() - validate_config_raise_exceptions(config, empty_build_list_ok=True) + r = validate_config(config, empty_build_list_ok=True) + valid_before = r == 0 + if not valid_before: + log.warning( + "Your cfbs.json fails validation before update " + + "(see messages above) - " + + "We will attempt update anyway, hoping newer " + + "versions of modules might fix the issues" + ) build = config["build"] # Update all modules in build if none specified @@ -582,6 +595,9 @@ def update_command(to_update) -> Result: module_updates = ModuleUpdates(config) index = None + old_modules = [] + new_modules = [] + update_objects = [] for update in to_update: old_module = config.get_module_from_build(update.name) assert ( @@ -611,7 +627,6 @@ def update_command(to_update) -> Result: if not old_module: log.warning("Module '%s' not in build. Skipping its update." % update.name) continue - if "url" in old_module: path, commit = clone_url_repo(old_module["url"]) remote_config = CFBSJson( @@ -663,11 +678,28 @@ def update_command(to_update) -> Result: continue new_module = index_info + update_objects.append(update) + old_modules.append(old_module) + new_modules.append(new_module) + + assert len(old_modules) == len(update_objects) + assert len(old_modules) == len(new_modules) + + # We don't validate old modules here because we want to allow + # cfbs update to fix invalid modules with a newer valid version. + + # Validate new modules, we don't want to add them unless they are valid: + for update, module in zip(update_objects, new_modules): + validate_single_module( + context="build", + name=update.name, + module=module, + config=None, + local_check=True, + ) + for old_module, new_module, update in zip(old_modules, new_modules, update_objects): update_module(old_module, new_module, module_updates, update) - - # add new items - updated.append(update) if module_updates.new_deps: @@ -677,6 +709,21 @@ def update_command(to_update) -> Result: for d in module_updates.new_deps ] config.add_with_dependencies(objects) + r = validate_config(config, empty_build_list_ok=False) + valid_after = r == 0 + if not valid_after: + if valid_before: + raise CFBSValidationError( + "The cfbs.json was valid before update, " + + "but is invalid after adding new versions " + + "of modules - aborting update " + + "(see validation error messages above)" + ) + raise CFBSValidationError( + "The cfbs.json was invalid before update, " + + "but updating modules did not fix it - aborting update" + + "(see validation error messages above)" + ) config.save() if module_updates.changes_made: diff --git a/cfbs/main.py b/cfbs/main.py index 4506a37d..d21f2f44 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -6,6 +6,8 @@ import logging as log import sys import os +import traceback +import pathlib from typing import Union from cfbs.result import Result @@ -272,19 +274,50 @@ def main() -> int: return r except CFBSValidationError as e: print("Error: " + str(e)) + return 1 except CFBSExitError as e: print("Error: " + str(e)) + return 1 except CFBSUserError as e: print("Error: " + str(e)) + return 1 except CFBSNetworkError as e: print("Error: " + str(e)) - except (AssertionError, CFBSProgrammerError) as e: - print("Error: " + str(e)) - print( - "This is an unexpected error indicating a bug, please create a ticket at:" + return 1 + # AssertionError and CFBSProgrammerError are not expected, print extra info: + except AssertionError as e: + tb = traceback.extract_tb(e.__traceback__) + frame = tb[-1] + this_file = pathlib.Path(__file__) + cfbs_prefix = os.path.abspath(this_file.parent.parent.resolve()) + filename = os.path.abspath(frame.filename) + # Opportunistically cut off beginning of path if possible: + if filename.startswith(cfbs_prefix): + filename = filename[len(cfbs_prefix) :] + if filename.startswith("/"): + filename = filename[1:] + line = frame.lineno + # Avoid using frame.colno - it was not available in python 3.5, + # and even in the latest version, it is not declared in the + # docstring, so you will get linting warnings; + # https://github.com/python/cpython/blob/v3.13.5/Lib/traceback.py#L276-L288 + # column = frame.colno + assertion = frame.line + explanation = str(e) + message = "Assertion failed - %s%s (%s:%s)" % ( + assertion, + (" - " + explanation) if explanation else "", + filename, + line, ) - print("https://northerntech.atlassian.net/") - print("(Rerun with CFBACKTRACE=1 in front of your command to show backtraces)") + print("Error: " + message) + except CFBSProgrammerError as e: + print("Error: " + str(e)) + print("This is an unexpected error indicating a bug, please create a ticket at:") + print("https://northerntech.atlassian.net/") + print( + "(Rerun with CFBACKTRACE=1 in front of your command to show the full backtrace)" + ) # TODO: Handle other exceptions return 1 diff --git a/cfbs/validate.py b/cfbs/validate.py index 5a3eae82..dbde1655 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -210,11 +210,11 @@ def validate_config_raise_exceptions(config, empty_build_list_ok=False): if "index" in raw_data and type(raw_data["index"]) in (dict, OrderedDict): for name, module in raw_data["index"].items(): - _validate_module_object("index", name, module, config) + validate_single_module("index", name, module, config) if "provides" in raw_data: for name, module in raw_data["provides"].items(): - _validate_module_object("provides", name, module, config) + validate_single_module("provides", name, module, config) if config["type"] == "module": validate_module_name_content(config["name"]) @@ -372,8 +372,12 @@ def _validate_module_alias(name, module, context, config): raise CFBSValidationError(name, '"alias" cannot reference another alias') -def _validate_module_name(name, module): +def _validate_module_name(name: str, module): assert "name" in module + assert name + assert type(name) is str + assert module["name"] + assert type(module["name"]) is str assert name == module["name"] if type(module["name"]) is not str: raise CFBSValidationError(name, '"name" must be of type string') @@ -416,8 +420,18 @@ def _validate_module_by(name, module): raise CFBSValidationError(name, '"by" must be non-empty') -def _validate_module_dependencies(name, module, config, context): - if context == "build": +def _validate_module_dependencies(name, module, config, context, local_check=False): + assert name + assert module + assert context in ("build", "provides", "index") + if local_check: + assert config is None + else: + assert config + + if local_check: + search_in = None + elif context == "build": search_in = ("build",) elif context == "provides": search_in = ("index", "provides") @@ -432,6 +446,9 @@ def _validate_module_dependencies(name, module, config, context): for dependency in module["dependencies"]: if type(dependency) is not str: raise CFBSValidationError(name, '"dependencies" must be a list of strings') + if local_check: + continue + assert config if not config.can_reach_dependency(dependency, search_in): raise CFBSValidationError( name, @@ -631,12 +648,33 @@ def _validate_module_input(name, module): ) -def _validate_module_object(context, name, module, config): +def validate_single_module(context, name, module, config, local_check=False): + """Function to validate one module object. + + Called repeatedly for each module in index, provides, and build. + Also called from other places, like in the update command, + before "accepting" new versions of a module object. + Planned to be used by cfbs add as well in the future. + + local_check can be set to True if you don't want to check + references to other parts of the cfbs.json i.e. to disable + checks for dependencies and aliases. In that case, the function must be + called with config = None. + """ assert context in ("index", "provides", "build") + if local_check: + assert config is None + else: + assert config # Step 1 - Handle special cases (alias): if "alias" in module: + if local_check: + raise CFBSValidationError( + "The 'alias' field is not allowed in '%s', inside '$%s'" + % (name, context) + ) # Needs to be validated first because it's missing the other fields: _validate_module_alias(name, module, context, config) return # alias entries would fail the other validation below @@ -682,7 +720,7 @@ def _validate_module_object(context, name, module, config): if "by" in module: _validate_module_by(name, module) if "dependencies" in module: - _validate_module_dependencies(name, module, config, context) + _validate_module_dependencies(name, module, config, context, local_check) if "index" in module: _validate_module_index(name, module) if "version" in module: @@ -722,7 +760,7 @@ def _validate_config_for_build_field(config, empty_build_list_ok=False): # If there are modules in "build" validate them: for index, module in enumerate(config["build"]): name = module["name"] if "name" in module else index - _validate_module_object("build", name, module, config) + validate_single_module("build", name, module, config) elif not empty_build_list_ok: raise CFBSExitError( "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'"