Skip to content

Commit

Permalink
Auto merge of #14004 - epage:deterministic, r=weihanglo
Browse files Browse the repository at this point in the history
fix(vendor): Ensure sort happens for vendor

### What does this PR try to resolve?

This is a follow up to #13989 which sorted in time for `cargo package` and `cargo publish` but too late for `cargo vendor`.  This moves the sorting earlier and puts it in a place that will only run when resolving.

Fixes #13988

### How should we test and review this PR?

I had assumed tests existed but just happened to write in a way that leads to sorting.  Apparently not. I've added some tests.

The `cargo vendor` test has to elide paths because `/`s are normalized during publish, rather than resolve, so the output varies across platforms, even with cargo-test-support normalizing slashes because `toml` will switch between `"` and `'`.

### Additional information
  • Loading branch information
bors committed Jun 3, 2024
2 parents 8114c9a + 150461c commit 7034e2a
Show file tree
Hide file tree
Showing 5 changed files with 261 additions and 10 deletions.
5 changes: 0 additions & 5 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2777,11 +2777,6 @@ fn prepare_targets_for_publish(
};
prepared.push(target);
}
// Ensure target order is deterministic, particularly for `cargo vendor` where re-vendoring
// shuld not cause changes.
//
// `unstable` should be deterministic because we enforce that `t.name` is unique
prepared.sort_unstable_by_key(|t| t.name.clone());

if prepared.is_empty() {
Ok(None)
Expand Down
10 changes: 8 additions & 2 deletions src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ fn toml_targets_and_inferred(
autodiscover_flag_name: &str,
) -> Vec<TomlTarget> {
let inferred_targets = inferred_to_toml_targets(inferred);
match toml_targets {
let mut toml_targets = match toml_targets {
None => {
if let Some(false) = autodiscover {
vec![]
Expand Down Expand Up @@ -819,7 +819,13 @@ https://github.com/rust-lang/cargo/issues/5330",

targets
}
}
};
// Ensure target order is deterministic, particularly for `cargo vendor` where re-vendoring
// should not cause changes.
//
// `unstable` should be deterministic because we enforce that `t.name` is unique
toml_targets.sort_unstable_by_key(|t| t.name.clone());
toml_targets
}

fn inferred_to_toml_targets(inferred: &[(String, PathBuf)]) -> Vec<TomlTarget> {
Expand Down
127 changes: 125 additions & 2 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3351,8 +3351,8 @@ See [..]
[WARNING] ignoring `package.build` as `build.rs` is not included in the published package
[WARNING] ignoring binary `foo` as `src/main.rs` is not included in the published package
[WARNING] ignoring example `ExampleFoo` as `examples/ExampleFoo.rs` is not included in the published package
[WARNING] ignoring test `explicitpath` as `tests/explicitpath.rs` is not included in the published package
[WARNING] ignoring test `ExplicitPath` as `tests/ExplicitPath.rs` is not included in the published package
[WARNING] ignoring test `explicitpath` as `tests/explicitpath.rs` is not included in the published package
[VERIFYING] foo v0.0.1 ([CWD])
[COMPILING] foo v0.0.1 ([CWD][..])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
Expand Down Expand Up @@ -3384,8 +3384,8 @@ See [..]
[WARNING] ignoring `package.build` as `build.rs` is not included in the published package
[WARNING] ignoring binary `foo` as `src/main.rs` is not included in the published package
[WARNING] ignoring example `ExampleFoo` as `examples/ExampleFoo.rs` is not included in the published package
[WARNING] ignoring test `explicitpath` as `tests/explicitpath.rs` is not included in the published package
[WARNING] ignoring test `ExplicitPath` as `tests/ExplicitPath.rs` is not included in the published package
[WARNING] ignoring test `explicitpath` as `tests/explicitpath.rs` is not included in the published package
[VERIFYING] foo v0.0.1 ([CWD])
[COMPILING] foo v0.0.1 ([CWD][..])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
Expand Down Expand Up @@ -4928,3 +4928,126 @@ path = "src/lib.rs"
)],
);
}

#[cargo_test]
fn deterministic_build_targets() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
edition = "2021"
license = "MIT"
description = "foo"
documentation = "docs.rs/foo"
authors = []
[[example]]
name = "c"
[[example]]
name = "b"
[[example]]
name = "a"
"#,
)
.file("src/lib.rs", "")
.file("examples/z.rs", "fn main() {}")
.file("examples/y.rs", "fn main() {}")
.file("examples/x.rs", "fn main() {}")
.file("examples/c.rs", "fn main() {}")
.file("examples/b.rs", "fn main() {}")
.file("examples/a.rs", "fn main() {}")
.build();

p.cargo("package")
.with_stdout("")
.with_stderr(
"\
[PACKAGING] foo v0.0.1 ([CWD])
[VERIFYING] foo v0.0.1 ([CWD])
[COMPILING] foo v0.0.1 ([CWD][..])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
[PACKAGED] 10 files, [..] ([..] compressed)
",
)
.run();

let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
validate_crate_contents(
f,
"foo-0.0.1.crate",
&[
"Cargo.lock",
"Cargo.toml",
"Cargo.toml.orig",
"src/lib.rs",
"examples/a.rs",
"examples/b.rs",
"examples/c.rs",
"examples/x.rs",
"examples/y.rs",
"examples/z.rs",
],
&[(
"Cargo.toml",
r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# "normalize" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g., crates.io) dependencies.
#
# If you are reading this file be aware that the original Cargo.toml
# will likely look very different (and much more reasonable).
# See Cargo.toml.orig for the original contents.
[package]
edition = "2021"
name = "foo"
version = "0.0.1"
authors = []
build = false
autobins = false
autoexamples = false
autotests = false
autobenches = false
description = "foo"
documentation = "docs.rs/foo"
readme = false
license = "MIT"
[lib]
name = "foo"
path = "src/lib.rs"
[[example]]
name = "a"
path = "examples/a.rs"
[[example]]
name = "b"
path = "examples/b.rs"
[[example]]
name = "c"
path = "examples/c.rs"
[[example]]
name = "x"
path = "examples/x.rs"
[[example]]
name = "y"
path = "examples/y.rs"
[[example]]
name = "z"
path = "examples/z.rs"
"#,
)],
);
}
2 changes: 1 addition & 1 deletion tests/testsuite/required_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1474,12 +1474,12 @@ fn truncated_install_warning_message() {
[FINISHED] `release` profile [optimized] target(s) in [..]
[WARNING] none of the package's binaries are available for install using the selected features
bin \"foo1\" requires the features: `feature1`, `feature2`, `feature3`
bin \"foo10\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5`
bin \"foo2\" requires the features: `feature2`
bin \"foo3\" requires the features: `feature3`
bin \"foo4\" requires the features: `feature4`, `feature1`
bin \"foo5\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5`
bin \"foo6\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5`
bin \"foo7\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5`
4 more targets also requires features not enabled. See them in the Cargo.toml file.
Consider enabling some of the needed features by passing, e.g., `--features=\"feature1 feature2 feature3\"`").run();
}
127 changes: 127 additions & 0 deletions tests/testsuite/vendor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
use std::fs;

use cargo_test_support::compare::assert_e2e;
use cargo_test_support::git;
use cargo_test_support::registry::{self, Package, RegistryBuilder};
use cargo_test_support::str;
use cargo_test_support::{basic_lib_manifest, basic_manifest, paths, project, Project};

#[cargo_test]
Expand Down Expand Up @@ -862,6 +864,131 @@ fn git_complex() {
.run();
}

#[cargo_test]
fn git_deterministic() {
let git_dep = git::new("git_dep", |p| {
p.file(
"Cargo.toml",
r#"
[package]
name = "git_dep"
version = "0.0.1"
edition = "2021"
license = "MIT"
description = "foo"
documentation = "docs.rs/foo"
authors = []
[[example]]
name = "c"
[[example]]
name = "b"
[[example]]
name = "a"
"#,
)
.file("src/lib.rs", "")
.file("examples/z.rs", "fn main() {}")
.file("examples/y.rs", "fn main() {}")
.file("examples/x.rs", "fn main() {}")
.file("examples/c.rs", "fn main() {}")
.file("examples/b.rs", "fn main() {}")
.file("examples/a.rs", "fn main() {}")
});

let p = project()
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
git_dep = {{ git = '{}' }}
"#,
git_dep.url()
),
)
.file("src/lib.rs", "")
.build();

let output = p
.cargo("vendor --respect-source-config")
.exec_with_output()
.unwrap();
let output = String::from_utf8(output.stdout).unwrap();
p.change_file(".cargo/config.toml", &output);

let git_dep_manifest = p.read_file("vendor/git_dep/Cargo.toml");
assert_e2e().eq(
git_dep_manifest,
str![[r##"
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
# "normalize" Cargo.toml files for maximal compatibility
# with all versions of Cargo and also rewrite `path` dependencies
# to registry (e.g., crates.io) dependencies.
#
# If you are reading this file be aware that the original Cargo.toml
# will likely look very different (and much more reasonable).
# See Cargo.toml.orig for the original contents.
bin = []
test = []
bench = []
[package]
edition = "2021"
name = "git_dep"
version = "0.0.1"
authors = []
build = false
autobins = false
autoexamples = false
autotests = false
autobenches = false
description = "foo"
documentation = "docs.rs/foo"
readme = false
license = "MIT"
[lib]
name = "git_dep"
path = [..]
[[example]]
name = "a"
path = [..]
[[example]]
name = "b"
path = [..]
[[example]]
name = "c"
path = [..]
[[example]]
name = "x"
path = [..]
[[example]]
name = "y"
path = [..]
[[example]]
name = "z"
path = [..]
"##]],
);
}

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

0 comments on commit 7034e2a

Please sign in to comment.