Skip to content

Commit bd8253f

Browse files
alexcrichtonalexcrichton
committed
Avoid reparsing env_args continuously
We only need to parse this information once, so calculate it when a `TargetInfo` is created and then just reuse that from then on out.
1 parent 7246028 commit bd8253f

File tree

6 files changed

+159
-153
lines changed

6 files changed

+159
-153
lines changed

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

Lines changed: 5 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
use std::collections::HashMap;
2-
use std::env;
32
use std::path::{Path, PathBuf};
43
use std::str;
54

@@ -9,7 +8,7 @@ use crate::core::profiles::Profiles;
98
use crate::core::{Dependency, Workspace};
109
use crate::core::{PackageId, PackageSet, Resolve};
1110
use crate::util::errors::CargoResult;
12-
use crate::util::{profile, Cfg, CfgExpr, Config, Rustc};
11+
use crate::util::{profile, Cfg, Config, Rustc};
1312

1413
use super::{BuildConfig, BuildOutput, Kind, Unit};
1514

@@ -157,26 +156,12 @@ impl<'a, 'cfg> BuildContext<'a, 'cfg> {
157156
self.build_config.jobs
158157
}
159158

160-
pub fn rustflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
161-
env_args(
162-
self.config,
163-
&self.build_config.requested_target,
164-
self.host_triple(),
165-
self.info(unit.kind).cfg(),
166-
unit.kind,
167-
"RUSTFLAGS",
168-
)
159+
pub fn rustflags_args(&self, unit: &Unit<'_>) -> &[String] {
160+
&self.info(unit.kind).rustflags
169161
}
170162

171-
pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> CargoResult<Vec<String>> {
172-
env_args(
173-
self.config,
174-
&self.build_config.requested_target,
175-
self.host_triple(),
176-
self.info(unit.kind).cfg(),
177-
unit.kind,
178-
"RUSTDOCFLAGS",
179-
)
163+
pub fn rustdocflags_args(&self, unit: &Unit<'_>) -> &[String] {
164+
&self.info(unit.kind).rustdocflags
180165
}
181166

182167
pub fn show_warnings(&self, pkg: PackageId) -> bool {
@@ -292,124 +277,3 @@ impl TargetConfig {
292277
Ok(ret)
293278
}
294279
}
295-
296-
/// Acquire extra flags to pass to the compiler from various locations.
297-
///
298-
/// The locations are:
299-
///
300-
/// - the `RUSTFLAGS` environment variable
301-
///
302-
/// then if this was not found
303-
///
304-
/// - `target.*.rustflags` from the manifest (Cargo.toml)
305-
/// - `target.cfg(..).rustflags` from the manifest
306-
///
307-
/// then if neither of these were found
308-
///
309-
/// - `build.rustflags` from the manifest
310-
///
311-
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
312-
/// scripts, ...), even if it is the same as the target.
313-
fn env_args(
314-
config: &Config,
315-
requested_target: &Option<String>,
316-
host_triple: &str,
317-
target_cfg: Option<&[Cfg]>,
318-
kind: Kind,
319-
name: &str,
320-
) -> CargoResult<Vec<String>> {
321-
// We *want* to apply RUSTFLAGS only to builds for the
322-
// requested target architecture, and not to things like build
323-
// scripts and plugins, which may be for an entirely different
324-
// architecture. Cargo's present architecture makes it quite
325-
// hard to only apply flags to things that are not build
326-
// scripts and plugins though, so we do something more hacky
327-
// instead to avoid applying the same RUSTFLAGS to multiple targets
328-
// arches:
329-
//
330-
// 1) If --target is not specified we just apply RUSTFLAGS to
331-
// all builds; they are all going to have the same target.
332-
//
333-
// 2) If --target *is* specified then we only apply RUSTFLAGS
334-
// to compilation units with the Target kind, which indicates
335-
// it was chosen by the --target flag.
336-
//
337-
// This means that, e.g., even if the specified --target is the
338-
// same as the host, build scripts in plugins won't get
339-
// RUSTFLAGS.
340-
let compiling_with_target = requested_target.is_some();
341-
let is_target_kind = kind == Kind::Target;
342-
343-
if compiling_with_target && !is_target_kind {
344-
// This is probably a build script or plugin and we're
345-
// compiling with --target. In this scenario there are
346-
// no rustflags we can apply.
347-
return Ok(Vec::new());
348-
}
349-
350-
// First try RUSTFLAGS from the environment
351-
if let Ok(a) = env::var(name) {
352-
let args = a
353-
.split(' ')
354-
.map(str::trim)
355-
.filter(|s| !s.is_empty())
356-
.map(str::to_string);
357-
return Ok(args.collect());
358-
}
359-
360-
let mut rustflags = Vec::new();
361-
362-
let name = name
363-
.chars()
364-
.flat_map(|c| c.to_lowercase())
365-
.collect::<String>();
366-
// Then the target.*.rustflags value...
367-
let target = requested_target
368-
.as_ref()
369-
.map(|s| s.as_str())
370-
.unwrap_or(host_triple);
371-
let key = format!("target.{}.{}", target, name);
372-
if let Some(args) = config.get_list_or_split_string(&key)? {
373-
let args = args.val.into_iter();
374-
rustflags.extend(args);
375-
}
376-
// ...including target.'cfg(...)'.rustflags
377-
if let Some(target_cfg) = target_cfg {
378-
if let Some(table) = config.get_table("target")? {
379-
let cfgs = table
380-
.val
381-
.keys()
382-
.filter(|key| CfgExpr::matches_key(key, target_cfg));
383-
384-
// Note that we may have multiple matching `[target]` sections and
385-
// because we're passing flags to the compiler this can affect
386-
// cargo's caching and whether it rebuilds. Ensure a deterministic
387-
// ordering through sorting for now. We may perhaps one day wish to
388-
// ensure a deterministic ordering via the order keys were defined
389-
// in files perhaps.
390-
let mut cfgs = cfgs.collect::<Vec<_>>();
391-
cfgs.sort();
392-
393-
for n in cfgs {
394-
let key = format!("target.{}.{}", n, name);
395-
if let Some(args) = config.get_list_or_split_string(&key)? {
396-
let args = args.val.into_iter();
397-
rustflags.extend(args);
398-
}
399-
}
400-
}
401-
}
402-
403-
if !rustflags.is_empty() {
404-
return Ok(rustflags);
405-
}
406-
407-
// Then the `build.rustflags` value.
408-
let key = format!("build.{}", name);
409-
if let Some(args) = config.get_list_or_split_string(&key)? {
410-
let args = args.val.into_iter();
411-
return Ok(args.collect());
412-
}
413-
414-
Ok(Vec::new())
415-
}

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

Lines changed: 145 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use std::cell::RefCell;
22
use std::collections::hash_map::{Entry, HashMap};
3+
use std::env;
34
use std::path::PathBuf;
45
use std::str::{self, FromStr};
56

6-
use super::env_args;
77
use super::Kind;
88
use crate::core::TargetKind;
9+
use crate::util::CfgExpr;
910
use crate::util::{CargoResult, CargoResultExt, Cfg, Config, ProcessBuilder, Rustc};
1011

1112
#[derive(Clone)]
@@ -14,6 +15,8 @@ pub struct TargetInfo {
1415
crate_types: RefCell<HashMap<String, Option<(String, String)>>>,
1516
cfg: Option<Vec<Cfg>>,
1617
pub sysroot_libdir: Option<PathBuf>,
18+
pub rustflags: Vec<String>,
19+
pub rustdocflags: Vec<String>,
1720
}
1821

1922
/// Type of each file generated by a Unit.
@@ -136,16 +139,34 @@ impl TargetInfo {
136139
}
137140

138141
let cfg = if has_cfg_and_sysroot {
139-
Some(lines.map(Cfg::from_str).collect::<CargoResult<_>>()?)
142+
Some(lines.map(Cfg::from_str).collect::<CargoResult<Vec<_>>>()?)
140143
} else {
141144
None
142145
};
143146

144147
Ok(TargetInfo {
145148
crate_type_process: Some(crate_type_process),
146149
crate_types: RefCell::new(map),
147-
cfg,
148150
sysroot_libdir,
151+
// recalculate `rustflags` from above now that we have `cfg`
152+
// information
153+
rustflags: env_args(
154+
config,
155+
requested_target,
156+
&rustc.host,
157+
cfg.as_ref().map(|v| v.as_ref()),
158+
kind,
159+
"RUSTFLAGS",
160+
)?,
161+
rustdocflags: env_args(
162+
config,
163+
requested_target,
164+
&rustc.host,
165+
cfg.as_ref().map(|v| v.as_ref()),
166+
kind,
167+
"RUSTDOCFLAGS",
168+
)?,
169+
cfg,
149170
})
150171
}
151172

@@ -289,3 +310,124 @@ fn parse_crate_type(
289310

290311
Ok(Some((prefix.to_string(), suffix.to_string())))
291312
}
313+
314+
/// Acquire extra flags to pass to the compiler from various locations.
315+
///
316+
/// The locations are:
317+
///
318+
/// - the `RUSTFLAGS` environment variable
319+
///
320+
/// then if this was not found
321+
///
322+
/// - `target.*.rustflags` from the manifest (Cargo.toml)
323+
/// - `target.cfg(..).rustflags` from the manifest
324+
///
325+
/// then if neither of these were found
326+
///
327+
/// - `build.rustflags` from the manifest
328+
///
329+
/// Note that if a `target` is specified, no args will be passed to host code (plugins, build
330+
/// scripts, ...), even if it is the same as the target.
331+
fn env_args(
332+
config: &Config,
333+
requested_target: &Option<String>,
334+
host_triple: &str,
335+
target_cfg: Option<&[Cfg]>,
336+
kind: Kind,
337+
name: &str,
338+
) -> CargoResult<Vec<String>> {
339+
// We *want* to apply RUSTFLAGS only to builds for the
340+
// requested target architecture, and not to things like build
341+
// scripts and plugins, which may be for an entirely different
342+
// architecture. Cargo's present architecture makes it quite
343+
// hard to only apply flags to things that are not build
344+
// scripts and plugins though, so we do something more hacky
345+
// instead to avoid applying the same RUSTFLAGS to multiple targets
346+
// arches:
347+
//
348+
// 1) If --target is not specified we just apply RUSTFLAGS to
349+
// all builds; they are all going to have the same target.
350+
//
351+
// 2) If --target *is* specified then we only apply RUSTFLAGS
352+
// to compilation units with the Target kind, which indicates
353+
// it was chosen by the --target flag.
354+
//
355+
// This means that, e.g., even if the specified --target is the
356+
// same as the host, build scripts in plugins won't get
357+
// RUSTFLAGS.
358+
let compiling_with_target = requested_target.is_some();
359+
let is_target_kind = kind == Kind::Target;
360+
361+
if compiling_with_target && !is_target_kind {
362+
// This is probably a build script or plugin and we're
363+
// compiling with --target. In this scenario there are
364+
// no rustflags we can apply.
365+
return Ok(Vec::new());
366+
}
367+
368+
// First try RUSTFLAGS from the environment
369+
if let Ok(a) = env::var(name) {
370+
let args = a
371+
.split(' ')
372+
.map(str::trim)
373+
.filter(|s| !s.is_empty())
374+
.map(str::to_string);
375+
return Ok(args.collect());
376+
}
377+
378+
let mut rustflags = Vec::new();
379+
380+
let name = name
381+
.chars()
382+
.flat_map(|c| c.to_lowercase())
383+
.collect::<String>();
384+
// Then the target.*.rustflags value...
385+
let target = requested_target
386+
.as_ref()
387+
.map(|s| s.as_str())
388+
.unwrap_or(host_triple);
389+
let key = format!("target.{}.{}", target, name);
390+
if let Some(args) = config.get_list_or_split_string(&key)? {
391+
let args = args.val.into_iter();
392+
rustflags.extend(args);
393+
}
394+
// ...including target.'cfg(...)'.rustflags
395+
if let Some(target_cfg) = target_cfg {
396+
if let Some(table) = config.get_table("target")? {
397+
let cfgs = table
398+
.val
399+
.keys()
400+
.filter(|key| CfgExpr::matches_key(key, target_cfg));
401+
402+
// Note that we may have multiple matching `[target]` sections and
403+
// because we're passing flags to the compiler this can affect
404+
// cargo's caching and whether it rebuilds. Ensure a deterministic
405+
// ordering through sorting for now. We may perhaps one day wish to
406+
// ensure a deterministic ordering via the order keys were defined
407+
// in files perhaps.
408+
let mut cfgs = cfgs.collect::<Vec<_>>();
409+
cfgs.sort();
410+
411+
for n in cfgs {
412+
let key = format!("target.{}.{}", n, name);
413+
if let Some(args) = config.get_list_or_split_string(&key)? {
414+
let args = args.val.into_iter();
415+
rustflags.extend(args);
416+
}
417+
}
418+
}
419+
}
420+
421+
if !rustflags.is_empty() {
422+
return Ok(rustflags);
423+
}
424+
425+
// Then the `build.rustflags` value.
426+
let key = format!("build.{}", name);
427+
if let Some(args) = config.get_list_or_split_string(&key)? {
428+
let args = args.val.into_iter();
429+
return Ok(args.collect());
430+
}
431+
432+
Ok(Vec::new())
433+
}

‎src/cargo/core/compiler/context/compilation_files.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -504,9 +504,9 @@ fn compute_metadata<'a, 'cfg>(
504504
// This helps when the target directory is a shared cache for projects with different cargo configs,
505505
// or if the user is experimenting with different rustflags manually.
506506
if unit.mode.is_doc() {
507-
cx.bcx.rustdocflags_args(unit).ok().hash(&mut hasher);
507+
cx.bcx.rustdocflags_args(unit).hash(&mut hasher);
508508
} else {
509-
cx.bcx.rustflags_args(unit).ok().hash(&mut hasher);
509+
cx.bcx.rustflags_args(unit).hash(&mut hasher);
510510
}
511511

512512
// Artifacts compiled for the host should have a different metadata

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,12 +238,12 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
238238
.collect()
239239
});
240240
}
241-
let rustdocflags = self.bcx.rustdocflags_args(unit)?;
241+
let rustdocflags = self.bcx.rustdocflags_args(unit);
242242
if !rustdocflags.is_empty() {
243243
self.compilation
244244
.rustdocflags
245245
.entry(unit.pkg.package_id())
246-
.or_insert(rustdocflags);
246+
.or_insert(rustdocflags.to_vec());
247247
}
248248

249249
super::output_depinfo(&mut self, unit)?;

0 commit comments

Comments
 (0)