From b942f16a3df877c44053abe372bbd088c6f783eb Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Fri, 10 Jul 2026 14:54:47 -0600 Subject: [PATCH 1/2] fix: Reject unsupported Cargo local packages --- crates/turborepo-repository/src/cargo.rs | 254 +++++++++++++++++- crates/turborepo/ARCHITECTURE.md | 28 +- .../turborepo/tests/cargo_workspace_test.rs | 57 ++++ 3 files changed, 324 insertions(+), 15 deletions(-) diff --git a/crates/turborepo-repository/src/cargo.rs b/crates/turborepo-repository/src/cargo.rs index 411dfcf19df73..56a89d8ef2198 100644 --- a/crates/turborepo-repository/src/cargo.rs +++ b/crates/turborepo-repository/src/cargo.rs @@ -96,6 +96,29 @@ pub enum Error { InvalidLockfile { stderr: String }, #[error("failed to validate Cargo.lock with `cargo metadata --locked`: {0}")] LockfileValidationSpawn(#[source] io::Error), + #[error( + "Cargo local package {name:?} at {manifest_path} is outside the repository and cannot be \ + cached, watched, or pruned safely. Move it into the repository and make it a workspace \ + member." + )] + OutsideRepositoryLocalPackage { name: String, manifest_path: String }, + #[error( + "Cargo local package {name:?} at {manifest_path} is not a workspace member and cannot be \ + hashed or pruned safely. Add it to `[workspace].members` and remove it from \ + `[workspace].exclude`." + )] + NonMemberLocalPackage { name: String, manifest_path: String }, + #[error( + "Cargo package {name:?} is defined in the root Cargo.toml, which Turborepo cannot model \ + as a package safely. Move it into a subdirectory and add it to `[workspace].members`." + )] + UnsupportedRootPackage { name: String }, + #[error("failed to resolve Cargo local package path {path}: {source}")] + LocalPackagePath { + path: String, + #[source] + source: turbopath::PathError, + }, #[error("failed to read workspace file: {0}")] WorkspaceFileRead(#[source] io::Error), #[error("failed to run `rustc -vV`: {0}")] @@ -191,9 +214,10 @@ pub fn external_closures( )?) } -/// Verify Cargo can resolve the workspace without changing Cargo.lock. +/// Verify Cargo can resolve the workspace without changing Cargo.lock and that +/// every resolved local package is an in-repository workspace member. /// Validation happens before task hashes and cache lookup, so artifacts are -/// always keyed by the dependency resolution Cargo will execute. +/// always keyed by sources Turborepo can hash, watch, and prune. pub fn validate_lockfile(repo_root: &AbsoluteSystemPath) -> Result<(), Error> { let lock_path = repo_root.join_component(CARGO_LOCK); match lock_path.read_to_string() { @@ -211,6 +235,7 @@ pub fn validate_lockfile(repo_root: &AbsoluteSystemPath) -> Result<(), Error> { "--format-version", "1", "--locked", + "--all-features", "--manifest-path", root_manifest_path.as_str(), ]) @@ -223,6 +248,55 @@ pub fn validate_lockfile(repo_root: &AbsoluteSystemPath) -> Result<(), Error> { }); } + let metadata: ResolvedMetadata = serde_json::from_slice(&output.stdout)?; + validate_resolved_local_packages(repo_root, metadata) +} + +fn validate_resolved_local_packages( + repo_root: &AbsoluteSystemPath, + metadata: ResolvedMetadata, +) -> Result<(), Error> { + let real_repo_root = repo_root + .to_realpath() + .map_err(|source| Error::LocalPackagePath { + path: repo_root.to_string(), + source, + })?; + let root_manifest_path = real_repo_root.join_component(CARGO_TOML); + for package in metadata.packages { + if package.source.is_some() { + continue; + } + let Some(manifest_path) = metadata_path(&package.manifest_path) else { + return Err(Error::OutsideRepositoryLocalPackage { + name: package.name, + manifest_path: package.manifest_path, + }); + }; + let real_manifest_path = + manifest_path + .to_realpath() + .map_err(|source| Error::LocalPackagePath { + path: package.manifest_path.clone(), + source, + })?; + if !real_repo_root.contains(&real_manifest_path) { + return Err(Error::OutsideRepositoryLocalPackage { + name: package.name, + manifest_path: package.manifest_path, + }); + } + if real_manifest_path == root_manifest_path { + return Err(Error::UnsupportedRootPackage { name: package.name }); + } + if !metadata.workspace_members.contains(&package.id) { + return Err(Error::NonMemberLocalPackage { + name: package.name, + manifest_path: package.manifest_path, + }); + } + } + Ok(()) } @@ -939,6 +1013,10 @@ impl Toolchain for CargoToolchain { let crates = workspace.crates; if crates.is_empty() { + if workspace.has_packages { + turborepo_rayon_compat::block_in_place(|| validate_lockfile(&self.repo_root)) + .map_err(|err| toolchain::Error::Failed(Box::new(err)))?; + } return Ok(Vec::new()); } @@ -1152,6 +1230,11 @@ pub struct DiscoveredWorkspace { /// without members don't demand a name for nothing. pub name: Option, pub crates: Vec, + /// Whether Cargo reported any workspace packages before Turborepo's + /// repository-boundary filtering. A workspace with packages that all get + /// filtered must still run full validation rather than be mistaken for a + /// memberless virtual workspace. + pub has_packages: bool, } /// Discover all Rust crates in the Cargo workspace rooted at `repo_root` by @@ -1172,6 +1255,7 @@ pub fn discover_crates(repo_root: &AbsoluteSystemPath) -> Result Result Result, } +/// The subset of full `cargo metadata --locked --all-features` output needed +/// to distinguish external packages, workspace members, and unsupported local +/// path packages. +#[derive(Debug, Deserialize)] +struct ResolvedMetadata { + packages: Vec, + workspace_members: HashSet, +} + +#[derive(Debug, Deserialize)] +struct ResolvedMetadataPackage { + id: String, + name: String, + source: Option, + manifest_path: String, +} + #[cfg(test)] mod test { - use turbopath::AbsoluteSystemPathBuf; + use turbopath::{AbsoluteSystemPathBuf, IntoUnix}; use super::*; @@ -1507,6 +1613,52 @@ mod test { std::fs::write(path.as_std_path(), contents).unwrap(); } + fn generate_lockfile(root: &AbsoluteSystemPath) { + let output = std::process::Command::new("cargo") + .arg("generate-lockfile") + .current_dir(root.as_std_path()) + .output() + .unwrap(); + assert!( + output.status.success(), + "failed to generate fixture lockfile: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + + fn write_local_dependency_workspace( + root: &AbsoluteSystemPathBuf, + dependency_table: &str, + exclude_local: bool, + ) { + let exclude = if exclude_local { + "exclude = [\"crates/local\"]\n" + } else { + "" + }; + write( + root, + &["Cargo.toml"], + &format!("[workspace]\nmembers = [\"crates/app\"]\n{exclude}resolver = \"2\"\n"), + ); + write( + root, + &["crates", "app", "Cargo.toml"], + &format!( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \ + \"2021\"\n\n{dependency_table}" + ), + ); + write(root, &["crates", "app", "src", "main.rs"], "fn main() {}\n"); + write( + root, + &["crates", "local", "Cargo.toml"], + "[package]\nname = \"local\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ); + write(root, &["crates", "local", "src", "lib.rs"], ""); + generate_lockfile(root); + } + /// Write a small workspace: `app` (bin) depends on `lib-a` (lib), plus a /// dev-dep cycle between `lib-a` and `lib-a-test-util`. fn write_fixture_workspace(root: &AbsoluteSystemPathBuf) { @@ -1587,6 +1739,100 @@ dependencies = ["lib-a"] assert!(matches!(error, Error::MissingLockfile)); } + #[test] + fn test_validate_lockfile_accepts_automatic_path_member() { + let (_tmp, root) = tempdir_root(); + write_local_dependency_workspace( + &root, + "[dependencies]\nlocal = { path = \"../local\" }\n", + false, + ); + + validate_lockfile(&root).unwrap(); + } + + #[test] + fn test_validate_lockfile_rejects_nonmember_path_dependency_kinds() { + for dependency_table in [ + "[dependencies]\nlocal = { path = \"../local\" }\n", + "[build-dependencies]\nlocal = { path = \"../local\" }\n", + "[dev-dependencies]\nlocal = { path = \"../local\" }\n", + "[target.'cfg(target_os = \"none\")'.dependencies]\nlocal = { path = \"../local\" }\n", + "[dependencies]\nlocal = { path = \"../local\", optional = true }\n", + ] { + let (_tmp, root) = tempdir_root(); + write_local_dependency_workspace(&root, dependency_table, true); + + let error = validate_lockfile(&root).unwrap_err(); + assert!( + matches!(error, Error::NonMemberLocalPackage { ref name, .. } if name == "local"), + "unexpected validation result for {dependency_table:?}: {error}" + ); + } + } + + #[test] + fn test_validate_lockfile_rejects_outside_repository_path_dependency() { + let (_tmp, root) = tempdir_root(); + let repo = root.join_component("repo"); + let outside = root.join_component("outside"); + write( + &repo, + &["Cargo.toml"], + "[workspace]\nmembers = [\"crates/app\"]\nresolver = \"2\"\n", + ); + write( + &repo, + &["crates", "app", "Cargo.toml"], + &format!( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\nedition = \ + \"2021\"\n\n[dependencies]\noutside = {{ path = '{}' }}\n", + outside.as_str().into_unix() + ), + ); + write( + &repo, + &["crates", "app", "src", "main.rs"], + "fn main() {}\n", + ); + write( + &outside, + &["Cargo.toml"], + "[package]\nname = \"outside\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ); + write(&outside, &["src", "lib.rs"], ""); + generate_lockfile(&repo); + + let error = validate_lockfile(&repo).unwrap_err(); + assert!( + matches!(error, Error::OutsideRepositoryLocalPackage { ref name, .. } if name == "outside"), + "unexpected validation result: {error}" + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn test_cargo_toolchain_rejects_root_package() { + let (_tmp, root) = tempdir_root(); + write( + &root, + &["Cargo.toml"], + "[package]\nname = \"root-package\"\nversion = \"0.1.0\"\nedition = \ + \"2021\"\n\n[workspace]\nmembers = []\nresolver = \"2\"\n", + ); + write(&root, &["src", "lib.rs"], ""); + generate_lockfile(&root); + + let error = CargoToolchain::new(root) + .discover_packages() + .await + .unwrap_err(); + assert!( + error.to_string().contains("root-package") + && error.to_string().contains("root Cargo.toml"), + "unexpected validation result: {error}" + ); + } + #[test] fn test_parse_rustc_identity_includes_host() { let identity = parse_rustc_identity( diff --git a/crates/turborepo/ARCHITECTURE.md b/crates/turborepo/ARCHITECTURE.md index 231ae360c4824..89c4b515f7fb7 100644 --- a/crates/turborepo/ARCHITECTURE.md +++ b/crates/turborepo/ARCHITECTURE.md @@ -177,11 +177,15 @@ whether anything changed; Cargo decides how and in what order to build.** (member globs, automatic path-dependency members, excludes, target-specific dependency tables, renames). Dev-dependency edges that would form a cycle are dropped (Cargo permits dev-dep cycles; crate edges must support - topological `^` ordering). Manifests outside the repository root are - skipped, crate names are validated, and a crate/JS package name collision - hard-errors. Crate path dependencies are synthesized as `workspace:*` - specifiers in the toolchain-neutral descriptor, so the existing dependency - splitter wires crate→crate edges. + topological `^` ordering). Crate names are validated, and a crate/JS package + name collision hard-errors. Crate path dependencies are synthesized as + `workspace:*` specifiers in the toolchain-neutral descriptor, so the existing + dependency splitter wires crate→crate edges. A second full `cargo metadata + --locked --all-features` pass validates resolution and every resolved local + package: automatic in-repository workspace members are supported, while + excluded/non-member, outside-repository, and root-manifest local packages + hard-error because Turborepo cannot hash, watch, or prune their sources + safely. - **Package shapes**: crates are classified via `CargoPackageKind`. *Entrypoints* (crates with `bin`/`cdylib`/`staticlib` targets) are the workspace's deliverables. *Libraries* exist in the package graph — so @@ -228,10 +232,11 @@ whether anything changed; Cargo decides how and in what order to build.** through hashed task arguments, `CARGO_BUILD_TARGET`, or repository Cargo configuration remain distinct. Failure to resolve the compiler identity is a hard error. Every non-empty Cargo workspace must have a current - `Cargo.lock`: discovery runs full `cargo metadata --locked` before hashing, - then computes per-crate closures. Missing, stale, unparsable, or incomplete - lockfiles are hard errors. Turborepo never creates or refreshes the source - lockfile; users do that explicitly with Cargo and commit the result. + `Cargo.lock`: discovery runs full `cargo metadata --locked --all-features` + before hashing, then computes per-crate closures. Missing, stale, unparsable, + or incomplete lockfiles are hard errors. Turborepo never creates or refreshes + the source lockfile; users do that explicitly with Cargo and commit the + result. - **Caching**: task caches store logs plus, for entrypoint builds, the deliverables: bins (`target/*/`) and cdylib/staticlib artifacts (`target/*/lib.{so,dylib,a}`, `.{dll,lib}` — all platform @@ -328,8 +333,9 @@ hint pointing at the flag. Released turbo versions hard-error on unknown End-to-end coverage lives in `crates/turborepo/tests/cargo_workspace_test.rs` against the `cargo_monorepo` fixture (a mixed npm + Cargo workspace): graph shape, execution, caching, deliverable restoration, cross-crate -invalidation, lockfile enforcement, uncached `run`/`dev` execution, and the -filter hint. `turbo query` serves Cargo packages through the same graph. +invalidation, lockfile enforcement, unsupported local-package rejection, +uncached `run`/`dev` execution, and the filter hint. `turbo query` serves Cargo +packages through the same graph. ### 3. Task Graph (`crates/turborepo-lib/src/engine/`) diff --git a/crates/turborepo/tests/cargo_workspace_test.rs b/crates/turborepo/tests/cargo_workspace_test.rs index 4f18296016ad7..215610fc3faee 100644 --- a/crates/turborepo/tests/cargo_workspace_test.rs +++ b/crates/turborepo/tests/cargo_workspace_test.rs @@ -124,6 +124,63 @@ fn test_cargo_workspace_rejects_stale_lockfile() { ); } +#[test] +fn test_cargo_workspace_rejects_excluded_path_dependency() { + let tempdir = tempfile::tempdir().unwrap(); + setup_cargo_monorepo(tempdir.path()); + + let root_manifest = tempdir.path().join("Cargo.toml"); + let contents = fs::read_to_string(&root_manifest).unwrap(); + fs::write( + &root_manifest, + contents.replace( + "resolver = \"2\"", + "exclude = [\"crates/local\"]\nresolver = \"2\"", + ), + ) + .unwrap(); + let app_manifest = tempdir.path().join("crates/app/Cargo.toml"); + let contents = fs::read_to_string(&app_manifest).unwrap(); + fs::write( + &app_manifest, + format!("{contents}local = {{ path = \"../local\" }}\n"), + ) + .unwrap(); + let local = tempdir.path().join("crates/local"); + fs::create_dir_all(local.join("src")).unwrap(); + fs::write( + local.join("Cargo.toml"), + "[package]\nname = \"local\"\nversion = \"0.1.0\"\nedition = \"2021\"\n", + ) + .unwrap(); + fs::write(local.join("src/lib.rs"), "pub fn local() {}\n").unwrap(); + let status = std::process::Command::new("cargo") + .arg("generate-lockfile") + .current_dir(tempdir.path()) + .status() + .expect("cargo generate-lockfile runs"); + assert!(status.success()); + + for args in [ + &["build", "--filter=app", "--dry-run=json"][..], + &["prune", "app"][..], + ] { + let output = run_turbo(tempdir.path(), args); + assert!(!output.status.success(), "unsupported path must fail"); + let combined = format!( + "{}{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + combined.contains("is not a workspace member") + && combined.contains("hashed") + && combined.contains("pruned safely"), + "expected actionable path dependency error: {combined}" + ); + } +} + #[test] fn test_cargo_build_executes_caches_and_restores() { let tempdir = tempfile::tempdir().unwrap(); From 8f8ba241844cbb8cbe23d6fd66b670f135f8a0bf Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Fri, 10 Jul 2026 15:08:01 -0600 Subject: [PATCH 2/2] test: Avoid diagnostic wrapping assumptions --- crates/turborepo/tests/cargo_workspace_test.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/turborepo/tests/cargo_workspace_test.rs b/crates/turborepo/tests/cargo_workspace_test.rs index 215610fc3faee..fcddf3fb0b553 100644 --- a/crates/turborepo/tests/cargo_workspace_test.rs +++ b/crates/turborepo/tests/cargo_workspace_test.rs @@ -173,9 +173,11 @@ fn test_cargo_workspace_rejects_excluded_path_dependency() { String::from_utf8_lossy(&output.stderr) ); assert!( - combined.contains("is not a workspace member") + combined.contains("local") + && combined.contains("workspace") + && combined.contains("member") && combined.contains("hashed") - && combined.contains("pruned safely"), + && combined.contains("pruned"), "expected actionable path dependency error: {combined}" ); }