From 55f0163fb2c4fce722c6e935bd974366d4a5d420 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Mon, 21 Aug 2023 10:37:37 +0800 Subject: [PATCH 1/3] Add support for `target.'cfg(..)'.linker` --- src/cargo/core/compiler/build_context/mod.rs | 10 ---- src/cargo/core/compiler/compilation.rs | 50 ++++++++++++++++++++ src/cargo/core/compiler/context/mod.rs | 2 +- src/cargo/core/compiler/custom_build.rs | 7 +-- src/cargo/core/compiler/fingerprint/mod.rs | 2 +- src/cargo/core/compiler/mod.rs | 5 +- src/cargo/util/config/target.rs | 1 + tests/testsuite/tool_paths.rs | 1 - 8 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/cargo/core/compiler/build_context/mod.rs b/src/cargo/core/compiler/build_context/mod.rs index f35084e2b7f..19dee718b02 100644 --- a/src/cargo/core/compiler/build_context/mod.rs +++ b/src/cargo/core/compiler/build_context/mod.rs @@ -10,7 +10,6 @@ use crate::util::errors::CargoResult; use crate::util::interning::InternedString; use crate::util::Rustc; use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; mod target_info; pub use self::target_info::{ @@ -120,15 +119,6 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> { &self.target_data.rustc } - /// Gets the user-specified linker for a particular host or target. - pub fn linker(&self, kind: CompileKind) -> Option { - self.target_data - .target_config(kind) - .linker - .as_ref() - .map(|l| l.val.clone().resolve_program(self.config)) - } - /// Gets the host architecture triple. /// /// For example, x86_64-unknown-linux-gnu, would be diff --git a/src/cargo/core/compiler/compilation.rs b/src/cargo/core/compiler/compilation.rs index b263119b039..e008bd0e685 100644 --- a/src/cargo/core/compiler/compilation.rs +++ b/src/cargo/core/compiler/compilation.rs @@ -104,6 +104,8 @@ pub struct Compilation<'cfg> { primary_rustc_process: Option, target_runners: HashMap)>>, + /// The linker to use for each host or target. + target_linkers: HashMap>, } impl<'cfg> Compilation<'cfg> { @@ -150,6 +152,13 @@ impl<'cfg> Compilation<'cfg> { .chain(Some(&CompileKind::Host)) .map(|kind| Ok((*kind, target_runner(bcx, *kind)?))) .collect::>>()?, + target_linkers: bcx + .build_config + .requested_kinds + .iter() + .chain(Some(&CompileKind::Host)) + .map(|kind| Ok((*kind, target_linker(bcx, *kind)?))) + .collect::>>()?, }) } @@ -221,6 +230,11 @@ impl<'cfg> Compilation<'cfg> { 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 { + 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`. @@ -441,3 +455,39 @@ 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> { + // 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.config)) + { + return Ok(Some(path)); + } + + // Try target.'cfg(...)'.linker. + let target_cfg = bcx.target_data.info(kind).cfg(); + let mut cfgs = bcx + .config + .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.config))) +} diff --git a/src/cargo/core/compiler/context/mod.rs b/src/cargo/core/compiler/context/mod.rs index 3f13f086c95..010fe2793dd 100644 --- a/src/cargo/core/compiler/context/mod.rs +++ b/src/cargo/core/compiler/context/mod.rs @@ -272,7 +272,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { unit: unit.clone(), args, unstable_opts, - linker: self.bcx.linker(unit.kind), + linker: self.compilation.target_linker(unit.kind).clone(), script_meta, env: artifact::get_env(&self, self.unit_deps(unit))?, }); diff --git a/src/cargo/core/compiler/custom_build.rs b/src/cargo/core/compiler/custom_build.rs index 85306aaac87..ca6c29c3d1e 100644 --- a/src/cargo/core/compiler/custom_build.rs +++ b/src/cargo/core/compiler/custom_build.rs @@ -299,11 +299,8 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult { cmd.env(&var, value); } - if let Some(linker) = &bcx.target_data.target_config(unit.kind).linker { - cmd.env( - "RUSTC_LINKER", - linker.val.clone().resolve_program(bcx.config), - ); + if let Some(linker) = &cx.compilation.target_linker(unit.kind) { + cmd.env("RUSTC_LINKER", linker); } if let Some(links) = unit.pkg.manifest().links() { diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs index 2e6fb7eed07..1a2dfe4eee5 100644 --- a/src/cargo/core/compiler/fingerprint/mod.rs +++ b/src/cargo/core/compiler/fingerprint/mod.rs @@ -1426,7 +1426,7 @@ fn calculate_normal(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult, cmd: &mut ProcessBuilder, unit: &Unit) cmd, "-C", "linker=", - bcx.linker(unit.kind).as_ref().map(|s| s.as_ref()), + cx.compilation + .target_linker(unit.kind) + .as_ref() + .map(|s| s.as_ref()), ); if incremental { let dir = cx.files().layout(unit.kind).incremental().as_os_str(); diff --git a/src/cargo/util/config/target.rs b/src/cargo/util/config/target.rs index b8aaf906d6f..6d6a6beffe6 100644 --- a/src/cargo/util/config/target.rs +++ b/src/cargo/util/config/target.rs @@ -12,6 +12,7 @@ use std::path::PathBuf; pub struct TargetCfgConfig { pub runner: OptValue, pub rustflags: OptValue, + pub linker: OptValue, // This is here just to ignore fields from normal `TargetConfig` because // all `[target]` tables are getting deserialized, whether they start with // `cfg(` or not. diff --git a/tests/testsuite/tool_paths.rs b/tests/testsuite/tool_paths.rs index a211b532837..8ddd358c6b1 100644 --- a/tests/testsuite/tool_paths.rs +++ b/tests/testsuite/tool_paths.rs @@ -393,7 +393,6 @@ fn cfg_ignored_fields() { [WARNING] unused key `ar` in [target] config table `cfg(not(target_os = \"none\"))` [WARNING] unused key `foo` in [target] config table `cfg(not(target_os = \"none\"))` [WARNING] unused key `invalid` in [target] config table `cfg(not(target_os = \"none\"))` -[WARNING] unused key `linker` in [target] config table `cfg(not(target_os = \"none\"))` [CHECKING] foo v0.0.1 ([..]) [FINISHED] [..] ", From 7cbc858b71925684e6c360e508743ebfc9df6b41 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 22 Aug 2023 11:01:33 +0800 Subject: [PATCH 2/3] Add some tests --- tests/testsuite/build_script.rs | 37 ++++++++++++++ tests/testsuite/tool_paths.rs | 87 +++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/tests/testsuite/build_script.rs b/tests/testsuite/build_script.rs index 400d10547c4..0ccbb4e27d7 100644 --- a/tests/testsuite/build_script.rs +++ b/tests/testsuite/build_script.rs @@ -453,6 +453,43 @@ fn custom_build_env_var_rustc_linker() { p.cargo("build --target").arg(&target).run(); } +// Only run this test on linux, since it's difficult to construct +// a case suitable for all platforms. +// See:https://github.com/rust-lang/cargo/pull/12535#discussion_r1306618264 +#[cargo_test] +#[cfg(target_os = "linux")] +fn custom_build_env_var_rustc_linker_with_target_cfg() { + if cross_compile::disabled() { + return; + } + + let target = cross_compile::alternate(); + let p = project() + .file( + ".cargo/config", + r#" + [target.'cfg(target_pointer_width = "32")'] + linker = "/path/to/linker" + "#, + ) + .file( + "build.rs", + r#" + use std::env; + + fn main() { + assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/linker")); + } + "#, + ) + .file("src/lib.rs", "") + .build(); + + // no crate type set => linker never called => build succeeds if and + // only if build.rs succeeds, despite linker binary not existing. + p.cargo("build --target").arg(&target).run(); +} + #[cargo_test] fn custom_build_env_var_rustc_linker_bad_host_target() { let target = rustc_host(); diff --git a/tests/testsuite/tool_paths.rs b/tests/testsuite/tool_paths.rs index 8ddd358c6b1..5428f9d016b 100644 --- a/tests/testsuite/tool_paths.rs +++ b/tests/testsuite/tool_paths.rs @@ -32,6 +32,93 @@ fn pathless_tools() { .run(); } +// can set a custom linker via `target.'cfg(..)'.linker` +#[cargo_test] +fn custom_linker_cfg() { + let foo = project() + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [target.'cfg(not(target_os = "none"))'] + linker = "nonexistent-linker" + "#, + ) + .build(); + + foo.cargo("build --verbose") + .with_stderr( + "\ +[COMPILING] foo v0.5.0 ([CWD]) +[RUNNING] `rustc [..] -C linker=nonexistent-linker [..]` +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +// custom linker set via `target.$triple.linker` have precede over `target.'cfg(..)'.linker` +#[cargo_test] +fn custom_linker_cfg_precedence() { + let target = rustc_host(); + + let foo = project() + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + &format!( + r#" + [target.'cfg(not(target_os = "none"))'] + linker = "ignored-linker" + [target.{}] + linker = "nonexistent-linker" + "#, + target + ), + ) + .build(); + + foo.cargo("build --verbose") + .with_stderr( + "\ +[COMPILING] foo v0.5.0 ([CWD]) +[RUNNING] `rustc [..] -C linker=nonexistent-linker [..]` +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..] +", + ) + .run(); +} + +#[cargo_test] +fn custom_linker_cfg_collision() { + let foo = project() + .file("Cargo.toml", &basic_lib_manifest("foo")) + .file("src/lib.rs", "") + .file( + ".cargo/config", + r#" + [target.'cfg(not(target_arch = "avr"))'] + linker = "nonexistent-linker1" + [target.'cfg(not(target_os = "none"))'] + linker = "nonexistent-linker2" + "#, + ) + .build(); + + foo.cargo("build --verbose") + .with_status(101) + .with_stderr(&format!( + "\ +[ERROR] several matching instances of `target.'cfg(..)'.linker` in configurations +first match `cfg(not(target_arch = \"avr\"))` located in [..]/foo/.cargo/config +second match `cfg(not(target_os = \"none\"))` located in [..]/foo/.cargo/config +", + )) + .run(); +} + #[cargo_test] fn absolute_tools() { let target = rustc_host(); From c9cc3b99daaae385cb7df462ed89235f3f65b714 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 25 Aug 2023 00:22:30 +0800 Subject: [PATCH 3/3] Add `target..linker` docs --- src/doc/src/reference/config.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/doc/src/reference/config.md b/src/doc/src/reference/config.md index d1f2b04d35e..3a21039b5ba 100644 --- a/src/doc/src/reference/config.md +++ b/src/doc/src/reference/config.md @@ -1098,6 +1098,12 @@ This option is deprecated and unused. Specifies the linker which is passed to `rustc` (via [`-C linker`]) when the [``] is being compiled for. By default, the linker is not overridden. +##### `target..linker` +This is similar to the [target linker](#targettriplelinker), but using +a [`cfg()` expression]. If both a [``] and `` runner match, +the `` will take precedence. It is an error if more than one +`` runner matches the current target. + ##### `target..runner` * Type: string or array of strings ([program path with args]) * Default: none