Properly use metadata to use artifact environment variables in doctests

This is the commit message #2:
------------------------------

Add test for resolver = "2" and build dependencies

Interestingly the 'host-features' flag must be set (as is seemingly
documented in the flags documentation as well), even though I am not
quite sure if this is the 100% correct solution. Should it rather
have an entry with this flag being false in its map? Probably not…
but I am not quite certain.

This is the commit message rust-lang#3:
------------------------------

set most if not all tests to use resolver = "2"

This allows to keep it working with the most recent version while
allowing to quickly test with "1" as well (which thus far was working
fine).

src/cargo/core/compiler/artifact.rs

@@ -1,19 +1,20 @@
 use crate::core::compiler::unit_graph::UnitDep;
-use crate::core::compiler::{Context, CrateType, FileFlavor, Unit};
+use crate::core::compiler::{Context, CrateType, FileFlavor, Metadata, Unit};
 use crate::core::TargetKind;
 use crate::CargoResult;
 use cargo_util::ProcessBuilder;
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 
 /// Adjust `cmd` to contain artifact environment variables and return all set key/value pairs for later use.
 pub fn set_env(
     cx: &Context<'_, '_>,
     dependencies: &[UnitDep],
     cmd: &mut ProcessBuilder,
-) -> CargoResult<Option<HashSet<(String, PathBuf)>>> {
-    let mut ret = HashSet::new();
+) -> CargoResult<Option<HashMap<Metadata, HashSet<(String, PathBuf)>>>> {
+    let mut ret = HashMap::new();
     for unit_dep in dependencies.iter().filter(|d| d.unit.artifact) {
+        let mut set = HashSet::new();
         for artifact_path in cx
             .outputs(&unit_dep.unit)?
             .iter()
@@ -26,7 +27,7 @@ pub fn set_env(
             let var = format!("CARGO_{}_DIR_{}", artifact_type_upper, dep_name_upper);
             let path = artifact_path.parent().expect("parent dir for artifacts");
             cmd.env(&var, path);
-            ret.insert((var, path.to_owned()));
+            set.insert((var, path.to_owned()));
 
             let var = format!(
                 "CARGO_{}_FILE_{}_{}",
@@ -35,14 +36,17 @@ pub fn set_env(
                 unit_dep.unit.target.name()
             );
             cmd.env(&var, artifact_path);
-            ret.insert((var, artifact_path.to_owned()));
+            set.insert((var, artifact_path.to_owned()));
 
             if unit_dep.unit.target.name() == dep_name.as_str() {
                 let var = format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,);
                 cmd.env(&var, artifact_path);
-                ret.insert((var, artifact_path.to_owned()));
+                set.insert((var, artifact_path.to_owned()));
             }
         }
+        if !set.is_empty() {
+            ret.insert(cx.files().metadata(&unit_dep.unit), set);
+        }
     }
     Ok((!ret.is_empty()).then(|| ret))
 }

src/cargo/core/compiler/compilation.rs

@@ -16,7 +16,7 @@ pub struct Doctest {
     /// What's being doctested
     pub unit: Unit,
     /// The above units metadata key for accessing artifact environment variables.
-    pub unit_meta: Metadata,
+    pub artifact_meta: Vec<Metadata>,
     /// Arguments needed to pass to rustdoc to run this test.
     pub args: Vec<OsString>,
     /// Whether or not -Zunstable-options is needed.
@@ -195,12 +195,16 @@ impl<'cfg> Compilation<'cfg> {
         &self,
         unit: &Unit,
         script_meta: Option<Metadata>,
-        unit_meta: Option<Metadata>,
+        unit_meta: Option<&[Metadata]>,
     ) -> CargoResult<ProcessBuilder> {
         let rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?);
         let cmd = fill_rustc_tool_env(rustdoc, unit);
         let mut cmd = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?;
-        if let Some(artifact_env) = unit_meta.and_then(|meta| self.artifact_env.get(&meta)) {
+        if let Some(artifact_env) = unit_meta.map(|meta| {
+            meta.iter()
+                .filter_map(|meta| self.artifact_env.get(&meta))
+                .flat_map(std::convert::identity)
+        }) {
             for (var, path) in artifact_env {
                 cmd.env(var, path);
             }

src/cargo/core/compiler/context/mod.rs

@@ -258,7 +258,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
                 self.compilation.to_doc_test.push(compilation::Doctest {
                     unit: unit.clone(),
-                    unit_meta: self.files().metadata(&unit),
+                    artifact_meta: self
+                        .unit_deps(unit)
+                        .iter()
+                        .filter_map(|dep| {
+                            dep.unit.artifact.then(|| self.files().metadata(&dep.unit))
+                        })
+                        .collect(),
                     args,
                     unstable_opts,
                     linker: self.bcx.linker(unit.kind),

src/cargo/core/compiler/mod.rs

@@ -1145,16 +1145,7 @@ fn build_deps_args(
     if let Some(vars) = artifact::set_env(cx, deps, cmd)?
         .and_then(|vars| cx.bcx.roots.contains(&unit).then(|| vars))
     {
-        let previous = cx
-            .compilation
-            .artifact_env
-            .insert(cx.files().metadata(unit), vars);
-        assert!(
-            previous.is_none(),
-            "BUG: Expecting no previous value to exist for {:?}, but got {:#?}.",
-            unit,
-            previous
-        );
+        cx.compilation.artifact_env.extend(vars);
     }
 
     // This will only be set if we're already using a feature

src/cargo/core/compiler/unit_dependencies.rs

@@ -502,7 +502,6 @@ fn compute_deps_custom_build(
     } else {
         let mut artifact_units: Vec<_> = build_artifact_requirements_to_units(
             unit,
-            script_unit_for,
             artifact_build_deps,
             state,
             CompileKind::Host, // TODO(ST): probably here we have to handle the artifact target more properly.
@@ -514,18 +513,20 @@ fn compute_deps_custom_build(
 
 fn build_artifact_requirements_to_units(
     parent: &Unit,
-    parent_unit_for: UnitFor,
     artifact_deps: Vec<(PackageId, &HashSet<Dependency>)>,
     state: &State<'_, '_>,
     compile_kind: CompileKind,
 ) -> CargoResult<Vec<UnitDep>> {
     let mut ret = Vec::new();
+    // So, this really wants to be true for build dependencies, otherwise resolver = "2" will fail.
+    let host_features = true;
+    let unit_for = UnitFor::new_host(host_features);
     for (dep_pkg_id, deps) in artifact_deps {
         let artifact_pkg = state.get(dep_pkg_id);
         for build_dep in deps.iter().filter(|d| d.is_build()) {
             ret.extend(artifact_targets_to_unit_deps(
                 parent,
-                parent_unit_for,
+                unit_for,
                 state,
                 compile_kind,
                 artifact_pkg,

src/cargo/ops/cargo_test.rs

@@ -163,7 +163,7 @@ fn run_doc_tests(
             args,
             unstable_opts,
             unit,
-            unit_meta,
+            artifact_meta,
             linker,
             script_meta,
         } = doctest_info;
@@ -191,7 +191,11 @@ fn run_doc_tests(
         }
 
         config.shell().status("Doc-tests", unit.target.name())?;
-        let mut p = compilation.rustdoc_process(unit, *script_meta, Some(*unit_meta))?;
+        let mut p = compilation.rustdoc_process(
+            unit,
+            *script_meta,
+            (!artifact_meta.is_empty()).then(|| artifact_meta.as_slice()),
+        )?;
         p.arg("--crate-name").arg(&unit.target.crate_name());
         p.arg("--test");
 

tests/testsuite/artifact_dep.rs

@@ -13,6 +13,7 @@ fn check_with_invalid_artifact_dependency() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "unknown" }
@@ -77,6 +78,7 @@ fn build_without_nightly_shows_warnings_and_ignores_them() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -136,6 +138,7 @@ fn disallow_artifact_and_no_artifact_dep_to_same_package_within_the_same_dep_cat
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -165,6 +168,7 @@ fn build_script_with_bin_artifacts() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [build-dependencies]
                 bar = { path = "bar/", artifact = ["bin", "staticlib", "cdylib"] }
@@ -274,6 +278,7 @@ fn build_script_with_bin_artifact_and_lib_false() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [build-dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -319,6 +324,7 @@ fn lib_with_bin_artifact_and_lib_false() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -362,6 +368,7 @@ fn build_script_with_selected_dashed_bin_artifact_and_lib_true() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [build-dependencies]
                 bar-baz = { path = "bar/", artifact = "bin:baz-suffix", lib = true }
@@ -460,6 +467,7 @@ fn lib_with_selected_dashed_bin_artifact_and_lib_true() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar-baz = { path = "bar/", artifact = ["bin:baz-suffix", "staticlib", "cdylib"], lib = true }
@@ -528,6 +536,7 @@ fn allow_artifact_and_no_artifact_dep_to_same_package_within_different_dep_categ
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -577,6 +586,7 @@ fn allow_dep_renames_with_multiple_versions() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [build-dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -622,6 +632,7 @@ fn allow_artifact_and_non_artifact_dependency_to_same_crate_if_these_are_not_the
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [build-dependencies]
                 bar = { path = "bar/", artifact = "bin", lib = false }
@@ -668,6 +679,7 @@ fn prevent_no_lib_warning_with_artifact_dependencies() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin" }
@@ -701,6 +713,7 @@ fn show_no_lib_warning_with_artifact_dependencies_that_have_no_lib_but_lib_true(
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [dependencies]
                 bar = { path = "bar/", artifact = "bin", lib = true }
@@ -719,6 +732,36 @@ fn show_no_lib_warning_with_artifact_dependencies_that_have_no_lib_but_lib_true(
         .run();
 }
 
+#[cargo_test]
+fn resolver_2_build_dep_without_lib() {
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.0"
+                authors = []
+                edition = "2021"
+                
+                [build-dependencies]
+                bar = { path = "bar/", artifact = "bin" }
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .file("build.rs", r#"
+                fn main() {
+                    let bar: std::path::PathBuf = std::env::var("CARGO_BIN_FILE_BAR").expect("CARGO_BIN_FILE_BAR").into();
+                    assert!(&bar.is_file()); 
+                }"#)
+        .file("bar/Cargo.toml", &basic_bin_manifest("bar"))
+        .file("bar/src/main.rs", "fn main() {}")
+        .build();
+    p.cargo("check -Z unstable-options -Z bindeps")
+        .masquerade_as_nightly_cargo()
+        .run();
+}
+
 #[cargo_test]
 fn check_missing_crate_type_in_package_fails() {
     for crate_type in &["cdylib", "staticlib", "bin"] {
@@ -762,6 +805,7 @@ fn env_vars_and_build_products_for_various_build_targets() {
                 name = "foo"
                 version = "0.0.0"
                 authors = []
+                resolver = "2"
                 
                 [lib]
                 doctest = true
@@ -839,6 +883,7 @@ fn env_vars_and_build_products_for_various_build_targets() {
 [FINISHED] test [unoptimized + debuginfo] target(s) in [..]
 [RUNNING] unittests [..]
 [RUNNING] tests/main.rs [..]
+[DOCTEST] foo
 ",
         )
         .run();
@@ -863,6 +908,7 @@ fn publish_artifact_dep() {
             documentation = "foo"
             homepage = "foo"
             repository = "foo"
+            resolver = "2"
 
             [dependencies]
             bar = { version = "1.0", artifact = "bin", lib = true }
@@ -942,6 +988,7 @@ homepage = "foo"
 documentation = "foo"
 license = "MIT"
 repository = "foo"
+resolver = "2"
 [dependencies.bar]
 version = "1.0"
 artifact = ["bin"]
@@ -965,6 +1012,7 @@ fn doc_lib_true() {
                 name = "foo"
                 version = "0.0.1"
                 authors = []
+                resolver = "2"
 
                 [dependencies.bar]
                 path = "bar"
@@ -1018,6 +1066,7 @@ fn rustdoc_works_on_libs_with_artifacts_and_lib_false() {
                 name = "foo"
                 version = "0.0.1"
                 authors = []
+                resolver = "2"
 
                 [dependencies.bar]
                 path = "bar"