Skip to content

Commit

Permalink
Auto merge of #10448 - aeikum:fix_vendor_sync, r=weihanglo
Browse files Browse the repository at this point in the history
vendor: Don't allow multiple values for --sync

The --sync argument to cargo vendor currently takes a list, which makes it easy for it to eat the final path argument:

````
cargo vendor --manifest-path foo/Cargo.toml -s bar/Cargo.toml ./test_vendor/
error: failed to read ./test_vendor/

Caused by:
  No such file or directory (os error 2)
````

Per discussion on #10441, this behavior is undesirable and hopefully used infrequently enough that we can change the UI for it. This patch will now only allow one value per --sync argument.

I didn't regenerate other doc files as it's not clear to me how/when that should be done.
  • Loading branch information
bors committed Mar 12, 2022
2 parents f955ddc + d41f5cc commit b816d82
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 10 deletions.
3 changes: 1 addition & 2 deletions src/bin/cargo/commands/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ pub fn cli() -> App {
.help("Additional `Cargo.toml` to sync and vendor")
.value_name("TOML")
.allow_invalid_utf8(true)
.multiple_occurrences(true)
.multiple_values(true),
.multiple_occurrences(true),
)
.arg(
Arg::new("respect-source-config")
Expand Down
4 changes: 2 additions & 2 deletions src/doc/man/cargo-vendor.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ to use the vendored sources, which you will need to add to `.cargo/config.toml`.
{{#options}}

{{#option "`-s` _manifest_" "`--sync` _manifest_" }}
Specify extra `Cargo.toml` manifests to workspaces which should also be
vendored and synced to the output.
Specify an extra `Cargo.toml` manifest to workspaces which should also be
vendored and synced to the output. May be specified multiple times.
{{/option}}

{{#option "`--no-delete`" }}
Expand Down
5 changes: 3 additions & 2 deletions src/doc/man/generated_txt/cargo-vendor.txt
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ DESCRIPTION
OPTIONS
Vendor Options
-s manifest, --sync manifest
Specify extra Cargo.toml manifests to workspaces which should also
be vendored and synced to the output.
Specify an extra Cargo.toml manifest to workspaces which should also
be vendored and synced to the output. May be specified multiple
times.

--no-delete
Don't delete the "vendor" directory when vendoring, but rather keep
Expand Down
4 changes: 2 additions & 2 deletions src/doc/src/commands/cargo-vendor.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ to use the vendored sources, which you will need to add to `.cargo/config.toml`.

<dt class="option-term" id="option-cargo-vendor--s"><a class="option-anchor" href="#option-cargo-vendor--s"></a><code>-s</code> <em>manifest</em></dt>
<dt class="option-term" id="option-cargo-vendor---sync"><a class="option-anchor" href="#option-cargo-vendor---sync"></a><code>--sync</code> <em>manifest</em></dt>
<dd class="option-desc">Specify extra <code>Cargo.toml</code> manifests to workspaces which should also be
vendored and synced to the output.</dd>
<dd class="option-desc">Specify an extra <code>Cargo.toml</code> manifest to workspaces which should also be
vendored and synced to the output. May be specified multiple times.</dd>


<dt class="option-term" id="option-cargo-vendor---no-delete"><a class="option-anchor" href="#option-cargo-vendor---no-delete"></a><code>--no-delete</code></dt>
Expand Down
4 changes: 2 additions & 2 deletions src/etc/man/cargo-vendor.1
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ to use the vendored sources, which you will need to add to \fB\&.cargo/config.to
\fB\-s\fR \fImanifest\fR,
\fB\-\-sync\fR \fImanifest\fR
.RS 4
Specify extra \fBCargo.toml\fR manifests to workspaces which should also be
vendored and synced to the output.
Specify an extra \fBCargo.toml\fR manifest to workspaces which should also be
vendored and synced to the output. May be specified multiple times.
.RE
.sp
\fB\-\-no\-delete\fR
Expand Down
66 changes: 66 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,72 @@ fn two_lockfiles() {
p.cargo("build").cwd("bar").run();
}

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

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

p.cargo("vendor --respect-source-config --manifest-path foo/Cargo.toml -s bar/Cargo.toml baz/Cargo.toml test_vendor")
.with_stderr("\
error: Found argument 'test_vendor' which wasn't expected, or isn't valid in this context
USAGE:
cargo[EXE] vendor [OPTIONS] [path]
For more information try --help",
)
.with_status(1)
.run();

p.cargo("vendor --respect-source-config --manifest-path foo/Cargo.toml -s bar/Cargo.toml -s baz/Cargo.toml test_vendor")
.run();

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

#[cargo_test]
fn delete_old_crates() {
let p = project()
Expand Down

0 comments on commit b816d82

Please sign in to comment.