From 3ad812dfef883ea85ca9bf2d437e08b0cc0af3ee Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Tue, 22 Jul 2025 13:57:24 +0200 Subject: [PATCH 1/7] Made a variable name less ambiguous Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index a7e80f08..37d3255a 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -173,7 +173,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if is_cfbs_repo(): raise CFBSUserError("Already initialized - look at %s" % cfbs_filename()) - name = prompt_user( + project_name = prompt_user( non_interactive, "Please enter the name of this CFEngine Build project", default="Example project", @@ -186,7 +186,7 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: config = OrderedDict( { - "name": name, + "name": project_name, "type": "policy-set", # TODO: Prompt whether user wants to make a module "description": description, "build": [], @@ -269,7 +269,9 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: return 1 print( - "Initialized an empty project called '{}' in '{}'".format(name, cfbs_filename()) + "Initialized an empty project called '{}' in '{}'".format( + project_name, cfbs_filename() + ) ) """ From 7eda8e46bf41e7d4c28364822098a77af8409985 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 13:26:58 +0200 Subject: [PATCH 2/7] Renamed variable Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 37d3255a..3e895a69 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -195,24 +195,24 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if index: config["index"] = index - do_git = get_args().git + use_git = get_args().git is_git = is_git_repo() - if do_git is None: + if use_git is None: if is_git: - do_git = prompt_user_yesno( + use_git = prompt_user_yesno( non_interactive, "This is a git repository. Do you want cfbs to make commits to it?", ) else: - do_git = prompt_user_yesno( + use_git = prompt_user_yesno( non_interactive, "Do you want cfbs to initialize a git repository and make commits to it?", ) else: - assert do_git in ("yes", "no") - do_git = True if do_git == "yes" else False + assert use_git in ("yes", "no") + use_git = True if use_git == "yes" else False - if do_git is True: + if use_git is True: if not git_exists(): print("Command 'git' was not found") return 1 @@ -249,14 +249,14 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: print("Failed to set Git user name and email") return 1 - config["git"] = do_git + config["git"] = use_git data = pretty(config, CFBS_DEFAULT_SORTING_RULES) + "\n" with open(cfbs_filename(), "w") as f: f.write(data) assert is_cfbs_repo() - if do_git: + if use_git: try: git_commit_maybe_prompt( "Initialized a new CFEngine Build project", From a4f55f54b0edb64704a29760179e74788a9274f7 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:21:04 +0200 Subject: [PATCH 3/7] Yes-no command-line arguments are now stored as booleans Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/args.py | 18 +++++++++++++++++- cfbs/commands.py | 3 --- cfbs/git_magic.py | 4 ++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cfbs/args.py b/cfbs/args.py index edd4bd87..bcf5271f 100644 --- a/cfbs/args.py +++ b/cfbs/args.py @@ -20,7 +20,7 @@ class ArgsTypesNamespace(argparse.Namespace): check: bool checksum: Union[str, None] keep_order: bool - git: Union[str, None] + git: Union[bool, None] git_user_name: Union[str, None] git_user_email: Union[str, None] git_commit_message: Union[str, None] @@ -63,6 +63,21 @@ def get_manual(): raise CFBSExitError("Manual file does not exist") +def yesno_to_bool(s: str): + if s == "yes": + return True + if s == "no": + return False + assert False + + +class YesNoToBool(argparse.Action): + def __call__(self, parser, namespace, values, option_string=None): + assert type(values) is str + values = yesno_to_bool(values) + setattr(namespace, self.dest, values) + + @cache def get_arg_parser(): command_list = commands.get_command_names() @@ -116,6 +131,7 @@ def get_arg_parser(): parser.add_argument( "--git", choices=("yes", "no"), + action=YesNoToBool, help="Override git option in cfbs.json", ) parser.add_argument( diff --git a/cfbs/commands.py b/cfbs/commands.py index 3e895a69..9d388351 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -208,9 +208,6 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: non_interactive, "Do you want cfbs to initialize a git repository and make commits to it?", ) - else: - assert use_git in ("yes", "no") - use_git = True if use_git == "yes" else False if use_git is True: if not git_exists(): diff --git a/cfbs/git_magic.py b/cfbs/git_magic.py index efb3f309..c6c9c51a 100644 --- a/cfbs/git_magic.py +++ b/cfbs/git_magic.py @@ -98,13 +98,13 @@ def decorated_fn(*args, **kwargs): if not should_commit: return ret - if do_git == "yes": + if do_git is True: if not is_git_repo(): log.error( "Used '--git=yes' option on what appears to not be a git repository" ) return ret - elif do_git == "no": + elif do_git is False: return ret else: assert do_git is None From 0c70eb3d97f729692d91219abd805d58b719871b Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:26:44 +0200 Subject: [PATCH 4/7] Allow configuring whether to use Git in the CFBS project initialization code Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 8 ++++++-- cfbs/main.py | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 9d388351..8260f22b 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -169,7 +169,12 @@ 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: +def init_command( + index=None, + masterfiles=None, + non_interactive=False, + use_git: Union[bool, None] = None, +) -> int: if is_cfbs_repo(): raise CFBSUserError("Already initialized - look at %s" % cfbs_filename()) @@ -195,7 +200,6 @@ def init_command(index=None, masterfiles=None, non_interactive=False) -> int: if index: config["index"] = index - use_git = get_args().git is_git = is_git_repo() if use_git is None: if is_git: diff --git a/cfbs/main.py b/cfbs/main.py index d21f2f44..c8e3211a 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -163,6 +163,7 @@ def _main() -> Union[int, Result]: index=args.index, masterfiles=args.masterfiles, non_interactive=args.non_interactive, + use_git=args.git, ) if args.command == "search": From 90fbf31ac141eb49a621492e58a6d29762dd7099 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:43:20 +0200 Subject: [PATCH 5/7] Move Git initialization code out of `commands.py` This improves modularity (useful in case Git initialization needs to also be done elsewhere), and helps deflate the currently overgrown `commands.py` file Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 46 +++++++--------------------------------------- cfbs/git.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 39 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 8260f22b..b74bba79 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -100,11 +100,8 @@ def search_command(terms: List[str]) -> int: ) from cfbs.index import _VERSION_INDEX, Index from cfbs.git import ( - git_exists, + git_configure_and_initialize, is_git_repo, - git_get_config, - git_set_config, - git_init, CFBSGitError, ls_remote, ) @@ -200,9 +197,8 @@ def init_command( if index: config["index"] = index - is_git = is_git_repo() if use_git is None: - if is_git: + if is_git_repo(): use_git = prompt_user_yesno( non_interactive, "This is a git repository. Do you want cfbs to make commits to it?", @@ -214,41 +210,13 @@ def init_command( ) if use_git is True: - if not git_exists(): - print("Command 'git' was not found") - return 1 - user_name = get_args().git_user_name - if not user_name: - user_name = git_get_config("user.name") - user_name = prompt_user( - non_interactive, - "Please enter user name to use for git commits", - default=user_name or "cfbs", - ) - user_email = get_args().git_user_email - if not user_email: - user_email = git_get_config("user.email") - node_name = os.uname().nodename - user_email = prompt_user( - non_interactive, - "Please enter user email to use for git commits", - default=user_email or ("cfbs@%s" % node_name), - ) - - if not is_git: - try: - git_init(user_name, user_email, description) - except CFBSGitError as e: - print(str(e)) - return 1 - else: - if not git_set_config("user.name", user_name) or not git_set_config( - "user.email", user_email - ): - print("Failed to set Git user name and email") - return 1 + r = git_configure_and_initialize( + user_name, user_email, non_interactive, description + ) + if r != 0: + return r config["git"] = use_git diff --git a/cfbs/git.py b/cfbs/git.py index 9bf77e54..93798a9f 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -14,6 +14,7 @@ from subprocess import check_call, check_output, run, PIPE, DEVNULL, CalledProcessError from typing import Iterable, Union +from cfbs.prompts import prompt_user from cfbs.utils import are_paths_equal @@ -115,6 +116,46 @@ def git_init(user_name=None, user_email=None, description=None, initial_branch=" f.write(description + "\n") +def git_configure_and_initialize( + user_name=None, user_email=None, non_interactive=False, description=None +): + if not git_exists(): + print("Command 'git' was not found") + return 1 + + if not user_name: + user_name = git_get_config("user.name") + user_name = prompt_user( + non_interactive, + "Please enter user name to use for git commits", + default=user_name or "cfbs", + ) + + if not user_email: + user_email = git_get_config("user.email") + node_name = os.uname().nodename + user_email = prompt_user( + non_interactive, + "Please enter user email to use for git commits", + default=user_email or ("cfbs@%s" % node_name), + ) + + if not is_git_repo(): + try: + git_init(user_name, user_email, description) + except CFBSGitError as e: + print(str(e)) + return 1 + else: + if not git_set_config("user.name", user_name) or not git_set_config( + "user.email", user_email + ): + print("Failed to set Git user name and email") + return 1 + + return 0 + + def git_commit( commit_msg, edit_commit_msg=False, From ef31a0cca7e9f43ffe3da3d3cedd9b06f5dea6eb Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 14:56:06 +0200 Subject: [PATCH 6/7] Rewrite a function to propagate errors using exceptions instead of printing and returning 0 or 1 Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 4 +--- cfbs/git.py | 16 ++++------------ cfbs/main.py | 4 ++++ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index b74bba79..d43462b6 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -212,11 +212,9 @@ def init_command( if use_git is True: user_name = get_args().git_user_name user_email = get_args().git_user_email - r = git_configure_and_initialize( + git_configure_and_initialize( user_name, user_email, non_interactive, description ) - if r != 0: - return r config["git"] = use_git diff --git a/cfbs/git.py b/cfbs/git.py index 93798a9f..1f9ff9e6 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -15,7 +15,7 @@ from typing import Iterable, Union from cfbs.prompts import prompt_user -from cfbs.utils import are_paths_equal +from cfbs.utils import CFBSExitError, are_paths_equal class CFBSGitError(Exception): @@ -120,8 +120,7 @@ def git_configure_and_initialize( user_name=None, user_email=None, non_interactive=False, description=None ): if not git_exists(): - print("Command 'git' was not found") - return 1 + raise CFBSExitError("Command 'git' was not found") if not user_name: user_name = git_get_config("user.name") @@ -141,19 +140,12 @@ def git_configure_and_initialize( ) if not is_git_repo(): - try: - git_init(user_name, user_email, description) - except CFBSGitError as e: - print(str(e)) - return 1 + git_init(user_name, user_email, description) else: if not git_set_config("user.name", user_name) or not git_set_config( "user.email", user_email ): - print("Failed to set Git user name and email") - return 1 - - return 0 + raise CFBSExitError("Failed to set Git user name and email") def git_commit( diff --git a/cfbs/main.py b/cfbs/main.py index c8e3211a..d78fd513 100644 --- a/cfbs/main.py +++ b/cfbs/main.py @@ -10,6 +10,7 @@ import pathlib from typing import Union +from cfbs.git import CFBSGitError from cfbs.result import Result from cfbs.validate import validate_index_string from cfbs.version import string as version @@ -285,6 +286,9 @@ def main() -> int: except CFBSNetworkError as e: print("Error: " + str(e)) return 1 + except CFBSGitError as e: + print("Error: " + str(e)) + return 1 # AssertionError and CFBSProgrammerError are not expected, print extra info: except AssertionError as e: tb = traceback.extract_tb(e.__traceback__) From 79933af11b9e47cefecf2ad7ae0d0b9e6ffd69d3 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Wed, 23 Jul 2025 15:00:00 +0200 Subject: [PATCH 7/7] Use exceptions instead of printing strings and returning integers Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index d43462b6..56ea6a68 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -230,10 +230,9 @@ def init_command( non_interactive, [cfbs_filename()], ) - except CFBSGitError as e: - print(str(e)) + except CFBSGitError: os.unlink(cfbs_filename()) - return 1 + raise print( "Initialized an empty project called '{}' in '{}'".format(