Add -Z json-target-spec (#16557)

This adds the `-Z json-target-spec` CLI flag to assist with using custom
`.json` target spec files. `rustc` recently switched so that it requires
`-Z unstable-options` when using custom spec files
(rust-lang/rust#151534). This can make it rather
awkward to use spec files with cargo because it then requires setting
RUSTFLAGS and RUSTDOCFLAGS to pass `-Zunstable-options`. It also ends up
causing some confusing error messages.

See the individual commits for more details.

This ended up being a larger diff than I wanted. I'm not 100% certain
this is worth it, but I think it significantly improves the experience
using `.json` files, so I I'm leaning towards it.

### How to test and review this PR?

Testing can be done with `rustc` built from
rust-lang/rust#151534 to ensure that everything
passes after that PR (including setting `CARGO_RUN_BUILD_STD_TESTS=1`).

Author: epage

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

@@ -200,9 +200,7 @@ impl TargetInfo {
                 process.inherit_jobserver(client);
             }
 
-            if let CompileKind::Target(target) = kind {
-                process.arg("--target").arg(target.rustc_target());
-            }
+            kind.add_target_arg(&mut process);
 
             let crate_type_process = process.clone();
             const KNOWN_CRATE_TYPES: &[CrateType] = &[
@@ -331,11 +329,7 @@ impl TargetInfo {
                     .args(&rustflags)
                     .env_remove("RUSTC_LOG");
 
-                if let CompileKind::Target(target) = kind {
-                    target_spec_process
-                        .arg("--target")
-                        .arg(target.rustc_target());
-                }
+                kind.add_target_arg(&mut target_spec_process);
 
                 #[derive(Deserialize)]
                 struct Metadata {
@@ -965,7 +959,7 @@ impl<'gctx> RustcTargetData<'gctx> {
         let mut target_config = HashMap::new();
         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_target = CompileTarget::new(&rustc.host, gctx.cli_unstable().json_target_spec)?;
         let host_info = TargetInfo::new(gctx, requested_kinds, &rustc, CompileKind::Host)?;
 
         // This config is used for link overrides and choosing a linker.

src/cargo/core/compiler/compile_kind.rs

@@ -5,6 +5,8 @@ use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
 use crate::util::{GlobalContext, StableHasher, try_canonicalize};
 use anyhow::Context as _;
+use anyhow::bail;
+use cargo_util::ProcessBuilder;
 use serde::Serialize;
 use std::collections::BTreeSet;
 use std::fs;
@@ -92,9 +94,15 @@ impl CompileKind {
 
                     if value.as_str() == "host-tuple" {
                         let host_triple = env!("RUST_HOST_TARGET");
-                        Ok(CompileKind::Target(CompileTarget::new(host_triple)?))
+                        Ok(CompileKind::Target(CompileTarget::new(
+                            host_triple,
+                            gctx.cli_unstable().json_target_spec,
+                        )?))
                     } else {
-                        Ok(CompileKind::Target(CompileTarget::new(value.as_str())?))
+                        Ok(CompileKind::Target(CompileTarget::new(
+                            value.as_str(),
+                            gctx.cli_unstable().json_target_spec,
+                        )?))
                     }
                 })
                 // First collect into a set to deduplicate any `--target` passed
@@ -132,6 +140,17 @@ impl CompileKind {
             CompileKind::Target(target) => target.fingerprint_hash(),
         }
     }
+
+    /// Adds the `--target` flag to the given [`ProcessBuilder`] if this is a
+    /// non-host build.
+    pub fn add_target_arg(&self, builder: &mut ProcessBuilder) {
+        if let CompileKind::Target(target) = self {
+            builder.arg("--target").arg(target.rustc_target());
+            if matches!(target, CompileTarget::Json { .. }) {
+                builder.arg("-Zunstable-options");
+            }
+        }
+    }
 }
 
 impl serde::ser::Serialize for CompileKind {
@@ -141,7 +160,7 @@ impl serde::ser::Serialize for CompileKind {
     {
         match self {
             CompileKind::Host => None::<&str>.serialize(s),
-            CompileKind::Target(t) => Some(t.name).serialize(s),
+            CompileKind::Target(t) => Some(t.rustc_target()).serialize(s),
         }
     }
 }
@@ -164,31 +183,39 @@ impl serde::ser::Serialize for CompileKind {
 /// file stem of JSON target files. For built-in rustc targets this is just an
 /// uninterpreted string basically.
 #[derive(PartialEq, Eq, Hash, Debug, Clone, Copy, PartialOrd, Ord, Serialize)]
-pub struct CompileTarget {
-    name: InternedString,
+pub enum CompileTarget {
+    Tuple(InternedString),
+    Json {
+        short: InternedString,
+        path: InternedString,
+    },
 }
 
 impl CompileTarget {
-    pub fn new(name: &str) -> CargoResult<CompileTarget> {
+    pub fn new(name: &str, unstable_json: bool) -> CargoResult<CompileTarget> {
         let name = name.trim();
         if name.is_empty() {
-            anyhow::bail!("target was empty");
+            bail!("target was empty");
         }
         if !name.ends_with(".json") {
-            return Ok(CompileTarget { name: name.into() });
+            return Ok(CompileTarget::Tuple(name.into()));
+        }
+
+        if !unstable_json {
+            bail!("`.json` target specs require -Zjson-target-spec");
         }
 
         // If `name` ends in `.json` then it's likely a custom target
         // specification. Canonicalize the path to ensure that different builds
         // with different paths always produce the same result.
-        let path = try_canonicalize(Path::new(name))
-            .with_context(|| format!("target path {:?} is not a valid file", name))?;
-
-        let name = path
-            .into_os_string()
-            .into_string()
-            .map_err(|_| anyhow::format_err!("target path is not valid unicode"))?;
-        Ok(CompileTarget { name: name.into() })
+        let p = try_canonicalize(Path::new(name))
+            .with_context(|| format!("target path `{name}` is not a valid file"))?;
+        let path = p
+            .to_str()
+            .ok_or_else(|| anyhow::format_err!("target path `{name}` is not valid unicode"))?
+            .into();
+        let short = p.file_stem().unwrap().to_str().unwrap().into();
+        Ok(CompileTarget::Json { short, path })
     }
 
     /// Returns the full unqualified name of this target, suitable for passing
@@ -198,7 +225,10 @@ impl CompileTarget {
     /// of JSON target files this will be a full canonicalized path name for the
     /// current filesystem.
     pub fn rustc_target(&self) -> InternedString {
-        self.name
+        match self {
+            CompileTarget::Tuple(name) => *name,
+            CompileTarget::Json { path, .. } => *path,
+        }
     }
 
     /// Returns a "short" version of the target name suitable for usage within
@@ -208,34 +238,25 @@ impl CompileTarget {
     /// JSON target files this returns just the file stem (e.g. `foo` out of
     /// `foo.json`) instead of the full path.
     pub fn short_name(&self) -> &str {
-        // Flexible target specifications often point at json files, so if it
-        // looks like we've got one of those just use the file stem (the file
-        // name without ".json") as a short name for this target. Note that the
-        // `unwrap()` here should never trigger since we have a nonempty name
-        // and it starts as utf-8 so it's always utf-8
-        if self.name.ends_with(".json") {
-            Path::new(&self.name).file_stem().unwrap().to_str().unwrap()
-        } else {
-            &self.name
+        match self {
+            CompileTarget::Tuple(name) => name,
+            CompileTarget::Json { short, .. } => short,
         }
     }
 
     /// See [`CompileKind::fingerprint_hash`].
     pub fn fingerprint_hash(&self) -> u64 {
         let mut hasher = StableHasher::new();
-        match self
-            .name
-            .ends_with(".json")
-            .then(|| fs::read_to_string(self.name))
-        {
-            Some(Ok(contents)) => {
+        match self {
+            CompileTarget::Tuple(name) => name.hash(&mut hasher),
+            CompileTarget::Json { path, .. } => {
                 // This may have some performance concerns, since it is called
                 // fairly often. If that ever seems worth fixing, consider
                 // embedding this in `CompileTarget`.
-                contents.hash(&mut hasher);
-            }
-            _ => {
-                self.name.hash(&mut hasher);
+                match fs::read_to_string(path) {
+                    Ok(contents) => contents.hash(&mut hasher),
+                    Err(_) => path.hash(&mut hasher),
+                }
             }
         }
         Hasher::finish(&hasher)

src/cargo/core/compiler/mod.rs

@@ -864,9 +864,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu
     add_path_args(bcx.ws, unit, &mut rustdoc);
     add_cap_lints(bcx, unit, &mut rustdoc);
 
-    if let CompileKind::Target(target) = unit.kind {
-        rustdoc.arg("--target").arg(target.rustc_target());
-    }
+    unit.kind.add_target_arg(&mut rustdoc);
     let doc_dir = build_runner.files().out_dir(unit);
     rustdoc.arg("-o").arg(&doc_dir);
     rustdoc.args(&features_args(unit));
@@ -1414,9 +1412,7 @@ fn build_base_args(
         }
     }
 
-    if let CompileKind::Target(n) = unit.kind {
-        cmd.arg("--target").arg(n.rustc_target());
-    }
+    unit.kind.add_target_arg(cmd);
 
     opt(
         cmd,

src/cargo/core/dependency.rs

@@ -496,6 +496,7 @@ impl Artifact {
         artifacts: &[impl AsRef<str>],
         is_lib: bool,
         target: Option<&str>,
+        unstable_json: bool,
     ) -> CargoResult<Self> {
         let kinds = ArtifactKind::validate(
             artifacts
@@ -506,7 +507,9 @@ impl Artifact {
         Ok(Artifact {
             inner: Arc::new(kinds),
             is_lib,
-            target: target.map(ArtifactTarget::parse).transpose()?,
+            target: target
+                .map(|name| ArtifactTarget::parse(name, unstable_json))
+                .transpose()?,
         })
     }
 
@@ -536,10 +539,10 @@ pub enum ArtifactTarget {
 }
 
 impl ArtifactTarget {
-    pub fn parse(target: &str) -> CargoResult<ArtifactTarget> {
+    pub fn parse(target: &str, unstable_json: bool) -> CargoResult<ArtifactTarget> {
         Ok(match target {
             "target" => ArtifactTarget::BuildDependencyAssumeTarget,
-            name => ArtifactTarget::Force(CompileTarget::new(name)?),
+            name => ArtifactTarget::Force(CompileTarget::new(name, unstable_json)?),
         })
     }
 

src/cargo/core/features.rs

@@ -870,6 +870,7 @@ unstable_cli_options!(
     #[serde(deserialize_with = "deserialize_gitoxide_features")]
     gitoxide: Option<GitoxideFeatures> = ("Use gitoxide for the given git interactions, or all of them if no argument is given"),
     host_config: bool = ("Enable the `[host]` section in the .cargo/config.toml file"),
+    json_target_spec: bool = ("Enable `.json` target spec files"),
     lockfile_path: bool = ("Enable the `resolver.lockfile-path` config option"),
     minimal_versions: bool = ("Resolve minimal dependency versions instead of maximum"),
     msrv_policy: bool = ("Enable rust-version aware policy within cargo"),
@@ -1409,6 +1410,7 @@ impl CliUnstable {
                 )?
             }
             "host-config" => self.host_config = parse_empty(k, v)?,
+            "json-target-spec" => self.json_target_spec = parse_empty(k, v)?,
             "lockfile-path" => self.lockfile_path = parse_empty(k, v)?,
             "next-lockfile-bump" => self.next_lockfile_bump = parse_empty(k, v)?,
             "minimal-versions" => self.minimal_versions = parse_empty(k, v)?,

src/cargo/core/resolver/features.rs

@@ -811,6 +811,7 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
             pkg_id: PackageId,
             dep: &Dependency,
             lib_fk: FeaturesFor,
+            unstable_json_spec: bool,
         ) -> CargoResult<Vec<FeaturesFor>> {
             let Some(artifact) = dep.artifact() else {
                 return Ok(vec![lib_fk]);
@@ -844,7 +845,9 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
                     ArtifactTarget::BuildDependencyAssumeTarget => {
                         for kind in this.requested_targets {
                             let target = match kind {
-                                CompileKind::Host => CompileTarget::new(&host_triple).unwrap(),
+                                CompileKind::Host => {
+                                    CompileTarget::new(&host_triple, unstable_json_spec).unwrap()
+                                }
                                 CompileKind::Target(target) => *target,
                             };
                             activate_target(target)?;
@@ -859,6 +862,7 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
             Ok(result)
         }
 
+        let unstable_json_spec = self.ws.gctx().cli_unstable().json_target_spec;
         self.resolve
             .deps(pkg_id)
             .map(|(dep_id, deps)| {
@@ -915,7 +919,8 @@ impl<'a, 'gctx> FeatureResolver<'a, 'gctx> {
                             fk
                         };
 
-                        let dep_fks = artifact_features_for(self, pkg_id, dep, lib_fk)?;
+                        let dep_fks =
+                            artifact_features_for(self, pkg_id, dep, lib_fk, unstable_json_spec)?;
                         Ok(dep_fks.into_iter().map(move |dep_fk| (dep, dep_fk)))
                     })
                     .flatten_ok()

src/cargo/ops/cargo_compile/mod.rs

@@ -221,9 +221,7 @@ pub fn print<'a>(
         if let Some(args) = target_rustc_args {
             process.args(args);
         }
-        if let CompileKind::Target(t) = kind {
-            process.arg("--target").arg(t.rustc_target());
-        }
+        kind.add_target_arg(&mut process);
         process.arg("--print").arg(print_opt_value);
         process.exec()?;
     }
@@ -400,7 +398,10 @@ pub fn create_bcx<'a, 'gctx>(
     // If `--target` has not been specified, then the unit graph is built
     // assuming `--target $HOST` was specified. See
     // `rebuild_unit_graph_shared` for more on why this is done.
-    let explicit_host_kind = CompileKind::Target(CompileTarget::new(&target_data.rustc.host)?);
+    let explicit_host_kind = CompileKind::Target(CompileTarget::new(
+        &target_data.rustc.host,
+        gctx.cli_unstable().json_target_spec,
+    )?);
     let explicit_host_kinds: Vec<_> = build_config
         .requested_kinds
         .iter()

src/cargo/ops/cargo_test.rs

@@ -1,4 +1,4 @@
-use crate::core::compiler::{Compilation, CompileKind, Doctest, Unit, UnitHash, UnitOutput};
+use crate::core::compiler::{Compilation, Doctest, Unit, UnitHash, UnitOutput};
 use crate::core::profiles::PanicStrategy;
 use crate::core::shell::ColorChoice;
 use crate::core::shell::Verbosity;
@@ -211,10 +211,7 @@ fn run_doc_tests(
         add_path_args(ws, unit, &mut p);
         p.arg("--test-run-directory").arg(unit.pkg.root());
 
-        if let CompileKind::Target(target) = unit.kind {
-            // use `rustc_target()` to properly handle JSON target paths
-            p.arg("--target").arg(target.rustc_target());
-        }
+        unit.kind.add_target_arg(&mut p);
 
         if let Some((runtool, runtool_args)) = compilation.target_runner(unit.kind) {
             p.arg("--test-runtool").arg(runtool);