Fix linker value with target-applies-to-host and implicit target by storing it in Unit

Author: gmorenz

src/cargo/core/compiler/build_context/target_info.rs

@@ -20,10 +20,12 @@ use serde::{Deserialize, Serialize};
 use std::cell::RefCell;
 use std::collections::hash_map::{Entry, HashMap};
 use std::path::{Path, PathBuf};
+use std::rc::Rc;
 use std::str::{self, FromStr};
 use std::sync::Arc;
 
-/// Information about the platform target gleaned from querying rustc.
+/// Information about the platform target gleaned from querying rustc and from
+/// merging configs.
 ///
 /// [`RustcTargetData`] keeps several of these, one for the host and the others
 /// for other specified targets. If no target is specified, it uses a clone from
@@ -54,6 +56,8 @@ pub struct TargetInfo {
     pub rustflags: Arc<[String]>,
     /// Extra flags to pass to `rustdoc`, see [`extra_args`].
     pub rustdocflags: Arc<[String]>,
+    /// Linker to use. If the value is `None` it is left up to rustc.
+    pub linker: Option<Rc<PathBuf>>,
     /// Whether or not rustc (stably) supports the `--check-cfg` flag.
     ///
     /// Can be removed once the minimum supported rustc version of Cargo is
@@ -157,6 +161,11 @@ impl TargetInfo {
         requested_kinds: &[CompileKind],
         rustc: &Rustc,
         kind: CompileKind,
+        // This config is used for `links_overrides` and `linker`.
+        //
+        // In the case of target_applies_to_host=true (default) it may contain
+        // incorrect rustflags.
+        target_config: &TargetConfig,
     ) -> CargoResult<TargetInfo> {
         let mut rustflags =
             extra_args(gctx, requested_kinds, &rustc.host, None, kind, Flags::Rust)?;
@@ -321,6 +330,7 @@ impl TargetInfo {
                     Flags::Rustdoc,
                 )?
                 .into(),
+                linker: target_linker(gctx, &cfg, target_config)?.map(Rc::new),
                 cfg,
                 support_split_debuginfo,
                 support_check_cfg,
@@ -826,6 +836,42 @@ fn rustflags_from_target(
     }
 }
 
+/// Gets the user-specified linker for a particular host or target from the configuration.
+fn target_linker(
+    gctx: &GlobalContext,
+    target_cfg: &[Cfg],
+    target_config: &TargetConfig,
+) -> CargoResult<Option<PathBuf>> {
+    // Try host.linker and target.{}.linker.
+    if let Some(path) = target_config
+        .linker
+        .as_ref()
+        .map(|l| l.val.clone().resolve_program(gctx))
+    {
+        return Ok(Some(path));
+    }
+
+    // Try target.'cfg(...)'.linker.
+    let mut cfgs = gctx
+        .target_cfgs()?
+        .iter()
+        .filter_map(|(key, cfg)| cfg.linker.as_ref().map(|linker| (key, linker)))
+        .filter(|(key, _linker)| CfgExpr::matches_key(key, target_cfg));
+    let matching_linker = cfgs.next();
+    if let Some((key, linker)) = cfgs.next() {
+        anyhow::bail!(
+            "several matching instances of `target.'cfg(..)'.linker` in configurations\n\
+             first match `{}` located in {}\n\
+             second match `{}` located in {}",
+            matching_linker.unwrap().0,
+            matching_linker.unwrap().1.definition,
+            key,
+            linker.definition
+        );
+    }
+    Ok(matching_linker.map(|(_k, linker)| linker.val.clone().resolve_program(gctx)))
+}
+
 /// Gets compiler flags from `[host]` section in the config.
 /// See [`extra_args`] for more.
 fn rustflags_from_host(
@@ -893,22 +939,28 @@ impl<'gctx> RustcTargetData<'gctx> {
         let mut target_info = HashMap::new();
         let target_applies_to_host = gctx.target_applies_to_host()?;
         let host_target = CompileTarget::new(&rustc.host)?;
-        let host_info = TargetInfo::new(gctx, requested_kinds, &rustc, CompileKind::Host)?;
 
         // This config is used for link overrides and choosing a linker.
         let host_config = if target_applies_to_host {
             gctx.target_cfg_triple(&rustc.host)?
         } else {
             gctx.host_cfg_triple(&rustc.host)?
         };
+        let host_info = TargetInfo::new(
+            gctx,
+            requested_kinds,
+            &rustc,
+            CompileKind::Host,
+            &host_config,
+        )?;
 
         // This is a hack. The unit_dependency graph builder "pretends" that
         // `CompileKind::Host` is `CompileKind::Target(host)` if the
         // `--target` flag is not specified. Since the unit_dependency code
         // needs access to the target config data, create a copy so that it
         // can be found. See `rebuild_unit_graph_shared` for why this is done.
         if requested_kinds.iter().any(CompileKind::is_host) {
-            target_config.insert(host_target, gctx.target_cfg_triple(&rustc.host)?);
+            let host_target_config = gctx.target_cfg_triple(&rustc.host)?;
 
             // If target_applies_to_host is true, the host_info is the target info,
             // otherwise we need to build target info for the target.
@@ -920,9 +972,12 @@ impl<'gctx> RustcTargetData<'gctx> {
                     requested_kinds,
                     &rustc,
                     CompileKind::Target(host_target),
+                    &host_target_config,
                 )?;
                 target_info.insert(host_target, host_target_info);
             }
+
+            target_config.insert(host_target, host_target_config);
         };
 
         let mut res = RustcTargetData {
@@ -969,14 +1024,20 @@ impl<'gctx> RustcTargetData<'gctx> {
     /// Insert `kind` into our `target_info` and `target_config` members if it isn't present yet.
     pub fn merge_compile_kind(&mut self, kind: CompileKind) -> CargoResult<()> {
         if let CompileKind::Target(target) = kind {
-            if !self.target_config.contains_key(&target) {
-                self.target_config
-                    .insert(target, self.gctx.target_cfg_triple(target.short_name())?);
-            }
+            let target_config: &TargetConfig = match self.target_config.entry(target) {
+                Entry::Occupied(o) => o.into_mut(),
+                Entry::Vacant(v) => v.insert(self.gctx.target_cfg_triple(target.short_name())?),
+            };
             if !self.target_info.contains_key(&target) {
                 self.target_info.insert(
                     target,
-                    TargetInfo::new(self.gctx, &self.requested_kinds, &self.rustc, kind)?,
+                    TargetInfo::new(
+                        self.gctx,
+                        &self.requested_kinds,
+                        &self.rustc,
+                        kind,
+                        target_config,
+                    )?,
                 );
             }
         }

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

@@ -284,7 +284,6 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
                     unit: unit.clone(),
                     args,
                     unstable_opts,
-                    linker: self.compilation.target_linker(unit.kind).clone(),
                     script_meta,
                     env: artifact::get_env(&self, self.unit_deps(unit))?,
                 });

src/cargo/core/compiler/compilation.rs

@@ -40,8 +40,6 @@ pub struct Doctest {
     pub args: Vec<OsString>,
     /// Whether or not -Zunstable-options is needed.
     pub unstable_opts: bool,
-    /// The -Clinker value to use.
-    pub linker: Option<PathBuf>,
     /// The script metadata, if this unit's package has a build script.
     ///
     /// This is used for indexing [`Compilation::extra_env`].
@@ -120,8 +118,6 @@ pub struct Compilation<'gctx> {
     primary_rustc_process: Option<ProcessBuilder>,
 
     target_runners: HashMap<CompileKind, Option<(PathBuf, Vec<String>)>>,
-    /// The linker to use for each host or target.
-    target_linkers: HashMap<CompileKind, Option<PathBuf>>,
 }
 
 impl<'gctx> Compilation<'gctx> {
@@ -162,13 +158,6 @@ impl<'gctx> Compilation<'gctx> {
                 .chain(Some(&CompileKind::Host))
                 .map(|kind| Ok((*kind, target_runner(bcx, *kind)?)))
                 .collect::<CargoResult<HashMap<_, _>>>()?,
-            target_linkers: bcx
-                .build_config
-                .requested_kinds
-                .iter()
-                .chain(Some(&CompileKind::Host))
-                .map(|kind| Ok((*kind, target_linker(bcx, *kind)?)))
-                .collect::<CargoResult<HashMap<_, _>>>()?,
         })
     }
 
@@ -240,11 +229,6 @@ impl<'gctx> Compilation<'gctx> {
         self.target_runners.get(&kind).and_then(|x| x.as_ref())
     }
 
-    /// Gets the user-specified linker for a particular host or target.
-    pub fn target_linker(&self, kind: CompileKind) -> Option<PathBuf> {
-        self.target_linkers.get(&kind).and_then(|x| x.clone())
-    }
-
     /// Returns a [`ProcessBuilder`] appropriate for running a process for the
     /// target platform. This is typically used for `cargo run` and `cargo
     /// test`.
@@ -484,39 +468,3 @@ fn target_runner(
         )
     }))
 }
-
-/// Gets the user-specified linker for a particular host or target from the configuration.
-fn target_linker(bcx: &BuildContext<'_, '_>, kind: CompileKind) -> CargoResult<Option<PathBuf>> {
-    // Try host.linker and target.{}.linker.
-    if let Some(path) = bcx
-        .target_data
-        .target_config(kind)
-        .linker
-        .as_ref()
-        .map(|l| l.val.clone().resolve_program(bcx.gctx))
-    {
-        return Ok(Some(path));
-    }
-
-    // Try target.'cfg(...)'.linker.
-    let target_cfg = bcx.target_data.info(kind).cfg();
-    let mut cfgs = bcx
-        .gctx
-        .target_cfgs()?
-        .iter()
-        .filter_map(|(key, cfg)| cfg.linker.as_ref().map(|linker| (key, linker)))
-        .filter(|(key, _linker)| CfgExpr::matches_key(key, target_cfg));
-    let matching_linker = cfgs.next();
-    if let Some((key, linker)) = cfgs.next() {
-        anyhow::bail!(
-            "several matching instances of `target.'cfg(..)'.linker` in configurations\n\
-             first match `{}` located in {}\n\
-             second match `{}` located in {}",
-            matching_linker.unwrap().0,
-            matching_linker.unwrap().1.definition,
-            key,
-            linker.definition
-        );
-    }
-    Ok(matching_linker.map(|(_k, linker)| linker.val.clone().resolve_program(bcx.gctx)))
-}

src/cargo/core/compiler/custom_build.rs

@@ -300,8 +300,8 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
         cmd.env(&var, value);
     }
 
-    if let Some(linker) = &build_runner.compilation.target_linker(unit.kind) {
-        cmd.env("RUSTC_LINKER", linker);
+    if let Some(linker) = &unit.linker {
+        cmd.env("RUSTC_LINKER", linker.as_ref());
     }
 
     if let Some(links) = unit.pkg.manifest().links() {

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

@@ -1454,8 +1454,8 @@ fn calculate_normal(
     let m = unit.pkg.manifest().metadata();
     let metadata = util::hash_u64((&m.authors, &m.description, &m.homepage, &m.repository));
     let mut config = StableHasher::new();
-    if let Some(linker) = build_runner.compilation.target_linker(unit.kind) {
-        linker.hash(&mut config);
+    if let Some(linker) = &unit.linker {
+        linker.as_ref().hash(&mut config);
     }
     if unit.mode.is_doc() && build_runner.bcx.gctx.cli_unstable().rustdoc_map {
         if let Ok(map) = build_runner.bcx.gctx.doc_extern_map() {

src/cargo/core/compiler/mod.rs

@@ -1156,11 +1156,7 @@ fn build_base_args(
         cmd,
         "-C",
         "linker=",
-        build_runner
-            .compilation
-            .target_linker(unit.kind)
-            .as_ref()
-            .map(|s| s.as_ref()),
+        unit.linker.as_ref().map(|s| s.as_ref().as_ref()),
     );
     if incremental {
         let dir = build_runner

src/cargo/core/compiler/standard_lib.rs

@@ -220,6 +220,7 @@ pub fn generate_std_roots(
                 target_data.info(*kind).rustflags.clone(),
                 target_data.info(*kind).rustdocflags.clone(),
                 target_data.target_config(*kind).links_overrides.clone(),
+                target_data.info(*kind).linker.clone(),
                 /*is_std*/ true,
                 /*dep_hash*/ 0,
                 IsArtifact::No,

src/cargo/core/compiler/unit.rs

@@ -13,6 +13,7 @@ use std::collections::{BTreeMap, HashSet};
 use std::fmt;
 use std::hash::{Hash, Hasher};
 use std::ops::Deref;
+use std::path::PathBuf;
 use std::rc::Rc;
 use std::sync::Arc;
 
@@ -90,6 +91,8 @@ pub struct UnitInner {
     /// running its build script and instead use the given output from the
     /// config file.
     pub links_overrides: Rc<BTreeMap<String, BuildOutput>>,
+    /// Linker (if any) to pass to `rustc`, if not specified rustc chooses a default.
+    pub linker: Option<Rc<PathBuf>>,
     // if `true`, the dependency is an artifact dependency, requiring special handling when
     // calculating output directories, linkage and environment variables provided to builds.
     pub artifact: IsArtifact,
@@ -185,6 +188,7 @@ impl fmt::Debug for Unit {
             .field("rustflags", &self.rustflags)
             .field("rustdocflags", &self.rustdocflags)
             .field("links_overrides", &self.links_overrides)
+            .field("linker", &self.linker)
             .field("artifact", &self.artifact.is_true())
             .field(
                 "artifact_target_for_features",
@@ -235,6 +239,7 @@ impl UnitInterner {
         rustflags: Arc<[String]>,
         rustdocflags: Arc<[String]>,
         links_overrides: Rc<BTreeMap<String, BuildOutput>>,
+        linker: Option<Rc<PathBuf>>,
         is_std: bool,
         dep_hash: u64,
         artifact: IsArtifact,
@@ -271,6 +276,7 @@ impl UnitInterner {
             rustflags,
             rustdocflags,
             links_overrides,
+            linker,
             is_std,
             dep_hash,
             artifact,

src/cargo/core/compiler/unit_dependencies.rs

@@ -862,6 +862,7 @@ fn new_unit_dep_with_profile(
             .target_config(kind)
             .links_overrides
             .clone(),
+        state.target_data.info(kind).linker.clone(),
         state.is_std,
         /*dep_hash*/ 0,
         artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes),

src/cargo/ops/cargo_compile/mod.rs

@@ -178,7 +178,17 @@ pub fn print<'a>(
         if index != 0 {
             drop_println!(gctx);
         }
-        let target_info = TargetInfo::new(gctx, &build_config.requested_kinds, &rustc, *kind)?;
+        let target_config = match kind {
+            CompileKind::Host => gctx.target_cfg_triple(&rustc.host),
+            CompileKind::Target(target) => gctx.target_cfg_triple(target.short_name()),
+        }?;
+        let target_info = TargetInfo::new(
+            gctx,
+            &build_config.requested_kinds,
+            &rustc,
+            *kind,
+            &target_config,
+        )?;
         let mut process = rustc.process();
         process.args(&target_info.rustflags);
         if let Some(args) = target_rustc_args {
@@ -699,6 +709,7 @@ fn traverse_and_share(
             unit.rustflags.clone(),
             unit.rustdocflags.clone(),
             unit.links_overrides.clone(),
+            unit.linker.clone(),
             unit.is_std,
             unit.dep_hash,
             unit.artifact,
@@ -727,6 +738,7 @@ fn traverse_and_share(
         unit.rustflags.clone(),
         unit.rustdocflags.clone(),
         unit.links_overrides.clone(),
+        unit.linker.clone(),
         unit.is_std,
         new_dep_hash,
         unit.artifact,
@@ -891,6 +903,7 @@ fn override_rustc_crate_types(
             unit.rustflags.clone(),
             unit.rustdocflags.clone(),
             unit.links_overrides.clone(),
+            unit.linker.clone(),
             unit.is_std,
             unit.dep_hash,
             unit.artifact,

src/cargo/ops/cargo_compile/unit_generator.rs

@@ -174,6 +174,7 @@ impl<'a> UnitGenerator<'a, '_> {
                     self.target_data.info(kind).rustflags.clone(),
                     self.target_data.info(kind).rustdocflags.clone(),
                     self.target_data.target_config(kind).links_overrides.clone(),
+                    self.target_data.info(kind).linker.clone(),
                     /*is_std*/ false,
                     /*dep_hash*/ 0,
                     IsArtifact::No,