From e36bb8a94943c991b2bd3bf369db5510e6c62152 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Wed, 15 Apr 2026 11:28:26 -0700 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A4=96=20Add=20ResolveResult=20to=20p?= =?UTF-8?q?reserve=20dependency=20graph=20from=20resolution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit resolve() previously returned a plain dict, discarding the dependency graph that resolvelib computed. Callers like _create_library_filelist() had to rebuild the graph by re-reading manifests from disk. ResolveResult now wraps the mapping with a graph (dict[str, set[str]]) and the set of direct dependencies, while providing dict-like convenience methods for backward compatibility. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fastsandpm/__init__.py | 5 +- src/fastsandpm/dependencies/__init__.py | 7 ++- src/fastsandpm/dependencies/__main__.py | 7 ++- src/fastsandpm/dependencies/provider.py | 72 +++++++++++++++++++++++-- src/fastsandpm/install.py | 30 ++++------- 5 files changed, 95 insertions(+), 26 deletions(-) diff --git a/src/fastsandpm/__init__.py b/src/fastsandpm/__init__.py index 60cc299..143db15 100644 --- a/src/fastsandpm/__init__.py +++ b/src/fastsandpm/__init__.py @@ -34,7 +34,7 @@ 'my-package' >>> resolved = fastsandpm.dependencies.resolve(manifest) >>> print(type(resolved)) - + >>> build_library(resolved, pathlib.Path("my-library")) Included Classes: @@ -42,6 +42,7 @@ - :py:class:`~manifest.Package`: Package metadata (name, version, description, authors). - :py:class:`~manifest.ManifestNotFoundError`: Raised when a manifest file cannot be found. - :py:class:`~manifest.ManifestParseError`: Raised when a manifest file cannot be parsed. + - :py:class:`~dependencies.ResolveResult`: Result of dependency resolution with graph. Functions: - :py:func:`~manifest.get_manifest`: Load and parse a manifest from a repository path. @@ -63,6 +64,7 @@ from fastsandpm import _info from fastsandpm.dependencies import ( + ResolveResult, resolve, ) from fastsandpm.install import build_library, library_from_manifest @@ -85,6 +87,7 @@ "ManifestNotFoundError", "ManifestParseError", "Package", + "ResolveResult", "resolve", "library_from_manifest", "build_library", diff --git a/src/fastsandpm/dependencies/__init__.py b/src/fastsandpm/dependencies/__init__.py index eeae07c..ba0995a 100644 --- a/src/fastsandpm/dependencies/__init__.py +++ b/src/fastsandpm/dependencies/__init__.py @@ -36,6 +36,10 @@ - :py:class:`~candidates.PathCandidate`: Candidate from a local filesystem path. - :py:class:`~candidates.GitCandidate`: Candidate from a git repository. +Included Classes (continued): + - :py:class:`~provider.ResolveResult`: Result of dependency resolution containing the resolved + packages and their dependency graph. + Included Functions: - :py:func:`~candidates.candidate_factory`: Singledispatch function to create candidates from requirements. @@ -52,7 +56,7 @@ PathCandidate, candidate_factory, ) -from .provider import resolve +from .provider import ResolveResult, resolve from .requirements import ( BranchGitRequirement, CommitGitRequirement, @@ -78,5 +82,6 @@ "PathCandidate", "GitCandidate", "candidate_factory", + "ResolveResult", "resolve", ] diff --git a/src/fastsandpm/dependencies/__main__.py b/src/fastsandpm/dependencies/__main__.py index d585891..e8d18c6 100644 --- a/src/fastsandpm/dependencies/__main__.py +++ b/src/fastsandpm/dependencies/__main__.py @@ -65,4 +65,9 @@ pkg_manifest = Manifest.model_validate(definition) pprint.pprint(pkg_manifest) print("") - pprint.pprint(resolve(pkg_manifest)) + result = resolve(pkg_manifest) + pprint.pprint(result.mapping) + print("\nDependency graph:") + pprint.pprint(result.graph) + print("\nDirect dependencies:") + pprint.pprint(result.direct_dependencies) diff --git a/src/fastsandpm/dependencies/provider.py b/src/fastsandpm/dependencies/provider.py index 4ede3f5..3edf262 100644 --- a/src/fastsandpm/dependencies/provider.py +++ b/src/fastsandpm/dependencies/provider.py @@ -35,7 +35,8 @@ from __future__ import annotations -from collections.abc import Iterable, Iterator, Mapping, Sequence +from collections.abc import ItemsView, Iterable, Iterator, Mapping, Sequence +from dataclasses import dataclass from typing import TYPE_CHECKING import resolvelib @@ -285,7 +286,50 @@ def narrow_requirement_selection( """Type alias for the resolution reporter.""" -def resolve(manifest: Manifest, optional_deps: list[str] | None = None) -> dict[str, Candidate]: +@dataclass(frozen=True) +class ResolveResult: + """Result of dependency resolution, containing resolved packages and their dependency graph. + + The mapping contains all resolved packages keyed by name. The graph preserves + the dependency relationships computed by the resolver, avoiding the need to + re-read manifests from disk to reconstruct them. + + Attributes: + mapping: Dictionary mapping package names to their resolved Candidate objects. + graph: Dictionary mapping each package name to the set of package names it + depends on. Only includes dependencies that are themselves in the resolved set. + direct_dependencies: The set of package names that were direct (user-specified) + dependencies, as opposed to transitive dependencies. + """ + + mapping: dict[str, Candidate] + graph: dict[str, set[str]] + direct_dependencies: frozenset[str] + + def items(self) -> ItemsView[str, Candidate]: + """Return items view of the mapping, for dict-like iteration.""" + return self.mapping.items() + + def __getitem__(self, key: str) -> Candidate: + """Get a candidate by package name.""" + return self.mapping[key] + + def __iter__(self) -> Iterator[str]: + """Iterate over package names.""" + return iter(self.mapping) + + def __len__(self) -> int: + """Return the number of resolved packages.""" + return len(self.mapping) + + def __contains__(self, key: object) -> bool: + """Check if a package name is in the resolved set.""" + return key in self.mapping + + +def resolve( + manifest: Manifest, optional_deps: list[str] | None = None +) -> ResolveResult: """Resolve all dependencies for a manifest. Creates a FastSandProvider with the manifest's registries and runs the @@ -297,7 +341,8 @@ def resolve(manifest: Manifest, optional_deps: list[str] | None = None) -> dict[ optional_deps: Optional dependency groups to include in the library. Returns: - A dictionary mapping package names to their resolved Candidate objects. + A ResolveResult containing the resolved packages, their dependency graph, + and the set of direct dependencies. Raises: resolvelib.ResolutionImpossible: If no compatible resolution exists. @@ -322,4 +367,23 @@ def resolve(manifest: Manifest, optional_deps: list[str] | None = None) -> dict[ dependencies.extend(manifest.optional_dependencies[group]) result = resolver.resolve(dependencies) - return result.mapping + + # Convert resolvelib's DirectedGraph to a plain dict, + # excluding the None root vertex. + dep_graph: dict[str, set[str]] = {} + for name in result.mapping: + dep_graph[name] = { + child for child in result.graph.iter_children(name) + if child is not None + } + + direct_deps = frozenset( + child for child in result.graph.iter_children(None) + if child is not None + ) + + return ResolveResult( + mapping=result.mapping, + graph=dep_graph, + direct_dependencies=direct_deps, + ) diff --git a/src/fastsandpm/install.py b/src/fastsandpm/install.py index 1d81131..93ccde0 100644 --- a/src/fastsandpm/install.py +++ b/src/fastsandpm/install.py @@ -56,7 +56,7 @@ from subprocess import CalledProcessError from fastsandpm import _git_utils -from fastsandpm.dependencies import Candidate, resolve +from fastsandpm.dependencies import ResolveResult, resolve from fastsandpm.dependencies.candidates import GitCandidate, PackageIndexCandidate, PathCandidate from fastsandpm.manifest import ( Manifest, @@ -93,7 +93,7 @@ def library_from_manifest( build_library(library, dest, clean) -def build_library(definition: dict[str, Candidate], dest: pathlib.Path, clean: bool = True) -> bool: +def build_library(definition: ResolveResult, dest: pathlib.Path, clean: bool = True) -> bool: """Build a library candidate from a manifest definition. The library will be placed in the destination directory with each dependency having it's own @@ -134,8 +134,8 @@ def build_library(definition: dict[str, Candidate], dest: pathlib.Path, clean: b a manifest. Args: - definition: The definition of the library to build. Where the key is the name of the - dependency and the value is the candidate for that dependency. + definition: The resolved dependency definition containing packages and their + dependency graph. dest: The destination directory for the library. clean: If True, clean the destination directory before building the library. @@ -420,33 +420,25 @@ def _install_path_candidate(candidate: PathCandidate, dep_dir: pathlib.Path, cle return False -def _create_library_filelist(definition: dict[str, Candidate], dest: pathlib.Path) -> None: +def _create_library_filelist(definition: ResolveResult, dest: pathlib.Path) -> None: """Create the library.f filelist with proper dependency ordering. Args: - definition: The library definition with candidates. + definition: The resolved dependency definition containing the dependency graph. dest: The destination directory for the library. - _logger: Logger for warnings and errors. """ - # Build dependency graph to determine ordering - dep_graph: dict[str, set[str]] = {} + # Use the dependency graph from resolution instead of rebuilding from disk + dep_graph = definition.graph dep_manifests: dict[str, Manifest] = {} - for name, _ in definition.items(): - dep_graph[name] = set() + for name in definition: dep_dir = dest / name - # Check if candidate has a manifest + # Read manifests to get flist paths try: dep_manifests[name] = get_manifest(dep_dir) - - # Add dependencies from manifest to graph - for dep in dep_manifests[name].dependencies: - if dep.name in definition: - dep_graph[name].add(dep.name) - - except ManifestNotFoundError as _: + except ManifestNotFoundError: _logger.debug("No manifest found for %s", name) except ManifestParseError as e: _logger.warning("Failed to read manifest for %s: %s", name, e) From 84104127b6ebe84a30ac96f424f0c487a5556ef8 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Wed, 15 Apr 2026 11:59:30 -0700 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A4=96=20Add=20topological=5Forder()?= =?UTF-8?q?=20method=20to=20ResolveResult?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move topological sort logic into ResolveResult so users can get dependency-ordered package names directly. Remove the private _topological_sort from install.py in favor of the new public method. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fastsandpm/dependencies/provider.py | 30 +++++++++++++++++++ src/fastsandpm/install.py | 40 ++----------------------- 2 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/fastsandpm/dependencies/provider.py b/src/fastsandpm/dependencies/provider.py index 3edf262..56b8801 100644 --- a/src/fastsandpm/dependencies/provider.py +++ b/src/fastsandpm/dependencies/provider.py @@ -326,6 +326,36 @@ def __contains__(self, key: object) -> bool: """Check if a package name is in the resolved set.""" return key in self.mapping + def topological_order(self) -> list[str]: + """Return package names in topological order (dependencies first). + + Packages that have no dependencies on other resolved packages appear + first, followed by packages whose dependencies have already appeared. + Ties are broken alphabetically for deterministic output. + + Returns: + List of package names sorted so that every package appears after + all of its dependencies. + """ + in_degree: dict[str, int] = {name: 0 for name in self.graph} + for name in self.graph: + for dep in self.graph[name]: + in_degree[dep] = in_degree.get(dep, 0) + 1 + + queue = sorted(name for name in self.graph if in_degree[name] == 0) + result: list[str] = [] + + while queue: + node = queue.pop(0) + result.append(node) + for dep in self.graph.get(node, set()): + in_degree[dep] -= 1 + if in_degree[dep] == 0: + queue.append(dep) + queue.sort() + + return result + def resolve( manifest: Manifest, optional_deps: list[str] | None = None diff --git a/src/fastsandpm/install.py b/src/fastsandpm/install.py index 93ccde0..35be4b6 100644 --- a/src/fastsandpm/install.py +++ b/src/fastsandpm/install.py @@ -428,8 +428,6 @@ def _create_library_filelist(definition: ResolveResult, dest: pathlib.Path) -> N dest: The destination directory for the library. """ - # Use the dependency graph from resolution instead of rebuilding from disk - dep_graph = definition.graph dep_manifests: dict[str, Manifest] = {} for name in definition: @@ -443,8 +441,8 @@ def _create_library_filelist(definition: ResolveResult, dest: pathlib.Path) -> N except ManifestParseError as e: _logger.warning("Failed to read manifest for %s: %s", name, e) - # Topological sort to order dependencies - ordered_deps = _topological_sort(dep_graph) + # Use the topological ordering from the resolve result + ordered_deps = definition.topological_order() # Create library.f file library_f_path = dest / "library.f" @@ -458,37 +456,3 @@ def _create_library_filelist(definition: ResolveResult, dest: pathlib.Path) -> N f.write(f"-F {name}/{name}.f\n") _logger.debug("Created library.f with %s dependencies", len(ordered_deps)) - - -def _topological_sort(graph: dict[str, set[str]]) -> list[str]: - """Perform topological sort on dependency graph. - - Args: - graph: Dictionary mapping node to set of its dependencies. - - Returns: - List of nodes in topologically sorted order (dependencies first). - """ - # Count incoming edges for each node - in_degree: dict[str, int] = {node: 0 for node in graph} - for node in graph: - for dep in graph[node]: - in_degree[dep] = in_degree.get(dep, 0) + 1 - - # Start with nodes that have no dependencies - queue = [node for node in graph if in_degree[node] == 0] - result = [] - - while queue: - # Sort to ensure deterministic ordering - queue.sort() - node = queue.pop(0) - result.append(node) - - # Remove edges from this node - for dep in graph.get(node, set()): - in_degree[dep] -= 1 - if in_degree[dep] == 0: - queue.append(dep) - - return result From e9f66f8929ef4e3796025c3ac35b02dd484bbbf9 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Wed, 15 Apr 2026 12:17:21 -0700 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=A4=96=20Add=20tests=20for=20ResolveR?= =?UTF-8?q?esult=20and=20fix=20topological=5Forder()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add 14 tests covering the dict-like interface and topological ordering. The tests caught a bug in topological_order() which was producing dependents-first instead of dependencies-first order. Fixed by reversing the traversal direction in Kahn's algorithm. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fastsandpm/dependencies/provider.py | 28 ++-- tests/dependencies/test_resolve_result.py | 173 ++++++++++++++++++++++ 2 files changed, 192 insertions(+), 9 deletions(-) create mode 100644 tests/dependencies/test_resolve_result.py diff --git a/src/fastsandpm/dependencies/provider.py b/src/fastsandpm/dependencies/provider.py index 56b8801..ac30a23 100644 --- a/src/fastsandpm/dependencies/provider.py +++ b/src/fastsandpm/dependencies/provider.py @@ -337,21 +337,31 @@ def topological_order(self) -> list[str]: List of package names sorted so that every package appears after all of its dependencies. """ - in_degree: dict[str, int] = {name: 0 for name in self.graph} - for name in self.graph: - for dep in self.graph[name]: - in_degree[dep] = in_degree.get(dep, 0) + 1 + # Count unresolved dependencies for each node + remaining_deps: dict[str, int] = { + name: len(deps) for name, deps in self.graph.items() + } + + # Build reverse lookup: for each dep, which nodes depend on it? + dependents: dict[str, set[str]] = {name: set() for name in self.graph} + for name, deps in self.graph.items(): + for dep in deps: + if dep in dependents: + dependents[dep].add(name) - queue = sorted(name for name in self.graph if in_degree[name] == 0) + # Start with nodes that have no dependencies + queue = sorted( + name for name in self.graph if remaining_deps[name] == 0 + ) result: list[str] = [] while queue: node = queue.pop(0) result.append(node) - for dep in self.graph.get(node, set()): - in_degree[dep] -= 1 - if in_degree[dep] == 0: - queue.append(dep) + for dependent in dependents.get(node, set()): + remaining_deps[dependent] -= 1 + if remaining_deps[dependent] == 0: + queue.append(dependent) queue.sort() return result diff --git a/tests/dependencies/test_resolve_result.py b/tests/dependencies/test_resolve_result.py new file mode 100644 index 0000000..3058d28 --- /dev/null +++ b/tests/dependencies/test_resolve_result.py @@ -0,0 +1,173 @@ +#################################################################################################### +# FastSandPM is a package management and dependency resolution tool for HDL Design and DV projects +# Copyright (C) 2026, Benjamin Davis +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see +# . +#################################################################################################### +"""Tests for the ResolveResult class.""" + +from __future__ import annotations + +import pathlib + +import pytest + +from fastsandpm.dependencies import PathCandidate, ResolveResult + + +def _make_candidate(name: str) -> PathCandidate: + """Create a minimal PathCandidate for testing.""" + return PathCandidate(name=name, version=None, path=pathlib.Path(f"/tmp/{name}")) + + +class TestResolveResult: + """Tests for the ResolveResult dict-like interface.""" + + @pytest.fixture + def result(self) -> ResolveResult: + """Create a ResolveResult with two independent packages.""" + a = _make_candidate("alpha") + b = _make_candidate("beta") + return ResolveResult( + mapping={"alpha": a, "beta": b}, + graph={"alpha": set(), "beta": set()}, + direct_dependencies=frozenset({"alpha", "beta"}), + ) + + def test_items_returns_mapping_items(self, result: ResolveResult) -> None: + """Test that items() delegates to the mapping.""" + items = dict(result.items()) + assert set(items.keys()) == {"alpha", "beta"} + + def test_getitem_returns_candidate(self, result: ResolveResult) -> None: + """Test that [] returns the correct candidate.""" + assert result["alpha"].name == "alpha" + assert result["beta"].name == "beta" + + def test_getitem_missing_raises_key_error(self, result: ResolveResult) -> None: + """Test that [] raises KeyError for unknown packages.""" + with pytest.raises(KeyError): + result["missing"] + + def test_iter_yields_keys(self, result: ResolveResult) -> None: + """Test that iterating yields package names.""" + assert set(result) == {"alpha", "beta"} + + def test_len_returns_count(self, result: ResolveResult) -> None: + """Test that len() returns the number of packages.""" + assert len(result) == 2 + + def test_contains_checks_membership(self, result: ResolveResult) -> None: + """Test that 'in' checks the mapping.""" + assert "alpha" in result + assert "missing" not in result + + def test_frozen(self, result: ResolveResult) -> None: + """Test that the dataclass is immutable.""" + with pytest.raises(AttributeError): + result.mapping = {} # type: ignore[misc] + + def test_empty_result(self) -> None: + """Test that an empty ResolveResult works correctly.""" + result = ResolveResult( + mapping={}, + graph={}, + direct_dependencies=frozenset(), + ) + assert len(result) == 0 + assert list(result) == [] + assert "anything" not in result + + +class TestResolveResultTopologicalOrder: + """Tests for the topological_order() method.""" + + def test_no_dependencies_returns_alphabetical(self) -> None: + """Test that independent packages are returned alphabetically.""" + result = ResolveResult( + mapping={ + "cherry": _make_candidate("cherry"), + "apple": _make_candidate("apple"), + "banana": _make_candidate("banana"), + }, + graph={"cherry": set(), "apple": set(), "banana": set()}, + direct_dependencies=frozenset({"cherry", "apple", "banana"}), + ) + assert result.topological_order() == ["apple", "banana", "cherry"] + + def test_linear_chain(self) -> None: + """Test a linear dependency chain: a -> b -> c.""" + result = ResolveResult( + mapping={ + "a": _make_candidate("a"), + "b": _make_candidate("b"), + "c": _make_candidate("c"), + }, + graph={"a": {"b"}, "b": {"c"}, "c": set()}, + direct_dependencies=frozenset({"a"}), + ) + order = result.topological_order() + assert order.index("c") < order.index("b") < order.index("a") + + def test_diamond(self) -> None: + """Test a diamond: a -> b, a -> c, b -> d, c -> d.""" + result = ResolveResult( + mapping={ + "a": _make_candidate("a"), + "b": _make_candidate("b"), + "c": _make_candidate("c"), + "d": _make_candidate("d"), + }, + graph={"a": {"b", "c"}, "b": {"d"}, "c": {"d"}, "d": set()}, + direct_dependencies=frozenset({"a"}), + ) + order = result.topological_order() + assert order.index("d") < order.index("b") + assert order.index("d") < order.index("c") + assert order.index("b") < order.index("a") + assert order.index("c") < order.index("a") + + def test_single_package(self) -> None: + """Test with a single package.""" + result = ResolveResult( + mapping={"only": _make_candidate("only")}, + graph={"only": set()}, + direct_dependencies=frozenset({"only"}), + ) + assert result.topological_order() == ["only"] + + def test_empty(self) -> None: + """Test with no packages.""" + result = ResolveResult( + mapping={}, + graph={}, + direct_dependencies=frozenset(), + ) + assert result.topological_order() == [] + + def test_multiple_roots(self) -> None: + """Test with two direct dependencies sharing a transitive dep.""" + result = ResolveResult( + mapping={ + "app_a": _make_candidate("app_a"), + "app_b": _make_candidate("app_b"), + "shared": _make_candidate("shared"), + }, + graph={"app_a": {"shared"}, "app_b": {"shared"}, "shared": set()}, + direct_dependencies=frozenset({"app_a", "app_b"}), + ) + order = result.topological_order() + assert order.index("shared") < order.index("app_a") + assert order.index("shared") < order.index("app_b") From 90230612bd0684b51808c7cec2eb04f9d11f0d30 Mon Sep 17 00:00:00 2001 From: Ben Davis Date: Wed, 15 Apr 2026 12:56:35 -0700 Subject: [PATCH 4/4] Fixed warnings from document generation which cause tox to fail --- src/fastsandpm/dependencies/provider.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/fastsandpm/dependencies/provider.py b/src/fastsandpm/dependencies/provider.py index ac30a23..5a7b3ec 100644 --- a/src/fastsandpm/dependencies/provider.py +++ b/src/fastsandpm/dependencies/provider.py @@ -293,18 +293,18 @@ class ResolveResult: The mapping contains all resolved packages keyed by name. The graph preserves the dependency relationships computed by the resolver, avoiding the need to re-read manifests from disk to reconstruct them. - - Attributes: - mapping: Dictionary mapping package names to their resolved Candidate objects. - graph: Dictionary mapping each package name to the set of package names it - depends on. Only includes dependencies that are themselves in the resolved set. - direct_dependencies: The set of package names that were direct (user-specified) - dependencies, as opposed to transitive dependencies. """ mapping: dict[str, Candidate] + """Dictionary mapping package names to their resolved Candidate objects.""" graph: dict[str, set[str]] + """Dictionary mapping each package name to the set of package names it depends on. + Only includes dependencies that are themselves in the resolved set. + """ direct_dependencies: frozenset[str] + """The set of package names that were direct (user-specified) dependencies, + as opposed to transitive dependencies. + """ def items(self) -> ItemsView[str, Candidate]: """Return items view of the mapping, for dict-like iteration."""