diff --git a/crates/dependable-fetch/src/tree.rs b/crates/dependable-fetch/src/tree.rs index c42e9aa..feb4a62 100644 --- a/crates/dependable-fetch/src/tree.rs +++ b/crates/dependable-fetch/src/tree.rs @@ -12,7 +12,7 @@ //! — a manifest declares a constraint, not a resolution — except where the //! constraint names exactly one release, which [`declared_pin`] reads off it. -use std::collections::{HashMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; use dependable_core::{ @@ -25,6 +25,13 @@ use dependable_core::{ use thiserror::Error; /// Directories never descended into while collecting member manifests. +/// +/// Deliberately **not** [`crate::discover::SKIP_DIRS`], despite listing the same +/// names today. That list bounds what `list` and `check` scan for manifests to +/// report on; this one bounds what the graph walk treats as workspace members. The +/// two answer different questions with different blast radii — adding a name here +/// silently drops crates from the graph, adding one there only narrows a report — +/// so they are free to diverge, and unifying them would couple the two decisions. const SKIP_DIRS: &[&str] = &["target", "node_modules", ".git", "vendor"]; /// Where a workspace graph's edges came from. @@ -109,7 +116,7 @@ pub fn build_workspace_graph( let root_content = read(&root_dir.join("Cargo.toml"))?; let excluded = excluded_dirs(&root_dir, &root_content); - let members = collect_members(&root_dir, &excluded); + let (members, scopes) = collect_members(&root_dir, &root_content, &excluded); let workspace_names: HashSet = members.iter().map(|member| member.name.clone()).collect(); @@ -136,7 +143,7 @@ pub fn build_workspace_graph( }); } - let graph = shallow_graph(&members, &workspace_names, &roots, &root_content); + let graph = shallow_graph(&members, &workspace_names, &roots, &scopes); Ok(WorkspaceGraph { graph, source: GraphSource::Manifests, @@ -169,58 +176,215 @@ fn excluded_dirs(root_dir: &Path, root_content: &str) -> HashSet { .unwrap_or_default() } +/// The authority one `[workspace]` root lends its members: both of Cargo's +/// inheritance tables, read off that root's manifest. +/// +/// A single scan can span more than one workspace — a `fuzz/` or `examples/` tree +/// with its own `[workspace]` is a root in its own right — so "the workspace root" +/// is not a single thing and each member must be read against the one that +/// actually governs it. The two tables travel together because a member's +/// `version.workspace = true` and its `dep.workspace = true` name the *same* root; +/// answering them from different manifests is the bug this type exists to prevent. +/// +/// The [`Default`] scope — both tables empty — is what an *opaque* boundary gets: +/// a root whose manifest cannot be read or parsed lends no authority at all, but +/// still stops the enclosing root's from reaching past it. +#[derive(Default)] +struct Scope { + /// `[workspace.package]`, the source of a member's `version.workspace = true`. + package_defaults: BTreeMap, + /// `[workspace.dependencies]`, the source of a member's `dep.workspace = true`. + declarations: Vec, +} + +/// Read a manifest's two workspace inheritance tables. +/// +/// A table that is absent, or a manifest that does not parse, yields an empty one +/// — which is the honest answer rather than a fallback: a root declaring no +/// `[workspace.package] version` resolves its members' `version.workspace = true` +/// to nothing, never to some other root's number. +fn scope_of(content: &str) -> Scope { + Scope { + package_defaults: parse_workspace(content) + .map(|ws| ws.package_defaults) + .unwrap_or_default(), + // A member's `dep.workspace = true` says nothing about what the crate *is* — + // its root's declaration does. Resolving against it is what tells a + // centrally-declared registry crate from a centrally-declared vendored path, + // which the member's own text cannot. + declarations: CargoTomlParser + .parse(content) + .map(|m| m.items) + .unwrap_or_default() + .into_iter() + .filter(|item| item.kind == DependencyKind::Workspace) + .collect(), + } +} + +/// What a directory's `Cargo.toml` tells the walk about the subtree beneath it. +/// +/// The distinction that matters is between *absent* and *unusable*. A directory +/// with no manifest is plainly still governed by the enclosing workspace root; a +/// directory whose manifest exists but cannot be read or parsed is not — it may +/// well declare a `[workspace]`, and there is no way to tell. Collapsing the two +/// into "not a workspace" is what would let a crate below an unreadable nested +/// root inherit a version from a root with no authority over it. +enum Boundary { + /// No `Cargo.toml` here. The enclosing scope still governs what is below. + Absent, + /// A `Cargo.toml` that was read and parses as TOML. + Manifest(String), + /// A `Cargo.toml` that exists but could not be read, or is not valid TOML. + Opaque, +} + +/// Classify a directory's `Cargo.toml` into a [`Boundary`]. +/// +/// Anything other than "the file is not there" is [`Boundary::Opaque`]: a +/// permission error, an unreadable device, a directory of that name, or a syntax +/// error all leave the manifest's contents unknown, and unknown is not the same as +/// empty. +fn boundary_at(dir: &Path) -> Boundary { + match std::fs::read_to_string(dir.join("Cargo.toml")) { + // `CargoTomlParser` fails only when the TOML itself does not parse, which is + // exactly the question being asked here. + Ok(content) if CargoTomlParser.parse(&content).is_ok() => Boundary::Manifest(content), + Ok(_) => Boundary::Opaque, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Boundary::Absent, + Err(_) => Boundary::Opaque, + } +} + /// A crate manifest found under the scan root. struct Member { /// The crate's `[package] name`. name: String, /// The manifest's text. content: String, - /// Whether the scan root is this crate's **nearest** `[workspace]` ancestor, - /// and so whether the root's `[workspace.package]` table governs it. + /// Index into the scan's [`Scope`] arena: the workspace root that governs this + /// crate, which is its **nearest** `[workspace]` ancestor. /// - /// False for a crate inside a nested, independent workspace — a `fuzz/` or - /// `examples/` directory with its own `[workspace]` table. Cargo resolves - /// those against *their* root; the scan root has no authority over them. - governed_by_root: bool, + /// The scan root is index 0. A crate inside a nested, independent workspace — a + /// `fuzz/` or `examples/` directory with its own `[workspace]` table — points at + /// that nested root instead, because Cargo resolves it against *that* one and the + /// scan root has no authority over it. An unreadable or unparseable manifest in + /// between opens a scope too — an empty one, per [`Boundary::Opaque`]. + scope: usize, } -/// Collect a [`Member`] for every crate under `root_dir`, deduplicated by name. -/// A crate is treated as in-workspace iff its `[package] name` appears here — -/// this sidesteps needing a glob engine. -fn collect_members(root_dir: &Path, excluded: &HashSet) -> Vec { - let mut out = Vec::new(); - let mut seen = HashSet::new(); - walk_members(root_dir, root_dir, excluded, &mut seen, &mut out, 64, true); - out +/// Collect a [`Member`] for every crate under `root_dir`, deduplicated by name, +/// together with the [`Scope`] arena those members index into. A crate is treated +/// as in-workspace iff its `[package] name` appears here — this sidesteps needing +/// a glob engine. +fn collect_members( + root_dir: &Path, + root_content: &str, + excluded: &HashSet, +) -> (Vec, Vec) { + let mut walk = Walk { + root_dir, + excluded, + seen: HashMap::new(), + members: Vec::new(), + // The scan root is index 0, and a nested root can only be pushed after the + // root that contains it — so a smaller index is always the outer scope. + scopes: vec![scope_of(root_content)], + }; + walk.descend(root_dir, 64, 0); + (walk.members, walk.scopes) } -/// Whether `dir` holds a `Cargo.toml` declaring a `[workspace]` table. -fn declares_workspace(dir: &Path) -> bool { - std::fs::read_to_string(dir.join("Cargo.toml")) - .is_ok_and(|content| parse_workspace(&content).is_some()) +/// One run of the member walk. +/// +/// The walk carries state across the whole recursion — the dedup index, the +/// members found, and the [`Scope`] arena that grows as nested workspace roots are +/// met — so it lives here rather than in an argument list threaded through every +/// call. Only what actually varies per directory stays an argument. +struct Walk<'a> { + /// The scan root, the one directory whose `[workspace]` does not open a new scope. + root_dir: &'a Path, + /// Absolute directories named in the scan root's `[workspace] exclude`. + excluded: &'a HashSet, + /// `[package] name` -> index into `members`; a crate name yields one member. + seen: HashMap, + members: Vec, + scopes: Vec, } -fn walk_members( - dir: &Path, - root_dir: &Path, - excluded: &HashSet, - seen: &mut HashSet, - out: &mut Vec, - depth_left: usize, - governed: bool, -) { - // A nested `[workspace]` is a workspace root in its own right. Cargo already - // ignores such a subtree, so nobody lists it in `[workspace] exclude`, and the - // walk still descends into it — but the scan root's `[workspace.package]` has - // no authority there, so nothing at or below this manifest may inherit from it. - let governed = governed && (dir == root_dir || !declares_workspace(dir)); - let Ok(entries) = std::fs::read_dir(dir) else { - return; - }; - for entry in entries.flatten() { - let path = entry.path(); - if path.is_dir() { - if depth_left == 0 || excluded.contains(&path) { +impl Walk<'_> { + /// Record `dir`'s crate, if it holds one, then descend into its subdirectories. + fn descend(&mut self, dir: &Path, depth_left: usize, scope: usize) { + // Read once: the same text answers both "is this a workspace root?" and "is + // this a crate?", and a `cargo fuzz` manifest is routinely both. + let manifest = boundary_at(dir); + // A nested `[workspace]` is a workspace root in its own right. Cargo already + // ignores such a subtree, so nobody lists it in `[workspace] exclude`, and the + // walk still descends into it — but the outer root's tables have no authority + // there. Everything at or below this manifest resolves against the nested root + // instead. The scope is switched *before* this directory's own `[package]` is + // read, so a manifest that is both a `[workspace]` and a `[package]` resolves + // against itself. + let scope = match &manifest { + Boundary::Manifest(content) + if dir != self.root_dir && parse_workspace(content).is_some() => + { + self.scopes.push(scope_of(content)); + self.scopes.len() - 1 + } + // A manifest that exists but cannot be read or parsed is an opaque + // boundary, not an absent one: it may declare a `[workspace]`, and nothing + // here can rule that out. Push an empty scope so the crates below it + // resolve their `workspace = true` fields to nothing, rather than silently + // borrowing an enclosing root's — a file that exists and cannot be read is + // not evidence that the outer root governs what is beneath it. + Boundary::Opaque if dir != self.root_dir => { + self.scopes.push(Scope::default()); + self.scopes.len() - 1 + } + _ => scope, + }; + if let Boundary::Manifest(content) = manifest + && let Some(name) = parse_package_name(&content) + { + match self.seen.get(&name).copied() { + // Two crates can share a `[package] name` across a nested-workspace + // boundary, and only one node can carry it. The outer scope wins: a + // nested root is only pushed after the root containing it, so a + // smaller index is the enclosing one. Between two crates in *sibling* + // nested workspaces neither encloses the other, and the smaller index + // is then the alphabetically earlier path — arbitrary, but fixed, + // which is the point. Without this the answer would follow whichever + // one the filesystem happened to hand back first. + Some(idx) => { + if scope < self.members[idx].scope { + self.members[idx] = Member { + name, + content, + scope, + }; + } + } + None => { + self.seen.insert(name.clone(), self.members.len()); + self.members.push(Member { + name, + content, + scope, + }); + } + } + } + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + // `read_dir` yields filesystem order, which can differ between machines holding + // identical contents. Descending in a fixed order is what makes the walk — and + // with it the duplicate-name rule above — reproducible. + let mut paths: Vec = entries.flatten().map(|entry| entry.path()).collect(); + paths.sort(); + for path in paths { + if !path.is_dir() || depth_left == 0 || self.excluded.contains(&path) { continue; } if let Some(name) = path.file_name().and_then(|n| n.to_str()) @@ -228,25 +392,7 @@ fn walk_members( { continue; } - walk_members( - &path, - root_dir, - excluded, - seen, - out, - depth_left - 1, - governed, - ); - } else if path.file_name().and_then(|n| n.to_str()) == Some("Cargo.toml") - && let Ok(content) = std::fs::read_to_string(&path) - && let Some(name) = parse_package_name(&content) - && seen.insert(name.clone()) - { - out.push(Member { - name, - content, - governed_by_root: governed, - }); + self.descend(&path, depth_left - 1, scope); } } } @@ -262,42 +408,31 @@ fn walk_members( /// which case the manifest has already resolved it and [`declared_pin`] reads it /// off. /// -/// `version.workspace = true` is resolved against the root's `[workspace.package]` -/// only for a member the root actually governs (see [`Member::governed_by_root`]). -/// A crate in a nested, independent workspace keeps no version at all: reporting -/// nothing is honest, where borrowing an unrelated root's number would not be. +/// Both kinds of `workspace = true` — a member's own `version` and its +/// dependencies — are resolved against [`Member::scope`], the crate's nearest +/// `[workspace]` ancestor, rather than against the scan root. A crate in a nested, +/// independent workspace therefore reports what *its* root declares; where that +/// root declares nothing, nothing is reported, because borrowing an unrelated +/// root's number would be a confidently wrong answer rather than an absent one. fn shallow_graph( members: &[Member], workspace_names: &HashSet, roots: &[String], - root_content: &str, + scopes: &[Scope], ) -> DependencyGraph { - // A member's `dep.workspace = true` says nothing about what the crate *is* — the - // root's declaration does, and the root is already in hand here. Resolving against - // it is what tells a centrally-declared registry crate from a centrally-declared - // vendored path, which the member's own text cannot. - let declarations: Vec = CargoTomlParser - .parse(root_content) - .map(|m| m.items) - .unwrap_or_default() - .into_iter() - .filter(|item| item.kind == DependencyKind::Workspace) - .collect(); - // `[workspace.package]`, the source a member's `version.workspace = true` - // inherits from. A member that inherits its version still has one. - let package_defaults = parse_workspace(root_content) - .map(|ws| ws.package_defaults) - .unwrap_or_default(); let mut member_pkgs: Vec = Vec::new(); let mut external_pkgs: Vec = Vec::new(); let mut external_seen: HashMap = HashMap::new(); for member in members { + // The root that governs *this* crate, which in a scan spanning more than one + // workspace is not necessarily the scan root. + let scope = &scopes[member.scope]; let mut items = CargoTomlParser .parse(&member.content) .map(|m| m.items) .unwrap_or_default(); - let _ = resolve_workspace_inheritance(&mut items, &declarations); + let _ = resolve_workspace_inheritance(&mut items, &scope.declarations); let mut deps: Vec = Vec::new(); for item in &items { deps.push(item.name.clone()); @@ -305,8 +440,8 @@ fn shallow_graph( continue; } // Items are inheritance-resolved by now, so a member's - // `dep.workspace = true` pointing at a root `= "1.0.200"` is read here - // as the pin the root declared. + // `dep.workspace = true` pointing at its own root's `= "1.0.200"` is read + // here as the pin that root declared. let pin = declared_pin(item, Ecosystem::Rust).map(str::to_owned); match external_seen.get(&item.name) { // One node for the name, so a version survives only where every @@ -348,15 +483,7 @@ fn shallow_graph( let version = parse_project(ManifestKind::CargoToml, &member.content) .version .as_ref() - .and_then(|field| { - if member.governed_by_root { - field.resolve(&package_defaults, "version") - } else { - // Not this root's crate to resolve — take only what its own - // manifest states outright. - field.literal() - } - }) + .and_then(|field| field.resolve(&scope.package_defaults, "version")) .map(str::to_owned); member_pkgs.push(LockedPackage::new(member.name.clone(), version, None, deps)); } diff --git a/crates/dependable-fetch/tests/tree.rs b/crates/dependable-fetch/tests/tree.rs index 52e416c..01b6f6c 100644 --- a/crates/dependable-fetch/tests/tree.rs +++ b/crates/dependable-fetch/tests/tree.rs @@ -326,10 +326,12 @@ version.workspace = true /// `examples/` tree with its own `[workspace]` — because Cargo already ignores such /// a subtree, so nobody lists it in `[workspace] exclude`. The outer root has no /// authority over those crates, so its `[workspace.package] version` must not be -/// handed to them: that would turn "no version" into a confidently wrong one. +/// handed to them. The nested root's own table is the one that governs them, and it +/// is one directory up from the crate: reading it is what turns "no version" into +/// the right one rather than a confidently wrong one. /// A version the nested crate states outright is still its own, and still reported. #[test] -fn a_nested_independent_workspace_does_not_inherit_the_outer_roots_version() { +fn a_nested_independent_workspaces_crate_resolves_against_its_own_root() { let tmp = TempDir::new().unwrap(); let dir = tmp.path(); fs::write( @@ -386,26 +388,424 @@ version = "7.7.7" let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); assert_eq!(built.source, GraphSource::Manifests); - let version_of = |name: &str| { - built - .graph - .nodes() - .iter() - .find(|n| n.name == name) - .unwrap_or_else(|| panic!("a node for {name}")) - .version - .clone() + let g = &built.graph; + assert_eq!(version_of(g, "a"), Some("1.0.0")); + assert_eq!( + version_of(g, "a-fuzz"), + Some("0.0.0"), + "a nested workspace's crate inherits from the nested root, never the outer one" + ); + assert_eq!( + version_of(g, "a-fuzz-stated"), + Some("7.7.7"), + "and a version the crate states outright is still its own" + ); +} + +/// The guard on the fix above: a nested root that declares a `[workspace]` and no +/// `[workspace.package]` resolves its members' `version.workspace = true` to +/// nothing. Reaching past it to the outer root's `1.0.0` would be the bug the +/// scope stack exists to prevent, dressed up as a fallback. +#[test] +fn a_nested_root_declaring_no_version_leaves_its_crate_without_one() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" +members = ["crates/a"] + +[workspace.package] +version = "1.0.0" +"#, + ) + .unwrap(); + let member = dir.join("crates").join("a"); + fs::create_dir_all(&member).unwrap(); + fs::write( + member.join("Cargo.toml"), + r#" +[package] +name = "a" +version.workspace = true +"#, + ) + .unwrap(); + let silent = dir.join("silent"); + fs::create_dir_all(&silent).unwrap(); + fs::write( + silent.join("Cargo.toml"), + r#" +[workspace] + +[package] +name = "silent" +version.workspace = true +"#, + ) + .unwrap(); + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + let g = &built.graph; + assert_eq!(version_of(g, "a"), Some("1.0.0")); + assert_eq!( + version_of(g, "silent"), + None, + "its own root declares no version, and the outer root's is not a fallback" + ); +} + +/// Workspaces nest more than one deep — a `fuzz/` tree that itself vendors an +/// example workspace. The governing root is the *nearest* `[workspace]` ancestor, +/// so the innermost one wins, for the crate that declares it and for everything +/// below it. +#[test] +fn the_innermost_workspace_root_governs_when_workspaces_nest_twice() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" + +[workspace.package] +version = "1.0.0" +"#, + ) + .unwrap(); + let outer_member = dir.join("crates").join("a"); + fs::create_dir_all(&outer_member).unwrap(); + fs::write( + outer_member.join("Cargo.toml"), + r#" +[package] +name = "a" +version.workspace = true +"#, + ) + .unwrap(); + let mid = dir.join("mid"); + fs::create_dir_all(&mid).unwrap(); + fs::write( + mid.join("Cargo.toml"), + r#" +[workspace] + +[workspace.package] +version = "2.0.0" + +[package] +name = "mid" +version.workspace = true +"#, + ) + .unwrap(); + let inner = mid.join("inner"); + fs::create_dir_all(&inner).unwrap(); + fs::write( + inner.join("Cargo.toml"), + r#" +[workspace] + +[workspace.package] +version = "3.0.0" + +[package] +name = "inner" +version.workspace = true +"#, + ) + .unwrap(); + let leaf = inner.join("crates").join("leaf"); + fs::create_dir_all(&leaf).unwrap(); + fs::write( + leaf.join("Cargo.toml"), + r#" +[package] +name = "leaf" +version.workspace = true +"#, + ) + .unwrap(); + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + let g = &built.graph; + assert_eq!(version_of(g, "a"), Some("1.0.0")); + // A manifest that is both a `[workspace]` and a `[package]` resolves against + // itself, not against the root that contains it. + assert_eq!(version_of(g, "mid"), Some("2.0.0")); + assert_eq!(version_of(g, "inner"), Some("3.0.0")); + assert_eq!( + version_of(g, "leaf"), + Some("3.0.0"), + "the nearest [workspace] ancestor governs, not the outermost" + ); +} + +/// `[workspace.dependencies]` is the second thing a root lends its members, and it +/// decides what a `dep.workspace = true` crate *is*. It is scoped exactly like +/// `[workspace.package]`: a nested workspace's crate takes its own root's +/// declaration, so a name that root vendors by path is a path crate, even though +/// the outer root declares the same shape of entry as a registry one. +/// +/// The two names are deliberately different: one node carries one name, so a name +/// declared on both sides could not distinguish which root was consulted. +#[test] +fn a_nested_workspaces_crate_inherits_dependencies_from_its_own_root() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" + +[workspace.dependencies] +outer-dep = "1" +nested-dep = "1" +"#, + ) + .unwrap(); + let outer_member = dir.join("crates").join("outer"); + fs::create_dir_all(&outer_member).unwrap(); + fs::write( + outer_member.join("Cargo.toml"), + r#" +[package] +name = "outer" +version = "0.1.0" + +[dependencies] +outer-dep.workspace = true +"#, + ) + .unwrap(); + let nested = dir.join("nested"); + fs::create_dir_all(&nested).unwrap(); + fs::write( + nested.join("Cargo.toml"), + r#" +[workspace] + +[workspace.dependencies] +nested-dep = { path = "../vendor/nested-dep" } + +[package] +name = "nested" +version = "0.1.0" + +[dependencies] +nested-dep.workspace = true +"#, + ) + .unwrap(); + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + let g = &built.graph; + assert_eq!(kind_of(g, "outer-dep"), NodeKind::Registry); + // The outer root declares `nested-dep = "1"` too, and consulting it would make + // this a registry crate. The nested root is the one with authority here. + assert_eq!( + kind_of(g, "nested-dep"), + NodeKind::Path, + "a nested workspace's crate inherits from the nested root's declarations" + ); +} + +/// Two crates can share a `[package] name` across a nested-workspace boundary, and +/// the graph keys nodes by name, so only one of them survives. The outer one does — +/// it is the crate the scan is actually about — and it does so whichever of the two +/// the directory walk reaches first, since a version that depends on filesystem +/// iteration order is not a resolution of anything. +#[test] +fn a_name_shared_across_a_nested_boundary_keeps_the_outer_crates_version() { + // The same two crates twice, with the directories named so the walk reaches the + // outer crate first in one layout and the nested crate first in the other. + let built_with = |outer_dir: &str, nested_dir: &str| { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" + +[workspace.package] +version = "1.0.0" +"#, + ) + .unwrap(); + let outer_member = dir.join(outer_dir); + fs::create_dir_all(&outer_member).unwrap(); + fs::write( + outer_member.join("Cargo.toml"), + r#" +[package] +name = "dup" +version.workspace = true +"#, + ) + .unwrap(); + let nested = dir.join(nested_dir); + fs::create_dir_all(&nested).unwrap(); + fs::write( + nested.join("Cargo.toml"), + r#" +[workspace] + +[workspace.package] +version = "9.9.9" +"#, + ) + .unwrap(); + let nested_member = nested.join("dup"); + fs::create_dir_all(&nested_member).unwrap(); + fs::write( + nested_member.join("Cargo.toml"), + r#" +[package] +name = "dup" +version.workspace = true +"#, + ) + .unwrap(); + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + version_of(&built.graph, "dup").map(str::to_owned) }; - assert_eq!(version_of("a").as_deref(), Some("1.0.0")); + + assert_eq!(built_with("aaa", "zzz").as_deref(), Some("1.0.0")); assert_eq!( - version_of("a-fuzz"), + built_with("zzz", "aaa").as_deref(), + Some("1.0.0"), + "and still the outer crate when the walk meets the nested one first" + ); +} + +/// Two crates in the *same* scope can share a `[package] name` too — `crates/dup` +/// and `examples/dup` in one workspace — and there no scope comparison can pick a +/// winner: both index the same root. First-wins then decides, and what "first" +/// means is the walk's own sorted descent, not whatever order `read_dir` happened +/// to return. Without that sort this answer would vary between two machines +/// holding byte-identical checkouts, which is what makes the sort load-bearing +/// rather than tidy. +#[test] +fn a_name_shared_within_one_scope_settles_on_the_alphabetically_earlier_path() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" +members = ["aaa", "zzz"] +"#, + ) + .unwrap(); + // Same name, same (root) scope, different literal versions — so which manifest + // the walk records is visible in the graph. Created in reverse alphabetical + // order deliberately: a filesystem that reports entries in creation order then + // hands back the *wrong* crate first, so the assertion below is answered by the + // walk's sort rather than by the directory happening to agree with it. + for (subdir, version) in [("zzz", "9.9.9"), ("aaa", "1.1.1")] { + let member = dir.join(subdir); + fs::create_dir_all(&member).unwrap(); + fs::write( + member.join("Cargo.toml"), + format!("[package]\nname = \"dup\"\nversion = \"{version}\"\n"), + ) + .unwrap(); + } + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + assert_eq!( + version_of(&built.graph, "dup"), + Some("1.1.1"), + "the sorted walk reaches `aaa` first, on every filesystem" + ); +} + +/// The scope stack's guarantee has to hold for a nested root that cannot be read at +/// all. A `Cargo.toml` with a TOML syntax error yields no `[workspace]` table — but +/// "it does not parse" is not "it is not a workspace", and treating the two alike +/// would hand every crate beneath it to the outer root, which is precisely the +/// confidently-wrong number the scope stack exists to prevent. An unusable manifest +/// is an opaque boundary: nothing is inherited across it, in either direction. +#[test] +fn a_crate_under_an_unparseable_nested_root_inherits_nothing() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path(); + fs::write( + dir.join("Cargo.toml"), + r#" +[workspace] +resolver = "2" +members = ["crates/a"] + +[workspace.package] +version = "1.0.0" + +[workspace.dependencies] +shared-dep = "1" +"#, + ) + .unwrap(); + let member = dir.join("crates").join("a"); + fs::create_dir_all(&member).unwrap(); + fs::write( + member.join("Cargo.toml"), + r#" +[package] +name = "a" +version.workspace = true +"#, + ) + .unwrap(); + // An unterminated table header: the file exists, and no reader can say whether + // it declares a `[workspace]`. + let broken = dir.join("fuzz"); + fs::create_dir_all(&broken).unwrap(); + fs::write(broken.join("Cargo.toml"), "[workspace\n").unwrap(); + let below = broken.join("crates").join("target-crate"); + fs::create_dir_all(&below).unwrap(); + fs::write( + below.join("Cargo.toml"), + r#" +[package] +name = "target-crate" +version.workspace = true + +[dependencies] +shared-dep.workspace = true +"#, + ) + .unwrap(); + + let built = build_workspace_graph(dir, &WorkspaceGraphOptions::default()).unwrap(); + assert_eq!(built.source, GraphSource::Manifests); + let g = &built.graph; + assert_eq!( + version_of(g, "a"), + Some("1.0.0"), + "the outer root still governs its own members" + ); + assert_eq!( + version_of(g, "target-crate"), None, - "the outer root does not govern a nested workspace's crate" + "a crate under an unreadable root inherits no version, least of all the outer root's 1.0.0" ); assert_eq!( - version_of("a-fuzz-stated").as_deref(), - Some("7.7.7"), - "but a version the crate states outright is still its own" + version_of(g, "shared-dep"), + None, + "and no dependency constraint either — the outer `[workspace.dependencies]` does not reach across" ); }