Fix cargo resolver pruning of optional/feature-gated dependencies

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"]}
@@ -113,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"]
@@ -173,20 +186,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. 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):
+                    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]:

test/test_cargo.py

@@ -1,6 +1,11 @@
+import json
+
 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, get_dependencies
+from it_depends.models import Dependency, Package, SourcePackage
+from it_depends.repository import SourceRepository
 
 
 @pytest.mark.parametrize(
@@ -20,3 +25,84 @@ 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_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
+    """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 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 `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",
+        source_repo=repo,
+        source="cargo",
+        dependencies=[_dep("libfoo"), _dep("libbar")],
+    )
+    libfoo = Package(
+        name="libfoo",
+        version="1.0.0",
+        source="cargo",
+        dependencies=[_dep("libqux")],
+    )
+
+    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 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("libqux"))