Repository: rust-lang/cargo (rust)

Issue:
Non determinist vendoring of git dependencies
### Problem

The output of `cargo vendor` is not determinist for git dependencies. Depending on the machine where the command is executed, the `Cargo.toml` of the vendored dependencies can be different. All tests were done on various versions of Ubuntu but with the same version of cargo. The difference is only about order of arrays of tables in the `Cargo.toml`.

I made a minimal reproduction example in [this repo](https://github.com/stormshield-guillaumed/vendor-repro) for which I ran the vendor command on my computer. You can see in [this CI run](https://github.com/stormshield-guillaumed/vendor-repro/actions/runs/9318120254) that running `cargo vendor` results in a diff with master. Also, the diff is not the same between the 3 jobs, so it doesn't seems like a mistake on my part.

### Steps

_No response_

### Possible Solution(s)

I'm not very familiar with the cargo codebase, but after digging a bit, it seems to only happen with git dependencies because they are treated differently. Those dependencies are not published on crates.io so their `Cargo.toml` is not normalised. Due to this, the `Cargo.toml` is normalised before the copy to the vendor directory. This normalisation yields different results depending on the machine.

### Notes

_No response_

### Version

```text
cargo 1.80.0-nightly (431db31d0 2024-05-28)
release: 1.80.0-nightly
commit-hash: 431db31d0dbeda320caf8ef8535ea48eb3093407
commit-date: 2024-05-28
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Ubuntu 22.4.0 (jammy) [64-bit]
```


Pull request description:
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



Patch:
diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs
index 9313a0ddd..088dd0196 100644
--- a/src/cargo/util/toml/mod.rs
+++ b/src/cargo/util/toml/mod.rs
@@ -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)
diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs
index 261ff67b3..166e8a700 100644
--- a/src/cargo/util/toml/targets.rs
+++ b/src/cargo/util/toml/targets.rs
@@ -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![]
@@ -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> {


Test patch:
diff --git a/tests/testsuite/package.rs b/tests/testsuite/package.rs
index 850fe9c50..fa5006d0c 100644
--- a/tests/testsuite/package.rs
+++ b/tests/testsuite/package.rs
@@ -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 [..]
@@ -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 [..]
@@ -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"
+"#,
+        )],
+    );
+}
diff --git a/tests/testsuite/required_features.rs b/tests/testsuite/required_features.rs
index f192be50d..10b552753 100644
--- a/tests/testsuite/required_features.rs
+++ b/tests/testsuite/required_features.rs
@@ -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();
 }
diff --git a/tests/testsuite/vendor.rs b/tests/testsuite/vendor.rs
index e5e56b396..4d1514734 100644
--- a/tests/testsuite/vendor.rs
+++ b/tests/testsuite/vendor.rs
@@ -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]
@@ -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()