From f737c54412243e3bc38f526d4cbce9a8b5abc5fa Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 13:45:03 +0200 Subject: [PATCH 1/8] cfbs validate: Changed warnings to errors We don't want to accidentally skip some things user specified, better be strict. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 3d336ba1..4dfd6a02 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -706,7 +706,7 @@ def validate_command(paths=None, index_arg=None) -> int: # `path` can be either a directory containing a CFBS project (cfbs.json) or path to cfbs.json if not os.path.basename(path) == "cfbs.json": if path.endswith(".json"): - log.warning( + raise GenericExitError( "Only cfbs.json files can be validated. Skipping validation" ) # if the user actually has an e.g. directory ending with `.json`, @@ -718,12 +718,12 @@ def validate_command(paths=None, index_arg=None) -> int: if not os.path.isfile(path): # either cfbs.json doesn't exist, or it's an e.g. directory if not os.path.exists(path): - log.warning( + raise GenericExitError( "%s is not a valid CFBS project path, skipping validation" % path ) else: - log.warning( + raise GenericExitError( "A non-file named cfbs.json detected. Skipping validation. " + "If it is not a mistake that cfbs.json is not a file, specify the full path to the cfbs.json file inside to validate." ) From 951c078043ba03c2d4ae537019825c2078ffa36b Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:02:19 +0200 Subject: [PATCH 2/8] cfbs validate: Enabled validation of differently named cfbs.json files We have some examples of this already in tests; example-cfbs.json. Also refactored error checking for missing files / folders. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 4dfd6a02..24d69508 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -703,32 +703,33 @@ def validate_command(paths=None, index_arg=None) -> int: ret_value = 0 for path in paths: - # `path` can be either a directory containing a CFBS project (cfbs.json) or path to cfbs.json - if not os.path.basename(path) == "cfbs.json": - if path.endswith(".json"): - raise GenericExitError( - "Only cfbs.json files can be validated. Skipping validation" - ) - # if the user actually has an e.g. directory ending with `.json`, - # they can always specify the path to the cfbs.json in that folder to force the validation - continue - # assume the provided path is a project directory - path = os.path.join(path, "cfbs.json") - - if not os.path.isfile(path): - # either cfbs.json doesn't exist, or it's an e.g. directory - if not os.path.exists(path): - raise GenericExitError( - "%s is not a valid CFBS project path, skipping validation" - % path + # Exit out early if we find anything wrong like missing files: + if not os.path.exists(path): + raise GenericExitError( + "Specified path '{}' does not exist".format(path) + ) + if path.endswith(".json") and not os.path.isfile(path): + raise GenericExitError( + "'{}' is not a file - Please specify a path to a cfbs project file, ending in .json, or a folder containing a cfbs.json".format( + path ) - else: - raise GenericExitError( - "A non-file named cfbs.json detected. Skipping validation. " - + "If it is not a mistake that cfbs.json is not a file, specify the full path to the cfbs.json file inside to validate." + ) + if not path.endswith(".json") and not os.path.isfile( + os.path.join(path, "cfbs.json") + ): + raise GenericExitError( + "No CFBS project file found at '{}'".format( + os.path.join(path, "cfbs.json") ) - continue + ) + + # Convert folder to folder/cfbs.json if appropriate: + if not path.endswith(".json"): + assert os.path.isdir(path) + path = os.path.join(path, "cfbs.json") + assert os.path.isfile(path) + # Actually opeb the file and perform validation: config = CFBSJson(path=path, index_argument=index_arg) r = validate_config(config) From a7d48478566c035c1fa4611d45a20466cef0d2b4 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:22:06 +0200 Subject: [PATCH 3/8] Re-introduced UserError exceptions There are more cases of GenericExitError which could be converted, but we can do those over time. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/cfbs_config.py | 5 +++-- cfbs/commands.py | 11 +++++------ cfbs/main.py | 28 +++++++++++++++------------- cfbs/man_generator.py | 4 ++-- cfbs/utils.py | 15 +++++++++++++++ 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 29613f51..67627d0d 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -8,6 +8,7 @@ from cfbs.result import Result from cfbs.utils import ( GenericExitError, + UserError, read_file, write_json, load_bundlenames, @@ -388,7 +389,7 @@ def add_command( checksum=None, ) -> Result: if not to_add: - raise GenericExitError("Must specify at least one module to add") + raise UserError("Must specify at least one module to add") modules_in_build_key = self.get("build", []) assert type(modules_in_build_key) is list @@ -404,7 +405,7 @@ def add_command( else: # for this `if` to be valid, module names containing `://` should be illegal if "://" in to_add[0]: - raise GenericExitError( + raise UserError( "URI scheme not supported. The supported URI schemes are: " + ", ".join(SUPPORTED_URI_SCHEMES) ) diff --git a/cfbs/commands.py b/cfbs/commands.py index 24d69508..92cf8d2b 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -18,6 +18,7 @@ from cfbs.updates import ModuleUpdates, update_module from cfbs.utils import ( FetchError, + UserError, cfbs_filename, is_cfbs_repo, read_json, @@ -126,7 +127,7 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: @cfbs_command("init") def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if is_cfbs_repo(): - raise GenericExitError("Already initialized - look at %s" % cfbs_filename()) + raise UserError("Already initialized - look at %s" % cfbs_filename()) name = prompt_user( non_interactive, @@ -705,11 +706,9 @@ def validate_command(paths=None, index_arg=None) -> int: for path in paths: # Exit out early if we find anything wrong like missing files: if not os.path.exists(path): - raise GenericExitError( - "Specified path '{}' does not exist".format(path) - ) + raise UserError("Specified path '{}' does not exist".format(path)) if path.endswith(".json") and not os.path.isfile(path): - raise GenericExitError( + raise UserError( "'{}' is not a file - Please specify a path to a cfbs project file, ending in .json, or a folder containing a cfbs.json".format( path ) @@ -717,7 +716,7 @@ def validate_command(paths=None, index_arg=None) -> int: if not path.endswith(".json") and not os.path.isfile( os.path.join(path, "cfbs.json") ): - raise GenericExitError( + raise UserError( "No CFBS project file found at '{}'".format( os.path.join(path, "cfbs.json") ) diff --git a/cfbs/main.py b/cfbs/main.py index 440b2463..dd6de214 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -11,7 +11,7 @@ from cfbs.result import Result from cfbs.validate import CFBSValidationError from cfbs.version import string as version -from cfbs.utils import GenericExitError, is_cfbs_repo, ProgrammerError +from cfbs.utils import GenericExitError, UserError, is_cfbs_repo, ProgrammerError from cfbs.cfbs_config import CFBSConfig from cfbs import commands from cfbs.args import get_args, print_help, get_manual @@ -63,62 +63,62 @@ def _main() -> Union[int, Result]: if not args.command: print_help() print("") - raise GenericExitError("No command given") + raise UserError("No command given") if args.command not in commands.get_command_names(): print_help() - raise GenericExitError("Command '%s' not found" % args.command) + raise UserError("Command '%s' not found" % args.command) if args.masterfiles and args.command != "init": - raise GenericExitError( + raise UserError( "The option --masterfiles is only for 'cfbs init', not 'cfbs %s'" % args.command ) if args.omit_download and args.command != "generate-release-information": - raise GenericExitError( + raise UserError( "The option --omit-download is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.check_against_git and args.command != "generate-release-information": - raise GenericExitError( + raise UserError( "The option --check-against-git is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.minimum_version and args.command != "generate-release-information": - raise GenericExitError( + raise UserError( "The option --from is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.masterfiles_dir and args.command not in ("analyze", "analyse"): - raise GenericExitError( + raise UserError( "The option --masterfiles-dir is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.reference_version and args.command not in ("analyze", "analyse"): - raise GenericExitError( + raise UserError( "The option --reference-version is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.to_json and args.command not in ("analyze", "analyse"): - raise GenericExitError( + raise UserError( "The option --to-json is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.ignored_path_components and args.command not in ("analyze", "analyse"): - raise GenericExitError( + raise UserError( "The option --ignored-path-components is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.offline and args.command not in ("analyze", "analyse"): - raise GenericExitError( + raise UserError( "The option --offline is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) @@ -131,7 +131,7 @@ def _main() -> Union[int, Result]: "update", "input", ): - raise GenericExitError( + raise UserError( "The option --non-interactive is not for cfbs %s" % (args.command) ) @@ -262,6 +262,8 @@ def main() -> int: print("Error: " + str(e)) except GenericExitError as e: print("Error: " + str(e)) + except UserError as e: + print("Error: " + str(e)) except (AssertionError, ProgrammerError) as e: print("Error: " + str(e)) print( diff --git a/cfbs/man_generator.py b/cfbs/man_generator.py index 0f15c61d..b33a1f07 100644 --- a/cfbs/man_generator.py +++ b/cfbs/man_generator.py @@ -1,11 +1,11 @@ import os -from utils import GenericExitError +from utils import UserError from args import get_arg_parser try: from build_manpages.manpage import Manpage # type: ignore except ImportError: - raise GenericExitError( + raise UserError( "Missing dependency, install from PyPI: 'pip install argparse-manpage setuptools'" ) diff --git a/cfbs/utils.py b/cfbs/utils.py index 32c4da74..00f662e3 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -19,10 +19,25 @@ class ProgrammerError(RuntimeError): + """Exception to use for cases where we as developers made a mistake. + + Situations which should never happen - similar to assertions. + """ + pass class GenericExitError(Exception): + """Generic errors which make the program exit. + + Most of these should be converted to more specific exception types.""" + + pass + + +class UserError(Exception): + """Exception for when the user did something wrong, such as specifying a file which does not exist.""" + pass From 93ee782ebcda1e65e316e2ae50513727aabdadf2 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:26:31 +0200 Subject: [PATCH 4/8] Renamed FetchError to NetworkError Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/analyze.py | 6 +++--- cfbs/commands.py | 4 ++-- cfbs/index.py | 8 ++++---- cfbs/internal_file_management.py | 4 ++-- cfbs/masterfiles/download_all_versions.py | 8 ++++---- cfbs/utils.py | 25 ++++++++++++++--------- 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/cfbs/analyze.py b/cfbs/analyze.py index 43588049..0374e005 100644 --- a/cfbs/analyze.py +++ b/cfbs/analyze.py @@ -8,7 +8,7 @@ version_as_comparable_list, ) from cfbs.utils import ( - FetchError, + NetworkError, cfbs_dir, deduplicate_list, fetch_url, @@ -136,7 +136,7 @@ def mpf_vcf_dicts(offline=False): try: latest_release_data = get_json(LATEST_RELEASE_API_URL) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine release information failed - check your Wi-Fi / network settings." ) @@ -157,7 +157,7 @@ def mpf_vcf_dicts(offline=False): archive_checksums_path = ri_version_path + "/checksums.txt" try: fetch_url(ri_checksums_url, archive_checksums_path) - except FetchError as e: + except NetworkError as e: raise GenericExitError(str(e)) with open(archive_checksums_path) as file: diff --git a/cfbs/commands.py b/cfbs/commands.py index 92cf8d2b..90f1f344 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -17,7 +17,7 @@ from cfbs.cfbs_json import CFBSJson from cfbs.updates import ModuleUpdates, update_module from cfbs.utils import ( - FetchError, + NetworkError, UserError, cfbs_filename, is_cfbs_repo, @@ -805,7 +805,7 @@ def _download_dependencies( else: try: versions = get_json(_VERSION_INDEX) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) diff --git a/cfbs/index.py b/cfbs/index.py index f4a49851..6dcf0570 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -4,7 +4,7 @@ from typing import Union from cfbs.module import Module -from cfbs.utils import FetchError, get_or_read_json, GenericExitError, get_json +from cfbs.utils import NetworkError, get_or_read_json, GenericExitError, get_json from cfbs.internal_file_management import local_module_name _DEFAULT_INDEX = ( @@ -91,7 +91,7 @@ def _expand_index(self): try: self._data = get_or_read_json(index) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading index '%s' failed - check your Wi-Fi / network settings." % index @@ -132,7 +132,7 @@ def exists(self, module): return name in self try: versions = get_json(_VERSION_INDEX) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -179,7 +179,7 @@ def get_module_object(self, module, added_by=None): if version: try: versions = get_json(_VERSION_INDEX) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index ae8c9c1d..09273afc 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -16,7 +16,7 @@ cfbs_dir, cp, fetch_url, - FetchError, + NetworkError, is_a_commit_hash, mkdir, pad_right, @@ -220,7 +220,7 @@ def fetch_archive( archive_path = os.path.join(downloads, archive_dir, archive_filename) try: archive_checksum = fetch_url(url, archive_path, checksum) - except FetchError as e: + except NetworkError as e: raise GenericExitError(str(e)) content_dir = os.path.join(downloads, archive_dir, archive_checksum) diff --git a/cfbs/masterfiles/download_all_versions.py b/cfbs/masterfiles/download_all_versions.py index dde31952..f0ac39e2 100644 --- a/cfbs/masterfiles/download_all_versions.py +++ b/cfbs/masterfiles/download_all_versions.py @@ -2,7 +2,7 @@ import shutil from cfbs.masterfiles.analyze import version_is_at_least -from cfbs.utils import FetchError, fetch_url, get_json, mkdir, GenericExitError +from cfbs.utils import NetworkError, fetch_url, get_json, mkdir, GenericExitError ENTERPRISE_RELEASES_URL = "https://cfengine.com/release-data/enterprise/releases.json" @@ -15,7 +15,7 @@ def get_download_urls_enterprise(min_version=None): try: data = get_json(ENTERPRISE_RELEASES_URL) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine release data failed - check your Wi-Fi / network settings." ) @@ -45,7 +45,7 @@ def get_download_urls_enterprise(min_version=None): release_url = release_data["URL"] try: subdata = get_json(release_url) - except FetchError: + except NetworkError: raise GenericExitError( "Downloading CFEngine release data for version %s failed - check your Wi-Fi / network settings." % version @@ -97,7 +97,7 @@ def download_versions_from_urls(download_path, download_urls, reported_checksums checksum = reported_checksums[version] try: fetch_url(url, tarball_path, checksum) - except FetchError as e: + except NetworkError as e: raise GenericExitError("For version " + version + ": " + str(e)) tarball_dir_path = os.path.join(version_path, "tarball") diff --git a/cfbs/utils.py b/cfbs/utils.py index 00f662e3..d0c749af 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -41,6 +41,15 @@ class UserError(Exception): pass +class NetworkError(Exception): + """Errors which generally can be attributed to a server our router being offline. + + Usually we'll advise the user to check their Wifi / network settings and/or try again later. + """ + + pass + + def _sh(cmd: str): # print(cmd) try: @@ -114,7 +123,7 @@ def get_json(url: str) -> OrderedDict: assert r.status >= 200 and r.status < 300 return json.loads(r.read().decode(), object_pairs_hook=OrderedDict) except urllib.error.URLError as e: - raise FetchError("Failed to get JSON from '%s'" % url) from e + raise NetworkError("Failed to get JSON from '%s'" % url) from e def get_or_read_json(path: str) -> Union[OrderedDict, None]: @@ -351,10 +360,6 @@ def file_sha256(file): return h.hexdigest() -class FetchError(Exception): - pass - - def fetch_url(url, target, checksum=None): if checksum is not None: if SHA1_RE.match(checksum): @@ -362,7 +367,7 @@ def fetch_url(url, target, checksum=None): elif SHA256_RE.match(checksum): sha = hashlib.sha256() else: - raise FetchError( + raise NetworkError( "Invalid checksum or unsupported checksum algorithm: '%s'" % checksum ) else: @@ -377,7 +382,7 @@ def fetch_url(url, target, checksum=None): with open(target, "wb") as f: with urllib.request.urlopen(request) as u: if not (200 <= u.status <= 300): - raise FetchError("Failed to fetch '%s': %s" % (url, u.reason)) + raise NetworkError("Failed to fetch '%s': %s" % (url, u.reason)) done = False while not done: chunk = u.read(512 * 1024) # 512 KiB @@ -393,7 +398,7 @@ def fetch_url(url, target, checksum=None): else: if os.path.exists(target): os.unlink(target) - raise FetchError( + raise NetworkError( "Checksum mismatch in fetched '%s': %s != %s" % (url, digest, checksum) ) @@ -402,11 +407,11 @@ def fetch_url(url, target, checksum=None): except urllib.error.URLError as e: if os.path.exists(target): os.unlink(target) - raise FetchError("Failed to fetch '%s': %s" % (url, e)) from e + raise NetworkError("Failed to fetch '%s': %s" % (url, e)) from e except OSError as e: if os.path.exists(target): os.unlink(target) - raise FetchError("Failed to fetch '%s' to '%s': %s" % (url, target, e)) from e + raise NetworkError("Failed to fetch '%s' to '%s': %s" % (url, target, e)) from e def is_a_commit_hash(commit): From 91749e7c3d709e153718a98a903b6e228c093d7f Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:28:42 +0200 Subject: [PATCH 5/8] Added top level handling of NetworkError exceptions Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/main.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cfbs/main.py b/cfbs/main.py index dd6de214..cf1c0aa9 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -11,7 +11,13 @@ from cfbs.result import Result from cfbs.validate import CFBSValidationError from cfbs.version import string as version -from cfbs.utils import GenericExitError, UserError, is_cfbs_repo, ProgrammerError +from cfbs.utils import ( + GenericExitError, + UserError, + is_cfbs_repo, + ProgrammerError, + NetworkError, +) from cfbs.cfbs_config import CFBSConfig from cfbs import commands from cfbs.args import get_args, print_help, get_manual @@ -264,6 +270,8 @@ def main() -> int: print("Error: " + str(e)) except UserError as e: print("Error: " + str(e)) + except NetworkError as e: + print("Error: " + str(e)) except (AssertionError, ProgrammerError) as e: print("Error: " + str(e)) print( From 5635ea701830e5000663ddde19b3cb2a7edc32cd Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:40:07 +0200 Subject: [PATCH 6/8] Moved CFBSValidationError to utils.py Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/main.py | 2 +- cfbs/utils.py | 31 +++++++++++++++++++++++++++++++ cfbs/validate.py | 19 +------------------ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/cfbs/main.py b/cfbs/main.py index cf1c0aa9..3cd5454c 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -9,9 +9,9 @@ from typing import Union from cfbs.result import Result -from cfbs.validate import CFBSValidationError from cfbs.version import string as version from cfbs.utils import ( + CFBSValidationError, GenericExitError, UserError, is_cfbs_repo, diff --git a/cfbs/utils.py b/cfbs/utils.py index d0c749af..6fda5905 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -50,6 +50,37 @@ class NetworkError(Exception): pass +class CFBSValidationError(Exception): + """Exception for when validation fails. + + Commonly this means some fields / values are wrong in cfbs.json, or that the referenced + files / modules don't exist. + + Mostly used by validate.py / cfbs validate command, but can be used in other places for + the same purpose. + + For example: If you inside the cfbs build logic want to check that "steps" exists, + you can raise a validation error if it doesn't. However, if you believe other + validation logic should have already checked this and it's not possible, + an assertion or programmer error is more appropriate.""" + + def __init__(self, name_or_message, message=None) -> None: + assert name_or_message + if message: + name = name_or_message + else: + name = None + message = name_or_message + if name is None: + super().__init__("Error in cfbs.json: " + message) + elif type(name) is int: + super().__init__( + "Error in cfbs.json for module at index %d: " % name + message + ) + else: + super().__init__("Error in cfbs.json for module '%s': " % name + message) + + def _sh(cmd: str): # print(cmd) try: diff --git a/cfbs/validate.py b/cfbs/validate.py index 09b56511..bf2d39b1 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -32,6 +32,7 @@ strip_left, strip_right_any, GenericExitError, + CFBSValidationError, ) from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS from cfbs.cfbs_config import CFBSConfig @@ -80,24 +81,6 @@ def step_has_valid_arg_count(args, expected): return True -class CFBSValidationError(Exception): - def __init__(self, name_or_message, message=None) -> None: - assert name_or_message - if message: - name = name_or_message - else: - name = None - message = name_or_message - if name is None: - super().__init__("Error in cfbs.json: " + message) - elif type(name) is int: - super().__init__( - "Error in cfbs.json for module at index %d: " % name + message - ) - else: - super().__init__("Error in cfbs.json for module '%s': " % name + message) - - def _validate_top_level_keys(config): # Convert the CFBSJson object to a simple dictionary with exactly # what was in the file. We don't want CFBSJson / CFBSConfig to do any From f1cc1a5e28572f2756f6fb60533485e3f54f0950 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 14:47:14 +0200 Subject: [PATCH 7/8] Renamed all custom exceptions to have CFBS prefix To make it clear which exceptions are custom to CFBS. To avoid potential collisions or confusion with other libraries' exceptions. Also dropped "Generic" in GenericExitError - CFBSExitError seems descriptive enough. Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/analyze.py | 18 +++--- cfbs/args.py | 8 +-- cfbs/build.py | 28 +++++----- cfbs/cfbs_config.py | 28 +++++----- cfbs/cfbs_json.py | 8 +-- cfbs/commands.py | 56 +++++++++---------- cfbs/index.py | 16 +++--- cfbs/internal_file_management.py | 38 ++++++------- cfbs/main.py | 46 ++++++++------- cfbs/man_generator.py | 4 +- .../masterfiles/check_download_matches_git.py | 4 +- cfbs/masterfiles/download_all_versions.py | 14 ++--- cfbs/updates.py | 4 +- cfbs/utils.py | 26 ++++----- cfbs/validate.py | 8 +-- 15 files changed, 150 insertions(+), 156 deletions(-) diff --git a/cfbs/analyze.py b/cfbs/analyze.py index 0374e005..a0e2e29d 100644 --- a/cfbs/analyze.py +++ b/cfbs/analyze.py @@ -8,7 +8,7 @@ version_as_comparable_list, ) from cfbs.utils import ( - NetworkError, + CFBSNetworkError, cfbs_dir, deduplicate_list, fetch_url, @@ -17,7 +17,7 @@ immediate_subdirectories, mkdir, read_json, - GenericExitError, + CFBSExitError, ) @@ -115,11 +115,11 @@ def mpf_vcf_dicts(offline=False): cfbs_ri_dir = os.path.join(cfbs_dir(), RI_SUBDIRS) if not os.path.exists(cfbs_ri_dir): - raise GenericExitError(ERROR_MESSAGE) + raise CFBSExitError(ERROR_MESSAGE) ri_versions = immediate_subdirectories(cfbs_ri_dir) if len(ri_versions) == 0: - raise GenericExitError(ERROR_MESSAGE) + raise CFBSExitError(ERROR_MESSAGE) ri_latest_version = max(ri_versions) mpf_vcf_path = os.path.join( @@ -136,8 +136,8 @@ def mpf_vcf_dicts(offline=False): try: latest_release_data = get_json(LATEST_RELEASE_API_URL) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine release information failed - check your Wi-Fi / network settings." ) @@ -157,8 +157,8 @@ def mpf_vcf_dicts(offline=False): archive_checksums_path = ri_version_path + "/checksums.txt" try: fetch_url(ri_checksums_url, archive_checksums_path) - except NetworkError as e: - raise GenericExitError(str(e)) + except CFBSNetworkError as e: + raise CFBSExitError(str(e)) with open(archive_checksums_path) as file: lines = [line.rstrip() for line in file] @@ -630,7 +630,7 @@ def analyze_policyset( + "\n ".join(possible_policyset_relpaths) + "\n" ) - raise GenericExitError( + raise CFBSExitError( "There doesn't seem to be a valid policy set in the supplied path.\n Usage: cfbs analyze path/to/policy-set\n" + extra_error_text ) diff --git a/cfbs/args.py b/cfbs/args.py index ac234f98..7f85db4d 100644 --- a/cfbs/args.py +++ b/cfbs/args.py @@ -3,7 +3,7 @@ from typing import List, Union from cfbs import commands -from cfbs.utils import cache, GenericExitError +from cfbs.utils import cache, CFBSExitError class ArgsTypesNamespace(argparse.Namespace): @@ -54,13 +54,13 @@ def get_manual(): with open(file_path, "r", encoding="utf-8") as man_file: man = man_file.read() if not man: - raise GenericExitError("Manual file is empty") + raise CFBSExitError("Manual file is empty") else: return man except OSError: - raise GenericExitError("Error reading manual file " + file_path) + raise CFBSExitError("Error reading manual file " + file_path) else: - raise GenericExitError("Manual file does not exist") + raise CFBSExitError("Manual file does not exist") @cache diff --git a/cfbs/build.py b/cfbs/build.py index f587b5cb..a77ca2d9 100644 --- a/cfbs/build.py +++ b/cfbs/build.py @@ -26,7 +26,7 @@ sh, strip_left, touch, - GenericExitError, + CFBSExitError, write_json, ) from cfbs.pretty import pretty, pretty_file @@ -92,9 +92,9 @@ def _perform_replace_step(n, a, b, filename): with open(filename, "r") as f: content = f.read() except FileNotFoundError: - raise GenericExitError("No such file '%s' in replace build step" % (filename,)) + raise CFBSExitError("No such file '%s' in replace build step" % (filename,)) except: - raise GenericExitError( + raise CFBSExitError( "Could not open/read '%s' in replace build step" % (filename,) ) new_content = previous_content = content @@ -102,7 +102,7 @@ def _perform_replace_step(n, a, b, filename): previous_content = new_content new_content = previous_content.replace(a, b, 1) if new_content == previous_content: - raise GenericExitError( + raise CFBSExitError( "replace build step could only replace '%s' in '%s' %s times, not %s times (required)" % (a, filename, i, n) ) @@ -114,12 +114,12 @@ def _perform_replace_step(n, a, b, filename): if new_content == previous_content: break if a in new_content: - raise GenericExitError("too many occurences of '%s' in '%s'" % (a, filename)) + raise CFBSExitError("too many occurences of '%s' in '%s'" % (a, filename)) try: with open(filename, "w") as f: f.write(new_content) except: - raise GenericExitError("Failed to write to '%s'" % (filename,)) + raise CFBSExitError("Failed to write to '%s'" % (filename,)) def _perform_build_step(module, i, step, max_length): @@ -160,7 +160,7 @@ def _perform_build_step(module, i, step, max_length): dst = "" print("%s json '%s' 'masterfiles/%s'" % (prefix, src, dst)) if not os.path.isfile(os.path.join(source, src)): - raise GenericExitError("'%s' is not a file" % src) + raise CFBSExitError("'%s' is not a file" % src) src, dst = os.path.join(source, src), os.path.join(destination, dst) extras, original = read_json(src), read_json(dst) if not extras: @@ -230,7 +230,7 @@ def _perform_build_step(module, i, step, max_length): extras = _generate_augment(name, extras) log.debug("Generated augment: %s", pretty(extras)) if not extras: - raise GenericExitError( + raise CFBSExitError( "Input data '%s' is incomplete: Skipping build step." % os.path.basename(src) ) @@ -253,7 +253,7 @@ def _perform_build_step(module, i, step, max_length): cf_files = find("out/masterfiles/" + file, extension=".cf") files += (strip_left(f, "out/masterfiles/") for f in cf_files) else: - raise GenericExitError( + raise CFBSExitError( "Unsupported filetype '%s' for build step '%s': " % (file, operation) + "Expected directory (*/) of policy file (*.cf)" @@ -308,7 +308,7 @@ def _perform_build_step(module, i, step, max_length): def perform_build(config) -> int: if not config.get("build"): - raise GenericExitError("No 'build' key found in the configuration") + raise CFBSExitError("No 'build' key found in the configuration") # mini-validation for module in config.get("build", []): @@ -316,25 +316,25 @@ def perform_build(config) -> int: operation, args = split_build_step(step) if step.split() != [operation] + args: - raise GenericExitError( + raise CFBSExitError( "Incorrect whitespace in the `%s` build step - singular spaces are required" % step ) if operation not in AVAILABLE_BUILD_STEPS: - raise GenericExitError("Unknown build step operation: %s" % operation) + raise CFBSExitError("Unknown build step operation: %s" % operation) expected = AVAILABLE_BUILD_STEPS[operation] actual = len(args) if not step_has_valid_arg_count(args, expected): if type(expected) is int: - raise GenericExitError( + raise CFBSExitError( "The `%s` build step expects %d arguments, %d were given" % (step, expected, actual) ) else: expected = int(expected[0:-1]) - raise GenericExitError( + raise CFBSExitError( "The `%s` build step expects %d or more arguments, %d were given" % (step, expected, actual) ) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 67627d0d..e7ac18aa 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -7,8 +7,8 @@ from cfbs.result import Result from cfbs.utils import ( - GenericExitError, - UserError, + CFBSExitError, + CFBSUserError, read_file, write_json, load_bundlenames, @@ -101,9 +101,9 @@ def add_with_dependencies( module_str = module module = (remote_config or self).get_module_for_build(module, str_added_by) if not module: - raise GenericExitError("Module '%s' not found" % module_str) + raise CFBSExitError("Module '%s' not found" % module_str) if not module: - raise GenericExitError("Module '%s' not found" % str(module)) + raise CFBSExitError("Module '%s' not found" % str(module)) assert "name" in module name = module["name"] assert "steps" in module @@ -152,7 +152,7 @@ def _add_using_url( if len(to_add) == 0: modules = list(provides.values()) if not any(modules): - raise GenericExitError("no modules available, nothing to do") + raise CFBSExitError("no modules available, nothing to do") print("Found %d modules in '%s':" % (len(modules), url)) for m in modules: deps = m.get("dependencies", []) @@ -170,7 +170,7 @@ def _add_using_url( else: missing = [k for k in to_add if k not in provides] if missing: - raise GenericExitError("Missing modules: " + ", ".join(missing)) + raise CFBSExitError("Missing modules: " + ", ".join(missing)) modules = [provides[k] for k in to_add] for i, module in enumerate(modules, start=1): @@ -389,7 +389,7 @@ def add_command( checksum=None, ) -> Result: if not to_add: - raise UserError("Must specify at least one module to add") + raise CFBSUserError("Must specify at least one module to add") modules_in_build_key = self.get("build", []) assert type(modules_in_build_key) is list @@ -405,7 +405,7 @@ def add_command( else: # for this `if` to be valid, module names containing `://` should be illegal if "://" in to_add[0]: - raise UserError( + raise CFBSUserError( "URI scheme not supported. The supported URI schemes are: " + ", ".join(SUPPORTED_URI_SCHEMES) ) @@ -460,7 +460,7 @@ def input_command(self, module_name, input_data): def _check_keys(keys, input_data): for key in keys: if key not in input_data: - raise GenericExitError( + raise CFBSExitError( "Expected attribute '%s' in input definition: %s" % (key, pretty(input_data)) ) @@ -479,7 +479,7 @@ def _input_elements(subtype): for element in subtype: _check_keys(["type", "label", "question", "key"], element) if element["type"] != "string": - raise GenericExitError( + raise CFBSExitError( "Subtype of type '%s' not supported for type list" % element["type"] ) @@ -506,7 +506,7 @@ def _input_list(input_data): elif isinstance(subtype, dict): _check_keys(["type", "label", "question"], subtype) if subtype["type"] != "string": - raise GenericExitError( + raise CFBSExitError( "Subtype of type '%s' not supported for type list" % subtype["type"] ) @@ -519,7 +519,7 @@ def _input_list(input_data): ).lower() in ("yes", "y"): result.append(_input_string(subtype)) return result - raise GenericExitError( + raise CFBSExitError( "Expected the value of attribute 'subtype' to be a JSON list or object, not: %s" % pretty(input_data["subtype"]) ) @@ -533,8 +533,6 @@ def _input_list(input_data): elif definition["type"] == "list": definition["response"] = _input_list(definition) else: - raise GenericExitError( - "Unsupported input type '%s'" % definition["type"] - ) + raise CFBSExitError("Unsupported input type '%s'" % definition["type"]) return None diff --git a/cfbs/cfbs_json.py b/cfbs/cfbs_json.py index 4e649090..4ea011fc 100644 --- a/cfbs/cfbs_json.py +++ b/cfbs/cfbs_json.py @@ -4,7 +4,7 @@ from cfbs.index import Index from cfbs.pretty import pretty, TOP_LEVEL_KEYS, MODULE_KEYS -from cfbs.utils import read_json, GenericExitError +from cfbs.utils import read_json, CFBSExitError def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): @@ -16,7 +16,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): module = OrderedDict() module["name"] = name if "description" not in data: - raise GenericExitError( + raise CFBSExitError( "missing required key 'description' in module definition: %s" % pretty(data) ) module["description"] = data["description"] @@ -31,7 +31,7 @@ def _construct_provided_module(name, data, url, commit, added_by="cfbs add"): if "input" in data: module["input"] = data["input"] if "steps" not in data: - raise GenericExitError( + raise CFBSExitError( "missing required key 'steps' in module definition: %s" % pretty(data) ) module["steps"] = data["steps"] @@ -154,7 +154,7 @@ def get_provides(self, added_by="cfbs add"): modules = OrderedDict() assert self._data is not None if "provides" not in self._data: - raise GenericExitError( + raise CFBSExitError( "missing required key 'provides' in module definition: %s" % pretty(self._data) ) diff --git a/cfbs/commands.py b/cfbs/commands.py index 90f1f344..3b63c8e4 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -17,15 +17,15 @@ from cfbs.cfbs_json import CFBSJson from cfbs.updates import ModuleUpdates, update_module from cfbs.utils import ( - NetworkError, - UserError, + CFBSNetworkError, + CFBSUserError, cfbs_filename, is_cfbs_repo, read_json, - GenericExitError, + CFBSExitError, strip_right, pad_right, - ProgrammerError, + CFBSProgrammerError, get_json, write_json, rm, @@ -97,13 +97,13 @@ def get_command_names(): @cfbs_command("pretty") def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: if not filenames: - raise GenericExitError("Filenames missing for cfbs pretty command") + raise CFBSExitError("Filenames missing for cfbs pretty command") sorting_rules = CFBS_DEFAULT_SORTING_RULES if keep_order else None num_files = 0 for f in filenames: if not f or not f.endswith(".json"): - raise GenericExitError( + raise CFBSExitError( "cfbs pretty command can only be used with .json files, not '%s'" % os.path.basename(f) ) @@ -115,9 +115,9 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: else: pretty_file(f, sorting_rules) except FileNotFoundError: - raise GenericExitError("File '%s' not found" % f) + raise CFBSExitError("File '%s' not found" % f) except json.decoder.JSONDecodeError as ex: - raise GenericExitError("Error reading json file '{}': {}".format(f, ex)) + raise CFBSExitError("Error reading json file '{}': {}".format(f, ex)) if check: print("Would reformat %d file(s)" % num_files) return 1 if num_files > 0 else 0 @@ -127,7 +127,7 @@ def pretty_command(filenames: list, check: bool, keep_order: bool) -> int: @cfbs_command("init") def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if is_cfbs_repo(): - raise UserError("Already initialized - look at %s" % cfbs_filename()) + raise CFBSUserError("Already initialized - look at %s" % cfbs_filename()) name = prompt_user( non_interactive, @@ -275,7 +275,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: remote = "https://github.com/cfengine/masterfiles" commit = ls_remote(remote, branch) if commit is None: - raise GenericExitError( + raise CFBSExitError( "Failed to find branch or tag %s at remote %s" % (branch, remote) ) log.debug("Current commit for masterfiles branch %s is %s" % (branch, commit)) @@ -404,7 +404,7 @@ def remove_command(to_remove: List[str]): config = CFBSConfig.get_instance() config.warn_about_unknown_keys() if "build" not in config: - raise GenericExitError( + raise CFBSExitError( 'Cannot remove any modules because the "build" key is missing from cfbs.json' ) modules = config["build"] @@ -462,7 +462,7 @@ def _get_modules_by_url(name) -> list: if name.startswith(SUPPORTED_URI_SCHEMES): matches = _get_modules_by_url(name) if not matches: - raise GenericExitError("Could not find module with URL '%s'" % name) + raise CFBSExitError("Could not find module with URL '%s'" % name) for module in matches: answer = _remove_module_user_prompt(module) if answer.lower() in ("yes", "y"): @@ -706,9 +706,9 @@ def validate_command(paths=None, index_arg=None) -> int: for path in paths: # Exit out early if we find anything wrong like missing files: if not os.path.exists(path): - raise UserError("Specified path '{}' does not exist".format(path)) + raise CFBSUserError("Specified path '{}' does not exist".format(path)) if path.endswith(".json") and not os.path.isfile(path): - raise UserError( + raise CFBSUserError( "'{}' is not a file - Please specify a path to a cfbs project file, ending in .json, or a folder containing a cfbs.json".format( path ) @@ -716,7 +716,7 @@ def validate_command(paths=None, index_arg=None) -> int: if not path.endswith(".json") and not os.path.isfile( os.path.join(path, "cfbs.json") ): - raise UserError( + raise CFBSUserError( "No CFBS project file found at '{}'".format( os.path.join(path, "cfbs.json") ) @@ -741,8 +741,8 @@ def validate_command(paths=None, index_arg=None) -> int: return ret_value if not is_cfbs_repo(): - # TODO change GenericExitError to UserError here - raise GenericExitError( + # TODO change CFBSExitError to CFBSUserError here + raise CFBSExitError( "Cannot validate: this is not a CFBS project. " + "Use `cfbs init` to start a new project in this directory, or provide a path to a CFBS project to validate." ) @@ -767,10 +767,10 @@ def _download_dependencies( counter += 1 continue if "commit" not in module: - raise GenericExitError("module %s must have a commit property" % name) + raise CFBSExitError("module %s must have a commit property" % name) commit = module["commit"] if not is_a_commit_hash(commit): - raise GenericExitError("'%s' is not a commit reference" % commit) + raise CFBSExitError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] url = strip_right(url, ".git") @@ -784,7 +784,7 @@ def _download_dependencies( if not os.path.exists(module_dir): if url.endswith(SUPPORTED_ARCHIVES): if os.path.exists(commit_dir) and "subdirectory" in module: - raise GenericExitError( + raise CFBSExitError( "Subdirectory '%s' for module '%s' was not found in fetched archive '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -795,7 +795,7 @@ def _download_dependencies( # - added by URL instead of name (no version property in module data) elif "index" in module or "url" in module or ignore_versions: if os.path.exists(commit_dir) and "subdirectory" in module: - raise GenericExitError( + raise CFBSExitError( "Subdirectory '%s' for module '%s' was not found in cloned repository '%s': " % (module["subdirectory"], name, url) + "Please check cfbs.json for possible typos." @@ -805,14 +805,14 @@ def _download_dependencies( else: try: versions = get_json(_VERSION_INDEX) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) try: checksum = versions[name][module["version"]]["archive_sha256"] except KeyError: - raise GenericExitError( + raise CFBSExitError( "Cannot verify checksum of the '%s' module" % name ) module_archive_url = os.path.join( @@ -870,7 +870,7 @@ def build_command(ignore_versions=False) -> int: @cfbs_command("install") def install_command(args) -> int: if len(args) > 1: - raise GenericExitError( + raise CFBSExitError( "Only one destination is allowed for command: cfbs install [destination]" ) if not os.path.exists("out/masterfiles"): @@ -896,7 +896,7 @@ def install_command(args) -> int: @cfbs_command("help") def help_command(): - raise ProgrammerError("help_command should not be called, as we use argparse") + raise CFBSProgrammerError("help_command should not be called, as we use argparse") def _print_module_info(data): @@ -927,7 +927,7 @@ def _print_module_info(data): @cfbs_command("info") def info_command(modules): if not modules: - raise GenericExitError( + raise CFBSExitError( "info/show command requires one or more module names as arguments" ) config = CFBSConfig.get_instance() @@ -997,7 +997,7 @@ def analyze_command( ) if not os.path.isdir(path): - raise GenericExitError("the provided policy set path is not a directory") + raise CFBSExitError("the provided policy set path is not a directory") if masterfiles_dir is None: masterfiles_dir = "masterfiles" diff --git a/cfbs/index.py b/cfbs/index.py index 6dcf0570..45cc7c12 100644 --- a/cfbs/index.py +++ b/cfbs/index.py @@ -4,7 +4,7 @@ from typing import Union from cfbs.module import Module -from cfbs.utils import NetworkError, get_or_read_json, GenericExitError, get_json +from cfbs.utils import CFBSNetworkError, get_or_read_json, CFBSExitError, get_json from cfbs.internal_file_management import local_module_name _DEFAULT_INDEX = ( @@ -91,8 +91,8 @@ def _expand_index(self): try: self._data = get_or_read_json(index) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading index '%s' failed - check your Wi-Fi / network settings." % index ) @@ -132,8 +132,8 @@ def exists(self, module): return name in self try: versions = get_json(_VERSION_INDEX) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) @@ -143,7 +143,7 @@ def check_existence(self, modules: list): for module in modules: assert isinstance(module, Module) if not self.exists(module): - raise GenericExitError( + raise CFBSExitError( "Module '%s'%s does not exist" % ( module.name, @@ -179,8 +179,8 @@ def get_module_object(self, module, added_by=None): if version: try: versions = get_json(_VERSION_INDEX) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine Build Module Index failed - check your Wi-Fi / network settings." ) new_values = versions[name][version] diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 09273afc..a7df8a67 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -16,14 +16,14 @@ cfbs_dir, cp, fetch_url, - NetworkError, + CFBSNetworkError, is_a_commit_hash, mkdir, pad_right, rm, sh, strip_right, - GenericExitError, + CFBSExitError, ) _SUPPORTED_TAR_TYPES = (".tar.gz", ".tgz") @@ -38,13 +38,13 @@ def local_module_name(module_path): if module.endswith((".cf", ".json", "/")) and not module.startswith("./"): module = "./" + module if not module.startswith("./"): - raise GenericExitError( + raise CFBSExitError( "Please prepend local files or folders with './' to avoid ambiguity" ) for illegal in ["//", "..", " ", "\n", "\t", " "]: if illegal in module: - raise GenericExitError("Module path cannot contain %s" % repr(illegal)) + raise CFBSExitError("Module path cannot contain %s" % repr(illegal)) if os.path.isdir(module) and not module.endswith("/"): module = module + "/" @@ -54,10 +54,10 @@ def local_module_name(module_path): assert os.path.exists(module) if os.path.isfile(module): if not module.endswith((".cf", ".json")): - raise GenericExitError("Only .cf and .json files supported currently") + raise CFBSExitError("Only .cf and .json files supported currently") else: if not os.path.isdir(module): - raise GenericExitError("'%s' must be either a directory or a file" % module) + raise CFBSExitError("'%s' must be either a directory or a file" % module) return module @@ -67,7 +67,7 @@ def get_download_path(module) -> str: commit = module["commit"] if not is_a_commit_hash(commit): - raise GenericExitError("'%s' is not a commit reference" % commit) + raise CFBSExitError("'%s' is not a commit reference" % commit) url = module.get("url") or module["repo"] if url.endswith(SUPPORTED_ARCHIVES): @@ -94,9 +94,9 @@ def _prettify_name(name): def local_module_copy(module, counter, max_length): name = module["name"] if not name.startswith("./"): - raise GenericExitError("module %s must start with ./" % name) + raise CFBSExitError("module %s must start with ./" % name) if not os.path.isfile(name) and not os.path.isdir(name): - raise GenericExitError("module %s does not exist" % name) + raise CFBSExitError("module %s does not exist" % name) pretty_name = _prettify_name(name) target = "out/steps/%03d_%s_local/" % (counter, pretty_name) module["_directory"] = target @@ -118,7 +118,7 @@ def local_module_copy(module, counter, max_length): def _get_path_from_url(url): if not url.startswith(SUPPORTED_URI_SCHEMES): if "://" in url: - raise GenericExitError("Unsupported URL protocol in '%s'" % url) + raise CFBSExitError("Unsupported URL protocol in '%s'" % url) else: # It's a path already, just remove trailing slashes (if any). return url.rstrip("/") @@ -163,7 +163,7 @@ def clone_url_repo(repo_url): # commit specified in the url repo_url, commit = repo_url.rsplit("@", 1) if not is_a_commit_hash(commit): - raise GenericExitError("'%s' is not a commit reference" % commit) + raise CFBSExitError("'%s' is not a commit reference" % commit) downloads = os.path.join(cfbs_dir(), "downloads") @@ -190,7 +190,7 @@ def clone_url_repo(repo_url): if os.path.exists(json_path): return (json_path, commit) else: - raise GenericExitError( + raise CFBSExitError( "Repository '%s' doesn't contain a valid cfbs.json index file" % repo_url ) @@ -209,7 +209,7 @@ def fetch_archive( archive_type = ext break else: - raise GenericExitError("Unsupported archive type: '%s'" % url) + raise CFBSExitError("Unsupported archive type: '%s'" % url) downloads = os.path.join(cfbs_dir(), "downloads") @@ -220,8 +220,8 @@ def fetch_archive( archive_path = os.path.join(downloads, archive_dir, archive_filename) try: archive_checksum = fetch_url(url, archive_path, checksum) - except NetworkError as e: - raise GenericExitError(str(e)) + except CFBSNetworkError as e: + raise CFBSExitError(str(e)) content_dir = os.path.join(downloads, archive_dir, archive_checksum) if extract_to_directory: @@ -240,14 +240,12 @@ def fetch_archive( if shutil.which("tar"): sh("cd %s; tar -xf %s" % (content_dir, archive_path)) else: - raise GenericExitError( - "Working with .tar archives requires the 'tar' utility" - ) + raise CFBSExitError("Working with .tar archives requires the 'tar' utility") elif archive_type == (".zip"): if shutil.which("unzip"): sh("cd %s; unzip %s" % (content_dir, archive_path)) else: - raise GenericExitError( + raise CFBSExitError( "Working with .zip archives requires the 'unzip' utility" ) else: @@ -276,7 +274,7 @@ def fetch_archive( if os.path.exists(index_path): return (index_path, archive_checksum) else: - raise GenericExitError( + raise CFBSExitError( "Archive '%s' doesn't contain a valid cfbs.json index file" % url ) else: diff --git a/cfbs/main.py b/cfbs/main.py index 3cd5454c..2efbae59 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -12,11 +12,11 @@ from cfbs.version import string as version from cfbs.utils import ( CFBSValidationError, - GenericExitError, - UserError, + CFBSExitError, + CFBSUserError, is_cfbs_repo, - ProgrammerError, - NetworkError, + CFBSProgrammerError, + CFBSNetworkError, ) from cfbs.cfbs_config import CFBSConfig from cfbs import commands @@ -69,62 +69,62 @@ def _main() -> Union[int, Result]: if not args.command: print_help() print("") - raise UserError("No command given") + raise CFBSUserError("No command given") if args.command not in commands.get_command_names(): print_help() - raise UserError("Command '%s' not found" % args.command) + raise CFBSUserError("Command '%s' not found" % args.command) if args.masterfiles and args.command != "init": - raise UserError( + raise CFBSUserError( "The option --masterfiles is only for 'cfbs init', not 'cfbs %s'" % args.command ) if args.omit_download and args.command != "generate-release-information": - raise UserError( + raise CFBSUserError( "The option --omit-download is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.check_against_git and args.command != "generate-release-information": - raise UserError( + raise CFBSUserError( "The option --check-against-git is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.minimum_version and args.command != "generate-release-information": - raise UserError( + raise CFBSUserError( "The option --from is only for 'cfbs generate-release-information', not 'cfbs %s'" % args.command ) if args.masterfiles_dir and args.command not in ("analyze", "analyse"): - raise UserError( + raise CFBSUserError( "The option --masterfiles-dir is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.reference_version and args.command not in ("analyze", "analyse"): - raise UserError( + raise CFBSUserError( "The option --reference-version is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.to_json and args.command not in ("analyze", "analyse"): - raise UserError( + raise CFBSUserError( "The option --to-json is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.ignored_path_components and args.command not in ("analyze", "analyse"): - raise UserError( + raise CFBSUserError( "The option --ignored-path-components is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) if args.offline and args.command not in ("analyze", "analyse"): - raise UserError( + raise CFBSUserError( "The option --offline is only for 'cfbs analyze', not 'cfbs %s'" % args.command ) @@ -137,7 +137,7 @@ def _main() -> Union[int, Result]: "update", "input", ): - raise UserError( + raise CFBSUserError( "The option --non-interactive is not for cfbs %s" % (args.command) ) @@ -184,9 +184,7 @@ def _main() -> Union[int, Result]: # Commands you cannot run outside a cfbs repo: if not is_cfbs_repo(): - raise GenericExitError( - "This is not a cfbs repo, to get started, type: cfbs init" - ) + raise CFBSExitError("This is not a cfbs repo, to get started, type: cfbs init") if args.command == "status": return commands.status_command() @@ -244,7 +242,7 @@ def _main() -> Union[int, Result]: finally: file.close() - raise ProgrammerError( + raise CFBSProgrammerError( "Command '%s' not handled appropriately by the code above" % args.command ) @@ -266,13 +264,13 @@ def main() -> int: return r except CFBSValidationError as e: print("Error: " + str(e)) - except GenericExitError as e: + except CFBSExitError as e: print("Error: " + str(e)) - except UserError as e: + except CFBSUserError as e: print("Error: " + str(e)) - except NetworkError as e: + except CFBSNetworkError as e: print("Error: " + str(e)) - except (AssertionError, ProgrammerError) as e: + except (AssertionError, CFBSProgrammerError) as e: print("Error: " + str(e)) print( "This is an unexpected error indicating a bug, please create a ticket at:" diff --git a/cfbs/man_generator.py b/cfbs/man_generator.py index b33a1f07..25cac4dd 100644 --- a/cfbs/man_generator.py +++ b/cfbs/man_generator.py @@ -1,11 +1,11 @@ import os -from utils import UserError +from utils import CFBSUserError from args import get_arg_parser try: from build_manpages.manpage import Manpage # type: ignore except ImportError: - raise UserError( + raise CFBSUserError( "Missing dependency, install from PyPI: 'pip install argparse-manpage setuptools'" ) diff --git a/cfbs/masterfiles/check_download_matches_git.py b/cfbs/masterfiles/check_download_matches_git.py index 2f1e8278..1898b5f6 100644 --- a/cfbs/masterfiles/check_download_matches_git.py +++ b/cfbs/masterfiles/check_download_matches_git.py @@ -2,7 +2,7 @@ from collections import OrderedDict from cfbs.masterfiles.analyze import version_as_comparable_list -from cfbs.utils import dict_diff, read_json, GenericExitError, write_json +from cfbs.utils import dict_diff, read_json, CFBSExitError, write_json def check_download_matches_git(versions): @@ -84,7 +84,7 @@ def check_download_matches_git(versions): write_json("differences.json", diffs_dict) if len(nonmatching_versions) > 0: - raise GenericExitError( + raise CFBSExitError( "The masterfiles downloaded from github.com and cfengine.com do not match - found " + str(extraneous_count) + " extraneous file" diff --git a/cfbs/masterfiles/download_all_versions.py b/cfbs/masterfiles/download_all_versions.py index f0ac39e2..968a8cb2 100644 --- a/cfbs/masterfiles/download_all_versions.py +++ b/cfbs/masterfiles/download_all_versions.py @@ -2,7 +2,7 @@ import shutil from cfbs.masterfiles.analyze import version_is_at_least -from cfbs.utils import NetworkError, fetch_url, get_json, mkdir, GenericExitError +from cfbs.utils import CFBSNetworkError, fetch_url, get_json, mkdir, CFBSExitError ENTERPRISE_RELEASES_URL = "https://cfengine.com/release-data/enterprise/releases.json" @@ -15,8 +15,8 @@ def get_download_urls_enterprise(min_version=None): try: data = get_json(ENTERPRISE_RELEASES_URL) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine release data failed - check your Wi-Fi / network settings." ) @@ -45,8 +45,8 @@ def get_download_urls_enterprise(min_version=None): release_url = release_data["URL"] try: subdata = get_json(release_url) - except NetworkError: - raise GenericExitError( + except CFBSNetworkError: + raise CFBSExitError( "Downloading CFEngine release data for version %s failed - check your Wi-Fi / network settings." % version ) @@ -97,8 +97,8 @@ def download_versions_from_urls(download_path, download_urls, reported_checksums checksum = reported_checksums[version] try: fetch_url(url, tarball_path, checksum) - except NetworkError as e: - raise GenericExitError("For version " + version + ": " + str(e)) + except CFBSNetworkError as e: + raise CFBSExitError("For version " + version + ": " + str(e)) tarball_dir_path = os.path.join(version_path, "tarball") shutil.unpack_archive(tarball_path, tarball_dir_path) diff --git a/cfbs/updates.py b/cfbs/updates.py index 532be763..972f1167 100644 --- a/cfbs/updates.py +++ b/cfbs/updates.py @@ -3,7 +3,7 @@ import logging as log from cfbs.prompts import YES_NO_CHOICES, prompt_user -from cfbs.utils import read_json, GenericExitError, write_json +from cfbs.utils import read_json, CFBSExitError, write_json class ModuleUpdates: @@ -112,7 +112,7 @@ def _update_variable(input_def, input_data): def_subtype, data_subtype, ("label", "question", "default") ) else: - raise GenericExitError( + raise CFBSExitError( "Unsupported subtype '%s' in input definition for module '%s'." % (type(def_subtype).__name__, module_name) ) diff --git a/cfbs/utils.py b/cfbs/utils.py index 6fda5905..aa5d1249 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -18,7 +18,7 @@ SHA256_RE = re.compile(r"^[0-9a-f]{64}$") -class ProgrammerError(RuntimeError): +class CFBSProgrammerError(RuntimeError): """Exception to use for cases where we as developers made a mistake. Situations which should never happen - similar to assertions. @@ -27,7 +27,7 @@ class ProgrammerError(RuntimeError): pass -class GenericExitError(Exception): +class CFBSExitError(Exception): """Generic errors which make the program exit. Most of these should be converted to more specific exception types.""" @@ -35,13 +35,13 @@ class GenericExitError(Exception): pass -class UserError(Exception): +class CFBSUserError(Exception): """Exception for when the user did something wrong, such as specifying a file which does not exist.""" pass -class NetworkError(Exception): +class CFBSNetworkError(Exception): """Errors which generally can be attributed to a server our router being offline. Usually we'll advise the user to check their Wifi / network settings and/or try again later. @@ -92,9 +92,7 @@ def _sh(cmd: str): stderr=subprocess.STDOUT, ) except subprocess.CalledProcessError as e: - raise GenericExitError( - "Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8")) - ) + raise CFBSExitError("Command failed - %s\n%s" % (cmd, e.stdout.decode("utf-8"))) def sh(cmd: str, directory=None): @@ -154,7 +152,7 @@ def get_json(url: str) -> OrderedDict: assert r.status >= 200 and r.status < 300 return json.loads(r.read().decode(), object_pairs_hook=OrderedDict) except urllib.error.URLError as e: - raise NetworkError("Failed to get JSON from '%s'" % url) from e + raise CFBSNetworkError("Failed to get JSON from '%s'" % url) from e def get_or_read_json(path: str) -> Union[OrderedDict, None]: @@ -398,7 +396,7 @@ def fetch_url(url, target, checksum=None): elif SHA256_RE.match(checksum): sha = hashlib.sha256() else: - raise NetworkError( + raise CFBSNetworkError( "Invalid checksum or unsupported checksum algorithm: '%s'" % checksum ) else: @@ -413,7 +411,7 @@ def fetch_url(url, target, checksum=None): with open(target, "wb") as f: with urllib.request.urlopen(request) as u: if not (200 <= u.status <= 300): - raise NetworkError("Failed to fetch '%s': %s" % (url, u.reason)) + raise CFBSNetworkError("Failed to fetch '%s': %s" % (url, u.reason)) done = False while not done: chunk = u.read(512 * 1024) # 512 KiB @@ -429,7 +427,7 @@ def fetch_url(url, target, checksum=None): else: if os.path.exists(target): os.unlink(target) - raise NetworkError( + raise CFBSNetworkError( "Checksum mismatch in fetched '%s': %s != %s" % (url, digest, checksum) ) @@ -438,11 +436,13 @@ def fetch_url(url, target, checksum=None): except urllib.error.URLError as e: if os.path.exists(target): os.unlink(target) - raise NetworkError("Failed to fetch '%s': %s" % (url, e)) from e + raise CFBSNetworkError("Failed to fetch '%s': %s" % (url, e)) from e except OSError as e: if os.path.exists(target): os.unlink(target) - raise NetworkError("Failed to fetch '%s' to '%s': %s" % (url, target, e)) from e + raise CFBSNetworkError( + "Failed to fetch '%s' to '%s': %s" % (url, target, e) + ) from e def is_a_commit_hash(commit): diff --git a/cfbs/validate.py b/cfbs/validate.py index bf2d39b1..c9e6678a 100644 --- a/cfbs/validate.py +++ b/cfbs/validate.py @@ -31,7 +31,7 @@ is_a_commit_hash, strip_left, strip_right_any, - GenericExitError, + CFBSExitError, CFBSValidationError, ) from cfbs.pretty import TOP_LEVEL_KEYS, MODULE_KEYS @@ -684,12 +684,12 @@ def validate_module_input(name, module): def _validate_config_for_build_field(config, empty_build_list_ok=False): """Validate that neccessary fields are in the config for the build/download commands to work""" if "build" not in config: - raise GenericExitError( + raise CFBSExitError( 'A "build" field is missing in ./cfbs.json' + " - The 'cfbs build' command loops through all modules in this list to find build steps to perform" ) if type(config["build"]) is not list: - raise GenericExitError( + raise CFBSExitError( 'The "build" field in ./cfbs.json must be a list (of modules involved in the build)' ) if len(config["build"]) > 0: @@ -698,7 +698,7 @@ def _validate_config_for_build_field(config, empty_build_list_ok=False): name = module["name"] if "name" in module else index _validate_module_object("build", name, module, config) elif not empty_build_list_ok: - raise GenericExitError( + raise CFBSExitError( "The \"build\" field in ./cfbs.json is empty - add modules with 'cfbs add'" ) From 0061c8641e312aea67834934ba54670db8673a22 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 11 Jul 2025 16:27:51 +0200 Subject: [PATCH 8/8] Fixed typos based on review comments Signed-off-by: Ole Herman Schumacher Elgesem --- cfbs/commands.py | 2 +- cfbs/utils.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 3b63c8e4..fd116dd3 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -728,7 +728,7 @@ def validate_command(paths=None, index_arg=None) -> int: path = os.path.join(path, "cfbs.json") assert os.path.isfile(path) - # Actually opeb the file and perform validation: + # Actually open the file and perform validation: config = CFBSJson(path=path, index_argument=index_arg) r = validate_config(config) diff --git a/cfbs/utils.py b/cfbs/utils.py index aa5d1249..b79280f3 100644 --- a/cfbs/utils.py +++ b/cfbs/utils.py @@ -42,7 +42,7 @@ class CFBSUserError(Exception): class CFBSNetworkError(Exception): - """Errors which generally can be attributed to a server our router being offline. + """Errors which generally can be attributed to a server or router being offline. Usually we'll advise the user to check their Wifi / network settings and/or try again later. """