Skip to content

Commit 30a86aa

Browse files
loadingaliasloadingalias
committed
fix(unify): keep workspace member deps path-pinned
1 parent 63dd986 commit 30a86aa

File tree

2 files changed

+111
-23
lines changed

2 files changed

+111
-23
lines changed

‎src/cargo/unify_analyzer.rs‎

Lines changed: 47 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,27 @@ impl UnifyAnalyzer {
9797
PathBuf::from(dep_path)
9898
}
9999

100+
/// Normalize a workspace member directory path relative to the workspace root.
101+
fn normalize_workspace_member_path(&self, member_manifest_path: &Path) -> PathBuf {
102+
let member_dir = member_manifest_path.parent().unwrap_or(member_manifest_path);
103+
104+
let canonical_workspace = self
105+
.workspace_root
106+
.canonicalize()
107+
.unwrap_or_else(|_| self.workspace_root.clone());
108+
let canonical_member = member_dir.canonicalize().unwrap_or_else(|_| member_dir.to_path_buf());
109+
110+
if let Ok(relative) = canonical_member.strip_prefix(&canonical_workspace) {
111+
return relative.to_path_buf();
112+
}
113+
114+
if let Ok(relative) = member_dir.strip_prefix(&self.workspace_root) {
115+
return relative.to_path_buf();
116+
}
117+
118+
member_dir.to_path_buf()
119+
}
120+
100121
/// Analyze workspace and generate unification plan
101122
pub fn analyze(&self) -> RailResult<UnificationPlan> {
102123
let dep_count = self.manifests.all_dependencies().len();
@@ -107,18 +128,20 @@ impl UnifyAnalyzer {
107128
let mut duplicates_cleaned = Vec::with_capacity(8);
108129
let mut version_mismatches = Vec::with_capacity(8);
109130

110-
// Pre-compute workspace member names for reuse
111-
let workspace_member_names: FxHashSet<Arc<str>> = self
112-
.metadata
113-
.workspace_packages()
114-
.iter()
115-
.map(|pkg| Arc::from(pkg.name.as_str()))
116-
.collect();
131+
// Pre-compute workspace member metadata for reuse.
132+
let mut workspace_member_names: FxHashSet<Arc<str>> = FxHashSet::default();
133+
let mut workspace_member_paths: FxHashMap<Arc<str>, PathBuf> = FxHashMap::default();
117134

118-
// Build member_paths mapping from metadata
135+
// Build member_paths mapping from metadata (manifest file paths)
119136
for pkg in self.metadata.workspace_packages() {
137+
let member_name = Arc::from(pkg.name.as_str());
120138
let manifest_path = pkg.manifest_path.clone().into_std_path_buf();
121-
member_paths.insert(Arc::from(pkg.name.as_str()), manifest_path);
139+
workspace_member_names.insert(Arc::clone(&member_name));
140+
workspace_member_paths.insert(
141+
Arc::clone(&member_name),
142+
self.normalize_workspace_member_path(&manifest_path),
143+
);
144+
member_paths.insert(member_name, manifest_path);
122145
}
123146

124147
progress!("Analyzing {} dependencies...", self.manifests.all_dependencies().len());
@@ -328,20 +351,17 @@ impl UnifyAnalyzer {
328351
// Get users - use Arc<str> to share allocations
329352
let users: FxHashSet<Arc<str>> = usage_sites.iter().map(|u| Arc::clone(&u.used_by)).collect();
330353

331-
// Check include_paths config before processing path dependencies
332-
let dep_path: Option<PathBuf> = if self.config.include_paths {
333-
// Check if any usage has a path (path dependencies)
334-
// If so, check if the dep is a workspace member to include path in workspace.dependencies
354+
// Workspace members must always carry `path` so member-to-member deps stay local.
355+
// This prevents dual-resolution in fresh lockfiles (local member + crates.io package).
356+
let dep_path: Option<PathBuf> = if workspace_member_names.contains(&dep_key.name) {
357+
workspace_member_paths.get(&dep_key.name).cloned()
358+
} else if self.config.include_paths {
359+
// Include explicit path deps for non-member packages only when requested.
335360
usage_sites.iter().find_map(|u| {
336361
u.path.as_ref().and_then(|p| {
337-
if !workspace_member_names.iter().any(|m| &**m == &*dep_key.name) {
338-
return None;
339-
}
340-
// Normalize path relative to workspace root using helper
341-
match &u.manifest_path {
342-
Some(manifest_path) => Some(self.normalize_dep_path(manifest_path, p)),
343-
None => Some(PathBuf::from(p)), // No manifest_path - use as-is (shouldn't happen)
344-
}
362+
u.manifest_path
363+
.as_ref()
364+
.map(|manifest_path| self.normalize_dep_path(manifest_path, p))
345365
})
346366
})
347367
} else {
@@ -365,8 +385,12 @@ impl UnifyAnalyzer {
365385
existing_features.sort();
366386
computed.sort();
367387

368-
// Also check if existing has path but we found one now
369-
let path_differs = dep_path.is_some() && existing.path.is_none();
388+
// Also check if existing path is missing or differs from resolved path.
389+
let path_differs = match (&dep_path, &existing.path) {
390+
(Some(new_path), Some(existing_path)) => Path::new(existing_path) != new_path,
391+
(Some(_), None) => true,
392+
_ => false,
393+
};
370394

371395
existing_features != computed || existing.default_features != default_features || path_differs
372396
}

‎tests/integration/test_unify.rs‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,70 @@ root = "."
852852
Ok(())
853853
}
854854

855+
#[test]
856+
fn test_unify_workspace_member_version_deps_always_get_path() -> Result<()> {
857+
let workspace = TestWorkspace::new_named("tokio-member-path")?;
858+
859+
// Workspace member that other members depend on via VERSION (Tokio-style).
860+
workspace.add_crate("tokio", "1.0.0", &[])?;
861+
workspace.add_crate("tokio-stream", "0.1.0", &[("tokio", r#""1.0.0""#)])?;
862+
workspace.add_crate(
863+
"tokio-util",
864+
"0.1.0",
865+
&[("tokio", r#"{ version = "1.0.0", features = ["rt"] }"#)],
866+
)?;
867+
868+
// Simulate Tokio's patch strategy where member deps are declared by version and patched locally.
869+
let root_manifest = workspace.path.join("Cargo.toml");
870+
let mut root = std::fs::read_to_string(&root_manifest)?;
871+
root.push_str("\n[patch.crates-io]\ntokio = { path = \"crates/tokio\" }\n");
872+
std::fs::write(&root_manifest, root)?;
873+
874+
// Critical: even with include_paths disabled, workspace member deps must still carry a path.
875+
std::fs::write(
876+
workspace.path.join(".config/rail.toml"),
877+
r#"[workspace]
878+
root = "."
879+
880+
[unify]
881+
include_paths = false
882+
"#,
883+
)?;
884+
885+
workspace.commit("Add tokio-style member dependencies with crates-io patch")?;
886+
887+
let output = run_cargo_rail(&workspace.path, &["rail", "unify"])?;
888+
assert!(
889+
output.status.success(),
890+
"unify should succeed.\nstdout:\n{}\nstderr:\n{}",
891+
String::from_utf8_lossy(&output.stdout),
892+
String::from_utf8_lossy(&output.stderr)
893+
);
894+
895+
let workspace_toml = std::fs::read_to_string(workspace.path.join("Cargo.toml"))?;
896+
assert!(
897+
workspace_toml.contains("tokio = {") && workspace_toml.contains("path = \"crates/tokio\""),
898+
"Workspace member dependency must include path for local resolution.\nCargo.toml:\n{}",
899+
workspace_toml
900+
);
901+
902+
let stream_toml = std::fs::read_to_string(workspace.path.join("crates/tokio-stream/Cargo.toml"))?;
903+
assert!(
904+
stream_toml.contains("tokio = { workspace = true"),
905+
"tokio-stream should inherit tokio from workspace.\nCargo.toml:\n{}",
906+
stream_toml
907+
);
908+
909+
let util_toml = std::fs::read_to_string(workspace.path.join("crates/tokio-util/Cargo.toml"))?;
910+
assert!(
911+
util_toml.contains("tokio = { workspace = true"),
912+
"tokio-util should inherit tokio from workspace.\nCargo.toml:\n{}",
913+
util_toml
914+
);
915+
916+
Ok(())
917+
}
918+
855919
// Config Options: include, pin_transitives, include_renamed
856920

857921
/// Test include config forces specific dependencies to be included

0 commit comments

Comments
 (0)