From a20fe4e3f7a958c41605d2cd7d7ee5bcd27ad1b3 Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Fri, 20 Mar 2026 15:52:18 +0100 Subject: [PATCH] Rough Draft of support for field `test_version` in component spec Commodore will first try to checkout `test_version` for components who set the field, and fall back to field `version` if the version given in `test_version` can't be checked out and isn't equal to `version`. This feature might be useful on development or Lab clusters where many users test various component feature branches, and generally the absence of a configured version indicates that the feature branch was merged and the component's main development branch (usually `master`) should be used again. Note that we probably should make this feature opt-in via a cluster fact or field in `parameters.commodore`. --- commodore/dependency_mgmt/__init__.py | 22 ++++++++++++++----- commodore/dependency_mgmt/version_parsing.py | 9 +++++++- .../lint_dependency_specification.py | 2 +- tests/test_dependency_mgmt.py | 16 +++++++------- tests/test_dependency_mgmt_version_parsing.py | 14 ++++++------ 5 files changed, 41 insertions(+), 22 deletions(-) diff --git a/commodore/dependency_mgmt/__init__.py b/commodore/dependency_mgmt/__init__.py index 1c673323e..dbca0abad 100644 --- a/commodore/dependency_mgmt/__init__.py +++ b/commodore/dependency_mgmt/__init__.py @@ -103,7 +103,7 @@ def fetch_components(cfg: Config): cn, work_dir=cfg.work_dir, dependency=cdep, - version=cspec.version, + version=cspec.test_version or cspec.version, sub_path=cspec.path, ) if c.checkout_is_dirty() and not cfg.force: @@ -111,7 +111,8 @@ def fetch_components(cfg: Config): f"Component {cn} has uncommitted changes. " + "Please specify `--force` to discard them" ) - deps.setdefault(cdep.url, []).append(c) + ci = (c, cspec.version) + deps.setdefault(cdep.url, []).append(ci) do_parallel(fetch_component, cfg, deps.values()) _setup_component_aliases(cfg, component_aliases, cspecs, set(deps.keys())) @@ -155,15 +156,26 @@ def _setup_component_aliases( do_parallel(setup_alias, cfg, aliases.values()) -def fetch_component(cfg: Config, dependencies: Iterable[Component]): +def fetch_component(cfg: Config, dependencies: Iterable[tuple[Component, str]]): """ Fetch all components of a MultiDependency object. """ - for c in dependencies: + for c, fallback_version in dependencies: try: c.checkout() except RefError as e: - raise ClickException(f"while fetching component {c.name}: {e}") + if c.version == fallback_version: + raise ClickException(f"while fetching component {c.name}: {e}") + else: + click.echo( + f" > Unable to checkout version '{c.version}' for component '{c.name}', " + + f"trying again with version '{fallback_version}'" + ) + c.version = fallback_version + try: + c.checkout() + except RefError as e: + raise ClickException(f"while fetching component {c.name}: {e}") cfg.register_component(c) create_component_symlinks(cfg, c) diff --git a/commodore/dependency_mgmt/version_parsing.py b/commodore/dependency_mgmt/version_parsing.py index 5318a901f..265f91e19 100644 --- a/commodore/dependency_mgmt/version_parsing.py +++ b/commodore/dependency_mgmt/version_parsing.py @@ -31,6 +31,7 @@ class DependencySpec: url: str version: str + test_version: Optional[str] path: str @classmethod @@ -52,13 +53,15 @@ def parse( if base_config: url = info.get("url", base_config.url) version = info.get("version", base_config.version) + test_version = info.get("test_version", base_config.test_version) if not path: path = base_config.path else: url = info["url"] version = info["version"] + test_version = info.get("test_version") - return DependencySpec(url, version, path) + return DependencySpec(url, version, test_version, path) def _read_versions( @@ -112,6 +115,10 @@ def _read_versions( if cfg.debug: click.echo(f" > URL for {depname}: {dep.url}") click.echo(f" > Version for {depname}: {dep.version}") + if dep.test_version: + click.echo( + f" > Test (override) version for {depname}: {dep.test_version}" + ) click.echo(f" > Subpath for {depname}: {dep.path}") dependencies[depname] = dep diff --git a/commodore/inventory/lint_dependency_specification.py b/commodore/inventory/lint_dependency_specification.py index c6145ed7e..9427fc09c 100644 --- a/commodore/inventory/lint_dependency_specification.py +++ b/commodore/inventory/lint_dependency_specification.py @@ -30,7 +30,7 @@ def lint_dependency_specification( ) errcount += 1 - unk_keys = set(dspec.keys()) - {"url", "version", "path"} + unk_keys = set(dspec.keys()) - {"url", "version", "test_version", "path"} if len(unk_keys) > 0: click.secho( f"> {deptype_str} specification for {d} " diff --git a/tests/test_dependency_mgmt.py b/tests/test_dependency_mgmt.py index 435d6fc63..a270bee20 100644 --- a/tests/test_dependency_mgmt.py +++ b/tests/test_dependency_mgmt.py @@ -73,7 +73,7 @@ def _prepare_repository( repo.index.add([f"{class_path}/defaults.yml", f"{class_path}/{component_name}.yml"]) repo.index.commit("component defaults") - return DependencySpec(url, version, sub_path) + return DependencySpec(url, version, None, sub_path) def test_create_component_symlinks_fails(config: Config, tmp_path: Path, mockdep): @@ -365,7 +365,7 @@ def test_register_components( component_dirs, other_dirs = _setup_register_components(tmp_path) patch_discover.return_value = (component_dirs, {}) patch_read.return_value = { - cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "") + cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "") for cn in component_dirs } @@ -389,7 +389,7 @@ def test_register_components_and_aliases( ) patch_discover.return_value = (component_dirs, alias_data) patch_read.return_value = { - cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "") + cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "") for cn in component_dirs } patch_read.return_value["fooer"] = patch_read.return_value["foo"] @@ -420,7 +420,7 @@ def test_register_components_and_aliases_raises( component_dirs, other_dirs = _setup_register_components(tmp_path) patch_discover.return_value = (component_dirs, alias_data) patch_read.return_value = { - cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "") + cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "") for cn in component_dirs } patch_read.return_value["fooer"] = patch_read.return_value["foo"] @@ -441,7 +441,7 @@ def test_register_unknown_components( component_dirs.extend(unknown_components) patch_discover.return_value = (component_dirs, {}) patch_read.return_value = { - cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "") + cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "") for cn in component_dirs } @@ -469,7 +469,7 @@ def test_register_dangling_aliases( patch_discover.return_value = (component_dirs, alias_data) patch_read.return_value = { - cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "") + cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "") for cn in component_dirs } patch_read.return_value["bazzer"] = patch_read.return_value["baz"] @@ -714,7 +714,7 @@ def _setup_packages( _setup_package_remote(p, upstream_path / f"{p}.git") url = f"file://{upstream_path}/{p}.git" version = "master" - package_specs[p] = DependencySpec(url, version, "") + package_specs[p] = DependencySpec(url, version, None, "") return package_specs @@ -833,7 +833,7 @@ def test_fetch_component_raises_clickexception(tmp_path: Path, config: Config): version=cspec.version, ) with pytest.raises(click.ClickException) as exc: - dependency_mgmt.fetch_component(config, [component]) + dependency_mgmt.fetch_component(config, [(component, cspec.version)]) assert ( "while fetching component test-component: Failed to checkout revision 'foo'" diff --git a/tests/test_dependency_mgmt_version_parsing.py b/tests/test_dependency_mgmt_version_parsing.py index bfc340649..2e46f97cd 100644 --- a/tests/test_dependency_mgmt_version_parsing.py +++ b/tests/test_dependency_mgmt_version_parsing.py @@ -144,7 +144,7 @@ def test_read_components_exc( ["test"], { "test": version_parsing.DependencySpec( - "https://git.example.com/pkg.git", "v1.0.0", "" + "https://git.example.com/pkg.git", "v1.0.0", None, "" ), }, ), @@ -153,10 +153,10 @@ def test_read_components_exc( ["test", "foo"], { "test": version_parsing.DependencySpec( - "https://git.example.com/pkg.git", "v1.0.0", "" + "https://git.example.com/pkg.git", "v1.0.0", None, "" ), "foo": version_parsing.DependencySpec( - "https://git.example.com/foo.git", "feat/initial", "" + "https://git.example.com/foo.git", "feat/initial", None, "" ), }, ), @@ -165,16 +165,16 @@ def test_read_components_exc( ["test", "foo", "bar", "baz"], { "test": version_parsing.DependencySpec( - "https://git.example.com/pkg.git", "v1.0.0", "" + "https://git.example.com/pkg.git", "v1.0.0", None, "" ), "foo": version_parsing.DependencySpec( - "https://git.example.com/foo.git", "feat/initial", "" + "https://git.example.com/foo.git", "feat/initial", None, "" ), "bar": version_parsing.DependencySpec( - "https://git.example.com/barbaz.git", "master", "bar" + "https://git.example.com/barbaz.git", "master", None, "bar" ), "baz": version_parsing.DependencySpec( - "https://git.example.com/barbaz.git", "master", "baz" + "https://git.example.com/barbaz.git", "master", None, "baz" ), }, ),