From 95ee18ff962bdd8659f9ccaf3e6d40890a045dc3 Mon Sep 17 00:00:00 2001 From: Evan Downing Date: Wed, 1 Jul 2026 08:27:33 -0400 Subject: [PATCH 1/2] Fix cargo resolver pruning of optional/feature-gated deps The cargo resolver marked every declared dependency of a package as already-resolved, including optional/feature-gated deps that `cargo metadata` does not activate under the default feature set. Those deps resolved to nothing and were pruned, collapsing it-depends' "resolve all possible versions" behavior to the single locked tree for cargo projects. Only mark a dependency resolved when `cargo metadata` actually added a matching package to the cache, so inactive optional deps fall through to normal resolution instead of being pruned. Also wrap the `cargo metadata` subprocess call in `get_dependencies` with CalledProcessError handling, mirroring pip.py (#125), so one unresolvable crate no longer aborts the entire resolution. Fixes #189 Co-Authored-By: Claude Opus 4.8 (1M context) --- src/it_depends/cargo.py | 38 +++++++++++++++++++---------- test/test_cargo.py | 53 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/src/it_depends/cargo.py b/src/it_depends/cargo.py index a18f944..9e27380 100644 --- a/src/it_depends/cargo.py +++ b/src/it_depends/cargo.py @@ -94,7 +94,14 @@ def get_dependencies( if cargo_path is None: cargo_path = resolve_executable("cargo") - metadata = json.loads(subprocess.check_output([cargo_path, "metadata", "--format-version", "1"], cwd=repo.path)) + try: + output = subprocess.check_output([cargo_path, "metadata", "--format-version", "1"], cwd=repo.path) + except subprocess.CalledProcessError: + # A single crate whose temp-project `cargo metadata` fails must not abort the whole + # resolution; mirror pip.py's handling of johnnydep failures (see #125). + logger.exception("Error running `cargo metadata` in %s", repo.path) + return + metadata = json.loads(output) if "workspace_members" in metadata: workspace_members = {_parse_workspace_member(m) for m in metadata["workspace_members"]} @@ -173,20 +180,25 @@ def resolve_from_source(self, repo: SourceRepository, cache: PackageCache | None if not self.can_resolve_from_source(repo): return None result = None - for package in get_dependencies(repo, cargo_path=self.tool_path): - if isinstance(package, SourcePackage): - result = package - elif cache is not None: - cache.add(package) + packages = list(get_dependencies(repo, cargo_path=self.tool_path)) + if cache is not None: + for package in packages: + if isinstance(package, SourcePackage): + result = package + else: + cache.add(package) + # Only mark a dependency as resolved when `cargo metadata` actually resolved a + # matching package. Optional/feature-gated deps that the active feature set does + # not enable never appear in the resolved package set, so they must fall through + # to normal resolution rather than being marked resolved-to-nothing and pruned. + for package in packages: for dep in package.dependencies: - if not cache.was_resolved(dep): + if not cache.was_resolved(dep) and next(cache.match(dep), None) is not None: cache.set_resolved(dep) - # Mark the SourcePackage's direct dependencies as resolved - # since cargo metadata already resolved them. - if result is not None and cache is not None: - for dep in result.dependencies: - if not cache.was_resolved(dep): - cache.set_resolved(dep) + else: + for package in packages: + if isinstance(package, SourcePackage): + result = package return result def resolve(self, dependency: Dependency) -> Iterator[Package]: diff --git a/test/test_cargo.py b/test/test_cargo.py index fdcc1bc..0c84fa5 100644 --- a/test/test_cargo.py +++ b/test/test_cargo.py @@ -1,6 +1,9 @@ import pytest -from it_depends.cargo import _parse_workspace_member +from it_depends.cache import InMemoryPackageCache +from it_depends.cargo import CargoResolver, _parse_workspace_member +from it_depends.models import Dependency, Package, SourcePackage +from it_depends.repository import SourceRepository @pytest.mark.parametrize( @@ -20,3 +23,51 @@ def test_parse_workspace_member(member: str, expected: str) -> None: def test_parse_workspace_member_unknown_format() -> None: result = _parse_workspace_member("something-unexpected") assert result == "something-unexpected" + + +def _dep(name: str) -> Dependency: + return Dependency(package=name, source="cargo", semantic_version=CargoResolver.parse_spec("*")) + + +def test_resolve_from_source_only_marks_resolved_deps(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: # noqa: ANN001 + """Optional/feature-gated deps with no matching package must not be marked resolved. + + Regression test for #189: the old shortcut marked every declared dependency as resolved, + including optional deps that `cargo metadata` never adds to the resolved package set. Those + deps then matched nothing and their subtrees were pruned, collapsing "all possible versions" + to the locked tree. + """ + repo = SourceRepository(path=tmp_path) + + # `libfoo` is actually resolved (a package is yielded); `libbar` and `rayon` are optional + # deps that cargo metadata never resolves, so no package is yielded for them. + source_pkg = SourcePackage( + name="myapp", + version="0.1.0", + source_repo=repo, + source="cargo", + dependencies=[_dep("libfoo"), _dep("libbar")], + ) + libfoo = Package( + name="libfoo", + version="1.0.0", + source="cargo", + dependencies=[_dep("rayon")], + ) + + monkeypatch.setattr(CargoResolver, "can_resolve_from_source", lambda self, repo: True) # noqa: ARG005 + monkeypatch.setattr("it_depends.cargo.get_dependencies", lambda repo, cargo_path=None: iter([source_pkg, libfoo])) # noqa: ARG005 + + resolver = CargoResolver() + resolver._tool_path = "cargo" # noqa: SLF001 # avoid resolve_executable in this unit test + + cache = InMemoryPackageCache() + with cache: + result = resolver.resolve_from_source(repo, cache) + + assert result is source_pkg + # libfoo has a matching package in the cache -> resolved. + assert cache.was_resolved(_dep("libfoo")) + # libbar and rayon resolved to nothing -> left unresolved so normal resolution can expand them. + assert not cache.was_resolved(_dep("libbar")) + assert not cache.was_resolved(_dep("rayon")) From e04e37cb7f1413eaa235b067f8bfa55abeaac187 Mon Sep 17 00:00:00 2001 From: Evan Downing Date: Tue, 7 Jul 2026 15:45:49 -0400 Subject: [PATCH 2/2] Ignore optional cargo dependencies that aren't explicitly required Per PR review: rather than expanding every optional/feature-gated dependency into the graph, drop optional deps that the active feature set does not require. A crate may declare a mutually exclusive set of optional deps, so forcing them all in can describe a configuration with no valid resolution. Filter optional deps in `get_dependencies` so they never become edges. The `resolve_from_source` match guard now only protects required deps that cargo failed to resolve (e.g. a subcrate whose `cargo metadata` errored), leaving them for normal resolution instead of pruning. Co-Authored-By: Claude Opus 4.8 --- src/it_depends/cargo.py | 12 ++++++--- test/test_cargo.py | 55 +++++++++++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/it_depends/cargo.py b/src/it_depends/cargo.py index 9e27380..fe45000 100644 --- a/src/it_depends/cargo.py +++ b/src/it_depends/cargo.py @@ -120,6 +120,12 @@ def get_dependencies( for dep in package["dependencies"]: if dep["kind"] is not None: continue + # Ignore optional (feature-gated) dependencies that are not explicitly required. + # A crate may declare a mutually exclusive set of optional deps, so forcing them all + # into the graph can describe a configuration that has no valid resolution. We only + # track dependencies cargo requires under the active feature set (see #189, #191). + if dep["optional"]: + continue if dep["name"] in dependencies: dependencies[dep["name"]].semantic_version = dependencies[ dep["name"] @@ -188,9 +194,9 @@ def resolve_from_source(self, repo: SourceRepository, cache: PackageCache | None else: cache.add(package) # Only mark a dependency as resolved when `cargo metadata` actually resolved a - # matching package. Optional/feature-gated deps that the active feature set does - # not enable never appear in the resolved package set, so they must fall through - # to normal resolution rather than being marked resolved-to-nothing and pruned. + # matching package. A required dep that cargo failed to resolve (e.g. a subcrate + # whose `cargo metadata` errored) must fall through to normal resolution rather + # than being marked resolved-to-nothing and pruned. for package in packages: for dep in package.dependencies: if not cache.was_resolved(dep) and next(cache.match(dep), None) is not None: diff --git a/test/test_cargo.py b/test/test_cargo.py index 0c84fa5..b9c7b99 100644 --- a/test/test_cargo.py +++ b/test/test_cargo.py @@ -1,7 +1,9 @@ +import json + import pytest from it_depends.cache import InMemoryPackageCache -from it_depends.cargo import CargoResolver, _parse_workspace_member +from it_depends.cargo import CargoResolver, _parse_workspace_member, get_dependencies from it_depends.models import Dependency, Package, SourcePackage from it_depends.repository import SourceRepository @@ -29,18 +31,51 @@ def _dep(name: str) -> Dependency: return Dependency(package=name, source="cargo", semantic_version=CargoResolver.parse_spec("*")) +def test_get_dependencies_ignores_optional_deps(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: # noqa: ANN001 + """Optional (feature-gated) deps are not tracked; only explicitly required deps survive. + + Regression test for #189/#191: a crate may declare a mutually exclusive set of optional + deps, so forcing them all into the graph can describe a configuration with no valid + resolution. `get_dependencies` keeps only the deps cargo requires under the active feature + set, dropping optional and dev/build deps. + """ + metadata = { + "packages": [ + { + "name": "myapp", + "version": "0.1.0", + "dependencies": [ + {"name": "serde", "req": "^1", "kind": None, "optional": False}, + {"name": "rayon", "req": "^1", "kind": None, "optional": True}, + {"name": "criterion", "req": "^0.5", "kind": "dev", "optional": False}, + ], + }, + ], + "workspace_members": ["myapp 0.1.0 (path+file:///tmp/myapp)"], + } + monkeypatch.setattr( + "it_depends.cargo.subprocess.check_output", + lambda *args, **kwargs: json.dumps(metadata).encode(), # noqa: ARG005 + ) + + packages = list(get_dependencies(SourceRepository(path=tmp_path), cargo_path="cargo")) + + assert len(packages) == 1 + assert {dep.package for dep in packages[0].dependencies} == {"serde"} + + def test_resolve_from_source_only_marks_resolved_deps(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None: # noqa: ANN001 - """Optional/feature-gated deps with no matching package must not be marked resolved. + """A required dep with no matching package must not be marked resolved. Regression test for #189: the old shortcut marked every declared dependency as resolved, - including optional deps that `cargo metadata` never adds to the resolved package set. Those - deps then matched nothing and their subtrees were pruned, collapsing "all possible versions" - to the locked tree. + including deps that `cargo metadata` never adds to the resolved package set (e.g. a subcrate + whose `cargo metadata` errored). Those deps then matched nothing and their subtrees were + pruned. Such deps must be left unresolved so normal resolution can retry them. """ repo = SourceRepository(path=tmp_path) - # `libfoo` is actually resolved (a package is yielded); `libbar` and `rayon` are optional - # deps that cargo metadata never resolves, so no package is yielded for them. + # `libfoo` is actually resolved (a package is yielded); `libbar` and `libqux` are required + # deps that cargo failed to resolve, so no package is yielded for them. source_pkg = SourcePackage( name="myapp", version="0.1.0", @@ -52,7 +87,7 @@ def test_resolve_from_source_only_marks_resolved_deps(monkeypatch: pytest.Monkey name="libfoo", version="1.0.0", source="cargo", - dependencies=[_dep("rayon")], + dependencies=[_dep("libqux")], ) monkeypatch.setattr(CargoResolver, "can_resolve_from_source", lambda self, repo: True) # noqa: ARG005 @@ -68,6 +103,6 @@ def test_resolve_from_source_only_marks_resolved_deps(monkeypatch: pytest.Monkey assert result is source_pkg # libfoo has a matching package in the cache -> resolved. assert cache.was_resolved(_dep("libfoo")) - # libbar and rayon resolved to nothing -> left unresolved so normal resolution can expand them. + # libbar and libqux resolved to nothing -> left unresolved so normal resolution can retry them. assert not cache.was_resolved(_dep("libbar")) - assert not cache.was_resolved(_dep("rayon")) + assert not cache.was_resolved(_dep("libqux"))