Refactor how compile targets are handled

Rename `Kind` to `CompileKind` to reflect that it's intended for
compilation. Additionally change the `Target` variant to have a newtype
`CompileTarget` instead of just being a raw string. This new
`CompileTarget` type has a fallible constructor and handles custom json
target files internally.

Two accessors are available for `CompileTarget`, one is `rustc_target()`
which goes straight to rustc and everything else uses `short_name()`
which is the raw target or file stem for json files. The `short_name` is
used everywhere in Cargo for all purposes like configuration, env vars,
target directory naming, etc.

Author: alexcrichton

src/cargo/core/compiler/build_config.rs

@@ -1,18 +1,16 @@
 use std::cell::RefCell;
-use std::path::Path;
 
 use serde::ser;
 
-use crate::core::InternedString;
+use crate::core::compiler::{CompileKind, CompileTarget};
 use crate::util::ProcessBuilder;
-use crate::util::{CargoResult, CargoResultExt, Config, RustfixDiagnosticServer};
+use crate::util::{CargoResult, Config, RustfixDiagnosticServer};
 
 /// Configuration information for a rustc build.
 #[derive(Debug)]
 pub struct BuildConfig {
-    /// The target arch triple.
-    /// Default: host arch.
-    pub requested_target: Option<InternedString>,
+    /// The requested kind of compilation for this session
+    pub requested_kind: CompileKind,
     /// Number of rustc jobs to run in parallel.
     pub jobs: u32,
     /// `true` if we are building for release.
@@ -47,36 +45,21 @@ impl BuildConfig {
         requested_target: &Option<String>,
         mode: CompileMode,
     ) -> CargoResult<BuildConfig> {
-        let requested_target = match requested_target {
-            &Some(ref target) if target.ends_with(".json") => {
-                let path = Path::new(target).canonicalize().chain_err(|| {
-                    failure::format_err!("Target path {:?} is not a valid file", target)
-                })?;
-                Some(
-                    path.into_os_string()
-                        .into_string()
-                        .map_err(|_| failure::format_err!("Target path is not valid unicode"))?,
-                )
-            }
-            other => other.clone(),
+        let requested_kind = match requested_target {
+            Some(s) => CompileKind::Target(CompileTarget::new(s)?),
+            None => match config.get_string("build.target")? {
+                Some(cfg) => {
+                    let value = if cfg.val.ends_with(".json") {
+                        let path = cfg.definition.root(config).join(&cfg.val);
+                        path.to_str().expect("must be utf-8 in toml").to_string()
+                    } else {
+                        cfg.val
+                    };
+                    CompileKind::Target(CompileTarget::new(&value)?)
+                }
+                None => CompileKind::Host,
+            },
         };
-        if let Some(ref s) = requested_target {
-            if s.trim().is_empty() {
-                failure::bail!("target was empty")
-            }
-        }
-        let cfg_target = match config.get_string("build.target")? {
-            Some(ref target) if target.val.ends_with(".json") => {
-                let path = target.definition.root(config).join(&target.val);
-                let path_string = path
-                    .into_os_string()
-                    .into_string()
-                    .map_err(|_| failure::format_err!("Target path is not valid unicode"));
-                Some(path_string?)
-            }
-            other => other.map(|t| t.val),
-        };
-        let target = requested_target.or(cfg_target);
 
         if jobs == Some(0) {
             failure::bail!("jobs must be at least 1")
@@ -92,7 +75,7 @@ impl BuildConfig {
         let jobs = jobs.or(cfg_jobs).unwrap_or(::num_cpus::get() as u32);
 
         Ok(BuildConfig {
-            requested_target: target.as_ref().map(|s| s.into()),
+            requested_kind,
             jobs,
             release: false,
             mode,

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

@@ -1,5 +1,6 @@
 use crate::core::compiler::unit::UnitInterner;
-use crate::core::compiler::{BuildConfig, BuildOutput, Kind, Unit};
+use crate::core::compiler::CompileTarget;
+use crate::core::compiler::{BuildConfig, BuildOutput, CompileKind, Unit};
 use crate::core::profiles::Profiles;
 use crate::core::{Dependency, InternedString, Workspace};
 use crate::core::{PackageId, PackageSet};
@@ -35,8 +36,8 @@ pub struct BuildContext<'a, 'cfg> {
     /// Build information for the host arch.
     host_config: TargetConfig,
     /// Build information for the target.
-    target_config: HashMap<InternedString, TargetConfig>,
-    target_info: HashMap<InternedString, TargetInfo>,
+    target_config: HashMap<CompileTarget, TargetConfig>,
+    target_info: HashMap<CompileTarget, TargetInfo>,
     host_info: TargetInfo,
     pub units: &'a UnitInterner<'a>,
 }
@@ -54,14 +55,24 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         let rustc = config.load_global_rustc(Some(ws))?;
 
         let host_config = TargetConfig::new(config, &rustc.host)?;
-        let host_info = TargetInfo::new(config, build_config.requested_target, &rustc, Kind::Host)?;
+        let host_info = TargetInfo::new(
+            config,
+            build_config.requested_kind,
+            &rustc,
+            CompileKind::Host,
+        )?;
         let mut target_config = HashMap::new();
         let mut target_info = HashMap::new();
-        if let Some(target) = build_config.requested_target {
-            target_config.insert(target, TargetConfig::new(config, &target)?);
+        if let CompileKind::Target(target) = build_config.requested_kind {
+            target_config.insert(target, TargetConfig::new(config, target.short_name())?);
             target_info.insert(
                 target,
-                TargetInfo::new(config, Some(target), &rustc, Kind::Target(target))?,
+                TargetInfo::new(
+                    config,
+                    build_config.requested_kind,
+                    &rustc,
+                    CompileKind::Target(target),
+                )?,
             );
         }
 
@@ -82,30 +93,30 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
     }
 
     /// Whether a dependency should be compiled for the host or target platform,
-    /// specified by `Kind`.
-    pub fn dep_platform_activated(&self, dep: &Dependency, kind: Kind) -> bool {
+    /// specified by `CompileKind`.
+    pub fn dep_platform_activated(&self, dep: &Dependency, kind: CompileKind) -> bool {
         // If this dependency is only available for certain platforms,
         // make sure we're only enabling it for that platform.
         let platform = match dep.platform() {
             Some(p) => p,
             None => return true,
         };
-        let name = self.target_triple(kind);
+        let name = kind.short_name(self);
         platform.matches(&name, self.cfg(kind))
     }
 
     /// Gets the user-specified linker for a particular host or target.
-    pub fn linker(&self, kind: Kind) -> Option<&Path> {
+    pub fn linker(&self, kind: CompileKind) -> Option<&Path> {
         self.target_config(kind).linker.as_ref().map(|s| s.as_ref())
     }
 
     /// Gets the user-specified `ar` program for a particular host or target.
-    pub fn ar(&self, kind: Kind) -> Option<&Path> {
+    pub fn ar(&self, kind: CompileKind) -> Option<&Path> {
         self.target_config(kind).ar.as_ref().map(|s| s.as_ref())
     }
 
     /// Gets the list of `cfg`s printed out from the compiler for the specified kind.
-    pub fn cfg(&self, kind: Kind) -> &[Cfg] {
+    pub fn cfg(&self, kind: CompileKind) -> &[Cfg] {
         self.info(kind).cfg()
     }
 
@@ -119,19 +130,11 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         self.rustc.host
     }
 
-    /// Returns the target triple associated with a `Kind`
-    pub fn target_triple(&self, kind: Kind) -> InternedString {
-        match kind {
-            Kind::Host => self.host_triple(),
-            Kind::Target(name) => name,
-        }
-    }
-
     /// Gets the target configuration for a particular host or target.
-    pub fn target_config(&self, kind: Kind) -> &TargetConfig {
+    pub fn target_config(&self, kind: CompileKind) -> &TargetConfig {
         match kind {
-            Kind::Host => &self.host_config,
-            Kind::Target(s) => &self.target_config[&s],
+            CompileKind::Host => &self.host_config,
+            CompileKind::Target(s) => &self.target_config[&s],
         }
     }
 
@@ -152,10 +155,10 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
         pkg.source_id().is_path() || self.config.extra_verbose()
     }
 
-    pub fn info(&self, kind: Kind) -> &TargetInfo {
+    pub fn info(&self, kind: CompileKind) -> &TargetInfo {
         match kind {
-            Kind::Host => &self.host_info,
-            Kind::Target(s) => &self.target_info[&s],
+            CompileKind::Host => &self.host_info,
+            CompileKind::Target(s) => &self.target_info[&s],
         }
     }
 
@@ -167,7 +170,7 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
     ///
     /// `lib_name` is the `links` library name and `kind` is whether it is for
     /// Host or Target.
-    pub fn script_override(&self, lib_name: &str, kind: Kind) -> Option<&BuildOutput> {
+    pub fn script_override(&self, lib_name: &str, kind: CompileKind) -> Option<&BuildOutput> {
         self.target_config(kind).overrides.get(lib_name)
     }
 }

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

@@ -4,8 +4,7 @@ use std::env;
 use std::path::PathBuf;
 use std::str::{self, FromStr};
 
-use crate::core::compiler::Kind;
-use crate::core::InternedString;
+use crate::core::compiler::CompileKind;
 use crate::core::TargetKind;
 use crate::util::{CargoResult, CargoResultExt, Config, ProcessBuilder, Rustc};
 use cargo_platform::{Cfg, CfgExpr};
@@ -37,7 +36,7 @@ pub struct TargetInfo {
     pub rustdocflags: Vec<String>,
 }
 
-/// Kind of each file generated by a Unit, part of `FileType`.
+/// CompileKind of each file generated by a Unit, part of `FileType`.
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum FileFlavor {
     /// Not a special file type.
@@ -81,18 +80,11 @@ impl FileType {
 impl TargetInfo {
     pub fn new(
         config: &Config,
-        requested_target: Option<InternedString>,
+        requested_kind: CompileKind,
         rustc: &Rustc,
-        kind: Kind,
+        kind: CompileKind,
     ) -> CargoResult<TargetInfo> {
-        let rustflags = env_args(
-            config,
-            requested_target,
-            &rustc.host,
-            None,
-            kind,
-            "RUSTFLAGS",
-        )?;
+        let rustflags = env_args(config, requested_kind, &rustc.host, None, kind, "RUSTFLAGS")?;
         let mut process = rustc.process();
         process
             .arg("-")
@@ -102,8 +94,8 @@ impl TargetInfo {
             .args(&rustflags)
             .env_remove("RUSTC_LOG");
 
-        if let Kind::Target(target) = kind {
-            process.arg("--target").arg(target);
+        if let CompileKind::Target(target) = kind {
+            process.arg("--target").arg(target.rustc_target());
         }
 
         let crate_type_process = process.clone();
@@ -137,18 +129,18 @@ impl TargetInfo {
         };
         let mut rustlib = PathBuf::from(line);
         let sysroot_libdir = match kind {
-            Kind::Host => {
+            CompileKind::Host => {
                 if cfg!(windows) {
                     rustlib.push("bin");
                 } else {
                     rustlib.push("lib");
                 }
                 rustlib
             }
-            Kind::Target(target) => {
+            CompileKind::Target(target) => {
                 rustlib.push("lib");
                 rustlib.push("rustlib");
-                rustlib.push(target);
+                rustlib.push(target.short_name());
                 rustlib.push("lib");
                 rustlib
             }
@@ -172,15 +164,15 @@ impl TargetInfo {
             // information
             rustflags: env_args(
                 config,
-                requested_target,
+                requested_kind,
                 &rustc.host,
                 Some(&cfg),
                 kind,
                 "RUSTFLAGS",
             )?,
             rustdocflags: env_args(
                 config,
-                requested_target,
+                requested_kind,
                 &rustc.host,
                 Some(&cfg),
                 kind,
@@ -378,10 +370,10 @@ fn output_err_info(cmd: &ProcessBuilder, stdout: &str, stderr: &str) -> String {
 /// scripts, ...), even if it is the same as the target.
 fn env_args(
     config: &Config,
-    requested_target: Option<InternedString>,
+    requested_kind: CompileKind,
     host_triple: &str,
     target_cfg: Option<&[Cfg]>,
-    kind: Kind,
+    kind: CompileKind,
     name: &str,
 ) -> CargoResult<Vec<String>> {
     // We *want* to apply RUSTFLAGS only to builds for the
@@ -403,8 +395,7 @@ fn env_args(
     // This means that, e.g., even if the specified --target is the
     // same as the host, build scripts in plugins won't get
     // RUSTFLAGS.
-    let compiling_with_target = requested_target.is_some();
-    if compiling_with_target && kind.is_host() {
+    if !requested_kind.is_host() && kind.is_host() {
         // This is probably a build script or plugin and we're
         // compiling with --target. In this scenario there are
         // no rustflags we can apply.
@@ -428,10 +419,10 @@ fn env_args(
         .flat_map(|c| c.to_lowercase())
         .collect::<String>();
     // Then the target.*.rustflags value...
-    let target = requested_target
-        .as_ref()
-        .map(|s| s.as_str())
-        .unwrap_or(host_triple);
+    let target = match &kind {
+        CompileKind::Host => host_triple,
+        CompileKind::Target(target) => target.short_name(),
+    };
     let key = format!("target.{}.{}", target, name);
     if let Some(args) = config.get_list_or_split_string(&key)? {
         let args = args.val.into_iter();

src/cargo/core/compiler/build_plan.rs

@@ -12,7 +12,7 @@ use std::path::PathBuf;
 use serde::Serialize;
 
 use super::context::OutputFile;
-use super::{CompileMode, Context, Kind, Unit};
+use super::{CompileKind, CompileMode, Context, Unit};
 use crate::core::TargetKind;
 use crate::util::{internal, CargoResult, ProcessBuilder};
 
@@ -21,7 +21,7 @@ struct Invocation {
     package_name: String,
     package_version: semver::Version,
     target_kind: TargetKind,
-    kind: Kind,
+    kind: CompileKind,
     compile_mode: CompileMode,
     deps: Vec<usize>,
     outputs: Vec<PathBuf>,