From 48c3c91468fc40293c20ea5489954a429392a1b9 Mon Sep 17 00:00:00 2001 From: Weihang Lo Date: Thu, 25 Jan 2024 15:44:01 -0500 Subject: [PATCH] feat(cargo-vendor): vendor path dep if it is not in any given workspaces Generally cargo don't vendor path dependencies. This seems quiet reasonable path dependencies are "local" comparing to git or registry dependencies, and usually under the user's control. However, it is not always the case. A workspace might contain * any `[patch]` to local path dependencies * a set of shared path dependencies outside the current workspace These use cases demonstrate that users might not have controls or permissions to those dependencies. When they want to create a reproducible tarball for their own workspace, `cargo vendor` is not a tool helping them achieve the goal. There is one workaround: Have a `[patch]` to a local git repository instead of a lcoal path dependency. This is not ergonomic and adds overhead of setting git repositories. This PR proposes that Cargo vendors path dependencies if they are not belong to any given workspaces. As a side effect, this exposes a new `[source]` kind `path`: ```toml [source."path+file:///path/to/package"] path = "/path/to/package" replace-with = "vendored-sources" ``` --- src/cargo/ops/vendor.rs | 27 +++++++++++++++++++++------ src/cargo/sources/config.rs | 6 ++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs index 4413786feb2..8a1e2253f9d 100644 --- a/src/cargo/ops/vendor.rs +++ b/src/cargo/ops/vendor.rs @@ -56,14 +56,13 @@ struct VendorConfig { } #[derive(Serialize)] -#[serde(rename_all = "lowercase", untagged)] +#[serde(rename_all = "kebab-case", rename_all_fields = "kebab-case", untagged)] enum VendorSource { Directory { directory: String, }, Registry { registry: Option, - #[serde(rename = "replace-with")] replace_with: String, }, Git { @@ -71,7 +70,10 @@ enum VendorSource { branch: Option, tag: Option, rev: Option, - #[serde(rename = "replace-with")] + replace_with: String, + }, + Path { + path: String, replace_with: String, }, } @@ -143,6 +145,10 @@ fn sync( // Next up let's actually download all crates and start storing internal // tables about them. + let members: BTreeSet<_> = workspaces + .iter() + .flat_map(|ws| ws.members().map(|p| p.package_id())) + .collect(); for ws in workspaces { let (packages, resolve) = ops::resolve_ws(ws).with_context(|| "failed to load pkg lockfile")?; @@ -152,9 +158,8 @@ fn sync( .with_context(|| "failed to download packages")?; for pkg in resolve.iter() { - // No need to vendor path crates since they're already in the - // repository - if pkg.source_id().is_path() { + // Don't vendor path deps unless it is outside any of the given workspaces. + if pkg.source_id().is_path() && !members.contains(&pkg) { continue; } ids.insert( @@ -289,6 +294,16 @@ fn sync( rev, replace_with: merged_source_name.to_string(), } + } else if source_id.is_path() { + VendorSource::Path { + path: source_id + .url() + .to_file_path() + .unwrap() + .to_string_lossy() + .replace("\\", "/"), + replace_with: merged_source_name.to_string(), + } } else { panic!("Invalid source ID: {}", source_id) }; diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index a2e8051c326..c6f7ae8d6ca 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -33,6 +33,8 @@ pub struct SourceConfigMap<'gctx> { struct SourceConfigDef { /// Indicates this source should be replaced with another of the given name. replace_with: OptValue, + /// A path source. + path: Option, /// A directory source. directory: Option, /// A registry source. Value is a URL. @@ -249,6 +251,10 @@ restore the source replacement configuration to continue the build let path = directory.resolve_path(self.gctx); srcs.push(SourceId::for_directory(&path)?); } + if let Some(path) = def.path { + let path = path.resolve_path(self.config); + srcs.push(SourceId::for_path(&path)?); + } if let Some(git) = def.git { let url = url(&git, &format!("source.{}.git", name))?; let reference = match def.branch {