Skip to content

Commit 8457356

Browse files
borsbors
committed
Auto merge of #13900 - gmorenz:target_applies_to_host_rustflags, r=weihanglo
Pass rustflags to artifacts built with implicit targets when using target-applies-to-host ## What this PR does This fixes #10744, a long-standing bug with `target-applies-to-host=false` where `RUSTFLAGS` are not being passed to artifact-units when built with `cargo build` (as opposed to `cargo build --target <host>`). It doesn't fix a similar problem with `linker` and `links`. If the architecture in this PR is accepted, I expect I will be able to fix `linker` and `links` in the same way in a subsequent PR. Below is a hopefully useful table of what flags are passed when, with new behavior bolded (without these changes the flag is not passed). I've only changed values in the top right cell, I've included the whole table for completeness and to hopefully make clear what exactly this feature is doing (which took me awhile to understand). The table was generated with a host of x86_64-unknown-linux-gnu. "Flag" refers to values in RUSTFLAGS. "Linker" refers to the value of `--config target.<host>.linker` . The table was built with [this repo](https://github.com/gmorenz/target_application_to_host_matrix) and then hand modify to bold changed values. | | target_applies_to_host=true | target_applies_to_host=false | |-|-|-| | no --target flag | Flag passed to bin<br/>Flag passed to shared dep from bin<br/>Linker passed to bin<br/><br/>Flag passed to proc macro<br/>Flag passed to shared dep from proc macro<br/>Linker passed to proc macro<br/><br/>Built with 5 invocations<br/>Without rustflags, built with 5 invocations<br/> | **Flag passed to bin**<br/>**Flag passed to shared dep from bin**<br/>Linker not passed to bin<br/><br/>Flag not passed to proc macro<br/>Flag not passed to shared dep from proc macro<br/>Linker not passed to proc macro<br/><br/>**Built with 6 invocations**<br/>Without rustflags, built with 5 invocations<br/> | | --target x86_64-unknown-linux-gnu | Flag passed to bin<br/>Flag passed to shared dep from bin<br/>Linker passed to bin<br/><br/>Flag not passed to proc macro<br/>Flag not passed to shared dep from proc macro<br/>Linker passed to proc macro<br/><br/>Built with 6 invocations<br/>Without rustflags, built with 6 invocations<br/> | Flag passed to bin<br/>Flag passed to shared dep from bin<br/>Linker passed to bin<br/><br/>Flag not passed to proc macro<br/>Flag not passed to shared dep from proc macro<br/>Linker not passed to proc macro<br/><br/>Built with 6 invocations<br/>Without rustflags, built with 6 invocations<br/> | | --target i686-unknown-linux-gnu | Flag passed to bin<br/>Flag passed to shared dep from bin<br/>Linker not passed to bin<br/><br/>Flag not passed to proc macro<br/>Flag not passed to shared dep from proc macro<br/>Linker passed to proc macro<br/><br/>Built with 6 invocations<br/>Without rustflags, built with 6 invocations<br/> | Flag passed to bin<br/>Flag passed to shared dep from bin<br/>Linker not passed to bin<br/><br/>Flag not passed to proc macro<br/>Flag not passed to shared dep from proc macro<br/>Linker not passed to proc macro<br/><br/>Built with 6 invocations<br/>Without rustflags, built with 6 invocations<br/> | ## How it is implemented There are two stages in the `UnitGraph`'s life. It gets built, with correct `CompileKind`: `CompileKind::Host` for host units, `CompileKind::Target(HostTriple)` for artifact units. Then it gets rebuilt with artifact units that are intended for the host having their kind changed to `CompileKind::Host`. This rebuilding step serves to de-duplicate host and artifact units. A step that we want to maintain where possible. Because de-duplicating host and artifact dependencies is only possible if their `rustflags` don't differ we must calculate `rustflags` for units before this step, and this step necessarily throws away the information necessary to calculate them. The possible rustflags have already been determined before creating units. "Calculating rustflags for units" just means determining which set of rustflags go with a particular unit, and storing that somewhere. The obvious place to do that is in the unit itself, so that's what this PR does, which has the convenient side affect of also handing the de-duplication logic for us. If the rustflags are the same, two units can be merged, if they differ, they cannot. In the above table that's why it takes 5 invocations to build the repo without rustflags, but 6 with them. The shared_dependency merges when it is built without rustflags, and not when it is built with rustflags. ## Related PRs, comments and issues fixes #10744 #9453 - Tracking issue #9753 - Stabilization PR #9634 - I believe this was another attempt (going down another implementation route) to fix the same issue Comments from Alex Crichton noting that this must be fixed [1](#10395 (comment)) [2](#9753 (comment)) [My own comment describing exactly how I ran into this](#9753 (comment)) [Documentation for the feature](https://doc.rust-lang.org/cargo/reference/unstable.html#target-applies-to-host)
2 parents 3a76b8c + 5be8044 commit 8457356

File tree

12 files changed

+136
-47
lines changed

12 files changed

+136
-47
lines changed

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

-26
Original file line numberDiff line numberDiff line change
@@ -134,32 +134,6 @@ impl<'a, 'gctx> BuildContext<'a, 'gctx> {
134134
self.build_config.jobs
135135
}
136136

137-
/// Extra compiler flags to pass to `rustc` for a given unit.
138-
///
139-
/// Although it depends on the caller, in the current Cargo implementation,
140-
/// these flags take precedence over those from [`BuildContext::extra_args_for`].
141-
///
142-
/// As of now, these flags come from environment variables and configurations.
143-
/// See [`TargetInfo.rustflags`] for more on how Cargo collects them.
144-
///
145-
/// [`TargetInfo.rustflags`]: TargetInfo::rustflags
146-
pub fn rustflags_args(&self, unit: &Unit) -> &[String] {
147-
&self.target_data.info(unit.kind).rustflags
148-
}
149-
150-
/// Extra compiler flags to pass to `rustdoc` for a given unit.
151-
///
152-
/// Although it depends on the caller, in the current Cargo implementation,
153-
/// these flags take precedence over those from [`BuildContext::extra_args_for`].
154-
///
155-
/// As of now, these flags come from environment variables and configurations.
156-
/// See [`TargetInfo.rustdocflags`] for more on how Cargo collects them.
157-
///
158-
/// [`TargetInfo.rustdocflags`]: TargetInfo::rustdocflags
159-
pub fn rustdocflags_args(&self, unit: &Unit) -> &[String] {
160-
&self.target_data.info(unit.kind).rustdocflags
161-
}
162-
163137
/// Extra compiler args for either `rustc` or `rustdoc`.
164138
///
165139
/// As of now, these flags come from the trailing args of either

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

+28-8
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use std::cell::RefCell;
2323
use std::collections::hash_map::{Entry, HashMap};
2424
use std::path::{Path, PathBuf};
2525
use std::str::{self, FromStr};
26+
use std::sync::Arc;
2627

2728
/// Information about the platform target gleaned from querying rustc.
2829
///
@@ -52,9 +53,9 @@ pub struct TargetInfo {
5253
/// target libraries.
5354
pub sysroot_target_libdir: PathBuf,
5455
/// Extra flags to pass to `rustc`, see [`extra_args`].
55-
pub rustflags: Vec<String>,
56+
pub rustflags: Arc<[String]>,
5657
/// Extra flags to pass to `rustdoc`, see [`extra_args`].
57-
pub rustdocflags: Vec<String>,
58+
pub rustdocflags: Arc<[String]>,
5859
/// Whether or not rustc (stably) supports the `--check-cfg` flag.
5960
///
6061
/// Can be removed once the minimum supported rustc version of Cargo is
@@ -312,15 +313,16 @@ impl TargetInfo {
312313
crate_types: RefCell::new(map),
313314
sysroot,
314315
sysroot_target_libdir,
315-
rustflags,
316+
rustflags: rustflags.into(),
316317
rustdocflags: extra_args(
317318
gctx,
318319
requested_kinds,
319320
&rustc.host,
320321
Some(&cfg),
321322
kind,
322323
Flags::Rustdoc,
323-
)?,
324+
)?
325+
.into(),
324326
cfg,
325327
support_split_debuginfo,
326328
support_check_cfg,
@@ -867,7 +869,10 @@ pub struct RustcTargetData<'gctx> {
867869

868870
/// Build information for the "host", which is information about when
869871
/// `rustc` is invoked without a `--target` flag. This is used for
870-
/// procedural macros, build scripts, etc.
872+
/// selecting a linker, and applying link overrides.
873+
///
874+
/// The configuration read into this depends on whether or not
875+
/// `target-applies-to-host=true`.
871876
host_config: TargetConfig,
872877
/// Information about the host platform.
873878
host_info: TargetInfo,
@@ -889,7 +894,10 @@ impl<'gctx> RustcTargetData<'gctx> {
889894
let mut target_config = HashMap::new();
890895
let mut target_info = HashMap::new();
891896
let target_applies_to_host = gctx.target_applies_to_host()?;
897+
let host_target = CompileTarget::new(&rustc.host)?;
892898
let host_info = TargetInfo::new(gctx, requested_kinds, &rustc, CompileKind::Host)?;
899+
900+
// This config is used for link overrides and choosing a linker.
893901
let host_config = if target_applies_to_host {
894902
gctx.target_cfg_triple(&rustc.host)?
895903
} else {
@@ -902,9 +910,21 @@ impl<'gctx> RustcTargetData<'gctx> {
902910
// needs access to the target config data, create a copy so that it
903911
// can be found. See `rebuild_unit_graph_shared` for why this is done.
904912
if requested_kinds.iter().any(CompileKind::is_host) {
905-
let ct = CompileTarget::new(&rustc.host)?;
906-
target_info.insert(ct, host_info.clone());
907-
target_config.insert(ct, gctx.target_cfg_triple(&rustc.host)?);
913+
target_config.insert(host_target, gctx.target_cfg_triple(&rustc.host)?);
914+
915+
// If target_applies_to_host is true, the host_info is the target info,
916+
// otherwise we need to build target info for the target.
917+
if target_applies_to_host {
918+
target_info.insert(host_target, host_info.clone());
919+
} else {
920+
let host_target_info = TargetInfo::new(
921+
gctx,
922+
requested_kinds,
923+
&rustc,
924+
CompileKind::Target(host_target),
925+
)?;
926+
target_info.insert(host_target, host_target_info);
927+
}
908928
};
909929

910930
let mut res = RustcTargetData {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
269269
}
270270
}
271271
}
272-
args.extend(self.bcx.rustdocflags_args(unit).iter().map(Into::into));
272+
args.extend(unit.rustdocflags.iter().map(Into::into));
273273

274274
use super::MessageFormat;
275275
let format = match self.bcx.build_config.message_format {

‎src/cargo/core/compiler/custom_build.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,7 @@ fn build_work(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResul
352352
cmd.env("RUSTC_WORKSPACE_WRAPPER", wrapper);
353353
}
354354
}
355-
cmd.env(
356-
"CARGO_ENCODED_RUSTFLAGS",
357-
bcx.rustflags_args(unit).join("\x1f"),
358-
);
355+
cmd.env("CARGO_ENCODED_RUSTFLAGS", unit.rustflags.join("\x1f"));
359356
cmd.env_remove("RUSTFLAGS");
360357

361358
if build_runner.bcx.ws.gctx().extra_verbose() {

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -1415,9 +1415,9 @@ fn calculate_normal(
14151415
// hashed to take up less space on disk as we just need to know when things
14161416
// change.
14171417
let extra_flags = if unit.mode.is_doc() || unit.mode.is_doc_scrape() {
1418-
build_runner.bcx.rustdocflags_args(unit)
1418+
&unit.rustdocflags
14191419
} else {
1420-
build_runner.bcx.rustflags_args(unit)
1420+
&unit.rustflags
14211421
}
14221422
.to_vec();
14231423

@@ -1512,7 +1512,7 @@ fn calculate_run_custom_build(
15121512
An I/O error happened. Please make sure you can access the file.
15131513
15141514
By default, if your project contains a build script, cargo scans all files in
1515-
it to determine whether a rebuild is needed. If you don't expect to access the
1515+
it to determine whether a rebuild is needed. If you don't expect to access the
15161516
file, specify `rerun-if-changed` in your build script.
15171517
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-changed for more information.";
15181518
pkg_fingerprint(build_runner.bcx, &unit.pkg).map_err(|err| {
@@ -1542,7 +1542,7 @@ See https://doc.rust-lang.org/cargo/reference/build-scripts.html#rerun-if-change
15421542
.collect::<CargoResult<Vec<_>>>()?
15431543
};
15441544

1545-
let rustflags = build_runner.bcx.rustflags_args(unit).to_vec();
1545+
let rustflags = unit.rustflags.to_vec();
15461546

15471547
Ok(Fingerprint {
15481548
local: Mutex::new(local),

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ fn prepare_rustc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResult
683683
base.inherit_jobserver(&build_runner.jobserver);
684684
build_deps_args(&mut base, build_runner, unit)?;
685685
add_cap_lints(build_runner.bcx, unit, &mut base);
686-
base.args(build_runner.bcx.rustflags_args(unit));
686+
base.args(&unit.rustflags);
687687
if build_runner.bcx.gctx.cli_unstable().binary_dep_depinfo {
688688
base.arg("-Z").arg("binary-dep-depinfo");
689689
}
@@ -780,7 +780,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu
780780

781781
rustdoc::add_output_format(build_runner, unit, &mut rustdoc)?;
782782

783-
rustdoc.args(bcx.rustdocflags_args(unit));
783+
rustdoc.args(&unit.rustdocflags);
784784

785785
if !crate_version_flag_already_present(&rustdoc) {
786786
append_crate_version_flag(unit, &mut rustdoc);

‎src/cargo/core/compiler/standard_lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ pub fn generate_std_roots(
178178
package_set: &PackageSet<'_>,
179179
interner: &UnitInterner,
180180
profiles: &Profiles,
181+
target_data: &RustcTargetData<'_>,
181182
) -> CargoResult<HashMap<CompileKind, Vec<Unit>>> {
182183
// Generate the root Units for the standard library.
183184
let std_ids = crates
@@ -216,6 +217,8 @@ pub fn generate_std_roots(
216217
*kind,
217218
mode,
218219
features.clone(),
220+
target_data.info(*kind).rustflags.clone(),
221+
target_data.info(*kind).rustdocflags.clone(),
219222
/*is_std*/ true,
220223
/*dep_hash*/ 0,
221224
IsArtifact::No,

‎src/cargo/core/compiler/unit.rs

+28
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::fmt;
1414
use std::hash::{Hash, Hasher};
1515
use std::ops::Deref;
1616
use std::rc::Rc;
17+
use std::sync::Arc;
1718

1819
/// All information needed to define a unit.
1920
///
@@ -59,6 +60,28 @@ pub struct UnitInner {
5960
/// The `cfg` features to enable for this unit.
6061
/// This must be sorted.
6162
pub features: Vec<InternedString>,
63+
/// Extra compiler flags to pass to `rustc` for a given unit.
64+
///
65+
/// Although it depends on the caller, in the current Cargo implementation,
66+
/// these flags take precedence over those from [`BuildContext::extra_args_for`].
67+
///
68+
/// As of now, these flags come from environment variables and configurations.
69+
/// See [`TargetInfo.rustflags`] for more on how Cargo collects them.
70+
///
71+
/// [`BuildContext::extra_args_for`]: crate::core::compiler::build_context::BuildContext::extra_args_for
72+
/// [`TargetInfo.rustflags`]: crate::core::compiler::build_context::TargetInfo::rustflags
73+
pub rustflags: Arc<[String]>,
74+
/// Extra compiler flags to pass to `rustdoc` for a given unit.
75+
///
76+
/// Although it depends on the caller, in the current Cargo implementation,
77+
/// these flags take precedence over those from [`BuildContext::extra_args_for`].
78+
///
79+
/// As of now, these flags come from environment variables and configurations.
80+
/// See [`TargetInfo.rustdocflags`] for more on how Cargo collects them.
81+
///
82+
/// [`BuildContext::extra_args_for`]: crate::core::compiler::build_context::BuildContext::extra_args_for
83+
/// [`TargetInfo.rustdocflags`]: crate::core::compiler::build_context::TargetInfo::rustdocflags
84+
pub rustdocflags: Arc<[String]>,
6285
// if `true`, the dependency is an artifact dependency, requiring special handling when
6386
// calculating output directories, linkage and environment variables provided to builds.
6487
pub artifact: IsArtifact,
@@ -151,6 +174,7 @@ impl fmt::Debug for Unit {
151174
.field("kind", &self.kind)
152175
.field("mode", &self.mode)
153176
.field("features", &self.features)
177+
.field("rustflags", &self.rustflags)
154178
.field("artifact", &self.artifact.is_true())
155179
.field(
156180
"artifact_target_for_features",
@@ -198,6 +222,8 @@ impl UnitInterner {
198222
kind: CompileKind,
199223
mode: CompileMode,
200224
features: Vec<InternedString>,
225+
rustflags: Arc<[String]>,
226+
rustdocflags: Arc<[String]>,
201227
is_std: bool,
202228
dep_hash: u64,
203229
artifact: IsArtifact,
@@ -231,6 +257,8 @@ impl UnitInterner {
231257
kind,
232258
mode,
233259
features,
260+
rustflags,
261+
rustdocflags,
234262
is_std,
235263
dep_hash,
236264
artifact,

‎src/cargo/core/compiler/unit_dependencies.rs

+2
Original file line numberDiff line numberDiff line change
@@ -859,6 +859,8 @@ fn new_unit_dep_with_profile(
859859
kind,
860860
mode,
861861
features,
862+
state.target_data.info(kind).rustflags.clone(),
863+
state.target_data.info(kind).rustdocflags.clone(),
862864
state.is_std,
863865
/*dep_hash*/ 0,
864866
artifact.map_or(IsArtifact::No, |_| IsArtifact::Yes),

‎src/cargo/ops/cargo_compile/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,7 @@ pub fn create_bcx<'a, 'gctx>(
360360
let generator = UnitGenerator {
361361
ws,
362362
packages: &to_builds,
363+
target_data: &target_data,
363364
filter,
364365
requested_kinds: &build_config.requested_kinds,
365366
explicit_host_kind,
@@ -399,6 +400,7 @@ pub fn create_bcx<'a, 'gctx>(
399400
&pkg_set,
400401
interner,
401402
&profiles,
403+
&target_data,
402404
)?
403405
} else {
404406
Default::default()
@@ -694,6 +696,8 @@ fn traverse_and_share(
694696
to_host.unwrap(),
695697
unit.mode,
696698
unit.features.clone(),
699+
unit.rustflags.clone(),
700+
unit.rustdocflags.clone(),
697701
unit.is_std,
698702
unit.dep_hash,
699703
unit.artifact,
@@ -719,6 +723,8 @@ fn traverse_and_share(
719723
canonical_kind,
720724
unit.mode,
721725
unit.features.clone(),
726+
unit.rustflags.clone(),
727+
unit.rustdocflags.clone(),
722728
unit.is_std,
723729
new_dep_hash,
724730
unit.artifact,
@@ -880,6 +886,8 @@ fn override_rustc_crate_types(
880886
unit.kind,
881887
unit.mode,
882888
unit.features.clone(),
889+
unit.rustflags.clone(),
890+
unit.rustdocflags.clone(),
883891
unit.is_std,
884892
unit.dep_hash,
885893
unit.artifact,

‎src/cargo/ops/cargo_compile/unit_generator.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use std::fmt::Write;
44

55
use crate::core::compiler::rustdoc::RustdocScrapeExamples;
66
use crate::core::compiler::unit_dependencies::IsArtifact;
7-
use crate::core::compiler::UnitInterner;
87
use crate::core::compiler::{CompileKind, CompileMode, Unit};
8+
use crate::core::compiler::{RustcTargetData, UnitInterner};
99
use crate::core::dependency::DepKind;
1010
use crate::core::profiles::{Profiles, UnitFor};
1111
use crate::core::resolver::features::{self, FeaturesFor};
@@ -47,6 +47,7 @@ struct Proposal<'a> {
4747
pub(super) struct UnitGenerator<'a, 'gctx> {
4848
pub ws: &'a Workspace<'gctx>,
4949
pub packages: &'a [&'a Package],
50+
pub target_data: &'a RustcTargetData<'gctx>,
5051
pub filter: &'a CompileFilter,
5152
pub requested_kinds: &'a [CompileKind],
5253
pub explicit_host_kind: CompileKind,
@@ -162,13 +163,16 @@ impl<'a> UnitGenerator<'a, '_> {
162163
unit_for,
163164
kind,
164165
);
166+
let kind = kind.for_target(target);
165167
self.interner.intern(
166168
pkg,
167169
target,
168170
profile,
169-
kind.for_target(target),
171+
kind,
170172
target_mode,
171173
features.clone(),
174+
self.target_data.info(kind).rustflags.clone(),
175+
self.target_data.info(kind).rustdocflags.clone(),
172176
/*is_std*/ false,
173177
/*dep_hash*/ 0,
174178
IsArtifact::No,

0 commit comments

Comments
 (0)