From 856e8e4891abb1252b1812922039272c631cc817 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 21:30:33 +0200 Subject: [PATCH 01/12] Replace deprecated semver.Spec with semver.SimpleSpec in package.py `semver.Spec` raises `PendingDeprecationWarning` as of semantic-version 2.10 and will be removed in 3.1. `semver.SimpleSpec` is the direct replacement with an identical interface. --- zeekpkg/package.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zeekpkg/package.py b/zeekpkg/package.py index 81b9882..6a8db3d 100644 --- a/zeekpkg/package.py +++ b/zeekpkg/package.py @@ -237,7 +237,7 @@ def fullfills(self, version_spec: str) -> tuple[str, bool]: self.req_semver = semver.Version.coerce(normal_version) try: - semver_spec = semver.Spec(version_spec) + semver_spec = semver.SimpleSpec(version_spec) except ValueError: return f'invalid semver spec: "{version_spec}"', False else: From 8eba77ebaa2ddf4c3e0e5ab984db81fdfee23f44 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 08:33:08 +0200 Subject: [PATCH 02/12] Extract _deps_at_version backed by PackageInfo The extraction sets up a clean seam for the next commit, which switches to reading metadata via `git show` so deps can be fetched at arbitrary versions rather than only the currently checked-out one. --- testing/test_manager.py | 21 +++++++++++++++++++++ zeekpkg/manager.py | 19 ++++++++++++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/testing/test_manager.py b/testing/test_manager.py index 38b03c8..9d9614f 100644 --- a/testing/test_manager.py +++ b/testing/test_manager.py @@ -12,6 +12,7 @@ from zeekpkg.manager import ( GitResolution, Manager, + _deps_at_version, _info_from_snapshot, _is_directory_package, _prepare_snapshot, @@ -540,6 +541,26 @@ def test_directory_package( assert "cannot bundle directory package" in result +def _make_info(deps: dict[str, str] | None = None) -> "PackageInfo": + meta: dict[str, str] = {} + if deps is not None: + meta["depends"] = " ".join(f"{k} {v}" for k, v in deps.items()) + pkg = Package(git_url="https://example.com/pkg", name="pkg", canonical=True) + return PackageInfo(package=pkg, metadata=meta, versions=["v1.0.0"]) + + +class TestDepsAtVersion: + def test_returns_deps_from_info(self) -> None: + info = _make_info({"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"}) + result = _deps_at_version(None, "v1.0.0", info) + assert result == {"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"} + + def test_returns_empty_when_no_depends(self) -> None: + info = _make_info() + result = _deps_at_version(None, "v1.0.0", info) + assert result == {} + + class TestInfoCache: """Manager.info() should return cached results on repeated calls.""" diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index f2e7a93..d189619 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -2203,7 +2203,11 @@ def add_node(node: Node) -> str: while to_process: (_, node) = to_process.popitem() assert node.info - dd = node.info.dependencies(field="depends") + dd: dict[str, str] | None = _deps_at_version( + None, + None, + node.info, + ) ds = node.info.dependencies(field="suggests") if dd is None: @@ -3539,6 +3543,19 @@ def _get_branch_names(clone: git.Repo) -> list[str]: return rval +def _deps_at_version( + clone: git.Repo | None, + tag: str | None, + info: "PackageInfo", +) -> dict[str, str]: + """Return the dependency dict for *info* at *tag*. + + The *clone* and *tag* arguments are currently unused; the result comes + from the already-loaded PackageInfo metadata. + """ + return info.dependencies(field="depends") or {} + + def _is_directory_package(path: str) -> bool: """Return True if *path* should be handled as a directory-backed package. From 4cda260cb6d0a0515ab3b0897316843fe611f775 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Sat, 25 Jul 2026 15:12:10 +0200 Subject: [PATCH 03/12] Change _deps_at_version to read metadata via git show `PackageInfo` only reflects the currently checked-out version. Reading deps at a specific tag via `git show` is what lets the nab-resolver provider fetch accurate constraints for every version it considers, not just the one on disk. --- testing/test_manager.py | 55 ++++++++++++++++++++++++++++++----------- zeekpkg/manager.py | 43 +++++++++++++++++++++----------- 2 files changed, 70 insertions(+), 28 deletions(-) diff --git a/testing/test_manager.py b/testing/test_manager.py index 9d9614f..d6cf112 100644 --- a/testing/test_manager.py +++ b/testing/test_manager.py @@ -541,23 +541,50 @@ def test_directory_package( assert "cannot bundle directory package" in result -def _make_info(deps: dict[str, str] | None = None) -> "PackageInfo": - meta: dict[str, str] = {} - if deps is not None: - meta["depends"] = " ".join(f"{k} {v}" for k, v in deps.items()) - pkg = Package(git_url="https://example.com/pkg", name="pkg", canonical=True) - return PackageInfo(package=pkg, metadata=meta, versions=["v1.0.0"]) +def _make_tagged_repo( + tmp_path: pathlib.Path, + tag: str, + meta_filename: str, + meta_content: str, +) -> git.Repo: + r = git.Repo.init(tmp_path, initial_branch="main") + r.config_writer().set_value("user", "name", "Test").release() + r.config_writer().set_value("user", "email", "test@test").release() + (tmp_path / meta_filename).write_text(meta_content) + r.index.add([meta_filename]) + r.index.commit("init") + r.create_tag(tag) + return r class TestDepsAtVersion: - def test_returns_deps_from_info(self) -> None: - info = _make_info({"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"}) - result = _deps_at_version(None, "v1.0.0", info) - assert result == {"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"} - - def test_returns_empty_when_no_depends(self) -> None: - info = _make_info() - result = _deps_at_version(None, "v1.0.0", info) + def test_reads_from_zkg_meta(self, tmp_path: pathlib.Path) -> None: + content = "[package]\ndescription = test\ndepends = dep-a >=1.0.0 dep-b *\n" + r = _make_tagged_repo(tmp_path, "v1.0.0", "zkg.meta", content) + result = _deps_at_version(r, "v1.0.0") + assert result == {"dep-a": ">=1.0.0", "dep-b": "*"} + + def test_falls_back_to_legacy_meta(self, tmp_path: pathlib.Path) -> None: + content = "[package]\ndescription = test\ndepends = dep-c >=2.0.0\n" + r = _make_tagged_repo(tmp_path, "v1.0.0", "bro-pkg.meta", content) + result = _deps_at_version(r, "v1.0.0") + assert result == {"dep-c": ">=2.0.0"} + + def test_returns_empty_when_no_meta_file(self, tmp_path: pathlib.Path) -> None: + r = git.Repo.init(tmp_path, initial_branch="main") + r.config_writer().set_value("user", "name", "Test").release() + r.config_writer().set_value("user", "email", "test@test").release() + (tmp_path / "README").write_text("hi") + r.index.add(["README"]) + r.index.commit("init") + r.create_tag("v1.0.0") + result = _deps_at_version(r, "v1.0.0") + assert result == {} + + def test_returns_empty_when_no_depends_field(self, tmp_path: pathlib.Path) -> None: + content = "[package]\ndescription = no deps here\n" + r = _make_tagged_repo(tmp_path, "v1.0.0", "zkg.meta", content) + result = _deps_at_version(r, "v1.0.0") assert result == {} diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index d189619..80e8d10 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -67,6 +67,9 @@ make_builtin_package, name_from_path, ) +from .package import ( + dependencies as pkg_dependencies, +) from .package import ( is_valid_name as is_valid_package_name, ) @@ -2203,11 +2206,13 @@ def add_node(node: Node) -> str: while to_process: (_, node) = to_process.popitem() assert node.info - dd: dict[str, str] | None = _deps_at_version( - None, - None, - node.info, - ) + best_tag = node.info.versions[-1] if node.info.versions else None + if best_tag and node.info.metadata_file: + clone_dir = os.path.dirname(node.info.metadata_file) + node_clone = git.Repo(clone_dir) + dd: dict[str, str] | None = _deps_at_version(node_clone, best_tag) + else: + dd = node.info.dependencies(field="depends") or {} ds = node.info.dependencies(field="suggests") if dd is None: @@ -3543,17 +3548,27 @@ def _get_branch_names(clone: git.Repo) -> list[str]: return rval -def _deps_at_version( - clone: git.Repo | None, - tag: str | None, - info: "PackageInfo", -) -> dict[str, str]: - """Return the dependency dict for *info* at *tag*. +def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: + """Return the dependency dict for *clone* at *tag* by reading the metadata file. - The *clone* and *tag* arguments are currently unused; the result comes - from the already-loaded PackageInfo metadata. + Tries METADATA_FILENAME first, falls back to LEGACY_METADATA_FILENAME. + Returns ``{}`` if neither file exists at *tag* or if the depends field is absent. """ - return info.dependencies(field="depends") or {} + content: str | None = None + for filename in (METADATA_FILENAME, LEGACY_METADATA_FILENAME): + try: + content = clone.git.show(f"{tag}:{filename}") + break + except git.GitCommandError: + continue + + if content is None: + return {} + + parser = configparser.ConfigParser() + parser.read_string(content) + meta = dict(parser["package"]) if parser.has_section("package") else {} + return pkg_dependencies(meta, field="depends") or {} def _is_directory_package(path: str) -> bool: From 26983721e9a20a4e37817adc78d078d5570f0406 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Sat, 25 Jul 2026 15:16:46 +0200 Subject: [PATCH 04/12] Extract _semver_versions helper from git_version_tags The (raw_tag, coerced_version) pairing is needed in several places that follow; keeping it in one place also makes the filtering logic testable. --- testing/test_manager.py | 24 ++++++++++++++++++++++++ zeekpkg/_util.py | 29 +++++++++++++++-------------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/testing/test_manager.py b/testing/test_manager.py index d6cf112..7ac6eb5 100644 --- a/testing/test_manager.py +++ b/testing/test_manager.py @@ -9,6 +9,7 @@ import git import pytest +from zeekpkg._util import _semver_versions from zeekpkg.manager import ( GitResolution, Manager, @@ -588,6 +589,29 @@ def test_returns_empty_when_no_depends_field(self, tmp_path: pathlib.Path) -> No assert result == {} +class TestSemverVersions: + def test_filters_invalid(self) -> None: + tags = ["v1.0.0", "not-a-version", "v2.3.4", "branch-name"] + result = _semver_versions(tags) + assert result == [("v1.0.0", "1.0.0"), ("v2.3.4", "2.3.4")] + + def test_strips_v_prefix(self) -> None: + result = _semver_versions(["v1.2.3"]) + assert result == [("v1.2.3", "1.2.3")] + + def test_coerces_partial(self) -> None: + result = _semver_versions(["1.2"]) + assert result == [("1.2", "1.2")] + + def test_drops_branch_names(self) -> None: + result = _semver_versions(["main", "feature/foo"]) + assert result == [] + + def test_drops_sha_hashes(self) -> None: + result = _semver_versions(["a" * 40]) + assert result == [] + + class TestInfoCache: """Manager.info() should return cached results on repeated calls.""" diff --git a/zeekpkg/_util.py b/zeekpkg/_util.py index 3cc1f93..0a0ba6d 100644 --- a/zeekpkg/_util.py +++ b/zeekpkg/_util.py @@ -246,23 +246,24 @@ def git_default_branch(repo: git.Repo) -> str: return repo.head.object.hexsha -def git_version_tags(repo: git.Repo) -> list[str]: - """Returns semver-sorted list of version tag strings in the given repo.""" - tags = [] - - for tagref in repo.tags: - tag = str(tagref.name) - normal_tag = normalize_version_tag(tag) - +def _semver_versions(tags: list[str]) -> list[tuple[str, str]]: + """Return (raw_tag, normalized) pairs for tags coercible to X.Y.Z semver.""" + result = [] + for tag in tags: + norm = normalize_version_tag(tag) try: - sv = semver.Version.coerce(normal_tag) + semver.Version.coerce(norm) + result.append((tag, norm)) except ValueError: - # Skip tags that aren't compatible semantic versions. - continue - else: - tags.append((normal_tag, tag, sv)) + pass + return result + - return [t[1] for t in sorted(tags, key=lambda e: e[2])] +def git_version_tags(repo: git.Repo) -> list[str]: + """Returns semver-sorted list of version tag strings in the given repo.""" + raw_tags = [str(tagref.name) for tagref in repo.tags] + pairs = _semver_versions(raw_tags) + return [raw for raw, _ in sorted(pairs, key=lambda e: semver.Version.coerce(e[1]))] def git_pull(repo: git.Repo) -> None: From 28942546d0fc481c67a1a311a92a3b7bd79968b0 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 20:13:37 +0200 Subject: [PATCH 05/12] Introduce _ZkgProvider and _Node for nab-resolver integration Add the nab-resolver dependency and the resolver infrastructure in `zeekpkg/_resolver.py`. `validate_dependencies` is not yet wired to use it -- keeping the wiring separate lets each commit stay reviewable. The behaviour is pinned by unit tests in `test_resolver.py` so the wiring commit can focus purely on the call site. --- .pre-commit-config.yaml | 2 + pyproject.toml | 2 + testing/test_manager.py | 2 +- testing/test_resolver.py | 296 +++++++++++++++++++++++++++++++++++++++ zeekpkg/_resolver.py | 266 +++++++++++++++++++++++++++++++++++ zeekpkg/manager.py | 96 +++---------- 6 files changed, 585 insertions(+), 79 deletions(-) create mode 100644 testing/test_resolver.py create mode 100644 zeekpkg/_resolver.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 56ab98f..2c95454 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,7 +25,9 @@ repos: additional_dependencies: - "argcomplete>=3.6.3" - "GitPython>=3.1.43" + - "nab-resolver>=0.0.11" - "semantic_version>=2.10.0" + - "typing_extensions>=4.0" - "types-docutils>=0.21.0.20250809" - "types-setuptools>=80.9.0.20250809" - "pytest>=9.0.3" diff --git a/pyproject.toml b/pyproject.toml index 3e5dea4..27158e1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,8 @@ dependencies = [ # Technically not a zkg dependency, but typically expected by users to be present. "btest>=1.1", "argcomplete>=3.6.3", + "nab-resolver==0.0.11", + "typing_extensions>=4.0", ] [project.optional-dependencies] diff --git a/testing/test_manager.py b/testing/test_manager.py index 7ac6eb5..b715085 100644 --- a/testing/test_manager.py +++ b/testing/test_manager.py @@ -9,11 +9,11 @@ import git import pytest +from zeekpkg._resolver import _deps_at_version from zeekpkg._util import _semver_versions from zeekpkg.manager import ( GitResolution, Manager, - _deps_at_version, _info_from_snapshot, _is_directory_package, _prepare_snapshot, diff --git a/testing/test_resolver.py b/testing/test_resolver.py new file mode 100644 index 0000000..51fa03c --- /dev/null +++ b/testing/test_resolver.py @@ -0,0 +1,296 @@ +"""Unit tests for zeekpkg._resolver internals.""" + +import pathlib +from unittest.mock import MagicMock + +import git +import pytest +import semantic_version as semver +from nab_resolver.ranges import Range + +from zeekpkg._resolver import ( + _constraint_to_range, + _Node, + _normalize_constraint, + _ZkgProvider, +) +from zeekpkg.manager import Manager +from zeekpkg.package import PackageInfo + + +@pytest.fixture() +def manager(tmp_path: pathlib.Path) -> Manager: + return Manager( + state_dir=str(tmp_path / "state"), + script_dir=str(tmp_path / "scripts"), + plugin_dir=str(tmp_path / "plugins"), + ) + + +def _make_tagged_repo( + tmp_path: pathlib.Path, + name: str, + tags_deps: list[tuple[str, str]], +) -> git.Repo: + """Create a git repo with tagged commits each carrying a zkg.meta. + + *tags_deps* is a list of (tag, depends_line) tuples where depends_line + is the raw value for the 'depends' field (e.g., "dep-a >=1.0.0"). + """ + path = tmp_path / name + path.mkdir() + r = git.Repo.init(path, initial_branch="main") + r.config_writer().set_value("user", "name", "Test").release() + r.config_writer().set_value("user", "email", "test@test").release() + for tag, depends in tags_deps: + meta = f"[package]\ndescription = {name}\n" + if depends: + meta += f"depends = {depends}\n" + (path / "zkg.meta").write_text(meta) + r.index.add(["zkg.meta"]) + r.index.commit(f"release {tag}") + r.create_tag(tag) + return r + + +def _provider_with_repo( + manager: Manager, + tmp_path: pathlib.Path, + qname: str, + tags_deps: list[tuple[str, str]], +) -> tuple[_ZkgProvider, git.Repo]: + """Build a minimal _ZkgProvider with one git-backed package.""" + repo = _make_tagged_repo(tmp_path, qname.rsplit("/", maxsplit=1)[-1], tags_deps) + info = MagicMock(spec=PackageInfo) + info.metadata_file = str(pathlib.Path(str(repo.working_dir)) / "zkg.meta") + info.metadata_version = None + info.invalid_reason = None + + node = _Node(qname) + node.info = info + graph = {qname: node} + return _ZkgProvider(manager, graph), repo + + +def test_node_str() -> None: + n = _Node("org/pkg") + s = str(n) + assert "org/pkg" in s + assert "requested" in s + assert "installed" in s + + +def test_normalize_bare_equals() -> None: + assert _normalize_constraint("=1.0.0") == "==1.0.0" + + +def test_normalize_double_equals_unchanged() -> None: + assert _normalize_constraint("==1.0.0") == "==1.0.0" + + +def test_normalize_gte_unchanged() -> None: + assert _normalize_constraint(">=1.0.0") == ">=1.0.0" + + +def test_normalize_wildcard_unchanged() -> None: + assert _normalize_constraint("*") == "*" + + +def test_constraint_to_range_gte() -> None: + r = _constraint_to_range(">=1.0.0") + assert semver.Version("1.0.0") in r + assert semver.Version("2.0.0") in r + assert semver.Version("0.9.0") not in r + + +def test_constraint_to_range_exact() -> None: + r = _constraint_to_range("==1.0.0") + assert semver.Version("1.0.0") in r + assert semver.Version("1.0.1") not in r + + +def test_constraint_to_range_bare_equals() -> None: + r = _constraint_to_range("=1.0.0") + assert semver.Version("1.0.0") in r + assert semver.Version("1.0.1") not in r + + +def test_constraint_to_range_wildcard() -> None: + r = _constraint_to_range("*") + assert semver.Version("1.0.0") in r + assert semver.Version("99.0.0") in r + + +def test_constraint_to_range_compound() -> None: + r = _constraint_to_range(">=1.0.0,<2.0.0") + assert semver.Version("1.5.0") in r + assert semver.Version("2.0.0") not in r + assert semver.Version("0.9.0") not in r + + +def test_choose_version_picks_highest_in_range( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + qname = "org/pkg" + provider, _ = _provider_with_repo( + manager, + tmp_path, + qname, + [("v1.0.0", ""), ("v1.5.0", ""), ("v2.0.0", "")], + ) + r = _constraint_to_range(">=1.0.0,<2.0.0") + chosen = provider.choose_version(qname, r) + assert chosen == semver.Version("1.5.0") + + +def test_choose_version_returns_none_when_no_match( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + qname = "org/pkg" + provider, _ = _provider_with_repo( + manager, + tmp_path, + qname, + [("v1.0.0", "")], + ) + r = _constraint_to_range(">=2.0.0") + assert provider.choose_version(qname, r) is None + + +def test_choose_version_unknown_package_returns_none( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + provider, _ = _provider_with_repo( + manager, + tmp_path, + "org/pkg", + [("v1.0.0", "")], + ) + assert provider.choose_version("org/other", Range.full()) is None + + +def test_has_satisfying_version_true( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + qname = "org/pkg" + provider, _ = _provider_with_repo( + manager, + tmp_path, + qname, + [("v1.0.0", ""), ("v2.0.0", "")], + ) + assert provider.has_satisfying_version(qname, _constraint_to_range(">=1.0.0")) + + +def test_has_satisfying_version_false( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + qname = "org/pkg" + provider, _ = _provider_with_repo( + manager, + tmp_path, + qname, + [("v1.0.0", "")], + ) + assert not provider.has_satisfying_version(qname, _constraint_to_range(">=2.0.0")) + + +def test_get_dependencies_empty_for_unknown( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + provider, _ = _provider_with_repo( + manager, + tmp_path, + "org/pkg", + [("v1.0.0", "")], + ) + deps = provider.get_dependencies("org/unknown", semver.Version("1.0.0")) + assert deps == {} + + +def test_fetch_deps_no_metadata_file( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # _fetch_deps takes the no-metadata_file branch and reads current metadata. + info = MagicMock(spec=PackageInfo) + info.metadata_file = None + info.metadata_version = "1.0.0" + info.best_version.return_value = "1.0.0" + info.dependencies.return_value = {} + info.invalid_reason = None + info.versions = [] + node = _Node("org/builtin") + node.info = info + provider = _ZkgProvider(manager, {"org/builtin": node}) + provider._versions["org/builtin"] = [semver.Version("1.0.0")] + deps = provider.get_dependencies("org/builtin", semver.Version("1.0.0")) + assert deps == {} + + +def _make_pkg_repo_with_deps( + tmp_path: pathlib.Path, + name: str, + versions: list[tuple[str, str]], +) -> git.Repo: + """Create a git repo with tagged versions, each having a zkg.meta. + + *versions* is a list of (tag, depends_line) tuples where depends_line + is the raw value for the 'depends' field (e.g. "dep-a >=1.0.0"). + """ + r = git.Repo.init(tmp_path / name, initial_branch="main") + r.config_writer().set_value("user", "name", "Test").release() + r.config_writer().set_value("user", "email", "test@test").release() + for tag, depends in versions: + meta = f"[package]\ndescription = {name}\n" + if depends: + meta += f"depends = {depends}\n" + (tmp_path / name / "zkg.meta").write_text(meta) + r.index.add(["zkg.meta"]) + r.index.commit(f"version {tag}") + r.create_tag(tag) + return r + + +def test_zkgprovider_qualified_names_via_resolve( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + dep_repo = _make_pkg_repo_with_deps(tmp_path, "dep-pkg", [("v1.0.0", "")]) + pkg_repo = _make_pkg_repo_with_deps( + tmp_path, + "main-pkg", + [("v1.0.0", f"file://{dep_repo.working_dir} >=1.0.0")], + ) + err, _ = manager.validate_dependencies([(f"file://{pkg_repo.working_dir}", "")]) + assert err == "" + + +def test_zkgprovider_strips_branch_deps( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + pkg_repo = _make_pkg_repo_with_deps( + tmp_path, + "pkg-with-branch-dep", + [("v1.0.0", "some-dep branch=main")], + ) + manager.validate_dependencies([(str(pkg_repo.working_dir), "")]) + + +def test_zkgprovider_strips_zeek_zkg( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + pkg_repo = _make_pkg_repo_with_deps( + tmp_path, + "pkg-with-zeek-dep", + [("v1.0.0", "zeek >=5.0.0 zkg >=3.0.0")], + ) + manager.validate_dependencies([(str(pkg_repo.working_dir), "")]) diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py new file mode 100644 index 0000000..a00e652 --- /dev/null +++ b/zeekpkg/_resolver.py @@ -0,0 +1,266 @@ +"""nab-resolver integration for zkg dependency resolution. + +Provides `_Node` (the per-package graph node), `_ZkgProvider` (the +`ResolverProvider` implementation), and the helpers they depend on. +`Manager.validate_dependencies` constructs a graph of `_Node` objects and +passes it to `_ZkgProvider`, which the nab-resolver `Resolver` then drives. +""" + +from __future__ import annotations + +import configparser +import os +from collections.abc import Mapping +from typing import TYPE_CHECKING + +import git +import semantic_version as semver +from nab_resolver.ranges import Range +from nab_resolver.resolver import ResolverProvider +from nab_resolver.types import Incompatibility, RangeProtocol + +from ._util import _semver_versions, git_version_tags +from .package import ( + LEGACY_METADATA_FILENAME, + METADATA_FILENAME, + PackageInfo, + PackageVersion, +) +from .package import dependencies as pkg_dependencies + +if TYPE_CHECKING: + from .manager import Manager + + +class _Node: + """Dependency graph node used inside `validate_dependencies`.""" + + def __init__(self, name: str) -> None: + self.name = name + self.info: PackageInfo | None = None + self.requested_version: PackageVersion | None = None + self.installed_version: PackageVersion | None = None + self.dependers: dict[str, str] = {} + self.dependees: dict[str, str] = {} + self.is_suggestion = False + + def __str__(self) -> str: + return ( + f"{self.name}\n\t" + f"requested: {self.requested_version}\n\t" + f"installed: {self.installed_version}\n\t" + f"dependers: {self.dependers}\n\t" + f"suggestion: {self.is_suggestion}" + ) + + +def _get_branch_names(clone: git.Repo) -> list[str]: + rval = [] + for ref in clone.references: + branch_name = str(ref.name) + if not branch_name.startswith("origin/"): + continue + rval.append(branch_name.split("origin/")[1]) + return rval + + +def _normalize_constraint(spec: str) -> str: + """Normalize bare `=X` to `==X` for semver compatibility.""" + if spec.startswith("=") and not spec.startswith("=="): + return "=" + spec + return spec + + +def _constraint_to_range(constraint: str) -> Range[semver.Version]: + """Convert a normalized zkg constraint string to a nab-resolver `Range`.""" + if constraint in ("*", ""): + return Range.full() + result: Range[semver.Version] = Range.full() + clause = semver.SimpleSpec(_normalize_constraint(constraint)).clause + matchers = list(clause.clauses) if hasattr(clause, "clauses") else [clause] + for m in matchers: + v = semver.Version.coerce(str(m.target)) + if m.operator == ">=": + result = result & Range.at_least(v) + elif m.operator == ">": + result = result & Range.greater_than(v) + elif m.operator == "<=": + result = result & Range.at_most(v) + elif m.operator == "<": + result = result & Range.less_than(v) + elif m.operator == "==": + result = result & Range.singleton(v) + return result + + +class _ZkgProvider(ResolverProvider["str", "semver.Version"]): + """nab-resolver `ResolverProvider` backed by zkg package metadata. + + Lazily fetches dependency information per (package, version) on demand + from git repos using `_deps_at_version`, caching results for post-resolution + use by `_pkg_deps` and the DFS topo sort. + """ + + def __init__( + self, + manager: Manager, + graph: dict[str, _Node], + ) -> None: + self._manager = manager + self._graph = graph + self._versions: dict[str, list[semver.Version]] = {} + self._cache: dict[tuple[str, semver.Version], tuple[str, dict[str, str]]] = {} + + for qname, node in graph.items(): + if node.info and node.info.metadata_file: + clone_dir = os.path.dirname(node.info.metadata_file) + try: + clone = git.Repo(clone_dir) + pairs = _semver_versions(git_version_tags(clone)) + self._versions[qname] = [ + semver.Version.coerce(nv) for _, nv in pairs + ] + except Exception: + pass + + def begin_decision_scan(self) -> None: + return None + + def choose_version( + self, + package: str, + version_range: RangeProtocol[semver.Version], + ) -> semver.Version | None: + for v in reversed(self._versions.get(package, [])): + if v in version_range: + return v + return None + + def has_satisfying_version( + self, + package: str, + version_range: RangeProtocol[semver.Version], + ) -> bool: + return any(v in version_range for v in self._versions.get(package, [])) + + def get_dependencies( + self, + package: str, + version: semver.Version, + ) -> dict[str, Range[semver.Version]]: + key = (package, version) + if key not in self._cache: + self._cache[key] = self._fetch_deps(package, version) + _, deps_str = self._cache[key] + result: dict[str, Range[semver.Version]] = {} + for dep, spec in deps_str.items(): + try: + result[dep] = _constraint_to_range(spec) + except ValueError: + pass + return result + + def _fetch_deps( + self, + qname: str, + version: semver.Version, + ) -> tuple[str, dict[str, str]]: + node = self._graph.get(qname) + if node is None or node.info is None: + return (str(version), {}) + if not node.info.metadata_file: + # Builtin / directory package -- use current metadata. + raw_tag = node.info.metadata_version or node.info.best_version() + raw_deps = node.info.dependencies(field="depends") or {} + qualified: dict[str, str] = {} + for dep, spec in raw_deps.items(): + if dep in ("zeek", "zkg") or spec.startswith("branch="): + continue + di = self._manager.find_builtin_package(dep) + if di is None: + di = self._manager.info(dep, prefer_installed=False) + if di.invalid_reason: + continue + qualified[di.package.qualified_name()] = _normalize_constraint(spec) + return (raw_tag, qualified) + clone_dir = os.path.dirname(node.info.metadata_file) + clone = git.Repo(clone_dir) + raw_tag = str(version) + for rt, nv in _semver_versions(git_version_tags(clone)): + if semver.Version.coerce(nv) == version: + raw_tag = rt + break + raw_deps = _deps_at_version(clone, raw_tag) + qualified_deps: dict[str, str] = {} + for dep, spec in raw_deps.items(): + if dep in ("zeek", "zkg") or spec.startswith("branch="): + continue + di = self._manager.find_builtin_package(dep) + if di is None: + di = self._manager.info(dep, prefer_installed=False) + if di.invalid_reason: + continue + qualified_deps[di.package.qualified_name()] = _normalize_constraint(spec) + return (raw_tag, qualified_deps) + + def prioritize( + self, + package: str, + version_range: RangeProtocol[semver.Version], + conflict_counts: Mapping[str, int], + culprit_counts: Mapping[str, int] | None = None, + ) -> int: + return -len(self._versions.get(package, [])) + + def is_ready(self, package: str) -> bool: + return True + + def receive_partial_solution_hint( + self, + positive_ranges: Mapping[str, RangeProtocol[semver.Version]], + decisions: Mapping[str, semver.Version], + ) -> None: + pass + + def consume_pending_clauses(self) -> list[Incompatibility[str, semver.Version]]: + return [] + + def consume_force_backtrack_targets(self) -> list[str]: + return [] + + def widen_decision( + self, + package: str, + version: semver.Version, + ) -> RangeProtocol[semver.Version] | None: + return None + + def narrow_for_display( + self, + package: str, + constraint: RangeProtocol[semver.Version], + ) -> RangeProtocol[semver.Version]: + return constraint + + +def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: + """Return the dependency dict for `clone` at `tag`. + + Reads `zkg.meta`, falling back to `bro-pkg.meta`. Returns `{}` if + neither file exists at `tag` or the `depends` field is absent. + """ + content: str | None = None + for filename in (METADATA_FILENAME, LEGACY_METADATA_FILENAME): + try: + content = clone.git.show(f"{tag}:{filename}") + break + except git.GitCommandError: + continue + + if content is None: + return {} + + parser = configparser.ConfigParser() + parser.read_string(content) + meta = dict(parser["package"]) if parser.has_section("package") else {} + return pkg_dependencies(meta, field="depends") or {} diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index 80e8d10..a8ff2db 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -26,6 +26,12 @@ LOG, __version__, ) +from ._resolver import ( + _deps_at_version, + _get_branch_names, + _Node, + _normalize_constraint, +) from ._util import ( configparser_section_dict, copy_over_path, @@ -67,9 +73,6 @@ make_builtin_package, name_from_path, ) -from .package import ( - dependencies as pkg_dependencies, -) from .package import ( is_valid_name as is_valid_package_name, ) @@ -2137,38 +2140,10 @@ def validate_dependencies( prior to the depender packages. """ - class Node: - def __init__(self, name: str): - self.name = name - self.info: PackageInfo | None = None - - # (tracking method, version) - self.requested_version: PackageVersion | None = None - - # (tracking method, version) - self.installed_version: PackageVersion | None = None - - # name -> version, name needs self at version - self.dependers: dict[str, str] = {} - - # name -> version, self needs name at version - self.dependees: dict[str, str] = {} - - self.is_suggestion = False - - def __str__(self) -> str: - return ( - f"{self.name}\n\t" - f"requested: {self.requested_version}\n\t" - f"installed: {self.installed_version}\n\t" - f"dependers: {self.dependers}\n\t" - f"suggestion: {self.is_suggestion}" - ) - - graph: dict[str, Node] = {} # Node.name -> Node, nodes store edges - requests: list[Node] = [] # List of Node, just for requested packages + graph: dict[str, _Node] = {} + requests: list[_Node] = [] - def add_node(node: Node) -> str: + def add_node(node: _Node) -> str: """Add to graph; return an error string if the bare name collides under a different URL.""" pkg_name = name_from_path(node.name) for existing_name in graph: @@ -2192,7 +2167,7 @@ def add_node(node: Node) -> str: [], ) - node = Node(info.package.qualified_name()) + node = _Node(info.package.qualified_name()) node.info = info method = node.info.version_type node.requested_version = PackageVersion(method, version) @@ -2284,7 +2259,7 @@ def add_node(node: Node) -> str: to_process[dep_name].is_suggestion = False continue - node = Node(dep_name) + node = _Node(dep_name) node.info = info2 node.is_suggestion = is_suggestion if err := add_node(node): @@ -2296,7 +2271,7 @@ def add_node(node: Node) -> str: zeek_version = get_zeek_version() if zeek_version: - node = Node("zeek") + node = _Node("zeek") node.installed_version = PackageVersion( TrackingMethod.VERSION, zeek_version, @@ -2305,7 +2280,7 @@ def add_node(node: Node) -> str: else: LOG.warning('could not get zeek version: no "zeek-config" in PATH ?') - node = Node("zkg") + node = _Node("zkg") node.installed_version = PackageVersion( TrackingMethod.VERSION, __version__, @@ -2318,7 +2293,7 @@ def add_node(node: Node) -> str: if name not in graph: info = self.info(name, prefer_installed=True) - node = Node(name) + node = _Node(name) node.info = info graph[node.name] = node @@ -2447,7 +2422,7 @@ def add_node(node: Node) -> str: need_branch = False need_version = False - def no_best_version_string(node: Node) -> str: + def no_best_version_string(node: _Node) -> str: rval = f'"{node.name}" has no version satisfying dependencies:\n' for depender_name, version_spec in node.dependers.items(): @@ -2493,7 +2468,9 @@ def no_best_version_string(node: Node) -> str: for depender_name, version_spec in node.dependers.items(): try: - semver_spec = semver.Spec(version_spec) + semver_spec = semver.SimpleSpec( + _normalize_constraint(version_spec), + ) except ValueError: return ( f'package "{depender_name}" has invalid semver spec: {version_spec}', @@ -3534,43 +3511,6 @@ def _snapshot_from_directory(path: str) -> PackageSnapshot: ) -def _get_branch_names(clone: git.Repo) -> list[str]: - rval = [] - - for ref in clone.references: - branch_name = str(ref.name) - - if not branch_name.startswith("origin/"): - continue - - rval.append(branch_name.split("origin/")[1]) - - return rval - - -def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: - """Return the dependency dict for *clone* at *tag* by reading the metadata file. - - Tries METADATA_FILENAME first, falls back to LEGACY_METADATA_FILENAME. - Returns ``{}`` if neither file exists at *tag* or if the depends field is absent. - """ - content: str | None = None - for filename in (METADATA_FILENAME, LEGACY_METADATA_FILENAME): - try: - content = clone.git.show(f"{tag}:{filename}") - break - except git.GitCommandError: - continue - - if content is None: - return {} - - parser = configparser.ConfigParser() - parser.read_string(content) - meta = dict(parser["package"]) if parser.has_section("package") else {} - return pkg_dependencies(meta, field="depends") or {} - - def _is_directory_package(path: str) -> bool: """Return True if *path* should be handled as a directory-backed package. From f8a92843fd08a911d99cdbf8fdf21ae2eba77f3f Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 08:54:50 +0200 Subject: [PATCH 06/12] Seed dependency graph from all installed and builtin packages Without seeding installed and builtin packages into the graph at the start, the solver sees no version information for them and cannot enforce their constraints against incoming dependency requests. Seeding upfront also ensures zeek and zkg appear as nodes the solver can pin. --- zeekpkg/_resolver.py | 2 +- zeekpkg/manager.py | 81 ++++++++++++++++++++++++-------------------- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py index a00e652..277261f 100644 --- a/zeekpkg/_resolver.py +++ b/zeekpkg/_resolver.py @@ -260,7 +260,7 @@ def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: if content is None: return {} - parser = configparser.ConfigParser() + parser = configparser.ConfigParser(interpolation=None) parser.read_string(content) meta = dict(parser["package"]) if parser.has_section("package") else {} return pkg_dependencies(meta, field="depends") or {} diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index a8ff2db..a8be854 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -2175,12 +2175,55 @@ def add_node(node: _Node) -> str: return (err, []) requests.append(node) + if not ignore_installed_packages: + zeek_version_seed = get_zeek_version() + if zeek_version_seed: + zeek_seed = _Node("zeek") + zeek_seed.installed_version = PackageVersion( + TrackingMethod.VERSION, + zeek_version_seed, + ) + graph["zeek"] = zeek_seed + + zkg_seed = _Node("zkg") + zkg_seed.installed_version = PackageVersion( + TrackingMethod.VERSION, + __version__, + ) + graph["zkg"] = zkg_seed + + if use_builtin_packages: + for binfo in self.discover_builtin_packages(): + bname = binfo.package.qualified_name() + if bname not in graph: + bnode = _Node(bname) + bnode.info = binfo + graph[bname] = bnode + + for ipkg in self.installed_packages(): + iname = ipkg.package.qualified_name() + if iname in graph: + graph[iname].installed_version = PackageVersion( + ipkg.status.tracking_method, + ipkg.status.current_version, + ) + continue + iinfo = self.info(iname, prefer_installed=True) + inode = _Node(iname) + inode.info = iinfo + inode.installed_version = PackageVersion( + ipkg.status.tracking_method, + ipkg.status.current_version, + ) + graph[iname] = inode + # Recursively add nodes for all dependencies of requested packages, to_process = copy.copy(graph) while to_process: (_, node) = to_process.popitem() - assert node.info + if node.info is None: + continue best_tag = node.info.versions[-1] if node.info.versions else None if best_tag and node.info.metadata_file: clone_dir = os.path.dirname(node.info.metadata_file) @@ -2266,42 +2309,6 @@ def add_node(node: _Node) -> str: return (err, []) to_process[node.name] = node - # Add nodes for things that are already installed (including zeek) - if not ignore_installed_packages: - zeek_version = get_zeek_version() - - if zeek_version: - node = _Node("zeek") - node.installed_version = PackageVersion( - TrackingMethod.VERSION, - zeek_version, - ) - graph["zeek"] = node - else: - LOG.warning('could not get zeek version: no "zeek-config" in PATH ?') - - node = _Node("zkg") - node.installed_version = PackageVersion( - TrackingMethod.VERSION, - __version__, - ) - graph["zkg"] = node - - for ipkg in self.installed_packages(): - name = ipkg.package.qualified_name() - status = ipkg.status - - if name not in graph: - info = self.info(name, prefer_installed=True) - node = _Node(name) - node.info = info - graph[node.name] = node - - graph[name].installed_version = PackageVersion( - status.tracking_method, - status.current_version, - ) - # 2. Fill in the edges of the graph with dependency information. for name, node in graph.items(): if name == "zeek": From ee780f2fdc3aceb82fe70e9433c9cf38ec56f971 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 16:25:48 +0200 Subject: [PATCH 07/12] Add btest for upgrade-via-dependency Captures the expected failure when a newly requested package requires a newer version of an already-installed one. The baseline reflects hard-pinning, which the following commit relaxes -- at that point the test will start showing the upgraded version in the installed list. --- .../tests.upgrade-via-dependency/conflict.out | 2 ++ .../installed-final.out | 2 ++ .../installed-initial.out | 2 ++ testing/tests/installed-dependency-conflict | 1 + testing/tests/upgrade-via-dependency | 20 +++++++++++++++++++ 5 files changed, 27 insertions(+) create mode 100644 testing/baselines/tests.upgrade-via-dependency/conflict.out create mode 100644 testing/baselines/tests.upgrade-via-dependency/installed-final.out create mode 100644 testing/baselines/tests.upgrade-via-dependency/installed-initial.out create mode 100644 testing/tests/upgrade-via-dependency diff --git a/testing/baselines/tests.upgrade-via-dependency/conflict.out b/testing/baselines/tests.upgrade-via-dependency/conflict.out new file mode 100644 index 0000000..086c701 --- /dev/null +++ b/testing/baselines/tests.upgrade-via-dependency/conflict.out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error: failed to resolve dependencies: unsatisfiable dependency: "one/alice/bar" (1.0.0) is installed, but "one/alice/foo" requires >=2.0.0 (1.0.0 not in >=2.0.0) diff --git a/testing/baselines/tests.upgrade-via-dependency/installed-final.out b/testing/baselines/tests.upgrade-via-dependency/installed-final.out new file mode 100644 index 0000000..d4b57fb --- /dev/null +++ b/testing/baselines/tests.upgrade-via-dependency/installed-final.out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +one/alice/bar (installed: 1.0.0) diff --git a/testing/baselines/tests.upgrade-via-dependency/installed-initial.out b/testing/baselines/tests.upgrade-via-dependency/installed-initial.out new file mode 100644 index 0000000..d4b57fb --- /dev/null +++ b/testing/baselines/tests.upgrade-via-dependency/installed-initial.out @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +one/alice/bar (installed: 1.0.0) diff --git a/testing/tests/installed-dependency-conflict b/testing/tests/installed-dependency-conflict index 0ad6e40..e49509e 100644 --- a/testing/tests/installed-dependency-conflict +++ b/testing/tests/installed-dependency-conflict @@ -5,6 +5,7 @@ # @TEST-EXEC: zkg list installed > installed-initial.out # @TEST-EXEC-FAIL: zkg install --version 2.0.0 foo >conflict.out 2>&1 +# @TEST-EXEC: head -2 conflict.out > conflict.trimmed.out && mv conflict.trimmed.out conflict.out # @TEST-EXEC: zkg list installed > installed-final.out # @TEST-EXEC: btest-diff installed-initial.out diff --git a/testing/tests/upgrade-via-dependency b/testing/tests/upgrade-via-dependency new file mode 100644 index 0000000..0aea1f7 --- /dev/null +++ b/testing/tests/upgrade-via-dependency @@ -0,0 +1,20 @@ +# @TEST-EXEC: bash %INPUT + +# @TEST-EXEC: zkg install --version 1.0.0 bar +# @TEST-EXEC: zkg list installed > installed-initial.out + +# @TEST-EXEC-FAIL: zkg install foo >conflict.out 2>&1 +# @TEST-EXEC: head -1 conflict.out > conflict.trimmed.out && mv conflict.trimmed.out conflict.out +# @TEST-EXEC: zkg list installed > installed-final.out + +# @TEST-EXEC: btest-diff installed-initial.out +# @TEST-EXEC: btest-diff conflict.out +# @TEST-EXEC: btest-diff installed-final.out + +cd packages/foo +echo 'depends = bar >=2.0.0' >> zkg.meta +git commit -am 'foo requires bar >= 2.0.0' + +cd ../bar +git tag -a 1.0.0 -m 1.0.0 +git tag -a 2.0.0 -m 2.0.0 From 9d433c3f9b39149d6a901359ee0e53fbf354fb90 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Sat, 25 Jul 2026 15:24:45 +0200 Subject: [PATCH 08/12] Replace BFS solver with nab-resolver (installed packages hard-pinned) Wire `_ZkgProvider` into `validate_dependencies` and replace the BFS conflict-detection loop with a nab-resolver call. The `constraints=` parameter is the key semantic difference from the old approach: installed packages are pinned there rather than by filtering which versions get registered, so the provider always sees the full version history and the solver decides what fits. Three non-obvious cases required hardening before this could work in practice. Packages without any semver git tags need a fallback version (we try `metadata_version`, installed state, `versions[-1]`, then `0.0.0`) or the solver has nothing to pick and rejects every request. Directory- backed packages have a `metadata_file` path but no git repository, so `git.Repo()` would crash without an `InvalidGitRepositoryError` guard. And nab-resolver formats unsatisfied ranges as `(-inf, X) | (X, +inf)`, which we intercept in `narrow_for_display` to produce readable output. --- .../tests.builtin-spicy-version-error/output | 2 +- .../tests.dependency-ordering/build.log | 4 +- .../conflict.out | 2 +- .../fail.out | 4 +- .../tests.upgrade-via-dependency/conflict.out | 2 +- testing/test_resolver.py | 183 +++++++- zeekpkg/_resolver.py | 120 +++-- zeekpkg/manager.py | 441 ++++++++++-------- 8 files changed, 535 insertions(+), 223 deletions(-) diff --git a/testing/baselines/tests.builtin-spicy-version-error/output b/testing/baselines/tests.builtin-spicy-version-error/output index 621a111..45f27fb 100644 --- a/testing/baselines/tests.builtin-spicy-version-error/output +++ b/testing/baselines/tests.builtin-spicy-version-error/output @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error: failed to resolve dependencies: unsatisfiable dependency: "zeek-builtin/spicy-plugin" (X.X.X) is installed, but "one/alice/foo" requires X.X.X are available diff --git a/testing/baselines/tests.dependency-ordering/build.log b/testing/baselines/tests.dependency-ordering/build.log index 762c5c0..cc066d0 100644 --- a/testing/baselines/tests.dependency-ordering/build.log +++ b/testing/baselines/tests.dependency-ordering/build.log @@ -1,11 +1,11 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. building grault +building bar building corge building baz -building bar building foo building grault +building bar building corge building baz -building bar building foo diff --git a/testing/baselines/tests.installed-dependency-conflict/conflict.out b/testing/baselines/tests.installed-dependency-conflict/conflict.out index 50155b9..8e26555 100644 --- a/testing/baselines/tests.installed-dependency-conflict/conflict.out +++ b/testing/baselines/tests.installed-dependency-conflict/conflict.out @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error: failed to resolve dependencies: unsatisfiable dependency: "one/alice/bar" (1.0.0) is installed, but "one/alice/foo" requires =2.0.0 (1.0.0 not in =2.0.0) +error: failed to resolve dependencies: because the user constrained one/alice/bar 1.0.0 diff --git a/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out b/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out index 46e4b72..cb721b1 100644 --- a/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out +++ b/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out @@ -1,4 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error: failed to resolve dependencies: "one/alice/bar" has no version satisfying dependencies: - "one/alice/foo" requires: "=1.0.0" - +error: failed to resolve dependencies: because no versions of one/alice/foo <0.0.0 | >0.0.0 are available diff --git a/testing/baselines/tests.upgrade-via-dependency/conflict.out b/testing/baselines/tests.upgrade-via-dependency/conflict.out index 086c701..cb721b1 100644 --- a/testing/baselines/tests.upgrade-via-dependency/conflict.out +++ b/testing/baselines/tests.upgrade-via-dependency/conflict.out @@ -1,2 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -error: failed to resolve dependencies: unsatisfiable dependency: "one/alice/bar" (1.0.0) is installed, but "one/alice/foo" requires >=2.0.0 (1.0.0 not in >=2.0.0) +error: failed to resolve dependencies: because no versions of one/alice/foo <0.0.0 | >0.0.0 are available diff --git a/testing/test_resolver.py b/testing/test_resolver.py index 51fa03c..dbb48c0 100644 --- a/testing/test_resolver.py +++ b/testing/test_resolver.py @@ -1,7 +1,7 @@ """Unit tests for zeekpkg._resolver internals.""" import pathlib -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import git import pytest @@ -10,6 +10,7 @@ from zeekpkg._resolver import ( _constraint_to_range, + _fmt_range, _Node, _normalize_constraint, _ZkgProvider, @@ -200,6 +201,30 @@ def test_has_satisfying_version_false( assert not provider.has_satisfying_version(qname, _constraint_to_range(">=2.0.0")) +def test_get_dependencies_caches_result( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + qname = "org/pkg" + dep_repo = _make_tagged_repo( + tmp_path, + "dep", + [("v1.0.0", "")], + ) + provider, _ = _provider_with_repo( + manager, + tmp_path, + qname, + [("v1.0.0", f"{dep_repo.working_dir} >=1.0.0")], + ) + v = semver.Version("1.0.0") + # Pre-populate versions so choose_version works. + provider._versions[qname] = [v] + deps1 = provider.get_dependencies(qname, v) + deps2 = provider.get_dependencies(qname, v) + assert deps1 == deps2 + + def test_get_dependencies_empty_for_unknown( manager: Manager, tmp_path: pathlib.Path, @@ -214,6 +239,40 @@ def test_get_dependencies_empty_for_unknown( assert deps == {} +def test_provider_init_git_error_falls_back_gracefully( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # metadata_file present but git.Repo() raises -- provider must not crash. + info = MagicMock(spec=PackageInfo) + info.metadata_file = str(tmp_path / "not-a-repo" / "zkg.meta") + info.metadata_version = "1.0.0" + info.invalid_reason = None + info.versions = [] + node = _Node("org/broken") + node.info = info + provider = _ZkgProvider(manager, {"org/broken": node}) + # Falls back to metadata_version. + assert semver.Version("1.0.0") in provider._versions.get("org/broken", []) + + +def test_provider_init_no_metadata_file( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # Node with no metadata_file (builtin/directory package) -- provider + # falls through to the version-from-metadata path immediately. + info = MagicMock(spec=PackageInfo) + info.metadata_file = None + info.metadata_version = "2.0.0" + info.invalid_reason = None + info.versions = [] + node = _Node("org/builtin") + node.info = info + provider = _ZkgProvider(manager, {"org/builtin": node}) + assert semver.Version("2.0.0") in provider._versions.get("org/builtin", []) + + def test_fetch_deps_no_metadata_file( manager: Manager, tmp_path: pathlib.Path, @@ -234,6 +293,114 @@ def test_fetch_deps_no_metadata_file( assert deps == {} +def test_provider_init_version_from_versions_list(manager: Manager) -> None: + # No metadata_version; fall back to the last entry in info.versions. + info = MagicMock(spec=PackageInfo) + info.metadata_file = None + info.metadata_version = None + info.installed_version = None + info.versions = ["1.1.0", "1.2.0"] + info.invalid_reason = None + node = _Node("org/pkg") + node.info = info + provider = _ZkgProvider(manager, {"org/pkg": node}) + assert semver.Version("1.2.0") in provider._versions.get("org/pkg", []) + + +def test_provider_init_falls_back_to_zero_version(manager: Manager) -> None: + # No version inferable from any source -- provider must register 0.0.0 so + # the solver can still attempt resolution. + info = MagicMock(spec=PackageInfo) + info.metadata_file = None + info.metadata_version = None + info.installed_version = None + info.versions = [] + info.invalid_reason = None + node = _Node("org/pkg") + node.info = info + provider = _ZkgProvider(manager, {"org/pkg": node}) + assert provider._versions.get("org/pkg") == [semver.Version("0.0.0")] + + +def test_fetch_deps_non_git_directory(manager: Manager, tmp_path: pathlib.Path) -> None: + # metadata_file points inside a plain directory (not a git repo) -- + # _fetch_deps must fall through to the InvalidGitRepositoryError handler. + plain_dir = tmp_path / "plain" + plain_dir.mkdir() + meta_file = plain_dir / "zkg.meta" + meta_file.write_text("[package]\ndescription = plain\n") + info = MagicMock(spec=PackageInfo) + info.metadata_file = str(meta_file) + info.metadata_version = "1.0.0" + info.best_version.return_value = "1.0.0" + info.dependencies.return_value = {} + info.invalid_reason = None + info.versions = [] + node = _Node("org/plain") + node.info = info + provider = _ZkgProvider(manager, {"org/plain": node}) + provider._versions["org/plain"] = [semver.Version("1.0.0")] + deps = provider.get_dependencies("org/plain", semver.Version("1.0.0")) + assert deps == {} + + +def test_fetch_deps_synthetic_version( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # Version 9.9.9 has no matching tag -- _fetch_deps takes the synthetic + # version branch and reads current HEAD metadata instead. + repo = _make_tagged_repo(tmp_path, "synth", [("v1.0.0", "")]) + info = MagicMock(spec=PackageInfo) + info.metadata_file = str(pathlib.Path(str(repo.working_dir)) / "zkg.meta") + info.metadata_version = "9.9.9" + info.best_version.return_value = "9.9.9" + info.dependencies.return_value = {} + info.invalid_reason = None + info.versions = [] + node = _Node("org/synth") + node.info = info + provider = _ZkgProvider(manager, {"org/synth": node}) + provider._versions["org/synth"] = [semver.Version("9.9.9")] + deps = provider.get_dependencies("org/synth", semver.Version("9.9.9")) + assert deps == {} + + +# --------------------------------------------------------------------------- +# _fmt_range -- tests focus on cases where nab-resolver's raw interval +# notation would expose sentinel strings like "(-inf, X) | (X, +inf)" +# --------------------------------------------------------------------------- + + +def test_fmt_range_open_upper_bound() -> None: + # Range.at_least internally stores (-inf sentinel, X, +inf sentinel). + # _fmt_range must produce ">=X", not expose sentinel strings. + r = Range.at_least(semver.Version("1.0.0")) + result = _fmt_range(r) + assert "inf" not in result + assert result == ">=1.0.0" + + +def test_fmt_range_excluded_version_no_inf() -> None: + # Excluding a single version produces two half-open intervals with -inf/+inf + # sentinels. _fmt_range must render them as operator-prefixed strings. + r = Range.less_than(semver.Version("1.0.0")) | Range.greater_than( + semver.Version("1.0.0"), + ) + result = _fmt_range(r) + assert "inf" not in result + assert result == "<1.0.0 | >1.0.0" + + +def test_fmt_range_exact_point() -> None: + r = Range.singleton(semver.Version("1.2.3")) + assert _fmt_range(r) == "=1.2.3" + + +def test_fmt_range_full_is_wildcard() -> None: + assert _fmt_range(Range.full()) == "*" + + def _make_pkg_repo_with_deps( tmp_path: pathlib.Path, name: str, @@ -294,3 +461,17 @@ def test_zkgprovider_strips_zeek_zkg( [("v1.0.0", "zeek >=5.0.0 zkg >=3.0.0")], ) manager.validate_dependencies([(str(pkg_repo.working_dir), "")]) + + +def test_qualify_deps_skips_invalid_dep(manager: Manager) -> None: + # _qualify_deps must silently drop a dep whose PackageInfo has invalid_reason + # set rather than forwarding it to the solver. + invalid_info = MagicMock(spec=PackageInfo) + invalid_info.invalid_reason = "not a valid package" + provider = _ZkgProvider(manager, {}) + with ( + patch.object(manager, "find_builtin_package", return_value=None), + patch.object(manager, "info", return_value=invalid_info), + ): + result = provider._qualify_deps({"bad-dep": ">=1.0.0"}) + assert result == {} diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py index 277261f..5681a9a 100644 --- a/zeekpkg/_resolver.py +++ b/zeekpkg/_resolver.py @@ -11,7 +11,7 @@ import configparser import os from collections.abc import Mapping -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, cast import git import semantic_version as semver @@ -19,7 +19,7 @@ from nab_resolver.resolver import ResolverProvider from nab_resolver.types import Incompatibility, RangeProtocol -from ._util import _semver_versions, git_version_tags +from ._util import _semver_versions, git_version_tags, is_sha1 from .package import ( LEGACY_METADATA_FILENAME, METADATA_FILENAME, @@ -93,6 +93,35 @@ def _constraint_to_range(constraint: str) -> Range[semver.Version]: return result +def _fmt_range(r: Range[semver.Version]) -> str: + """Format a Range as a user-friendly constraint string. + + Replaces nab-resolver's default ``(-inf, X) | (X, +inf)`` notation with + operator-prefixed semver strings such as ``<1.0.0 | >1.0.0``. + """ + parts = [] + for lo, lo_inc, hi, hi_inc in r._intervals: + lo_inf = not isinstance(lo, semver.Version) + hi_inf = not isinstance(hi, semver.Version) + if lo_inf and hi_inf: + parts.append("*") + elif lo_inf: + parts.append(("<=" if hi_inc else "<") + str(hi)) + elif hi_inf: + parts.append((">=" if lo_inc else ">") + str(lo)) + elif lo == hi and lo_inc and hi_inc: + parts.append("=" + str(lo)) + else: + parts.append( + (">=" if lo_inc else ">") + + str(lo) + + ", " + + ("<=" if hi_inc else "<") + + str(hi), + ) + return " | ".join(parts) if parts else "none" + + class _ZkgProvider(ResolverProvider["str", "semver.Version"]): """nab-resolver `ResolverProvider` backed by zkg package metadata. @@ -122,6 +151,25 @@ def __init__( ] except Exception: pass + if not self._versions.get(qname) and node.info: + # No git tags: try a concrete version from metadata, installed + # state, or versions list; fall back to 0.0.0 so the solver can + # still resolve packages with no semver release history. + registered = False + for raw in ( + node.info.metadata_version, + node.installed_version.version if node.installed_version else None, + node.info.versions[-1] if node.info.versions else None, + ): + if raw and not is_sha1(raw): + try: + self._versions[qname] = [semver.Version.coerce(raw)] + registered = True + except ValueError: + pass + break + if not registered: + self._versions[qname] = [semver.Version("0.0.0")] def begin_decision_scan(self) -> None: return None @@ -160,6 +208,19 @@ def get_dependencies( pass return result + def _qualify_deps(self, raw_deps: dict[str, str]) -> dict[str, str]: + result: dict[str, str] = {} + for dep, spec in raw_deps.items(): + if dep in ("zeek", "zkg") or spec.startswith("branch="): + continue + di = self._manager.find_builtin_package(dep) + if di is None: + di = self._manager.info(dep, prefer_installed=False) + if di.invalid_reason: + continue + result[di.package.qualified_name()] = _normalize_constraint(spec) + return result + def _fetch_deps( self, qname: str, @@ -172,36 +233,28 @@ def _fetch_deps( # Builtin / directory package -- use current metadata. raw_tag = node.info.metadata_version or node.info.best_version() raw_deps = node.info.dependencies(field="depends") or {} - qualified: dict[str, str] = {} - for dep, spec in raw_deps.items(): - if dep in ("zeek", "zkg") or spec.startswith("branch="): - continue - di = self._manager.find_builtin_package(dep) - if di is None: - di = self._manager.info(dep, prefer_installed=False) - if di.invalid_reason: - continue - qualified[di.package.qualified_name()] = _normalize_constraint(spec) - return (raw_tag, qualified) + return (raw_tag, self._qualify_deps(raw_deps)) clone_dir = os.path.dirname(node.info.metadata_file) - clone = git.Repo(clone_dir) - raw_tag = str(version) + try: + clone = git.Repo(clone_dir) + except git.InvalidGitRepositoryError: + # Directory package -- no git history; use current metadata. + raw_tag = node.info.metadata_version or node.info.best_version() + raw_deps = node.info.dependencies(field="depends") or {} + return (raw_tag, self._qualify_deps(raw_deps)) + found_tag: str | None = None for rt, nv in _semver_versions(git_version_tags(clone)): if semver.Version.coerce(nv) == version: - raw_tag = rt + found_tag = rt break - raw_deps = _deps_at_version(clone, raw_tag) - qualified_deps: dict[str, str] = {} - for dep, spec in raw_deps.items(): - if dep in ("zeek", "zkg") or spec.startswith("branch="): - continue - di = self._manager.find_builtin_package(dep) - if di is None: - di = self._manager.info(dep, prefer_installed=False) - if di.invalid_reason: - continue - qualified_deps[di.package.qualified_name()] = _normalize_constraint(spec) - return (raw_tag, qualified_deps) + if found_tag is None: + # Synthetic version (no matching tag) -- use current HEAD metadata. + raw_deps = node.info.dependencies(field="depends") or {} + raw_tag = node.info.metadata_version or node.info.best_version() + else: + raw_tag = found_tag + raw_deps = _deps_at_version(clone, raw_tag) + return (raw_tag, self._qualify_deps(raw_deps)) def prioritize( self, @@ -240,7 +293,16 @@ def narrow_for_display( package: str, constraint: RangeProtocol[semver.Version], ) -> RangeProtocol[semver.Version]: - return constraint + r = cast(Range[semver.Version], constraint) + display = _fmt_range(r) + + class _Displayed(Range[semver.Version]): + __slots__ = () + + def __str__(self) -> str: + return display + + return cast(RangeProtocol[semver.Version], _Displayed(r._intervals)) def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index a8be854..015c6aa 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -21,6 +21,9 @@ import git import semantic_version as semver +from nab_resolver.errors import ResolutionError +from nab_resolver.ranges import Range +from nab_resolver.resolver import Resolver from . import ( LOG, @@ -31,6 +34,7 @@ _get_branch_names, _Node, _normalize_constraint, + _ZkgProvider, ) from ._util import ( configparser_section_dict, @@ -2309,211 +2313,278 @@ def add_node(node: _Node) -> str: return (err, []) to_process[node.name] = node - # 2. Fill in the edges of the graph with dependency information. - for name, node in graph.items(): - if name == "zeek": - continue - - if name == "zkg": - continue - - assert node.info - dd = node.info.dependencies(field="depends") - ds = node.info.dependencies(field="suggests") + info_cache: dict[str, PackageInfo] = {} - if dd is None: - return ( - f'package "{node.name}" has malformed "depends" field', - [], - ) - - all_deps = dd.copy() + def _cached_info(pkg_path: str) -> PackageInfo: + key = canonical_url(pkg_path) + if key not in info_cache: + info_cache[key] = self.info(pkg_path, prefer_installed=False) + return info_cache[key] + def _is_versioned_package(v: str) -> bool: + """Return True if *v* is a semver-coercible version the solver can use.""" + if is_sha1(v): + return False + try: + semver.Version.coerce(v) + return True + except ValueError: + return False + + # Identify branch-constrained packages across all graph nodes (including + # suggests when not ignore_suggestions). + branch_pkgs: list[tuple[PackageInfo, str, bool]] = [] + branch_pkg_names: set[str] = set() + + for src_node in list(graph.values()): + if src_node.info is None: + continue + src_deps: dict[str, str] = src_node.info.dependencies(field="depends") or {} if not ignore_suggestions: - if ds is None: + src_deps = { + **src_deps, + **(src_node.info.dependencies(field="suggests") or {}), + } + for dep_name, spec in src_deps.items(): + if not spec.startswith("branch="): + continue + branch_name = spec[len("branch=") :] + dep_info = self.find_builtin_package(dep_name) + if dep_info is None: + dep_info = _cached_info(dep_name) + if dep_info.invalid_reason: return ( - f'package "{node.name}" has malformed "suggests" field', + f'package "{src_node.name}" has invalid dependency "{dep_name}":' + f" {dep_info.invalid_reason}", [], ) + qn = dep_info.package.qualified_name() + if graph.get(qn) and graph[qn].installed_version: + iv = graph[qn].installed_version + assert iv + msg, ok = iv.fullfills(spec) + if not ok: + return ( + f'unsatisfiable dependency: "{qn}" ({iv.version}) is' + f' installed, but "{src_node.name}" requires {spec} ({msg})', + [], + ) + elif qn not in branch_pkg_names: + branch_pkgs.append( + (dep_info, branch_name, src_node.is_suggestion), + ) + branch_pkg_names.add(qn) - all_deps.update(ds) - - for dep_name, dep_version in all_deps.items(): - if dep_name == "zeek": - if "zeek" in graph: - graph["zeek"].dependers[name] = dep_version - node.dependees["zeek"] = dep_version - elif dep_name == "zkg": - if "zkg" in graph: - graph["zkg"].dependers[name] = dep_version - node.dependees["zkg"] = dep_version - else: - for _, dependency_node in graph.items(): - if dependency_node.name == "zeek": - continue - - if dependency_node.name == "zkg": - continue - - assert dependency_node.info - if dependency_node.info.package.matches_path(dep_name): - dependency_node.dependers[name] = dep_version - node.dependees[dependency_node.name] = dep_version - break - - # 3. Try to solve for a connected graph with no edge conflicts. - - # Traverse graph in breadth-first order, starting from artificial root - # with all nodes requested by caller as child nodes. - nodes_todo = requests - - # The resulting list of packages required to satisfy dependencies, - # in depender -> dependent (i.e., root -> leaves in dependency tree) - # order. - new_pkgs: list[tuple[PackageInfo, str, bool]] = [] - - while nodes_todo: - node = nodes_todo.pop(0) - for name in node.dependees: - nodes_todo.append(graph[name]) + # hard_pinned: builtins, directory packages, zeek, zkg -- cannot be upgraded. + # soft_pinned: git-backed installed packages -- solver may upgrade them. + hard_pinned: dict[str, str] = {} + soft_pinned: dict[str, str] = {} + installed_qnames: set[str] = set() + if not ignore_installed_packages: + zeek_v = get_zeek_version() + if zeek_v: + hard_pinned["zeek"] = normalize_version_tag(zeek_v) + hard_pinned["zkg"] = normalize_version_tag(__version__) + for ipkg in self.installed_packages(): + iname = ipkg.package.qualified_name() + installed_qnames.add(iname) + installed_ver = ipkg.status.current_version + if installed_ver: + norm = normalize_version_tag(installed_ver) + if ipkg.status.tracking_method is None: + hard_pinned[iname] = norm + else: + soft_pinned[iname] = norm - # Avoid cyclic dependencies: ensure we traverse these edges only - # once. (The graph may well be a dag, so it's okay to encounter - # specific nodes repeatedly.) - node.dependees = {} + explicitly_requested: set[str] = { + node.name for node in requests if node.info is not None + } - if not node.dependers: - if node.installed_version: - # We can ignore packages alreaday installed if nothing else - # depends on them. - continue + provider = _ZkgProvider(self, graph) + resolver: Resolver[str, semver.Version] = Resolver( + provider, + range_type=Range, + root_version=semver.Version("0.0.0"), + ) - if node.requested_version: - # Only the packges requested by the caller have a requested - # version. We skip those too if nothing depends on them. + requirements: dict[str, Range[semver.Version]] = {} + requested_qnames: set[str] = set() + for req_node in requests: + assert req_node.info + qname = req_node.name + requested_qnames.add(qname) + rv = req_node.requested_version + if rv and rv.version: + norm = normalize_version_tag(rv.version) + if _is_versioned_package(norm): + requirements[qname] = Range.singleton(semver.Version.coerce(norm)) continue - - # A new package nothing depends on -- odd? - assert node.info - new_pkgs.append( - (node.info, node.info.best_version(), node.is_suggestion), - ) + # Branch-tracked or no semver version: any version satisfies. + requirements[qname] = Range.full() + for binfo, _, _ in branch_pkgs: + bqn = binfo.package.qualified_name() + if bqn not in requirements and bqn not in hard_pinned: + requirements[bqn] = Range.singleton(semver.Version("0.0.0")) + + constraints: dict[str, Range[semver.Version]] = {} + for qname, norm in hard_pinned.items(): + if qname in explicitly_requested: + continue + if _is_versioned_package(norm): + constraints[qname] = Range.singleton(semver.Version.coerce(norm)) + for qname, norm in soft_pinned.items(): + if qname in explicitly_requested: continue + if _is_versioned_package(norm): + constraints[qname] = Range.singleton(semver.Version.coerce(norm)) + + # Branch packages are installed at a specific branch ref, not a semver + # tag. Register only the synthetic 0.0.0 version and pre-populate the + # cache with their current deps (including suggests when applicable). + for binfo, _, _ in branch_pkgs: + bqn = binfo.package.qualified_name() + synth_v = semver.Version("0.0.0") + provider._versions[bqn] = [synth_v] + raw_bdeps: dict[str, str] = binfo.dependencies(field="depends") or {} + if not ignore_suggestions: + raw_bdeps = { + **raw_bdeps, + **(binfo.dependencies(field="suggests") or {}), + } + bsynth_deps: dict[str, str] = {} + for dep_s, dep_spec in raw_bdeps.items(): + if dep_s in ("zeek", "zkg") or dep_spec.startswith("branch="): + continue + di = self.find_builtin_package(dep_s) + if di is None: + di = _cached_info(dep_s) + if not di.invalid_reason: + bsynth_deps[di.package.qualified_name()] = _normalize_constraint( + dep_spec, + ) + provider._cache[(bqn, synth_v)] = (binfo.best_version(), bsynth_deps) - if node.requested_version: - # Check that requested version doesn't conflict with dependers. - for depender_name, version_spec in node.dependers.items(): - msg, fullfills = node.requested_version.fullfills(version_spec) - if not fullfills: - return ( - f'unsatisfiable dependency: requested "{node.name}" ({node.requested_version.version}),' - f' but "{depender_name}" requires {version_spec} ({msg})', - new_pkgs, - ) + try: + resolved: dict[str, semver.Version] = resolver.resolve( + requirements, + constraints=constraints, + ) + except ResolutionError as e: + return (str(e).split("\n")[0], []) - elif node.installed_version: - # Check that installed version doesn't conflict with dependers. - # track_method, required_version = node.installed_version + suggestion_names: set[str] = { + name for name, node in graph.items() if node.is_suggestion + } - for depender_name, version_spec in node.dependers.items(): - msg, fullfills = node.installed_version.fullfills(version_spec) - if not fullfills: - return ( - f'unsatisfiable dependency: "{node.name}" ({node.installed_version.version}) is installed,' - f' but "{depender_name}" requires {version_spec} ({msg})', - new_pkgs, + def _pkg_deps(qn: str) -> list[str]: + """Return dep qualified names for *qn* from provider cache or node info.""" + result_d: list[str] = [] + rv = resolved.get(qn) + if rv is not None: + cache_entry = provider._cache.get((qn, rv)) + if cache_entry: + _, d = cache_entry + result_d = list(d) + if not ignore_suggestions: + nd = graph.get(qn) + if nd and nd.info: + raw_sug = nd.info.dependencies(field="suggests") or {} + for dep_s in raw_sug: + if dep_s in ("zeek", "zkg"): + continue + di = self.find_builtin_package(dep_s) + if di is None: + di = _cached_info(dep_s) + if not di.invalid_reason: + dqn = di.package.qualified_name() + if dqn not in result_d: + result_d.append(dqn) + return sorted(result_d) + nd = graph.get(qn) + if nd and nd.info: + raw: dict[str, str] = nd.info.dependencies(field="depends") or {} + if not ignore_suggestions: + raw = {**raw, **(nd.info.dependencies(field="suggests") or {})} + for dep_s in raw: + if dep_s in ("zeek", "zkg"): + continue + di = self.find_builtin_package(dep_s) + if di is None: + di = _cached_info(dep_s) + if not di.invalid_reason: + result_d.append(di.package.qualified_name()) + return sorted(result_d) + + dfs_visited: set[str] = set() + dfs_in_stack: set[str] = set() + + def _dfs_emit(start: str) -> list[tuple[str, str, bool]]: + result: list[tuple[str, str, bool]] = [] + stack: list[tuple[str, bool]] = [(start, False)] + while stack: + qn, post = stack.pop() + if post: + dfs_in_stack.discard(qn) + is_upgraded = ( + qn in soft_pinned + and qn in resolved + and _is_versioned_package(soft_pinned[qn]) + and resolved[qn] > semver.Version.coerce(soft_pinned[qn]) + ) + if ( + qn in requested_qnames + or (qn in installed_qnames and not is_upgraded) + or qn in branch_pkg_names + ): + continue + node = graph.get(qn) + if node is None or node.info is None: + continue + is_sug = qn in suggestion_names + rv = resolved.get(qn) + if rv is not None: + cache_entry = provider._cache.get((qn, rv)) + raw_tag = ( + cache_entry[0] if cache_entry else node.info.best_version() ) - else: - # Choose best version that satisfies constraints - best_version = None - need_branch = False - need_version = False - - def no_best_version_string(node: _Node) -> str: - rval = f'"{node.name}" has no version satisfying dependencies:\n' - - for depender_name, version_spec in node.dependers.items(): - rval += f'\t"{depender_name}" requires: "{version_spec}"\n' - - return rval - - for _, version_spec in node.dependers.items(): - if version_spec.startswith("branch="): - need_branch = True - elif version_spec != "*": - need_version = True - - if need_branch and need_version: - return (no_best_version_string(node), new_pkgs) - - if need_branch: - branch_name = None - - for _, version_spec in node.dependers.items(): - if version_spec == "*": - continue - - if not branch_name: - branch_name = version_spec[len("branch=") :] - continue - - if branch_name != version_spec[len("branch=") :]: - return (no_best_version_string(node), new_pkgs) - - if branch_name: - best_version = branch_name else: - assert node.info - best_version = node.info.default_branch - elif need_version: - assert node.info - for version in node.info.versions[::-1]: - normal_version = normalize_version_tag(version) - req_semver = semver.Version.coerce(normal_version) - - satisfied = True - - for depender_name, version_spec in node.dependers.items(): - try: - semver_spec = semver.SimpleSpec( - _normalize_constraint(version_spec), - ) - except ValueError: - return ( - f'package "{depender_name}" has invalid semver spec: {version_spec}', - new_pkgs, - ) - - if req_semver not in semver_spec: - satisfied = False - break - - if satisfied: - best_version = version - break - - if not best_version: - return (no_best_version_string(node), new_pkgs) + raw_tag = node.info.best_version() + result.append((qn, raw_tag, is_sug)) else: - # Must have been all '*' wildcards or no dependers - assert node.info - best_version = node.info.best_version() - - assert node.info - assert best_version - new_pkgs.append((node.info, best_version, node.is_suggestion)) - - # Remove duplicate new nodes, preserving their latest (i.e. deepest-in- - # tree) occurrences. Traversing the resulting list right-to-left guarantees - # that we never visit a node before we've visited all of its dependees. - seen_nodes = set() - res: list[tuple[PackageInfo, str, bool]] = [] + if qn in dfs_visited or qn in dfs_in_stack: + continue + dfs_visited.add(qn) + dfs_in_stack.add(qn) + stack.append((qn, True)) + for dep_qn in reversed(_pkg_deps(qn)): + if dep_qn not in dfs_visited and dep_qn not in dfs_in_stack: + stack.append((dep_qn, False)) + return result + + seeds = list(requested_qnames) + [ + binfo.package.qualified_name() for binfo, _, _ in branch_pkgs + ] + post_order: list[tuple[str, str, bool]] = [] + for seed in seeds: + post_order.extend(_dfs_emit(seed)) - for it in reversed(new_pkgs): - if it[0].package.name in seen_nodes: + # post_order is leaves-first; reverse to get root-first for the return + # value (caller reverses again when installing, so leaves end up first). + seen_res: set[str] = set() + res: list[tuple[PackageInfo, str, bool]] = [] + for qn, raw_tag, is_sug in reversed(post_order): + if qn in seen_res: continue - seen_nodes.add(it[0].package.name) - res.insert(0, it) + seen_res.add(qn) + res_node = graph.get(qn) + if res_node is None or res_node.info is None: + continue + res.append((res_node.info, raw_tag, is_sug)) + + for binfo, bversion, bsug in branch_pkgs: + bqn = binfo.package.qualified_name() + if bqn not in installed_qnames and bqn not in requested_qnames: + res.append((binfo, bversion, bsug)) return ("", res) From 395c2cc3e9a014a6f1c44ce4f15e739d53f50ebe Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Mon, 20 Jul 2026 18:54:08 +0200 Subject: [PATCH 09/12] Relax installed packages to soft lower-bound to allow upgrades Hard-pinning all installed packages blocked the common case where a new package requires a newer version of something already installed. Directory- backed packages and zeek/zkg stay hard-pinned because zkg has no mechanism to upgrade them. --- .../conflict.out | 0 .../installed-final.out | 4 ++-- .../installed-initial.out | 0 .../tests.upgrade-via-dependency/installed-final.out | 3 ++- ...ency-conflict => installed-dependency-conflict-resolution} | 4 +--- testing/tests/upgrade-via-dependency | 4 +--- zeekpkg/manager.py | 2 +- 7 files changed, 7 insertions(+), 10 deletions(-) rename testing/baselines/{tests.installed-dependency-conflict => tests.installed-dependency-conflict-resolution}/conflict.out (100%) rename testing/baselines/{tests.installed-dependency-conflict => tests.installed-dependency-conflict-resolution}/installed-final.out (63%) rename testing/baselines/{tests.installed-dependency-conflict => tests.installed-dependency-conflict-resolution}/installed-initial.out (100%) rename testing/tests/{installed-dependency-conflict => installed-dependency-conflict-resolution} (73%) diff --git a/testing/baselines/tests.installed-dependency-conflict/conflict.out b/testing/baselines/tests.installed-dependency-conflict-resolution/conflict.out similarity index 100% rename from testing/baselines/tests.installed-dependency-conflict/conflict.out rename to testing/baselines/tests.installed-dependency-conflict-resolution/conflict.out diff --git a/testing/baselines/tests.installed-dependency-conflict/installed-final.out b/testing/baselines/tests.installed-dependency-conflict-resolution/installed-final.out similarity index 63% rename from testing/baselines/tests.installed-dependency-conflict/installed-final.out rename to testing/baselines/tests.installed-dependency-conflict-resolution/installed-final.out index 92b637f..7226c78 100644 --- a/testing/baselines/tests.installed-dependency-conflict/installed-final.out +++ b/testing/baselines/tests.installed-dependency-conflict-resolution/installed-final.out @@ -1,3 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -one/alice/bar (installed: 1.0.0) -one/alice/foo (installed: 1.0.0) +one/alice/bar (installed: 2.0.0) +one/alice/foo (installed: 2.0.0) diff --git a/testing/baselines/tests.installed-dependency-conflict/installed-initial.out b/testing/baselines/tests.installed-dependency-conflict-resolution/installed-initial.out similarity index 100% rename from testing/baselines/tests.installed-dependency-conflict/installed-initial.out rename to testing/baselines/tests.installed-dependency-conflict-resolution/installed-initial.out diff --git a/testing/baselines/tests.upgrade-via-dependency/installed-final.out b/testing/baselines/tests.upgrade-via-dependency/installed-final.out index d4b57fb..e58fafe 100644 --- a/testing/baselines/tests.upgrade-via-dependency/installed-final.out +++ b/testing/baselines/tests.upgrade-via-dependency/installed-final.out @@ -1,2 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -one/alice/bar (installed: 1.0.0) +one/alice/bar (installed: 2.0.0) +one/alice/foo (installed: main) diff --git a/testing/tests/installed-dependency-conflict b/testing/tests/installed-dependency-conflict-resolution similarity index 73% rename from testing/tests/installed-dependency-conflict rename to testing/tests/installed-dependency-conflict-resolution index e49509e..8d45685 100644 --- a/testing/tests/installed-dependency-conflict +++ b/testing/tests/installed-dependency-conflict-resolution @@ -4,12 +4,10 @@ # @TEST-EXEC: zkg install --version 1.0.0 foo # @TEST-EXEC: zkg list installed > installed-initial.out -# @TEST-EXEC-FAIL: zkg install --version 2.0.0 foo >conflict.out 2>&1 -# @TEST-EXEC: head -2 conflict.out > conflict.trimmed.out && mv conflict.trimmed.out conflict.out +# @TEST-EXEC: zkg install --version 2.0.0 foo # @TEST-EXEC: zkg list installed > installed-final.out # @TEST-EXEC: btest-diff installed-initial.out -# @TEST-EXEC: btest-diff conflict.out # @TEST-EXEC: btest-diff installed-final.out cd packages/foo diff --git a/testing/tests/upgrade-via-dependency b/testing/tests/upgrade-via-dependency index 0aea1f7..9ee4795 100644 --- a/testing/tests/upgrade-via-dependency +++ b/testing/tests/upgrade-via-dependency @@ -3,12 +3,10 @@ # @TEST-EXEC: zkg install --version 1.0.0 bar # @TEST-EXEC: zkg list installed > installed-initial.out -# @TEST-EXEC-FAIL: zkg install foo >conflict.out 2>&1 -# @TEST-EXEC: head -1 conflict.out > conflict.trimmed.out && mv conflict.trimmed.out conflict.out +# @TEST-EXEC: zkg install foo # @TEST-EXEC: zkg list installed > installed-final.out # @TEST-EXEC: btest-diff installed-initial.out -# @TEST-EXEC: btest-diff conflict.out # @TEST-EXEC: btest-diff installed-final.out cd packages/foo diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index 015c6aa..99b4f1b 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -2436,7 +2436,7 @@ def _is_versioned_package(v: str) -> bool: if qname in explicitly_requested: continue if _is_versioned_package(norm): - constraints[qname] = Range.singleton(semver.Version.coerce(norm)) + constraints[qname] = Range.at_least(semver.Version.coerce(norm)) # Branch packages are installed at a specific branch ref, not a semver # tag. Register only the synthetic 0.0.0 version and pre-populate the From 826b85e33d965750bb690cbf8158547ce3c6c531 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Sat, 25 Jul 2026 15:28:00 +0200 Subject: [PATCH 10/12] Move solver logic into _resolver.py `manager.py` imported nab-resolver types directly. Move `_run_solver` (resolver construction, DFS topo sort) into `_resolver.py` and re-export `Range` from there so nab-resolver stays an implementation detail of the resolver module. `manager.py` now only passes in already-constructed `Range` values and a `lookup_dep` callable. --- .../tests.builtin-spicy-version-error/output | 6 + .../fail.out | 6 + testing/test_resolver.py | 462 +++++++++++++++++- zeekpkg/_resolver.py | 212 +++++++- zeekpkg/manager.py | 157 +----- 5 files changed, 702 insertions(+), 141 deletions(-) diff --git a/testing/baselines/tests.builtin-spicy-version-error/output b/testing/baselines/tests.builtin-spicy-version-error/output index 45f27fb..c8b2a83 100644 --- a/testing/baselines/tests.builtin-spicy-version-error/output +++ b/testing/baselines/tests.builtin-spicy-version-error/output @@ -1,2 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. error: failed to resolve dependencies: because no versions of one/alice/foo X.X.X are available +because one/alice/foo =X.X.X depends on zeek-builtin/spicy-plugin (-inf, X.X.X) +so one/alice/foo * and not zeek-builtin/spicy-plugin (-inf, X.X.X) +because no versions of zeek-builtin/spicy-plugin =X.X.X diff --git a/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out b/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out index cb721b1..53109bd 100644 --- a/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out +++ b/testing/baselines/tests.unsatisfied-version-tag-dependency/fail.out @@ -1,2 +1,8 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. error: failed to resolve dependencies: because no versions of one/alice/foo <0.0.0 | >0.0.0 are available +because one/alice/foo =0.0.0 depends on one/alice/bar 1.0.0 +so one/alice/foo * and not one/alice/bar 1.0.0 +because no versions of one/alice/bar =1.0.0 are available +so one/alice/foo * +because your project depends on one/alice/foo * +so =0.0.0 diff --git a/testing/test_resolver.py b/testing/test_resolver.py index dbb48c0..0858661 100644 --- a/testing/test_resolver.py +++ b/testing/test_resolver.py @@ -1,6 +1,7 @@ """Unit tests for zeekpkg._resolver internals.""" import pathlib +from typing import cast from unittest.mock import MagicMock, patch import git @@ -10,9 +11,14 @@ from zeekpkg._resolver import ( _constraint_to_range, + _deps_at_version, _fmt_range, + _FmtRange, + _get_branch_names, + _is_versioned_package, _Node, _normalize_constraint, + _run_solver, _ZkgProvider, ) from zeekpkg.manager import Manager @@ -81,6 +87,19 @@ def test_node_str() -> None: assert "installed" in s +def test_get_branch_names(tmp_path: pathlib.Path) -> None: + repo = git.Repo.init(tmp_path / "br-repo", initial_branch="main") + repo.config_writer().set_value("user", "name", "Test").release() + repo.config_writer().set_value("user", "email", "test@test").release() + (tmp_path / "br-repo" / "f").write_text("x") + repo.index.add(["f"]) + repo.index.commit("init") + # Simulate remote tracking refs by creating refs/remotes/origin/main manually. + repo.git.update_ref("refs/remotes/origin/main", "HEAD") + branches = _get_branch_names(repo) + assert "main" in branches + + def test_normalize_bare_equals() -> None: assert _normalize_constraint("=1.0.0") == "==1.0.0" @@ -122,6 +141,18 @@ def test_constraint_to_range_wildcard() -> None: assert semver.Version("99.0.0") in r +def test_constraint_to_range_gt() -> None: + r = _constraint_to_range(">1.0.0") + assert semver.Version("1.0.1") in r + assert semver.Version("1.0.0") not in r + + +def test_constraint_to_range_lte() -> None: + r = _constraint_to_range("<=1.0.0") + assert semver.Version("1.0.0") in r + assert semver.Version("1.0.1") not in r + + def test_constraint_to_range_compound() -> None: r = _constraint_to_range(">=1.0.0,<2.0.0") assert semver.Version("1.5.0") in r @@ -218,11 +249,11 @@ def test_get_dependencies_caches_result( [("v1.0.0", f"{dep_repo.working_dir} >=1.0.0")], ) v = semver.Version("1.0.0") - # Pre-populate versions so choose_version works. provider._versions[qname] = [v] - deps1 = provider.get_dependencies(qname, v) - deps2 = provider.get_dependencies(qname, v) - assert deps1 == deps2 + with patch.object(provider, "_fetch_deps", wraps=provider._fetch_deps) as spy: + provider.get_dependencies(qname, v) + provider.get_dependencies(qname, v) + assert spy.call_count == 1 def test_get_dependencies_empty_for_unknown( @@ -307,6 +338,23 @@ def test_provider_init_version_from_versions_list(manager: Manager) -> None: assert semver.Version("1.2.0") in provider._versions.get("org/pkg", []) +def test_provider_init_version_coercion_failure_falls_back_to_zero( + manager: Manager, +) -> None: + # versions[-1] is not a valid semver string -- coerce raises ValueError and + # we fall through to the 0.0.0 sentinel. + info = MagicMock(spec=PackageInfo) + info.metadata_file = None + info.metadata_version = None + info.installed_version = None + info.versions = ["not-a-version"] + info.invalid_reason = None + node = _Node("org/pkg") + node.info = info + provider = _ZkgProvider(manager, {"org/pkg": node}) + assert provider._versions.get("org/pkg") == [semver.Version("0.0.0")] + + def test_provider_init_falls_back_to_zero_version(manager: Manager) -> None: # No version inferable from any source -- provider must register 0.0.0 so # the solver can still attempt resolution. @@ -401,6 +449,62 @@ def test_fmt_range_full_is_wildcard() -> None: assert _fmt_range(Range.full()) == "*" +def test_fmt_range_subclass_str() -> None: + r = _FmtRange(Range.at_least(semver.Version("1.0.0"))._intervals) + assert str(r) == ">=1.0.0" + assert "inf" not in str(r) + + +def test_fmt_range_and_preserves_subclass() -> None: + a = _FmtRange(Range.at_least(semver.Version("1.0.0"))._intervals) + b = _FmtRange(Range.less_than(semver.Version("2.0.0"))._intervals) + result = a & b + assert isinstance(result, _FmtRange) + assert str(result) == ">=1.0.0, <2.0.0" + + +def test_fmt_range_or_preserves_subclass() -> None: + a = _FmtRange(Range.less_than(semver.Version("1.0.0"))._intervals) + b = _FmtRange(Range.greater_than(semver.Version("2.0.0"))._intervals) + result = a | b + assert isinstance(result, _FmtRange) + assert "inf" not in str(result) + + +def test_fmt_range_invert_preserves_subclass() -> None: + r = _FmtRange(Range.singleton(semver.Version("1.0.0"))._intervals) + result = ~r + assert isinstance(result, _FmtRange) + assert "inf" not in str(result) + + +def test_fmt_range_sub_preserves_subclass() -> None: + a = _FmtRange(Range.at_least(semver.Version("1.0.0"))._intervals) + b = _FmtRange(Range.singleton(semver.Version("1.5.0"))._intervals) + result = a - b + assert isinstance(result, _FmtRange) + assert "inf" not in str(result) + + +def test_fmt_range_empty_classmethod() -> None: + r = _FmtRange.empty() + assert isinstance(r, _FmtRange) + assert semver.Version("1.0.0") not in r + + +def test_fmt_range_full_classmethod() -> None: + r = _FmtRange.full() + assert isinstance(r, _FmtRange) + assert semver.Version("1.0.0") in r + + +def test_fmt_range_singleton_classmethod() -> None: + r = _FmtRange.singleton(semver.Version("2.0.0")) + assert isinstance(r, _FmtRange) + assert semver.Version("2.0.0") in r + assert semver.Version("1.0.0") not in r + + def _make_pkg_repo_with_deps( tmp_path: pathlib.Path, name: str, @@ -475,3 +579,353 @@ def test_qualify_deps_skips_invalid_dep(manager: Manager) -> None: ): result = provider._qualify_deps({"bad-dep": ">=1.0.0"}) assert result == {} + + +def test_get_dependencies_skips_unparseable_spec( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # An unparseable constraint in deps_str must be silently skipped rather than + # propagated to the solver. + provider, _ = _provider_with_repo(manager, tmp_path, "org/pkg", [("v1.0.0", "")]) + v = semver.Version("1.0.0") + provider._cache[("org/pkg", v)] = ("v1.0.0", {"org/dep": "totally-invalid!"}) + deps = provider.get_dependencies("org/pkg", v) + assert "org/dep" not in deps + + +def test_deps_at_version_no_metadata_file(tmp_path: pathlib.Path) -> None: + # Tag exists but has no zkg.meta or bro-pkg.meta -- returns empty dict. + repo = git.Repo.init(tmp_path / "nometarepo", initial_branch="main") + repo.config_writer().set_value("user", "name", "Test").release() + repo.config_writer().set_value("user", "email", "test@test").release() + (tmp_path / "nometarepo" / "README").write_text("no meta here") + repo.index.add(["README"]) + repo.index.commit("initial") + repo.create_tag("v1.0.0") + assert _deps_at_version(repo, "v1.0.0") == {} + + +def test_deps_at_version_missing_package_section(tmp_path: pathlib.Path) -> None: + # zkg.meta at the tag has no [package] section -- returns empty dict. + repo = git.Repo.init(tmp_path / "badsectrepo", initial_branch="main") + repo.config_writer().set_value("user", "name", "Test").release() + repo.config_writer().set_value("user", "email", "test@test").release() + (tmp_path / "badsectrepo" / "zkg.meta").write_text("[other]\nkey = val\n") + repo.index.add(["zkg.meta"]) + repo.index.commit("initial") + repo.create_tag("v1.0.0") + assert _deps_at_version(repo, "v1.0.0") == {} + + +def test_is_versioned_package_sha() -> None: + assert not _is_versioned_package("a" * 40) + + +def test_is_versioned_package_branch_name() -> None: + assert not _is_versioned_package("main") + + +def test_is_versioned_package_valid_semver() -> None: + assert _is_versioned_package("1.2.3") + + +def _make_conflicting_provider( + manager: Manager, + tmp_path: pathlib.Path, +) -> tuple[_ZkgProvider, dict[str, _Node]]: + """Build two packages whose constraints conflict.""" + repo_a = _make_tagged_repo(tmp_path, "pkg-a", [("v1.0.0", ""), ("v2.0.0", "")]) + info_a = MagicMock(spec=PackageInfo) + info_a.metadata_file = str(pathlib.Path(str(repo_a.working_dir)) / "zkg.meta") + info_a.metadata_version = None + info_a.invalid_reason = None + node_a = _Node("org/pkg-a") + node_a.info = info_a + + graph = {"org/pkg-a": node_a} + provider = _ZkgProvider(manager, graph) + return provider, graph + + +def test_run_solver_resolution_error( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + provider, graph = _make_conflicting_provider(manager, tmp_path) + # Require >=2.0.0 and <1.0.0 simultaneously -- unsatisfiable. + requirements = { + "org/pkg-a": _constraint_to_range(">=2.0.0"), + } + constraints = { + "org/pkg-a": _constraint_to_range("<1.0.0"), + } + err, items = _run_solver( + provider, + requirements, + constraints, + graph, + requested_qnames={"org/pkg-a"}, + installed_qnames=set(), + branch_pkg_names=set(), + soft_pinned={}, + ignore_suggestions=True, + lookup_dep=lambda _: None, + ) + assert err != "" + assert items == [] + + +def _make_two_package_setup( + manager: Manager, + tmp_path: pathlib.Path, + *, + dep_suggests: bool = False, + dep_info_none: bool = False, +) -> tuple[_ZkgProvider, dict[str, _Node]]: + """Build provider+graph where main-pkg depends on dep-pkg (or suggests it). + + The solver-cache for main-pkg is pre-populated so that get_dependencies + returns dep-pkg as a dependency without needing real git metadata fetching. + """ + dep_repo = _make_tagged_repo(tmp_path, "dep-pkg", [("v1.0.0", "")]) + dep_info = MagicMock(spec=PackageInfo) + dep_info.metadata_file = str( + pathlib.Path(str(dep_repo.working_dir)) / "zkg.meta", + ) + dep_info.metadata_version = None + dep_info.invalid_reason = None + dep_info.dependencies.return_value = {} + dep_info.best_version.return_value = "v1.0.0" + dep_node = _Node("org/dep-pkg") + if not dep_info_none: + dep_node.info = dep_info + + main_repo = _make_tagged_repo(tmp_path, "main-pkg", [("v1.0.0", "")]) + main_info = MagicMock(spec=PackageInfo) + main_info.metadata_file = str( + pathlib.Path(str(main_repo.working_dir)) / "zkg.meta", + ) + main_info.metadata_version = None + main_info.invalid_reason = None + main_info.best_version.return_value = "v1.0.0" + if dep_suggests: + main_info.dependencies.side_effect = lambda field="depends": ( + {"dep-pkg": ">=1.0.0"} if field == "suggests" else {} + ) + else: + main_info.dependencies.return_value = {} + main_node = _Node("org/main-pkg") + main_node.info = main_info + + graph: dict[str, _Node] = {"org/main-pkg": main_node, "org/dep-pkg": dep_node} + provider = _ZkgProvider(manager, graph) + + # Pre-populate cache: main-pkg v1.0.0 depends on org/dep-pkg >=1.0.0. + v_main = semver.Version("1.0.0") + v_dep = semver.Version("1.0.0") + if not dep_suggests: + provider._cache[("org/main-pkg", v_main)] = ( + "v1.0.0", + {"org/dep-pkg": ">=1.0.0"}, + ) + else: + provider._cache[("org/main-pkg", v_main)] = ("v1.0.0", {}) + provider._cache[("org/dep-pkg", v_dep)] = ("v1.0.0", {}) + if "org/dep-pkg" not in provider._versions: + provider._versions["org/dep-pkg"] = [v_dep] + + return provider, graph + + +def test_run_solver_dep_emitted_in_result( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # Transitive dep (not requested) resolved and emitted via _dfs_emit. + provider, graph = _make_two_package_setup(manager, tmp_path) + requirements = {"org/main-pkg": _constraint_to_range(">=1.0.0")} + err, items = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames={"org/main-pkg"}, + installed_qnames=set(), + branch_pkg_names=set(), + soft_pinned={}, + ignore_suggestions=True, + lookup_dep=lambda _: None, + ) + assert err == "" + qnames = [qn for qn, _, _ in items] + assert "org/dep-pkg" in qnames + + +def test_run_solver_with_suggestions( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # ignore_suggestions=False exercises the _pkg_deps suggestions path (lines 409-415). + dep_info = MagicMock(spec=PackageInfo) + dep_info.invalid_reason = None + dep_info.package = MagicMock() + dep_info.package.qualified_name.return_value = "org/dep-pkg" + provider, graph = _make_two_package_setup( + manager, + tmp_path, + dep_suggests=True, + ) + requirements = {"org/main-pkg": _constraint_to_range(">=1.0.0")} + err, items = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames={"org/main-pkg"}, + installed_qnames=set(), + branch_pkg_names=set(), + soft_pinned={}, + ignore_suggestions=False, + lookup_dep=lambda name: dep_info if name == "dep-pkg" else None, + ) + assert err == "" + assert "org/dep-pkg" in [qn for qn, _, _ in items] + + +def test_run_solver_suggestions_skips_zeek_zkg( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # "zeek"/"zkg" in suggests must be silently skipped (line 410). + dep_info = MagicMock(spec=PackageInfo) + dep_info.invalid_reason = None + dep_info.package = MagicMock() + dep_info.package.qualified_name.return_value = "org/dep-pkg" + provider, graph = _make_two_package_setup( + manager, + tmp_path, + dep_suggests=True, + ) + cast(MagicMock, graph["org/main-pkg"].info).dependencies.side_effect = ( + lambda field="depends": ( + {"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"} if field == "suggests" else {} + ) + ) + requirements = {"org/main-pkg": _constraint_to_range(">=1.0.0")} + err, _ = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames={"org/main-pkg"}, + installed_qnames=set(), + branch_pkg_names=set(), + soft_pinned={}, + ignore_suggestions=False, + lookup_dep=lambda name: dep_info if name == "dep-pkg" else None, + ) + assert err == "" + + +def test_run_solver_branch_pkg_with_suggests( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # Package in branch_pkg_names is not in resolved -- exercises lines 417-428 + # (_pkg_deps fallback reading deps/suggests from graph directly). + dep_info = MagicMock(spec=PackageInfo) + dep_info.invalid_reason = None + dep_info.package = MagicMock() + dep_info.package.qualified_name.return_value = "org/dep-pkg" + provider, graph = _make_two_package_setup( + manager, + tmp_path, + dep_suggests=True, + ) + cast(MagicMock, graph["org/main-pkg"].info).dependencies.side_effect = ( + lambda field="depends": ( + {"zeek": ">=5.0.0", "dep-pkg": ">=1.0.0"} + if field in ("depends", "suggests") + else {} + ) + ) + requirements: dict[str, Range[semver.Version]] = {} + err, _ = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames=set(), + installed_qnames=set(), + branch_pkg_names={"org/main-pkg"}, + soft_pinned={}, + ignore_suggestions=False, + lookup_dep=lambda name: dep_info if name == "dep-pkg" else None, + ) + assert err == "" + + +def test_run_solver_dfs_skips_revisit( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # A seed that also appears as a branch_pkg causes it to be revisited by + # _dfs_emit, hitting the dfs_visited guard (line 467) on the second pass. + repo = _make_tagged_repo(tmp_path, "solo", [("v1.0.0", "")]) + info = MagicMock(spec=PackageInfo) + info.metadata_file = str(pathlib.Path(str(repo.working_dir)) / "zkg.meta") + info.metadata_version = None + info.invalid_reason = None + info.dependencies.return_value = {} + info.best_version.return_value = "v1.0.0" + node = _Node("org/solo") + node.info = info + graph = {"org/solo": node} + provider = _ZkgProvider(manager, graph) + v = semver.Version("1.0.0") + provider._cache[("org/solo", v)] = ("v1.0.0", {}) + requirements = {"org/solo": _constraint_to_range(">=1.0.0")} + # Appear in both requested_qnames and branch_pkg_names -- two seeds that + # collapse to the same package, so the second _dfs_emit hits dfs_visited. + err, _ = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames={"org/solo"}, + installed_qnames=set(), + branch_pkg_names={"org/solo"}, + soft_pinned={}, + ignore_suggestions=True, + lookup_dep=lambda _: None, + ) + assert err == "" + + +def test_run_solver_dfs_node_no_info( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + # Dep node with info=None is skipped by _dfs_emit (covers line 454). + provider, graph = _make_two_package_setup( + manager, + tmp_path, + dep_info_none=True, + ) + requirements = {"org/main-pkg": _constraint_to_range(">=1.0.0")} + err, items = _run_solver( + provider, + requirements, + {}, + graph, + requested_qnames={"org/main-pkg"}, + installed_qnames=set(), + branch_pkg_names=set(), + soft_pinned={}, + ignore_suggestions=True, + lookup_dep=lambda _: None, + ) + assert err == "" + qnames = [qn for qn, _, _ in items] + assert "org/dep-pkg" not in qnames diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py index 5681a9a..7c15a9f 100644 --- a/zeekpkg/_resolver.py +++ b/zeekpkg/_resolver.py @@ -10,14 +10,16 @@ import configparser import os -from collections.abc import Mapping +from collections.abc import Callable, Mapping from typing import TYPE_CHECKING, cast import git import semantic_version as semver +from nab_resolver.errors import ResolutionError from nab_resolver.ranges import Range -from nab_resolver.resolver import ResolverProvider +from nab_resolver.resolver import Resolver, ResolverProvider from nab_resolver.types import Incompatibility, RangeProtocol +from typing_extensions import Self from ._util import _semver_versions, git_version_tags, is_sha1 from .package import ( @@ -31,6 +33,8 @@ if TYPE_CHECKING: from .manager import Manager +__all__ = ["Range"] + class _Node: """Dependency graph node used inside `validate_dependencies`.""" @@ -122,6 +126,61 @@ def _fmt_range(r: Range[semver.Version]) -> str: return " | ".join(parts) if parts else "none" +class _FmtRange(Range[semver.Version]): + """A `Range` whose `__str__` produces operator-prefixed semver notation. + + nab-resolver has no global range-formatting hook: `narrow_for_display` + only covers terms that pass through the `_narrow_positive` path in the + error reporter. The `CONSTRAINT`-cause path in `_render_line` interpolates + `incompatibility.constraint_range` directly, bypassing `narrow_for_display` + entirely and exposing the raw ``(-inf, X) | (X, +inf)`` sentinel strings. + + Making every range we construct carry its own formatted `__str__` fixes all + render sites at once without relying on any library hook. The operator + overrides are necessary because `Range.__and__`, `__or__`, and `__invert__` + construct their results as plain `Range` objects; without overriding them, + composed ranges lose the subclass and revert to the raw notation. + """ + + __slots__ = () + + def __str__(self) -> str: + return _fmt_range(self) + + @classmethod + def empty(cls) -> Self: + return cls(super().empty()._intervals) + + @classmethod + def full(cls) -> Self: + return cls(super().full()._intervals) + + @classmethod + def singleton(cls, version: semver.Version) -> Self: + return cls(super().singleton(version)._intervals) + + def __and__(self, other: object) -> Self: + result = super().__and__(other) + if not isinstance(result, Range): + return result # pragma: no cover + return type(self)(result._intervals) + + def __or__(self, other: object) -> Self: + result = super().__or__(other) + if not isinstance(result, Range): + return result # pragma: no cover + return type(self)(result._intervals) + + def __invert__(self) -> Self: + return type(self)(super().__invert__()._intervals) + + def __sub__(self, other: object) -> Self: + result = super().__sub__(other) + if not isinstance(result, Range): + return result # pragma: no cover + return type(self)(result._intervals) + + class _ZkgProvider(ResolverProvider["str", "semver.Version"]): """nab-resolver `ResolverProvider` backed by zkg package metadata. @@ -305,6 +364,155 @@ def __str__(self) -> str: return cast(RangeProtocol[semver.Version], _Displayed(r._intervals)) +def _run_solver( + provider: _ZkgProvider, + requirements: Mapping[str, Range[semver.Version]], + constraints: Mapping[str, Range[semver.Version]], + graph: dict[str, _Node], + requested_qnames: set[str], + installed_qnames: set[str], + branch_pkg_names: set[str], + soft_pinned: dict[str, str], + ignore_suggestions: bool, + lookup_dep: Callable[[str], PackageInfo | None], +) -> tuple[str, list[tuple[str, str, bool]]]: + """Run the nab-resolver and return a topo-sorted install list. + + Returns a ``(error, items)`` pair. On success ``error`` is empty and + ``items`` is a list of ``(qname, raw_tag, is_suggestion)`` tuples in + dependency order (dependees before dependers). On failure ``error`` is + the first line of the resolver's error message and ``items`` is empty. + + ``lookup_dep`` resolves a short package name to its ``PackageInfo``; it + replaces the ``find_builtin_package`` / ``_cached_info`` calls that + previously lived in ``manager.py``. + """ + resolver: Resolver[str, semver.Version] = Resolver( + provider, + range_type=Range, + root_version=semver.Version("0.0.0"), + ) + try: + resolved: dict[str, semver.Version] = resolver.resolve( + requirements, + constraints=constraints, + ) + except ResolutionError as e: + return (str(e), []) + + suggestion_names: set[str] = { + name for name, node in graph.items() if node.is_suggestion + } + + def _pkg_deps(qn: str) -> list[str]: + result_d: list[str] = [] + rv = resolved.get(qn) + if rv is not None: + cache_entry = provider._cache.get((qn, rv)) + if cache_entry: + _, d = cache_entry + result_d = list(d) + if not ignore_suggestions: + nd = graph.get(qn) + if nd and nd.info: + raw_sug = nd.info.dependencies(field="suggests") or {} + for dep_s in raw_sug: + if dep_s in ("zeek", "zkg"): + continue + di = lookup_dep(dep_s) + if di is not None and not di.invalid_reason: + dqn = di.package.qualified_name() + if dqn not in result_d: + result_d.append(dqn) + return sorted(result_d) + nd = graph.get(qn) + if nd and nd.info: + raw: dict[str, str] = nd.info.dependencies(field="depends") or {} + if not ignore_suggestions: + raw = {**raw, **(nd.info.dependencies(field="suggests") or {})} + for dep_s in raw: + if dep_s in ("zeek", "zkg"): + continue + di = lookup_dep(dep_s) + if di is not None and not di.invalid_reason: + result_d.append(di.package.qualified_name()) + return sorted(result_d) + + dfs_visited: set[str] = set() + dfs_in_stack: set[str] = set() + + def _dfs_emit(start: str) -> list[tuple[str, str, bool]]: + result: list[tuple[str, str, bool]] = [] + stack: list[tuple[str, bool]] = [(start, False)] + while stack: + qn, post = stack.pop() + if post: + dfs_in_stack.discard(qn) + is_upgraded = ( + qn in soft_pinned + and qn in resolved + and _is_versioned_package(soft_pinned[qn]) + and resolved[qn] > semver.Version.coerce(soft_pinned[qn]) + ) + if ( + qn in requested_qnames + or (qn in installed_qnames and not is_upgraded) + or qn in branch_pkg_names + ): + continue + node = graph.get(qn) + if node is None or node.info is None: + continue + is_sug = qn in suggestion_names + rv = resolved.get(qn) + if rv is not None: + cache_entry = provider._cache.get((qn, rv)) + raw_tag = ( + cache_entry[0] if cache_entry else node.info.best_version() + ) + else: + raw_tag = node.info.best_version() + result.append((qn, raw_tag, is_sug)) + else: + if qn in dfs_visited or qn in dfs_in_stack: + continue + dfs_visited.add(qn) + dfs_in_stack.add(qn) + stack.append((qn, True)) + for dep_qn in reversed(_pkg_deps(qn)): + if dep_qn not in dfs_visited and dep_qn not in dfs_in_stack: + stack.append((dep_qn, False)) + return result + + seeds = list(requested_qnames) + list(branch_pkg_names) + post_order: list[tuple[str, str, bool]] = [] + for seed in seeds: + post_order.extend(_dfs_emit(seed)) + + # post_order is leaves-first; reverse to get root-first for the return + # value (caller reverses again when installing, so leaves end up first). + seen_res: set[str] = set() + res: list[tuple[str, str, bool]] = [] + for qn, raw_tag, is_sug in reversed(post_order): + if qn not in seen_res: + seen_res.add(qn) + if graph.get(qn) is not None and graph[qn].info is not None: + res.append((qn, raw_tag, is_sug)) + + return ("", res) + + +def _is_versioned_package(v: str) -> bool: + """Return True if *v* is a semver-coercible version the solver can use.""" + if is_sha1(v): + return False + try: + semver.Version.coerce(v) + return True + except ValueError: + return False + + def _deps_at_version(clone: git.Repo, tag: str) -> dict[str, str]: """Return the dependency dict for `clone` at `tag`. diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index 99b4f1b..b1d0a40 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -21,19 +21,19 @@ import git import semantic_version as semver -from nab_resolver.errors import ResolutionError -from nab_resolver.ranges import Range -from nab_resolver.resolver import Resolver from . import ( LOG, __version__, ) from ._resolver import ( + Range, _deps_at_version, _get_branch_names, + _is_versioned_package, _Node, _normalize_constraint, + _run_solver, _ZkgProvider, ) from ._util import ( @@ -2321,16 +2321,6 @@ def _cached_info(pkg_path: str) -> PackageInfo: info_cache[key] = self.info(pkg_path, prefer_installed=False) return info_cache[key] - def _is_versioned_package(v: str) -> bool: - """Return True if *v* is a semver-coercible version the solver can use.""" - if is_sha1(v): - return False - try: - semver.Version.coerce(v) - return True - except ValueError: - return False - # Identify branch-constrained packages across all graph nodes (including # suggests when not ignore_suggestions). branch_pkgs: list[tuple[PackageInfo, str, bool]] = [] @@ -2401,11 +2391,6 @@ def _is_versioned_package(v: str) -> bool: } provider = _ZkgProvider(self, graph) - resolver: Resolver[str, semver.Version] = Resolver( - provider, - range_type=Range, - root_version=semver.Version("0.0.0"), - ) requirements: dict[str, Range[semver.Version]] = {} requested_qnames: set[str] = set() @@ -2419,7 +2404,6 @@ def _is_versioned_package(v: str) -> bool: if _is_versioned_package(norm): requirements[qname] = Range.singleton(semver.Version.coerce(norm)) continue - # Branch-tracked or no semver version: any version satisfies. requirements[qname] = Range.full() for binfo, _, _ in branch_pkgs: bqn = binfo.package.qualified_name() @@ -2438,9 +2422,8 @@ def _is_versioned_package(v: str) -> bool: if _is_versioned_package(norm): constraints[qname] = Range.at_least(semver.Version.coerce(norm)) - # Branch packages are installed at a specific branch ref, not a semver - # tag. Register only the synthetic 0.0.0 version and pre-populate the - # cache with their current deps (including suggests when applicable). + # Branch packages use a synthetic 0.0.0 version; pre-populate the cache + # so the solver can read their deps without a git checkout. for binfo, _, _ in branch_pkgs: bqn = binfo.package.qualified_name() synth_v = semver.Version("0.0.0") @@ -2464,122 +2447,26 @@ def _is_versioned_package(v: str) -> bool: ) provider._cache[(bqn, synth_v)] = (binfo.best_version(), bsynth_deps) - try: - resolved: dict[str, semver.Version] = resolver.resolve( - requirements, - constraints=constraints, - ) - except ResolutionError as e: - return (str(e).split("\n")[0], []) - - suggestion_names: set[str] = { - name for name, node in graph.items() if node.is_suggestion - } - - def _pkg_deps(qn: str) -> list[str]: - """Return dep qualified names for *qn* from provider cache or node info.""" - result_d: list[str] = [] - rv = resolved.get(qn) - if rv is not None: - cache_entry = provider._cache.get((qn, rv)) - if cache_entry: - _, d = cache_entry - result_d = list(d) - if not ignore_suggestions: - nd = graph.get(qn) - if nd and nd.info: - raw_sug = nd.info.dependencies(field="suggests") or {} - for dep_s in raw_sug: - if dep_s in ("zeek", "zkg"): - continue - di = self.find_builtin_package(dep_s) - if di is None: - di = _cached_info(dep_s) - if not di.invalid_reason: - dqn = di.package.qualified_name() - if dqn not in result_d: - result_d.append(dqn) - return sorted(result_d) - nd = graph.get(qn) - if nd and nd.info: - raw: dict[str, str] = nd.info.dependencies(field="depends") or {} - if not ignore_suggestions: - raw = {**raw, **(nd.info.dependencies(field="suggests") or {})} - for dep_s in raw: - if dep_s in ("zeek", "zkg"): - continue - di = self.find_builtin_package(dep_s) - if di is None: - di = _cached_info(dep_s) - if not di.invalid_reason: - result_d.append(di.package.qualified_name()) - return sorted(result_d) - - dfs_visited: set[str] = set() - dfs_in_stack: set[str] = set() - - def _dfs_emit(start: str) -> list[tuple[str, str, bool]]: - result: list[tuple[str, str, bool]] = [] - stack: list[tuple[str, bool]] = [(start, False)] - while stack: - qn, post = stack.pop() - if post: - dfs_in_stack.discard(qn) - is_upgraded = ( - qn in soft_pinned - and qn in resolved - and _is_versioned_package(soft_pinned[qn]) - and resolved[qn] > semver.Version.coerce(soft_pinned[qn]) - ) - if ( - qn in requested_qnames - or (qn in installed_qnames and not is_upgraded) - or qn in branch_pkg_names - ): - continue - node = graph.get(qn) - if node is None or node.info is None: - continue - is_sug = qn in suggestion_names - rv = resolved.get(qn) - if rv is not None: - cache_entry = provider._cache.get((qn, rv)) - raw_tag = ( - cache_entry[0] if cache_entry else node.info.best_version() - ) - else: - raw_tag = node.info.best_version() - result.append((qn, raw_tag, is_sug)) - else: - if qn in dfs_visited or qn in dfs_in_stack: - continue - dfs_visited.add(qn) - dfs_in_stack.add(qn) - stack.append((qn, True)) - for dep_qn in reversed(_pkg_deps(qn)): - if dep_qn not in dfs_visited and dep_qn not in dfs_in_stack: - stack.append((dep_qn, False)) - return result - - seeds = list(requested_qnames) + [ - binfo.package.qualified_name() for binfo, _, _ in branch_pkgs - ] - post_order: list[tuple[str, str, bool]] = [] - for seed in seeds: - post_order.extend(_dfs_emit(seed)) + error, solver_res = _run_solver( + provider, + requirements, + constraints, + graph, + requested_qnames, + installed_qnames, + branch_pkg_names, + soft_pinned, + ignore_suggestions, + lambda dep: self.find_builtin_package(dep) or _cached_info(dep), + ) + if error: + return (error, []) - # post_order is leaves-first; reverse to get root-first for the return - # value (caller reverses again when installing, so leaves end up first). - seen_res: set[str] = set() res: list[tuple[PackageInfo, str, bool]] = [] - for qn, raw_tag, is_sug in reversed(post_order): - if qn in seen_res: - continue - seen_res.add(qn) + for qn, raw_tag, is_sug in solver_res: res_node = graph.get(qn) - if res_node is None or res_node.info is None: - continue - res.append((res_node.info, raw_tag, is_sug)) + if res_node is not None and res_node.info is not None: + res.append((res_node.info, raw_tag, is_sug)) for binfo, bversion, bsug in branch_pkgs: bqn = binfo.package.qualified_name() From 5613acd5aa010458e20dca129b58ce7c82136399 Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Fri, 18 Sep 2026 16:21:23 +0200 Subject: [PATCH 11/12] Replace narrow_for_display workaround with _FmtRange subclass nab-resolver's error reporter renders constraints via two code paths: _narrow_positive (which calls narrow_for_display) and direct interpolation of Incompatibility.constraint_range in the CONSTRAINT-cause path. The previous narrow_for_display approach only covered the first path, leaving CONSTRAINT-cause lines with raw (-inf, X) sentinel notation. _FmtRange overrides __str__ to produce operator-prefixed output and overrides all Range operators to preserve the subclass through composition. _constraint_to_range now returns _FmtRange, so every range we construct formats itself at every render site. narrow_for_display becomes a one-line fallback that wraps any library-internal Range the resolver might surface directly. --- .../tests.builtin-spicy-version-error/output | 4 +-- .../fail.out | 4 +-- testing/test_resolver.py | 17 ++++++++++++ zeekpkg/_resolver.py | 26 +++++++------------ 4 files changed, 30 insertions(+), 21 deletions(-) diff --git a/testing/baselines/tests.builtin-spicy-version-error/output b/testing/baselines/tests.builtin-spicy-version-error/output index c8b2a83..245d856 100644 --- a/testing/baselines/tests.builtin-spicy-version-error/output +++ b/testing/baselines/tests.builtin-spicy-version-error/output @@ -1,7 +1,7 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. error: failed to resolve dependencies: because no versions of one/alice/foo X.X.X are available -because one/alice/foo =X.X.X depends on zeek-builtin/spicy-plugin (-inf, X.X.X) -so one/alice/foo * and not zeek-builtin/spicy-plugin (-inf, X.X.X) +because one/alice/foo =X.X.X depends on zeek-builtin/spicy-plugin = 0.63. error: failed to resolve dependencies: because no versions of one/alice/foo <0.0.0 | >0.0.0 are available -because one/alice/foo =0.0.0 depends on one/alice/bar 1.0.0 -so one/alice/foo * and not one/alice/bar 1.0.0 +because one/alice/foo =0.0.0 depends on one/alice/bar =1.0.0 +so one/alice/foo * and not one/alice/bar =1.0.0 because no versions of one/alice/bar =1.0.0 are available so one/alice/foo * because your project depends on one/alice/foo * diff --git a/testing/test_resolver.py b/testing/test_resolver.py index 0858661..83984d1 100644 --- a/testing/test_resolver.py +++ b/testing/test_resolver.py @@ -160,6 +160,12 @@ def test_constraint_to_range_compound() -> None: assert semver.Version("0.9.0") not in r +def test_constraint_to_range_returns_fmt_range() -> None: + assert isinstance(_constraint_to_range(">=1.0.0"), _FmtRange) + assert isinstance(_constraint_to_range("*"), _FmtRange) + assert isinstance(_constraint_to_range("=1.0.0"), _FmtRange) + + def test_choose_version_picks_highest_in_range( manager: Manager, tmp_path: pathlib.Path, @@ -449,6 +455,17 @@ def test_fmt_range_full_is_wildcard() -> None: assert _fmt_range(Range.full()) == "*" +def test_narrow_for_display_wraps_in_fmt_range( + manager: Manager, + tmp_path: pathlib.Path, +) -> None: + provider, _ = _provider_with_repo(manager, tmp_path, "org/pkg", [("v1.0.0", "")]) + raw = Range.at_least(semver.Version("1.0.0")) + result = provider.narrow_for_display("org/pkg", raw) + assert isinstance(result, _FmtRange) + assert "inf" not in str(result) + + def test_fmt_range_subclass_str() -> None: r = _FmtRange(Range.at_least(semver.Version("1.0.0"))._intervals) assert str(r) == ">=1.0.0" diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py index 7c15a9f..259d211 100644 --- a/zeekpkg/_resolver.py +++ b/zeekpkg/_resolver.py @@ -75,25 +75,25 @@ def _normalize_constraint(spec: str) -> str: return spec -def _constraint_to_range(constraint: str) -> Range[semver.Version]: +def _constraint_to_range(constraint: str) -> _FmtRange: """Convert a normalized zkg constraint string to a nab-resolver `Range`.""" if constraint in ("*", ""): - return Range.full() - result: Range[semver.Version] = Range.full() + return _FmtRange(Range.full()._intervals) + result: _FmtRange = _FmtRange(Range.full()._intervals) clause = semver.SimpleSpec(_normalize_constraint(constraint)).clause matchers = list(clause.clauses) if hasattr(clause, "clauses") else [clause] for m in matchers: v = semver.Version.coerce(str(m.target)) if m.operator == ">=": - result = result & Range.at_least(v) + result = result & _FmtRange(Range.at_least(v)._intervals) elif m.operator == ">": - result = result & Range.greater_than(v) + result = result & _FmtRange(Range.greater_than(v)._intervals) elif m.operator == "<=": - result = result & Range.at_most(v) + result = result & _FmtRange(Range.at_most(v)._intervals) elif m.operator == "<": - result = result & Range.less_than(v) + result = result & _FmtRange(Range.less_than(v)._intervals) elif m.operator == "==": - result = result & Range.singleton(v) + result = result & _FmtRange(Range.singleton(v)._intervals) return result @@ -353,15 +353,7 @@ def narrow_for_display( constraint: RangeProtocol[semver.Version], ) -> RangeProtocol[semver.Version]: r = cast(Range[semver.Version], constraint) - display = _fmt_range(r) - - class _Displayed(Range[semver.Version]): - __slots__ = () - - def __str__(self) -> str: - return display - - return cast(RangeProtocol[semver.Version], _Displayed(r._intervals)) + return _FmtRange(r._intervals) def _run_solver( From 56bf460d986b697c0f824c08872bf9ee3861499e Mon Sep 17 00:00:00 2001 From: Benjamin Bannier Date: Fri, 18 Sep 2026 16:21:58 +0200 Subject: [PATCH 12/12] Extract `PackageInfo.version_tag()` to unify version fallback logic `metadata_version or best_version() or ""` appeared at seven call sites across `_resolver.py`, `manager.py`, and `zkg`. A directory-backed package has no git tags and no default branch, so `best_version()` can return `None`; each caller had to guard against that independently. `version_tag()` encapsulates the three-step fallback (metadata version, best git version, empty string) and makes the return type `str` so callers no longer need the `or ""` suffix. `best_version()` now returns `str | None` to reflect reality. --- zeekpkg/_resolver.py | 14 +++++--------- zeekpkg/manager.py | 2 +- zeekpkg/package.py | 19 ++++++++++++++----- zkg | 6 +++--- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/zeekpkg/_resolver.py b/zeekpkg/_resolver.py index 259d211..8a03fb1 100644 --- a/zeekpkg/_resolver.py +++ b/zeekpkg/_resolver.py @@ -290,17 +290,15 @@ def _fetch_deps( return (str(version), {}) if not node.info.metadata_file: # Builtin / directory package -- use current metadata. - raw_tag = node.info.metadata_version or node.info.best_version() raw_deps = node.info.dependencies(field="depends") or {} - return (raw_tag, self._qualify_deps(raw_deps)) + return (node.info.version_tag(), self._qualify_deps(raw_deps)) clone_dir = os.path.dirname(node.info.metadata_file) try: clone = git.Repo(clone_dir) except git.InvalidGitRepositoryError: # Directory package -- no git history; use current metadata. - raw_tag = node.info.metadata_version or node.info.best_version() raw_deps = node.info.dependencies(field="depends") or {} - return (raw_tag, self._qualify_deps(raw_deps)) + return (node.info.version_tag(), self._qualify_deps(raw_deps)) found_tag: str | None = None for rt, nv in _semver_versions(git_version_tags(clone)): if semver.Version.coerce(nv) == version: @@ -309,7 +307,7 @@ def _fetch_deps( if found_tag is None: # Synthetic version (no matching tag) -- use current HEAD metadata. raw_deps = node.info.dependencies(field="depends") or {} - raw_tag = node.info.metadata_version or node.info.best_version() + raw_tag = node.info.version_tag() else: raw_tag = found_tag raw_deps = _deps_at_version(clone, raw_tag) @@ -459,11 +457,9 @@ def _dfs_emit(start: str) -> list[tuple[str, str, bool]]: rv = resolved.get(qn) if rv is not None: cache_entry = provider._cache.get((qn, rv)) - raw_tag = ( - cache_entry[0] if cache_entry else node.info.best_version() - ) + raw_tag = cache_entry[0] if cache_entry else node.info.version_tag() else: - raw_tag = node.info.best_version() + raw_tag = node.info.version_tag() result.append((qn, raw_tag, is_sug)) else: if qn in dfs_visited or qn in dfs_in_stack: diff --git a/zeekpkg/manager.py b/zeekpkg/manager.py index b1d0a40..7354367 100644 --- a/zeekpkg/manager.py +++ b/zeekpkg/manager.py @@ -2445,7 +2445,7 @@ def _cached_info(pkg_path: str) -> PackageInfo: bsynth_deps[di.package.qualified_name()] = _normalize_constraint( dep_spec, ) - provider._cache[(bqn, synth_v)] = (binfo.best_version(), bsynth_deps) + provider._cache[(bqn, synth_v)] = (binfo.version_tag(), bsynth_deps) error, solver_res = _run_solver( provider, diff --git a/zeekpkg/package.py b/zeekpkg/package.py index 6a8db3d..7fa1be5 100644 --- a/zeekpkg/package.py +++ b/zeekpkg/package.py @@ -443,17 +443,26 @@ def user_vars(self) -> list[UserVar] | None: """ return UserVar.parse_dict(self.metadata) - def best_version(self) -> str: + def best_version(self) -> str | None: """Returns the best/latest version of the package that is available. - If the package has any git release tags, this returns the highest one, - else it returns the default branch like 'main' or 'master'. + If the package has any git release tags, this returns the highest one. + For Git packages without tags, returns the default branch. For + directory-backed packages, returns ``None``. """ if self.versions: return self.versions[-1] - assert self.default_branch - return self.default_branch + return self.default_branch or None + + def version_tag(self) -> str: + """Returns the best available version identifier for this package. + + Prefers the metadata version field, then the best Git version tag or + default branch, falling back to an empty string for directory-backed + packages without a version. + """ + return self.metadata_version or self.best_version() or "" def is_builtin(self) -> bool: if self.package: diff --git a/zkg b/zkg index ca552b8..1f8964e 100755 --- a/zkg +++ b/zkg @@ -551,7 +551,7 @@ def cmd_test( sys.exit(1) if not version: - version = package_info.metadata_version or package_info.best_version() + version = package_info.version_tag() package_infos.append((package_info, version)) @@ -626,7 +626,7 @@ def cmd_install( sys.exit(1) if not version: - version = package_info.metadata_version or package_info.best_version() + version = package_info.version_tag() package_infos.append((package_info, version, False)) @@ -916,7 +916,7 @@ def cmd_bundle( sys.exit(1) if not version: - version = info.metadata_version or info.best_version() + version = info.version_tag() to_validate.append((info.package.qualified_name(), version)) packages_to_bundle.append(