Skip to content

Commit

Permalink
feat(cargo-vendor): vendor path dep if it is not in any given workspaces
Browse files Browse the repository at this point in the history
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"
```
  • Loading branch information
weihanglo committed Jan 25, 2024
1 parent b2e1d3b commit c939843
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 6 deletions.
27 changes: 21 additions & 6 deletions src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,24 @@ 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<String>,
#[serde(rename = "replace-with")]
replace_with: String,
},
Git {
git: String,
branch: Option<String>,
tag: Option<String>,
rev: Option<String>,
#[serde(rename = "replace-with")]
replace_with: String,
},
Path {
path: String,
replace_with: String,
},
}
Expand Down Expand Up @@ -147,6 +149,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")?;
Expand All @@ -156,9 +162,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(
Expand Down Expand Up @@ -293,6 +298,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)
};
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ pub struct SourceConfigMap<'cfg> {
struct SourceConfigDef {
/// Indicates this source should be replaced with another of the given name.
replace_with: OptValue<String>,
/// A path source.
path: Option<ConfigRelativePath>,
/// A directory source.
directory: Option<ConfigRelativePath>,
/// A registry source. Value is a URL.
Expand Down Expand Up @@ -249,6 +251,10 @@ restore the source replacement configuration to continue the build
let path = directory.resolve_path(self.config);
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 {
Expand Down

0 comments on commit c939843

Please sign in to comment.