Skip to content

Commit

Permalink
Auto merge of #7631 - jsgf:explicit-version, r=alexcrichton
Browse files Browse the repository at this point in the history
vendor: implement --versioned-dirs

Implement `--explicit-version` from standalone cargo-vendor. This helps with vendoring
performance as it avoids redundantly deleting and re-copying already vendored packages.

For example, re-vendoring cargo's dependencies it makes a big difference in wallclock
time. For initial vendoring it makes no difference, but re-vendoring (ie, when most or all dependencies haven't changed) without explicit versions is actually slightly slower
(5.8s -> 6s), but with explicit versions it goes from 5.8s -> 1.6s.

Timings:

Without explicit versions, initial vendor
real	0m5.810s
user	0m0.924s
sys	0m2.491s

Re-vendor:
real	0m6.083s
user	0m0.937s
sys	0m2.654s

With explicit versions, initial vendor:
real	0m5.810s
user	0m0.937s
sys	0m2.461s

Re-vendor:
real	0m1.567s
user	0m0.578s
sys	0m0.967s

Its interesting to look at the syscall summary:

Without explicit versions:
```
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 25.17    1.104699          18     59432      1065 openat
 19.86    0.871574          21     41156     13825 unlink
 13.64    0.598739           2    210510           lstat
  9.02    0.395948          29     13208           copy_file_range
  8.00    0.351242          11     30245           read
  6.36    0.279005           3     72487      4476 statx
  5.35    0.235027           6     37219           write
  4.02    0.176267           3     58368           close
```
with explicit versions:
```
 29.38    0.419068          15     27798     13825 unlink
 25.52    0.364021           1    209586           lstat
 20.67    0.294788          16     17967      1032 openat
 10.42    0.148586           4     35646           write
  3.53    0.050350           3     13825           chmod
  3.14    0.044786           2     16701      1622 statx
  2.19    0.031171           1     16936           close
  1.86    0.026538          24      1078           rmdir
```

Specifically, there are a lot fewer opens, copy_file_ranges, and unlinks.
  • Loading branch information
bors committed Dec 16, 2019
2 parents 19a0de2 + 2214cf1 commit 414ec70
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 11 deletions.
14 changes: 6 additions & 8 deletions src/bin/cargo/commands/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ pub fn cli() -> App {
.help("Respect `[source]` config in `.cargo/config`")
.multiple(true),
)
.arg(
Arg::with_name("versioned-dirs")
.long("versioned-dirs")
.help("Always include version in subdir name"),
)
.arg(
Arg::with_name("no-merge-sources")
.long("no-merge-sources")
Expand All @@ -42,12 +47,6 @@ pub fn cli() -> App {
.long("only-git-deps")
.hidden(true),
)
.arg(
Arg::with_name("explicit-version")
.short("-x")
.long("explicit-version")
.hidden(true),
)
.arg(
Arg::with_name("disallow-duplicates")
.long("disallow-duplicates")
Expand Down Expand Up @@ -85,8 +84,6 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
Some("--relative-path")
} else if args.is_present("only-git-deps") {
Some("--only-git-deps")
} else if args.is_present("explicit-version") {
Some("--explicit-version")
} else if args.is_present("disallow-duplicates") {
Some("--disallow-duplicates")
} else {
Expand Down Expand Up @@ -116,6 +113,7 @@ https://github.com/rust-lang/cargo/issues/new
&ops::VendorOptions {
no_delete: args.is_present("no-delete"),
destination: &path,
versioned_dirs: args.is_present("versioned-dirs"),
extra: args
.values_of_os("tomls")
.unwrap_or_default()
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf};

pub struct VendorOptions<'a> {
pub no_delete: bool,
pub versioned_dirs: bool,
pub destination: &'a Path,
pub extra: Vec<PathBuf>,
}
Expand Down Expand Up @@ -186,7 +187,7 @@ fn sync(
.parent()
.expect("manifest_path should point to a file");
let max_version = *versions[&id.name()].iter().rev().next().unwrap().0;
let dir_has_version_suffix = id.version() != max_version;
let dir_has_version_suffix = opts.versioned_dirs || id.version() != max_version;
let dst_name = if dir_has_version_suffix {
// Eg vendor/futures-0.1.13
format!("{}-{}", id.name(), id.version())
Expand Down
7 changes: 7 additions & 0 deletions src/doc/man/cargo-vendor.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ to use the vendored sources, which you will need to add to `.cargo/config`.
Instead of ignoring `[source]` configuration by default in `.cargo/config`
read it and use it when downloading crates from crates.io, for example

*--versioned-dirs*::
Normally versions are only added to disambiguate multiple versions of the
same package. This option causes all directories in the "vendor" directory
to be versioned, which makes it easier to track the history of vendored
packages over time, and can help with the performance of re-vendoring when
only a subset of the packages have changed.

=== Manifest Options

include::options-manifest-path.adoc[]
Expand Down
8 changes: 8 additions & 0 deletions src/doc/man/generated/cargo-vendor.html
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ <h3 id="cargo_vendor_owner_options">Owner Options</h3>
<p>Instead of ignoring <code>[source]</code> configuration by default in <code>.cargo/config</code>
read it and use it when downloading crates from crates.io, for example</p>
</dd>
<dt class="hdlist1"><strong>--versioned-dirs</strong></dt>
<dd>
<p>Normally versions are only added to disambiguate multiple versions of the
same package. This option causes all directories in the "vendor" directory
to be versioned, which makes it easier to track the history of vendored
packages over time, and can help with the performance of re-vendoring when
only a subset of the packages have changed.</p>
</dd>
</dl>
</div>
</div>
Expand Down
13 changes: 11 additions & 2 deletions src/etc/man/cargo-vendor.1
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
.\" Title: cargo-vendor
.\" Author: [see the "AUTHOR(S)" section]
.\" Generator: Asciidoctor 2.0.10
.\" Date: 2019-09-08
.\" Date: 2019-12-09
.\" Manual: \ \&
.\" Source: \ \&
.\" Language: English
.\"
.TH "CARGO\-VENDOR" "1" "2019-09-08" "\ \&" "\ \&"
.TH "CARGO\-VENDOR" "1" "2019-12-09" "\ \&" "\ \&"
.ie \n(.g .ds Aq \(aq
.el .ds Aq '
.ss \n[.ss] 0
Expand Down Expand Up @@ -62,6 +62,15 @@ existing contents of the vendor directory
Instead of ignoring \fB[source]\fP configuration by default in \fB.cargo/config\fP
read it and use it when downloading crates from crates.io, for example
.RE
.sp
\fB\-\-versioned\-dirs\fP
.RS 4
Normally versions are only added to disambiguate multiple versions of the
same package. This option causes all directories in the "vendor" directory
to be versioned, which makes it easier to track the history of vendored
packages over time, and can help with the performance of re\-vendoring when
only a subset of the packages have changed.
.RE
.SS "Manifest Options"
.sp
\fB\-\-manifest\-path\fP \fIPATH\fP
Expand Down
45 changes: 45 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,51 @@ fn two_versions() {
p.cargo("build").run();
}

#[cargo_test]
fn two_explicit_versions() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bitflags = "0.8.0"
bar = { path = "bar" }
"#,
)
.file("src/lib.rs", "")
.file(
"bar/Cargo.toml",
r#"
[package]
name = "bar"
version = "0.1.0"
[dependencies]
bitflags = "0.7.0"
"#,
)
.file("bar/src/lib.rs", "")
.build();

Package::new("bitflags", "0.7.0").publish();
Package::new("bitflags", "0.8.0").publish();

p.cargo("vendor --respect-source-config --versioned-dirs")
.run();

let lock = p.read_file("vendor/bitflags-0.8.0/Cargo.toml");
assert!(lock.contains("version = \"0.8.0\""));
let lock = p.read_file("vendor/bitflags-0.7.0/Cargo.toml");
assert!(lock.contains("version = \"0.7.0\""));

add_vendor_config(&p);
p.cargo("build").run();
}

#[cargo_test]
fn help() {
let p = project().build();
Expand Down

0 comments on commit 414ec70

Please sign in to comment.