From a4cec703932e0631ffb49e901ab1cbf140de85ea Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 23 Oct 2025 11:22:41 +0200 Subject: [PATCH 1/8] Renamed git-checkout variable This function could be used not just for commits but also for branches or tags, and so the current name is not precisely correct Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/internal_file_management.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 666bae48..5dc960d1 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -148,11 +148,11 @@ def _get_git_repo_commit_sha(repo_path): return f.read().strip() -def _clone_and_checkout(url, path, commit): +def _clone_and_checkout(url, path, treeish): # NOTE: If any of these shell (git) commands fail, we will exit if not os.path.exists(os.path.join(path, ".git")): sh("git clone --no-checkout %s %s" % (url, path)) - sh("git checkout " + commit, directory=path) + sh("git checkout " + treeish, directory=path) def clone_url_repo(repo_url: str): From 71cef82ee94be447774d3459b689dd071a4714f4 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 23 Oct 2025 11:42:01 +0200 Subject: [PATCH 2/8] Removed outdated doc and redundant logic The comment is outdated after b451f92a5b20cecdf6b352095853e982d20267d4 Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/commands.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cfbs/commands.py b/cfbs/commands.py index 8dab3250..2bcd8bbb 100644 --- a/cfbs/commands.py +++ b/cfbs/commands.py @@ -262,7 +262,6 @@ def init_command( """ CFBSConfig.reload() - branch = None to_add = [] if masterfiles is None: if prompt_user_yesno( @@ -282,16 +281,8 @@ def init_command( log.debug("--masterfiles=%s appears to be a version number" % masterfiles) to_add = ["masterfiles@%s" % masterfiles] elif masterfiles != "no": - """This appears to be a branch. Thus we'll add masterfiles normally - and try to do the necessary modifications needed afterwards. I.e. - changing the 'repo' attribute to be 'url' and changing the commit to - be the current HEAD of the upstream branch.""" - log.debug("--masterfiles=%s appears to be a branch" % masterfiles) branch = masterfiles - to_add = ["masterfiles"] - - if branch is not None: remote = "https://github.com/cfengine/masterfiles" commit = ls_remote(remote, branch) if commit is None: From 86f61e903cefe875d29cf4f8679b83ba98ac24d2 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Thu, 23 Oct 2025 12:05:12 +0200 Subject: [PATCH 3/8] Fixed a throw when more than one version is given for a module in cfbs commands Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/module.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cfbs/module.py b/cfbs/module.py index 6f8875eb..9106945c 100644 --- a/cfbs/module.py +++ b/cfbs/module.py @@ -1,6 +1,7 @@ from collections import OrderedDict from cfbs.pretty import pretty +from cfbs.utils import CFBSUserError def is_module_added_manually(added_by: str): @@ -24,6 +25,10 @@ def __init__( """Initialize from argument with format `NAME[@VERSION]`""" assert isinstance(arg, str) if "@" in arg: + if arg.count("@") > 1: + raise CFBSUserError( + "Cannot specify more than one version of the same module" + ) self.name, self.version = arg.split("@") else: self.name = arg From e52650f7f0abedc53091bce01449af047ae01670 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 31 Oct 2025 14:18:37 +0100 Subject: [PATCH 4/8] Rewrote `clone_url_repo` to take a URL and commit as separate arguments Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/cfbs_config.py | 13 +++++++++---- cfbs/internal_file_management.py | 11 +++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index ae53415b..ac389d35 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -25,6 +25,7 @@ from cfbs.utils import ( CFBSExitError, CFBSUserError, + is_a_commit_hash, read_file, write_json, load_bundlenames, @@ -154,11 +155,15 @@ def _add_using_url( config_path, url_commit = fetch_archive(url, checksum) else: assert url.startswith(SUPPORTED_URI_SCHEMES) - config_path, url_commit = clone_url_repo(url) - if "@" in url and (url.rindex("@") > url.rindex(".")): - assert url.split("@")[-1] == url_commit - url = url[0 : url.rindex("@")] + commit = None + if "@" in url and (url.rindex("@") > url.rindex(".")): + # commit specified in the url + url, commit = url.rsplit("@", 1) + if not is_a_commit_hash(commit): + raise CFBSExitError("'%s' is not a commit reference" % commit) + + config_path, url_commit = clone_url_repo(url, commit) remote_config = CFBSJson(path=config_path, url=url, url_commit=url_commit) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 5dc960d1..b004b632 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -11,6 +11,7 @@ import os import re import shutil +from typing import Optional from cfbs.utils import ( cfbs_dir, @@ -155,15 +156,9 @@ def _clone_and_checkout(url, path, treeish): sh("git checkout " + treeish, directory=path) -def clone_url_repo(repo_url: str): +def clone_url_repo(repo_url: str, commit: Optional[str] = None): assert repo_url.startswith(SUPPORTED_URI_SCHEMES) - - commit = None - if "@" in repo_url and (repo_url.rindex("@") > repo_url.rindex(".")): - # commit specified in the url - repo_url, commit = repo_url.rsplit("@", 1) - if not is_a_commit_hash(commit): - raise CFBSExitError("'%s' is not a commit reference" % commit) + assert "@" not in repo_url or (repo_url.rindex("@") < repo_url.rindex(".")) downloads = os.path.join(cfbs_dir(), "downloads") From a63494b6fcfa0ad8268bec8105d4ac33e3425d43 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 31 Oct 2025 14:21:51 +0100 Subject: [PATCH 5/8] Documented the "`url`" argument in `_add_using_url` Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/cfbs_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index ac389d35..62089291 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -150,6 +150,7 @@ def _add_using_url( checksum=None, explicit_build_steps=None, ): + """The `url` argument can optionally also have the form `@`.""" url_commit = None if url.endswith(SUPPORTED_ARCHIVES): config_path, url_commit = fetch_archive(url, checksum) From 8467104f2b9751085a67bf01783dd34153c368d1 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 31 Oct 2025 14:23:01 +0100 Subject: [PATCH 6/8] Disallowed specifying multiple commits when adding modules by URL Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/cfbs_config.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index 62089291..6a98c99b 100644 --- a/cfbs/cfbs_config.py +++ b/cfbs/cfbs_config.py @@ -161,6 +161,10 @@ def _add_using_url( if "@" in url and (url.rindex("@") > url.rindex(".")): # commit specified in the url url, commit = url.rsplit("@", 1) + if "@" in url and (url.rindex("@") > url.rindex(".")): + raise CFBSUserError( + "Cannot specify more than one commit for one add URL" + ) if not is_a_commit_hash(commit): raise CFBSExitError("'%s' is not a commit reference" % commit) From 72b2586de26a9b50b5c33f2ae3c729d45559f2c4 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 31 Oct 2025 14:45:28 +0100 Subject: [PATCH 7/8] Documented how `ls_remote` does error handling Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/git.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cfbs/git.py b/cfbs/git.py index 335515bb..c46ee679 100644 --- a/cfbs/git.py +++ b/cfbs/git.py @@ -33,6 +33,7 @@ def git_exists(): def ls_remote(remote, branch): """Returns the hash of the commit that the current HEAD of a given branch on a given remote is pointing to. + Returns `None` in case of error (e.g. the branch does not exist). :param remote: the remote to list :param branch: the branch on the remote From a718040666a461656992983574b5cf6b27257d83 Mon Sep 17 00:00:00 2001 From: jakub-nt <175944085+jakub-nt@users.noreply.github.com> Date: Fri, 31 Oct 2025 15:01:36 +0100 Subject: [PATCH 8/8] Documented the `clone_url_repo` function Signed-off-by: jakub-nt <175944085+jakub-nt@users.noreply.github.com> --- cfbs/internal_file_management.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index b004b632..dcb0f4e2 100644 --- a/cfbs/internal_file_management.py +++ b/cfbs/internal_file_management.py @@ -157,6 +157,10 @@ def _clone_and_checkout(url, path, treeish): def clone_url_repo(repo_url: str, commit: Optional[str] = None): + """Clones a Git repository at `repo_url` URL, optionally checking out the `commit` commit. + + Returns path to the `cfbs.json` located in the cloned Git repository, and the Git commit hash. + """ assert repo_url.startswith(SUPPORTED_URI_SCHEMES) assert "@" not in repo_url or (repo_url.rindex("@") < repo_url.rindex("."))