diff --git a/cfbs/cfbs_config.py b/cfbs/cfbs_config.py index ae53415b..6a98c99b 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, @@ -149,16 +150,25 @@ 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) 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 "@" 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) + + 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/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: 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 diff --git a/cfbs/internal_file_management.py b/cfbs/internal_file_management.py index 666bae48..dcb0f4e2 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, @@ -148,22 +149,20 @@ 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): - assert repo_url.startswith(SUPPORTED_URI_SCHEMES) +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. - 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) + 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(".")) downloads = os.path.join(cfbs_dir(), "downloads") 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