verify that multiple versions are allowed for artifact deps as well.

also: remove redundant test

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

Properly choose which dependencies take part in artifact handling

Previously it would include them very generously without considering
the compatible dependency types.

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

a more complex test which includes dev-dependencies

It also shows that doc-tests don't yet work as rustdoc is run outside of
the system into which we integrate right now.

It should be possible to write our environment variable configuration
in terms of this 'finished compilation' though, hopefully with
most code reused.

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

A first stab at storing artifact environment variables for packages…

…however, it seems like the key for this isn't necessarily correct
under all circumstances. Maybe it should be something more specific,
don't know.

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

Adjust key for identifying units to Metadata

This one is actually unique and feels much better.

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

Attempt to make use of artifact environment information…

…but fail as the metadata won't match as the doctest unit is, of course,
its separate unit. Now I wonder if its possible to find the artifact
units in question that have the metadata.

src/cargo/core/compiler/artifact.rs

@@ -3,12 +3,16 @@ use crate::core::compiler::{Context, CrateType, FileFlavor, Unit};
 use crate::core::TargetKind;
 use crate::CargoResult;
 use cargo_util::ProcessBuilder;
+use std::collections::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<()> {
+) -> CargoResult<Option<HashSet<(String, PathBuf)>>> {
+    let mut ret = HashSet::new();
     for unit_dep in dependencies.iter().filter(|d| d.unit.artifact) {
         for artifact_path in cx
             .outputs(&unit_dep.unit)?
@@ -18,28 +22,29 @@ pub fn set_env(
             let artifact_type_upper = unit_artifact_type_name_upper(&unit_dep.unit);
             let dep_name = unit_dep.dep_name.unwrap_or(unit_dep.unit.pkg.name());
             let dep_name_upper = dep_name.to_uppercase().replace("-", "_");
-            cmd.env(
-                &format!("CARGO_{}_DIR_{}", artifact_type_upper, dep_name_upper),
-                artifact_path.parent().expect("parent dir for artifacts"),
-            );
-            cmd.env(
-                &format!(
-                    "CARGO_{}_FILE_{}_{}",
-                    artifact_type_upper,
-                    dep_name_upper,
-                    unit_dep.unit.target.name()
-                ),
-                artifact_path,
+
+            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()));
+
+            let var = format!(
+                "CARGO_{}_FILE_{}_{}",
+                artifact_type_upper,
+                dep_name_upper,
+                unit_dep.unit.target.name()
             );
+            cmd.env(&var, artifact_path);
+            ret.insert((var, artifact_path.to_owned()));
+
             if unit_dep.unit.target.name() == dep_name.as_str() {
-                cmd.env(
-                    &format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,),
-                    artifact_path,
-                );
+                let var = format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,);
+                cmd.env(&var, artifact_path);
+                ret.insert((var, artifact_path.to_owned()));
             }
         }
     }
-    Ok(())
+    Ok((!ret.is_empty()).then(|| ret))
 }
 
 fn unit_artifact_type_name_upper(unit: &Unit) -> &'static str {

src/cargo/core/compiler/compilation.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeSet, HashMap};
+use std::collections::{BTreeSet, HashMap, HashSet};
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::path::PathBuf;
@@ -15,6 +15,8 @@ use crate::util::{config, CargoResult, Config};
 pub struct Doctest {
     /// What's being doctested
     pub unit: Unit,
+    /// The above units metadata key for accessing artifact environment variables.
+    pub unit_meta: Metadata,
     /// Arguments needed to pass to rustdoc to run this test.
     pub args: Vec<OsString>,
     /// Whether or not -Zunstable-options is needed.
@@ -82,6 +84,11 @@ pub struct Compilation<'cfg> {
     /// `RunCustomBuild` unit that generated these env vars.
     pub extra_env: HashMap<Metadata, Vec<(String, String)>>,
 
+    /// Environment variables generated artifact dependencies for consumption by future invocations of programs.
+    ///
+    /// The key is the artifact units metadata to uniquely identify the unit that created them.
+    pub artifact_env: HashMap<Metadata, HashSet<(String, PathBuf)>>,
+
     /// Libraries to test with rustdoc.
     pub to_doc_test: Vec<Doctest>,
 
@@ -141,6 +148,7 @@ impl<'cfg> Compilation<'cfg> {
             cdylibs: Vec::new(),
             root_crate_names: Vec::new(),
             extra_env: HashMap::new(),
+            artifact_env: HashMap::new(),
             to_doc_test: Vec::new(),
             config: bcx.config,
             host: bcx.host_triple().to_string(),
@@ -187,17 +195,23 @@ impl<'cfg> Compilation<'cfg> {
         &self,
         unit: &Unit,
         script_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 p = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?;
-        unit.target.edition().cmd_edition_arg(&mut p);
+        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)) {
+            for (var, path) in artifact_env {
+                cmd.env(var, path);
+            }
+        }
+        unit.target.edition().cmd_edition_arg(&mut cmd);
 
         for crate_type in unit.target.rustc_crate_types() {
-            p.arg("--crate-type").arg(crate_type.as_str());
+            cmd.arg("--crate-type").arg(crate_type.as_str());
         }
 
-        Ok(p)
+        Ok(cmd)
     }
 
     /// Returns a [`ProcessBuilder`] appropriate for running a process for the

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

@@ -258,6 +258,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
                 self.compilation.to_doc_test.push(compilation::Doctest {
                     unit: unit.clone(),
+                    unit_meta: self.files().metadata(&unit),
                     args,
                     unstable_opts,
                     linker: self.bcx.linker(unit.kind),

src/cargo/core/compiler/mod.rs

@@ -1,4 +1,4 @@
-mod artifact;
+pub mod artifact;
 mod build_config;
 mod build_context;
 mod build_plan;
@@ -627,7 +627,7 @@ fn prepare_rustc(
 fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     let bcx = cx.bcx;
     // script_metadata is not needed here, it is only for tests.
-    let mut rustdoc = cx.compilation.rustdoc_process(unit, None)?;
+    let mut rustdoc = cx.compilation.rustdoc_process(unit, None, None)?;
     rustdoc.inherit_jobserver(&cx.jobserver);
     let crate_name = unit.target.crate_name();
     rustdoc.arg("--crate-name").arg(&crate_name);
@@ -1107,8 +1107,6 @@ fn build_deps_args(
 
     let deps = cx.unit_deps(unit);
 
-    artifact::set_env(cx, deps, cmd)?;
-
     // If there is not one linkable target but should, rustc fails later
     // on if there is an `extern crate` for it. This may turn into a hard
     // error in the future (see PR #4797).
@@ -1144,6 +1142,21 @@ fn build_deps_args(
         cmd.arg(arg);
     }
 
+    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
+        );
+    }
+
     // This will only be set if we're already using a feature
     // requiring nightly rust
     if unstable_opts {

src/cargo/core/compiler/unit_dependencies.rs

@@ -244,7 +244,7 @@ fn compute_deps(
     }
 
     let id = unit.pkg.package_id();
-    let filtered_deps = state.deps_filtered(unit, unit_for, &|dep| {
+    let dep_filter = &|dep: &Dependency| {
         // If this target is a build command, then we only want build
         // dependencies, otherwise we want everything *other than* build
         // dependencies.
@@ -266,7 +266,8 @@ fn compute_deps(
         // If we've gotten past all that, then this dependency is
         // actually used!
         true
-    });
+    };
+    let filtered_deps = state.deps_filtered(unit, unit_for, dep_filter);
 
     let mut ret = Vec::new();
     let mut dev_deps = Vec::new();
@@ -276,7 +277,7 @@ fn compute_deps(
         // as 'library as well'. We don't filter in the closure above as we still want to get a chance
         // to process them as pure non-lib artifact dependencies.
         let (has_artifact, artifact_lib) =
-            calc_artifact_deps(unit, unit_for, id, deps, state, &mut ret)?;
+            calc_artifact_deps(unit, unit_for, id, deps, state, dep_filter, &mut ret)?;
 
         let lib = package_lib(dep_pkg, has_artifact, artifact_lib);
         let lib = match lib {
@@ -416,13 +417,15 @@ fn calc_artifact_deps(
     dep_id: PackageId,
     deps: &HashSet<Dependency>,
     state: &State<'_, '_>,
+    filter: &dyn Fn(&Dependency) -> bool,
     ret: &mut Vec<UnitDep>,
 ) -> CargoResult<(bool, bool)> {
     let mut has_artifact = false;
     let mut artifact_lib = false;
     let artifact_pkg = state.get(dep_id);
     for (dep, artifact) in deps
         .iter()
+        .filter(|dep| filter(dep))
         .filter_map(|dep| dep.artifact().map(|a| (dep, a)))
     {
         has_artifact = true;
@@ -612,8 +615,9 @@ fn match_artifacts_kind_with_targets<'a>(
         };
         if !found {
             anyhow::bail!(
-                "Dependency `{}` in crate `{}` requires a `{}` artifact to be present.",
+                "Dependency `{} = \"{}\"` in crate `{}` requires a `{}` artifact to be present.",
                 artifact_dep.name_in_toml(),
+                artifact_dep.version_req(),
                 unit.pkg.name(),
                 artifact_kind
             );
@@ -628,15 +632,16 @@ fn compute_deps_doc(
     state: &mut State<'_, '_>,
     unit_for: UnitFor,
 ) -> CargoResult<Vec<UnitDep>> {
-    let deps = state.deps(unit, unit_for);
+    let dep_filter = &|dep: &Dependency| dep.kind() == DepKind::Normal;
+    let deps = state.deps_filtered(unit, unit_for, dep_filter);
 
     // To document a library, we depend on dependencies actually being
     // built. If we're documenting *all* libraries, then we also depend on
     // the documentation of the library being built.
     let mut ret = Vec::new();
     for (id, deps) in deps {
         let (has_artifact, artifact_lib) =
-            calc_artifact_deps(unit, unit_for, id, deps, state, &mut ret)?;
+            calc_artifact_deps(unit, unit_for, id, deps, state, dep_filter, &mut ret)?;
 
         let dep_pkg = state.get(id);
         let lib = package_lib(dep_pkg, has_artifact, artifact_lib);

src/cargo/ops/cargo_test.rs

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