Skip to content

Commit d7d8ca1

Browse files
alexcrichtonalexcrichton
committed
Consolidate build key configuration
Add a typed structure which lists all `build` key configuration throughout Cargo.
1 parent 09d9165 commit d7d8ca1

File tree

11 files changed

+115
-55
lines changed

11 files changed

+115
-55
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,15 +62,16 @@ impl BuildConfig {
6262
requested_target: &Option<String>,
6363
mode: CompileMode,
6464
) -> CargoResult<BuildConfig> {
65+
let cfg = config.build_config()?;
6566
let requested_kind = match requested_target {
6667
Some(s) => CompileKind::Target(CompileTarget::new(s)?),
67-
None => match config.get_string("build.target")? {
68-
Some(cfg) => {
69-
let value = if cfg.val.ends_with(".json") {
70-
let path = cfg.definition.root(config).join(&cfg.val);
68+
None => match &cfg.target {
69+
Some(val) => {
70+
let value = if val.raw_value().ends_with(".json") {
71+
let path = val.clone().resolve_path(config);
7172
path.to_str().expect("must be utf-8 in toml").to_string()
7273
} else {
73-
cfg.val
74+
val.raw_value().to_string()
7475
};
7576
CompileKind::Target(CompileTarget::new(&value)?)
7677
}
@@ -88,8 +89,7 @@ impl BuildConfig {
8889
its environment, ignoring the `-j` parameter",
8990
)?;
9091
}
91-
let cfg_jobs: Option<u32> = config.get("build.jobs")?;
92-
let jobs = jobs.or(cfg_jobs).unwrap_or(::num_cpus::get() as u32);
92+
let jobs = jobs.or(cfg.jobs).unwrap_or(::num_cpus::get() as u32);
9393

9494
Ok(BuildConfig {
9595
requested_kind,

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::str::{self, FromStr};
77
use crate::core::compiler::CompileKind;
88
use crate::core::TargetKind;
99
use crate::util::{CargoResult, CargoResultExt, Config, ProcessBuilder, Rustc};
10+
use crate::util::config::StringList;
1011
use cargo_platform::{Cfg, CfgExpr};
1112

1213
/// Information about the platform target gleaned from querying rustc.
@@ -427,9 +428,8 @@ fn env_args(
427428
CompileKind::Target(target) => target.short_name(),
428429
};
429430
let key = format!("target.{}.{}", target, name);
430-
if let Some(args) = config.get_list_or_split_string(&key)? {
431-
let args = args.val.into_iter();
432-
rustflags.extend(args);
431+
if let Some(args) = config.get::<Option<StringList>>(&key)? {
432+
rustflags.extend(args.as_slice().iter().cloned());
433433
}
434434
// ...including target.'cfg(...)'.rustflags
435435
if let Some(target_cfg) = target_cfg {
@@ -450,9 +450,8 @@ fn env_args(
450450

451451
for n in cfgs {
452452
let key = format!("target.{}.{}", n, name);
453-
if let Some(args) = config.get_list_or_split_string(&key)? {
454-
let args = args.val.into_iter();
455-
rustflags.extend(args);
453+
if let Some(args) = config.get::<Option<StringList>>(&key)? {
454+
rustflags.extend(args.as_slice().iter().cloned());
456455
}
457456
}
458457
}
@@ -463,10 +462,14 @@ fn env_args(
463462
}
464463

465464
// Then the `build.rustflags` value.
466-
let key = format!("build.{}", name);
467-
if let Some(args) = config.get_list_or_split_string(&key)? {
468-
let args = args.val.into_iter();
469-
return Ok(args.collect());
465+
let build = config.build_config()?;
466+
let list = if name == "rustflags" {
467+
&build.rustflags
468+
} else {
469+
&build.rustdocflags
470+
};
471+
if let Some(list) = list {
472+
return Ok(list.as_slice().to_vec())
470473
}
471474

472475
Ok(Vec::new())

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
9999
}
100100
};
101101

102-
let pipelining = bcx
103-
.config
104-
.get::<Option<bool>>("build.pipelining")?
105-
.unwrap_or(true);
102+
let pipelining = bcx.config.build_config()?.pipelining.unwrap_or(true);
106103

107104
Ok(Self {
108105
bcx,

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,10 @@ pub fn output_depinfo<'a, 'b>(cx: &mut Context<'a, 'b>, unit: &Unit<'a>) -> Carg
8181
let mut visited = HashSet::new();
8282
let success = add_deps_for_unit(&mut deps, cx, unit, &mut visited).is_ok();
8383
let basedir_string;
84-
let basedir = match bcx.config.get_path("build.dep-info-basedir")? {
84+
let basedir = match bcx.config.build_config()?.dep_info_basedir.clone() {
8585
Some(value) => {
8686
basedir_string = value
87-
.val
87+
.resolve_path(bcx.config)
8888
.as_os_str()
8989
.to_str()
9090
.ok_or_else(|| internal("build.dep-info-basedir path not utf-8"))?

‎src/cargo/core/profiles.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ impl Profiles {
3838

3939
let incremental = match env::var_os("CARGO_INCREMENTAL") {
4040
Some(v) => Some(v == "1"),
41-
None => config.get::<Option<bool>>("build.incremental")?,
41+
None => config.build_config()?.incremental,
4242
};
4343

4444
if !features.is_enabled(Feature::named_profiles()) {

‎src/cargo/ops/registry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ pub fn configure_http_handle(config: &Config, handle: &mut Easy) -> CargoResult<
420420
handle.proxy(&proxy)?;
421421
}
422422
if let Some(cainfo) = http.cainfo.clone() {
423-
let cainfo = cainfo.resolve(config);
423+
let cainfo = cainfo.resolve_path(config);
424424
handle.cainfo(&cainfo)?;
425425
}
426426
if let Some(check) = http.check_revoke {

‎src/cargo/util/config/mod.rs

Lines changed: 59 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ pub struct Config {
9797
/// Cached configuration parsed by Cargo
9898
http_config: LazyCell<CargoHttpConfig>,
9999
net_config: LazyCell<CargoNetConfig>,
100+
build_config: LazyCell<CargoBuildConfig>,
100101
}
101102

102103
impl Config {
@@ -157,6 +158,7 @@ impl Config {
157158
package_cache_lock: RefCell::new(None),
158159
http_config: LazyCell::new(),
159160
net_config: LazyCell::new(),
161+
build_config: LazyCell::new(),
160162
}
161163
}
162164

@@ -338,12 +340,12 @@ impl Config {
338340
}
339341

340342
pub fn target_dir(&self) -> CargoResult<Option<Filesystem>> {
341-
if let Some(ref dir) = self.target_dir {
343+
if let Some(dir) = &self.target_dir {
342344
Ok(Some(dir.clone()))
343345
} else if let Some(dir) = env::var_os("CARGO_TARGET_DIR") {
344346
Ok(Some(Filesystem::new(self.cwd.join(dir))))
345-
} else if let Some(val) = self.get_path("build.target-dir")? {
346-
let val = self.cwd.join(val.val);
347+
} else if let Some(val) = &self.build_config()?.target_dir {
348+
let val = val.resolve_path(self);
347349
Ok(Some(Filesystem::new(val)))
348350
} else {
349351
Ok(None)
@@ -469,7 +471,7 @@ impl Config {
469471
pub fn get_path(&self, key: &str) -> CargoResult<OptValue<PathBuf>> {
470472
self.get::<Option<Value<ConfigRelativePath>>>(key).map(|v| {
471473
v.map(|v| Value {
472-
val: v.val.resolve(self),
474+
val: v.val.resolve_program(self),
473475
definition: v.definition,
474476
})
475477
})
@@ -513,27 +515,13 @@ impl Config {
513515
}
514516
}
515517

516-
pub fn get_list_or_split_string(&self, key: &str) -> CargoResult<OptValue<Vec<String>>> {
517-
#[derive(Deserialize)]
518-
#[serde(untagged)]
519-
enum Target {
520-
String(String),
521-
List(Vec<String>),
522-
}
523-
524-
match self.get::<Option<Value<Target>>>(key)? {
518+
fn get_list_or_split_string(&self, key: &str) -> CargoResult<OptValue<Vec<String>>> {
519+
match self.get::<Option<Value<StringList>>>(key)? {
525520
None => Ok(None),
526-
Some(Value {
527-
val: Target::String(s),
528-
definition,
529-
}) => Ok(Some(Value {
530-
val: s.split(' ').map(str::to_string).collect(),
531-
definition,
521+
Some(val) => Ok(Some(Value {
522+
val: val.val.list,
523+
definition: val.definition,
532524
})),
533-
Some(Value {
534-
val: Target::List(val),
535-
definition,
536-
}) => Ok(Some(Value { val, definition })),
537525
}
538526
}
539527

@@ -927,6 +915,11 @@ impl Config {
927915
.try_borrow_with(|| Ok(self.get::<CargoNetConfig>("net")?))
928916
}
929917

918+
pub fn build_config(&self) -> CargoResult<&CargoBuildConfig> {
919+
self.build_config
920+
.try_borrow_with(|| Ok(self.get::<CargoBuildConfig>("build")?))
921+
}
922+
930923
pub fn crates_io_source_id<F>(&self, f: F) -> CargoResult<SourceId>
931924
where
932925
F: FnMut() -> CargoResult<SourceId>,
@@ -1463,3 +1456,46 @@ pub struct CargoNetConfig {
14631456
#[serde(rename = "git-fetch-with-cli")]
14641457
pub git_fetch_with_cli: Option<bool>,
14651458
}
1459+
1460+
#[derive(Debug, Deserialize)]
1461+
pub struct CargoBuildConfig {
1462+
pub pipelining: Option<bool>,
1463+
#[serde(rename = "dep-info-basedir")]
1464+
pub dep_info_basedir: Option<ConfigRelativePath>,
1465+
#[serde(rename = "target-dir")]
1466+
pub target_dir: Option<ConfigRelativePath>,
1467+
pub incremental: Option<bool>,
1468+
pub target: Option<ConfigRelativePath>,
1469+
pub jobs: Option<u32>,
1470+
pub rustflags: Option<StringList>,
1471+
pub rustdocflags: Option<StringList>,
1472+
}
1473+
1474+
#[derive(Debug)]
1475+
pub struct StringList {
1476+
list: Vec<String>,
1477+
}
1478+
1479+
impl StringList {
1480+
pub fn as_slice(&self) -> &[String] {
1481+
&self.list
1482+
}
1483+
}
1484+
1485+
impl<'de> serde::de::Deserialize<'de> for StringList {
1486+
fn deserialize<D: serde::de::Deserializer<'de>>(d: D) -> Result<Self, D::Error> {
1487+
#[derive(Deserialize)]
1488+
#[serde(untagged)]
1489+
enum Target {
1490+
String(String),
1491+
List(Vec<String>),
1492+
}
1493+
1494+
Ok(match Target::deserialize(d)? {
1495+
Target::String(s) => StringList {
1496+
list: s.split_whitespace().map(str::to_string).collect(),
1497+
},
1498+
Target::List(list) => StringList { list },
1499+
})
1500+
}
1501+
}

‎src/cargo/util/config/path.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,26 @@ use std::path::PathBuf;
1010
pub struct ConfigRelativePath(Value<String>);
1111

1212
impl ConfigRelativePath {
13-
pub fn resolve(self, config: &Config) -> PathBuf {
13+
/// Returns the raw underlying configuration value for this key.
14+
pub fn raw_value(&self) -> &str {
15+
&self.0.val
16+
}
17+
18+
/// Resolves this configuration-relative path to an absolute path.
19+
///
20+
/// This will always return an absolute path where it's relative to the
21+
/// location for configuration for this value.
22+
pub fn resolve_path(&self, config: &Config) -> PathBuf {
23+
self.0.definition.root(config).join(&self.0.val)
24+
}
25+
26+
/// Resolves this configuration-relative path to either an absolute path or
27+
/// something appropriate to execute from `PATH`.
28+
///
29+
/// Values which don't look like a filesystem path (don't contain `/` or
30+
/// `\`) will be returned as-is, and everything else will fall through to an
31+
/// absolute path.
32+
pub fn resolve_program(self, config: &Config) -> PathBuf {
1433
config.string_to_path(self.0.val, &self.0.definition)
1534
}
1635
}

‎src/cargo/util/errors.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,5 +394,5 @@ pub fn display_causes(error: &Error) -> String {
394394
.iter_chain()
395395
.map(|e| e.to_string())
396396
.collect::<Vec<_>>()
397-
.join("\nCaused by:\n ")
397+
.join("\n\nCaused by:\n ")
398398
}

‎tests/testsuite/config.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -560,10 +560,13 @@ fn config_bad_toml() {
560560
config.get::<i32>("foo").unwrap_err(),
561561
"\
562562
could not load Cargo configuration
563+
563564
Caused by:
564565
could not parse TOML configuration in `[..]/.cargo/config`
566+
565567
Caused by:
566568
could not parse input as TOML
569+
567570
Caused by:
568571
expected an equals, found eof at line 1 column 5",
569572
);
@@ -735,35 +738,35 @@ abs = '{}'
735738
config
736739
.get::<config::ConfigRelativePath>("p1")
737740
.unwrap()
738-
.resolve(&config),
741+
.resolve_path(&config),
739742
paths::root().join("foo/bar")
740743
);
741744
assert_eq!(
742745
config
743746
.get::<config::ConfigRelativePath>("p2")
744747
.unwrap()
745-
.resolve(&config),
748+
.resolve_path(&config),
746749
paths::root().join("../abc")
747750
);
748751
assert_eq!(
749752
config
750753
.get::<config::ConfigRelativePath>("p3")
751754
.unwrap()
752-
.resolve(&config),
755+
.resolve_path(&config),
753756
paths::root().join("d/e")
754757
);
755758
assert_eq!(
756759
config
757760
.get::<config::ConfigRelativePath>("abs")
758761
.unwrap()
759-
.resolve(&config),
762+
.resolve_path(&config),
760763
paths::home()
761764
);
762765
assert_eq!(
763766
config
764767
.get::<config::ConfigRelativePath>("epath")
765768
.unwrap()
766-
.resolve(&config),
769+
.resolve_path(&config),
767770
paths::root().join("a/b")
768771
);
769772
}

0 commit comments

Comments
 (0)