Skip to content

feat(fetch): resolve a nested workspace's members against their own root - #137

Open
justin13888 wants to merge 6 commits into
feat/107-exact-pin-versionfrom
feat/110-nested-workspace-scope
Open

feat(fetch): resolve a nested workspace's members against their own root#137
justin13888 wants to merge 6 commits into
feat/107-exact-pin-versionfrom
feat/110-nested-workspace-scope

Conversation

@justin13888

@justin13888 justin13888 commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Closes #110.

dependable tree falls back to a shallow, manifest-only graph when the scan root has no Cargo.lock. That walk collects every Cargo.toml beneath the root, which sweeps in crates belonging to a nested, independent workspace — a cargo fuzz tree, an examples/inner, an xtask with its own [workspace]. Cargo ignores such a subtree entirely, so nobody lists it in the outer root's exclude, and the walk reaches it.

Those crates were resolved against a single set of workspace tables: the scan root's. Refusing to hand the outer root's [workspace.package] version to a nested crate stopped it reporting a number that was never its own, but left it with no version at all — while the version it does have sits one directory up, in its own root's [workspace.package], unread. The same single-root assumption routed every member's dep.workspace = true through the outer root's [workspace.dependencies], so a name a nested root vendors by path was classified from whatever the outer root happened to say about that name, or from nothing.

This carries a scope per workspace root instead.

$ dependable tree root      # before          # after
a v1.0.0 (workspace)        a v1.0.0          a v1.0.0
a-fuzz (workspace)          a-fuzz            a-fuzz v0.0.0

What changed

  • A private Scope holds one root's two inheritance tables — [workspace.package] and [workspace.dependencies] — because a member's version.workspace = true and its dep.workspace = true name the same root; answering them from different manifests is the defect.
  • The member walk owns a Scope arena seeded with the scan root at index 0. On meeting a manifest that declares a [workspace], it pushes that root's scope and uses it for that directory and everything below. The switch happens before the directory's own [package] is read, so a manifest that is both a [workspace] and a [package] — the cargo fuzz shape — resolves against itself.
  • Member carries that scope index in place of a governed_by_root: bool; shallow_graph resolves both kinds of inheritance against it and no longer takes the root's content at all.
  • Innermost-wins falls out of the arena: a [workspace] under a [workspace] pushes another scope. The guarantee that a crate never borrows an unrelated root's version is preserved by construction, not by a special case: a nested root declaring no [workspace.package] version yields a table with no version key, which resolves to nothing. There is a test for exactly that.
  • The one hole in that guarantee is closed rather than caveated. A directory's Cargo.toml is classified into absent, readable, or opaque, where opaque means the file exists but could not be read (permissions, a device error) or is not valid TOML. Both failures used to collapse into "not a workspace" — read_to_string(..).ok() and ImDocument::parse(..).ok()? each yield None — so no scope was pushed and every crate below an unreadable nested root kept the outer scope and resolved version.workspace = true against a root with no authority over it. An opaque boundary now pushes a scope of its own with both tables empty: a file that exists and cannot be read is not evidence that the enclosing root governs what is beneath it. A directory with no Cargo.toml at all still inherits the enclosing scope, which is correct and unchanged. (The old behaviour was not introduced here — the base branch's declares_workspace collapsed the same two cases — but the claim above is only true once it is fixed.)
  • The walk now descends in sorted order and settles a [package] name duplicated across a nested-workspace boundary in favour of the outer crate. This change is the first to make the outcome differ by which scope won, so leaving the existing first-wins-over-unsorted-read_dir rule would have shipped a version that varied with filesystem iteration order between machines holding identical contents.

All private to tree.rs. No public API, no TreeError or GraphSource variant, no CLI flag, no config key, no exit code, no JSON schema field, no dependency change.

Decisions taken

Recorded under the autonomy contract for a run the user declared unattended, in their own words: "autonomously. create everything end-to-end" and "finish everything and finalize me all the final products I will test myself". There was no channel to ask on; each fork below was settled rather than raised, and each names what would reverse it.

1. Deliverable boundary — how the nested scope is resolved

Taken: a full scope stack carrying BOTH [workspace.package] and [workspace.dependencies]. The walk carries each member's governing scope; version.workspace = true resolves against it, and resolve_workspace_inheritance receives that member's own scope's declarations.

Rejected: a scope stack for [workspace.package] only — it closes the issue's literal complaint and knowingly leaves the second authority leak in place, because shallow_graph calls resolve_workspace_inheritance(&mut items, &declarations) for EVERY member with declarations derived from the outer root_content alone. A nested crate's dep.workspace = true would still take the outer root's declaration and therefore its PackageSource and NodeKind. Shipping a Scope struct that carries one of the two tables invites the reader to ask why. The marginal cost is one field and one argument, and the code that builds declarations already exists verbatim at the top of shallow_graph and is being lifted anyway.

Rejected: excluding nested-workspace crates from the outer graph entirely, as Cargo does — it is the largest behaviour change of the four: nodes present in today's output disappear, and tree is a discovery tool where a user may well want to see the fuzz crate they forgot about. It answers the issue by deleting the question rather than by reading the 0.0.0 sitting one directory up, and it requires rewriting the base branch's newest test into its negation.

Rejected: growing SKIP_DIRS — strictly less precise than machinery the file already has. Detecting a nested workspace from its [workspace] table identifies one STRUCTURALLY; a name list guesses. xtask is canonically a real member of the outer workspace, so listing it would drop a governed crate — the mirror image of the bug being fixed — and examples/<name>/Cargo.toml is likewise a member in many workspaces. The issue's own tests/fixtures candidate is not even expressible, because matching is on the bare directory name. The issue speculates this "may make the first unnecessary"; it cannot, because a nested workspace in an unlisted directory still needs the flag.

Reverses: drop declarations from Scope and restore the single outer-root resolve_workspace_inheritance call (gives the version-only variant); or return early at a nested [workspace] in the walk (gives the Cargo-like exclusion).

Filed: the "should a nested crate be visually distinguishable" question, as #138.

2. Which crate wins a duplicate [package] name across a nested boundary

Taken: the outer crate, kept as one node. The scan root is the workspace the user asked about, so its member keeps its own name. Concretely: the walk deduplicates by name with a seen index, and a later member replaces an earlier one only when its scope index is smaller — a nested root can only be pushed after the root containing it, so a smaller index is the enclosing scope. Directory entries are also sorted before recursing; this change is the first that makes the OUTCOME differ by which crate won, so leaving the answer to read_dir order would ship a version that varies between machines holding identical contents.

Rejected: dropping the nested crate from the graph once its name collides. It is the reading Cargo itself takes — the nested subtree is simply not part of this workspace — but tree is a discovery tool, and silently omitting a crate is the opposite of what a user runs it for.

Rejected: keeping both as distinct nodes, the way the lockfile path already keeps duplicate versions of one crate apart. The shallow graph keys nodes by bare name with no path component, so two nodes named dup would be indistinguishable in every renderer — the tree, the JSON, the TUI — and the ambiguity would move from the builder to the reader rather than being settled.

Rejected: sorting alone — determinism is not correctness; alphabetical order does not systematically favour the outer root, it only makes the wrong answer stable.

Reverses: drop the scope < members[idx].scope comparison for plain first-wins, or key seen on the member's path rather than its name.

3. Between two SIBLING nested workspaces, the alphabetically earlier directory wins

Taken: arbitrary, but fixed. Neither sibling encloses the other, so scope < members[idx].scope compares two indices with no enclosure relation between them, and the smaller one is simply whichever the sorted walk reached first. No scope has a claim here; stability across machines is the only property that matters, and the sorted descent supplies it.

Rejected: reporting the collision. tree has no diagnostic channel for a graph-construction ambiguity — the builder returns a graph or a TreeError, and neither carries warnings — and adding one for a case that requires two nested workspaces sharing a crate name is disproportionate to the case.

Reverses: emit a notice when seen rejects a member whose scope neither encloses nor is enclosed by the winner's.

4. A nested root is authoritative for its inheritance tables but not for its exclude, and its crates still count as members of the outer workspace

Taken: half-honouring the nested root, deliberately, as the current step rather than the end state. The two inheritance tables are what #110 is about. Honouring a nested root's [workspace] exclude and reconsidering whether its crates belong in the outer graph at all are separate questions with their own blast radii, and pretending otherwise would widen this change into both.

Rejected: honouring the nested root's exclude in the same change. collect_members documents that it treats a crate as in-workspace iff its [package] name appears under the root, precisely to sidestep a glob engine; exclude entries are globs, so honouring them reintroduces exactly what that design avoids — for the nested root and then, by symmetry, for the scan root too.

Filed: the visual half — whether a nested crate should be distinguishable in the rendered tree — is #138.

Reverses: honour the nested root's [workspace] exclude in excluded_dirs.

5. Test surface

Taken: TempDir tests in crates/dependable-fetch/tests/tree.rs only.

Rejected: adding a committed lockfile-less fixture plus CLI assertions — crates/dependable/tests/fixture_tree.rs is documented as "the graph comes from the fixture's Cargo.lock", which a lockfile-less fixture would make untrue, and it widens the change into crates/dependable/. The reachability risk that would have justified it does not apply: tests/tree.rs already drives shallow_graph through TempDirs, and the earlier nested-workspace test reaches both branches of the governed flag.

Reverses: add the fixture directory and fixture_tree.rs assertions.

6. Taken during implementation, not planned

clippy::too_many_arguments fires at the 8th argument, which the scope arena added to the recursive walk. Rather than suppress the lint, the walk's cross-recursion state — the scan root, the exclude set, the dedup index, the members, and the arena — moved onto a private Walk struct with the recursion as a method, leaving only what varies per directory (dir, depth_left, scope) as arguments. This is a private restructuring inside tree.rs; nothing re-exported moves. Reverses: restore the free function and thread the arena through its parameters, which requires an #[allow] the repository's -D warnings posture does not otherwise carry.

Residual, knowingly left

tree.rs keeps a private SKIP_DIRS that lists the same four names as discover::SKIP_DIRS, and its inline SKIP_DIRS.contains(&name) || name.starts_with('.') is discover::is_skipped_dir verbatim. The two can drift: adding dist to discover's list would change what list and check scan without changing what tree walks. Leaving the duplication is the right call for this PR — the two bound different scans with genuinely different consequences, since a name added to discover narrows a report while a name added here silently drops crates from the graph — so unifying them would couple two decisions that should stay separate. A comment on tree.rs's SKIP_DIRS now records that the divergence is intentional, so the next reader does not helpfully merge them.

Also settled, and why it is not here

Whether a declared pin should be distinguishable from a lockfile resolution was settled NO on the base branch (#120, decision 4), for the same reason it is declined here: it needs a new field on Node, which spans dependable-core, the TUI, and the JSON schema.

Tests

Seven tests in crates/dependable-fetch/tests/tree.rs. Each was confirmed to discriminate by mutating the implementation back toward the old behaviour and observing the failure — a test that passes against both is evidence of nothing.

Test Proves Fails when
a_nested_independent_workspaces_crate_resolves_against_its_own_root (rewritten from ..._does_not_inherit_the_outer_roots_version) a = 1.0.0, a-fuzz = 0.0.0 (was None), a-fuzz-stated = 7.7.7 resolution forced to scope 0
a_nested_root_declaring_no_version_leaves_its_crate_without_one the regression guard: a nested [workspace] with no [workspace.package] yields None, never the outer root's 1.0.0 resolution forced to scope 0
the_innermost_workspace_root_governs_when_workspaces_nest_twice 1.0.0 / 2.0.0 / 3.0.0 across two levels, including a crate below the innermost root resolution forced to scope 0
a_nested_workspaces_crate_inherits_dependencies_from_its_own_root a name the nested root vendors by path is NodeKind::Path even though the outer root declares the same name as a registry entry declarations forced to scope 0
a_name_shared_across_a_nested_boundary_keeps_the_outer_crates_version the same two crates built twice, with directories named so the walk meets the outer crate first in one layout and the nested one first in the other; 1.0.0 both times the scope comparison is removed (yields 9.9.9 in the second layout)
a_name_shared_within_one_scope_settles_on_the_alphabetically_earlier_path the case the scope comparison cannot answer: aaa/ and zzz/ both name a crate dup in the same (root) scope, with different literal versions, so first-wins decides and the sorted walk is the only thing that makes "first" mean anything. aaa's 1.1.1 wins. The two directories are created in reverse alphabetical order on purpose, so a filesystem reporting entries in creation order hands the walk the wrong crate first paths.sort() is removed — observed failing 30 out of 30 runs on the development machine, returning 9.9.9
a_crate_under_an_unparseable_nested_root_inherits_nothing an unterminated [workspace table header as a nested root's Cargo.toml, with a crate below it declaring version.workspace = true and shared-dep.workspace = true. Both resolve to None; the outer root's 1.0.0 and its [workspace.dependencies] do not reach across. The outer root's own member still gets 1.0.0 an unreadable manifest is classified Absent instead of Opaque — the crate then reports the outer root's 1.0.0

falls_back_to_shallow_graph_without_lockfile and a_member_inheriting_its_version_from_the_workspace_root_still_reports_one are unchanged and pass.

Validation

Every command below completed in this run, colour disabled (env -u FORCE_COLOR -u COLORTERM), on the final tree. No failures, so nothing to classify.

Command Outcome
cargo test -p dependable-fetch --test tree 16 passed, 0 failed
cargo test -p dependable-fetch 87 + 23 + 17 + 20 + 8 + 15 + 16 + 1 passed, 0 failed
mise run test exit 0, 0 failed
mise run fmt:check exit 0
mise run lint (clippy --workspace --all-targets -D warnings) exit 0
convco check over the series passing

Not proven

  • The tree CLI rendering of a nested crate's newly present version is not asserted at the CLI level, per decision 3; the graph the renderer consumes is.
  • [workspace] members globs are still not evaluated, a nested root's own exclude is still not honoured (decision 4), and the depth bound of 64 and the scan root's exclude are untouched. All were out of scope by design.
  • The opaque-boundary path is proven for the unparseable case only. A Cargo.toml that exists but cannot be read — mode 000, an EIO — takes the same branch in boundary_at, but no test creates one, because a permission-denied fixture is not reliable across the environments this suite runs in (root, containers, and filesystems that ignore the mode bits). The classification is one match arm away from the tested one.

Base

Opened against feat/107-exact-pin-version (#120), which is where the exact-pin narrowing in this same file lives. Its arm of shallow_graph — the external-dependency loop — is untouched here.

A scan can span more than one workspace: a `fuzz/` or `examples/` tree with
its own `[workspace]` is a root in its own right, and the walk descends into
it because Cargo already ignores such a subtree, so nobody lists it in the
outer root's `exclude`.

The shallow graph resolved every member against one root's tables — the scan
root's. Refusing to hand the outer root's `[workspace.package] version` to a
nested crate stopped it reporting a number that was never its own, but left
it with no version at all, when the version it does have sits one directory
up in its own root. The same single-root assumption routed every member's
`dep.workspace = true` through the outer root's `[workspace.dependencies]`,
so a name that a nested root vendors by path was classified from whatever
the outer root happened to say about that name, or from nothing.

Carry a `Scope` per workspace root instead — both inheritance tables
together, since a member's `version.workspace = true` and its
`dep.workspace = true` name the same root — and give each member the index
of the nearest `[workspace]` ancestor that governs it. A nested root
declaring no version still yields none: an absent table resolves to nothing
rather than to some other root's number, so a crate can never borrow a
version from a workspace it is not in.

The walk's growing state moves onto a `Walk` struct so the recursion carries
context rather than an argument list.
…dary

The member walk deduplicates by `[package] name` and takes the first crate
it meets. Its `read_dir` yields filesystem order, so two crates sharing a
name — one in the scanned workspace, one under a nested, independent root —
were settled by whichever the filesystem happened to hand back first.

That was invisible while both resolved against the same tables. Now that
each resolves against its own root, the two answers differ, and the one
reported would differ between machines holding identical contents.

Descend in sorted order and keep the outer crate: a nested root is only
pushed onto the scope arena after the root containing it, so the smaller
scope index is the enclosing one. Sorting alone would only make a wrong
answer stable — it is here so that the tie between two crates in sibling
nested workspaces, where neither encloses the other, is fixed rather than
arbitrary.
The member walk read a directory's `Cargo.toml` with `read_to_string(..).ok()`
and asked `parse_workspace` whether it opened a new scope. Both collapse failure
into `None`, so a nested root that could not be read — mode 000, or a TOML syntax
error — was indistinguishable from a directory with no manifest at all, and every
crate beneath it kept the *outer* scope. Such a crate resolved
`version.workspace = true` against a root with no authority over it, reporting a
number the outer root's `[workspace.package]` happened to carry.

A file that exists and cannot be read is not evidence that the enclosing root
governs what is below it: it may well declare a `[workspace]`, and nothing can
rule that out. Classify the manifest into absent, readable, or opaque, and give
an opaque one a scope of its own with both inheritance tables empty. Crates below
it now inherit nothing, in either direction, while a directory with no
`Cargo.toml` keeps inheriting the enclosing scope as before.
Two crates can also share a `[package] name` without a workspace boundary
between them — `crates/dup` and `examples/dup` in one workspace — and the scope
comparison cannot settle that: both index the same root, so first-wins decides.
What "first" means is the walk's sorted descent, and nothing pinned it. The
existing cross-boundary test passes with or without `paths.sort()`, because the
scope comparison picks the same winner in either encounter order.

Assert that the alphabetically earlier path wins, with the two directories
created in reverse alphabetical order so a filesystem reporting entries in
creation order hands the walk the wrong crate first. Removing `paths.sort()`
fails this test on every run rather than on some machines.
`tree`'s private `SKIP_DIRS` lists the same four names as `discover::SKIP_DIRS`,
and the inline filter beside it is `discover::is_skipped_dir` verbatim, which
reads as an oversight worth unifying. It is not: the two bound different scans
with different consequences — adding a name to `discover`'s list narrows what
`list` and `check` report on, adding one here silently drops crates from the
graph. Say so where the next reader will be tempted.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant