Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support generic target tables and env variables #9603

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/cargo/util/config/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,13 @@ enum KeyKind {
impl<'config> ConfigMapAccess<'config> {
fn new_map(de: Deserializer<'config>) -> Result<ConfigMapAccess<'config>, ConfigError> {
let mut fields = Vec::new();
if let Some(mut v) = de.config.get_table(&de.key)? {
// `v: Value<HashMap<String, CV>>`
for (key, _value) in v.val.drain() {
fields.push(KeyKind::CaseSensitive(key));
let key_is_table = de.config.is_table(&de.key)?;
if key_is_table.is_some() && key_is_table.unwrap() {
if let Some(mut v) = de.config.get_table(&de.key)? {
// `v: Value<HashMap<String, CV>>`
for (key, _value) in v.val.drain() {
fields.push(KeyKind::CaseSensitive(key));
}
}
}
if de.config.cli_unstable().advanced_env {
Expand Down
34 changes: 34 additions & 0 deletions src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,16 @@ impl Config {
Ok(false)
}

fn has_env_key(&self, key: &ConfigKey) -> bool {
if self.env.contains_key(key.as_env_key()) {
return true;
}

self.check_environment_key_case_mismatch(key);

false
}

fn check_environment_key_case_mismatch(&self, key: &ConfigKey) {
if let Some(env_key) = self.upper_case_env.get(key.as_env_key()) {
let _ = self.shell().warn(format!(
Expand Down Expand Up @@ -901,6 +911,14 @@ impl Config {
Ok(())
}

fn is_table(&self, key: &ConfigKey) -> CargoResult<Option<bool>> {
match self.get_cv(key)? {
Some(CV::Table(_, _def)) => Ok(Some(true)),
Some(_) => Ok(Some(false)),
None => Ok(None),
}
}

/// Low-level method for getting a config value as an `OptValue<HashMap<String, CV>>`.
///
/// NOTE: This does not read from env. The caller is responsible for that.
Expand Down Expand Up @@ -1719,6 +1737,15 @@ impl Config {
T::deserialize(d).map_err(|e| e.into())
}

pub fn get_env_only<'de, T: serde::de::Deserialize<'de>>(&self, key: &str) -> CargoResult<T> {
let d = Deserializer {
config: self,
key: ConfigKey::from_str(key),
env_prefix_ok: false,
};
T::deserialize(d).map_err(|e| e.into())
}

pub fn assert_package_cache_locked<'a>(&self, f: &'a Filesystem) -> &'a Path {
let ret = f.as_path_unlocked();
assert!(
Expand Down Expand Up @@ -2023,6 +2050,13 @@ impl ConfigValue {
}
}

pub fn is_string(&self) -> CargoResult<bool> {
match self {
CV::String(_, _def) => Ok(true),
_ => Ok(false),
}
}

pub fn table(&self, key: &str) -> CargoResult<(&HashMap<String, ConfigValue>, &Definition)> {
match self {
CV::Table(table, def) => Ok((table, def)),
Expand Down
67 changes: 51 additions & 16 deletions src/cargo/util/config/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,7 @@ pub(super) fn get_target_applies_to_host(config: &Config) -> CargoResult<bool> {
/// Loads a single `[host]` table for the given triple.
pub(super) fn load_host_triple(config: &Config, triple: &str) -> CargoResult<TargetConfig> {
if config.cli_unstable().host_config {
let host_triple_prefix = format!("host.{}", triple);
let host_triple_key = ConfigKey::from_str(&host_triple_prefix);
let host_prefix = match config.get_cv(&host_triple_key)? {
Some(_) => host_triple_prefix,
None => "host".to_string(),
};
load_config_table(config, &host_prefix)
load_config_table(config, "host", triple)
} else {
Ok(TargetConfig {
runner: None,
Expand All @@ -104,21 +98,56 @@ pub(super) fn load_host_triple(config: &Config, triple: &str) -> CargoResult<Tar

/// Loads a single `[target]` table for the given triple.
pub(super) fn load_target_triple(config: &Config, triple: &str) -> CargoResult<TargetConfig> {
load_config_table(config, &format!("target.{}", triple))
load_config_table(config, "target", triple)
}

fn load_config_val<'de, T: serde::de::Deserialize<'de>>(
config: &Config,
key: &str,
prefix: &str,
triple: &str,
) -> CargoResult<T> {
let triple_str = &format!("{}.{}.{}", prefix, triple, key);
let triple_key = &ConfigKey::from_str(triple_str);
if config.has_env_key(triple_key) {
return config.get_env_only(triple_str);
}

let generic_str = &format!("{}.{}", prefix, key);
let generic_key = &ConfigKey::from_str(generic_str);
if config.has_env_key(generic_key) {
return config.get_env_only(generic_str);
}

let generic_table = config.is_table(&ConfigKey::from_str(&format!("{}", prefix)))?;
let triple_table = config.is_table(&ConfigKey::from_str(&format!("{}.{}", prefix, triple)))?;
if !triple_table.is_some() && generic_table.is_some() && generic_table.unwrap() {
return config.get(generic_str);
}
config.get(triple_str)
}

/// Loads a single table for the given prefix.
fn load_config_table(config: &Config, prefix: &str) -> CargoResult<TargetConfig> {
fn load_config_table(config: &Config, prefix: &str, triple: &str) -> CargoResult<TargetConfig> {
// This needs to get each field individually because it cannot fetch the
// struct all at once due to `links_overrides`. Can't use `serde(flatten)`
// because it causes serde to use `deserialize_map` which means the config
// deserializer does not know which keys to deserialize, which means
// environment variables would not work.
let runner: OptValue<PathAndArgs> = config.get(&format!("{}.runner", prefix))?;
let rustflags: OptValue<StringList> = config.get(&format!("{}.rustflags", prefix))?;
let linker: OptValue<ConfigRelativePath> = config.get(&format!("{}.linker", prefix))?;
let runner: OptValue<PathAndArgs> = load_config_val(config, "runner", prefix, triple)?;
let rustflags: OptValue<StringList> = load_config_val(config, "rustflags", prefix, triple)?;
let linker: OptValue<ConfigRelativePath> = load_config_val(config, "linker", prefix, triple)?;
// Links do not support environment variables.
let target_key = ConfigKey::from_str(prefix);
let generic_key = ConfigKey::from_str(&format!("{}", prefix));
let triple_key = ConfigKey::from_str(&format!("{}.{}", prefix, triple));
let generic_table = config.is_table(&generic_key)?;
let triple_table = config.is_table(&triple_key)?;
let target_key = if !triple_table.is_some() && generic_table.is_some() && generic_table.unwrap()
{
generic_key
} else {
triple_key
};
let links_overrides = match config.get_table(&target_key)? {
Some(links) => parse_links_overrides(&target_key, links.val, config)?,
None => BTreeMap::new(),
Expand Down Expand Up @@ -150,7 +179,11 @@ fn parse_links_overrides(
_ => {}
}
let mut output = BuildOutput::default();
let table = value.table(&format!("{}.{}", target_key, lib_name))?.0;
let table_key = &format!("{}.{}", target_key, lib_name);
let table = value
.table(table_key)
.map_err(|e| anyhow::anyhow!("invalid configuration for key `{}`\n{}", table_key, e))?
.0;
// We require deterministic order of evaluation, so we must sort the pairs by key first.
let mut pairs = Vec::new();
for (k, value) in table {
Expand Down Expand Up @@ -227,8 +260,10 @@ fn parse_links_overrides(
anyhow::bail!("`{}` is not supported in build script overrides", key);
}
_ => {
let val = value.string(key)?.0;
output.metadata.push((key.clone(), val.to_string()));
if value.is_string()? {
let val = value.string(key)?.0;
output.metadata.push((key.clone(), val.to_string()));
}
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ fn bad1() {
.with_status(101)
.with_stderr(
"\
[ERROR] expected table for configuration key `target.nonexistent-target`, \
but found string in [..]config
[ERROR] invalid configuration for key `target.nonexistent-target`
expected a table, but found a string for `target.nonexistent-target` \
in [..]config
",
)
.run();
Expand Down
162 changes: 162 additions & 0 deletions tests/testsuite/build_script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,119 @@ fn custom_build_env_var_rustc_linker() {
p.cargo("build --target").arg(&target).run();
}

#[cargo_test]
fn custom_build_env_var_rustc_generic_linker() {
let target = rustc_host();
let p = project()
.file(
".cargo/config",
r#"
[target]
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.
if cargo_test_support::is_nightly() {
p.cargo("build -Z target-applies-to-host -Z host-config --target")
.arg(&target)
.masquerade_as_nightly_cargo(&["target-applies-to-host", "host-config"])
.run();
}
}

#[cargo_test]
fn custom_build_env_var_rustc_triple_overrides_generic_linker() {
let target = rustc_host();
let p = project()
.file(
".cargo/config",
&format!(
r#"
[target]
linker = "/path/to/generic/linker"
[target.{}]
linker = "/path/to/linker"
"#,
target
),
)
.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.
if cargo_test_support::is_nightly() {
p.cargo("build -Z target-applies-to-host -Z host-config --target")
.arg(&target)
.masquerade_as_nightly_cargo(&["target-applies-to-host", "host-config"])
.run();
}
}

#[cargo_test]
fn custom_build_env_var_rustc_generic_env_overrides_tables() {
let target = rustc_host();
let p = project()
.file(
".cargo/config",
&format!(
r#"
[target]
linker = "/path/to/generic/table/linker"
[target.{}]
linker = "/path/to/triple/table/linker"
"#,
target
),
)
.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.
if cargo_test_support::is_nightly() {
p.cargo("build -Z target-applies-to-host -Z host-config --target")
.env("CARGO_TARGET_LINKER", "/path/to/linker")
.arg(&target)
.masquerade_as_nightly_cargo(&["target-applies-to-host", "host-config"])
.run();
}
}

#[cargo_test]
fn custom_build_env_var_rustc_linker_bad_host_target() {
let target = rustc_host();
Expand Down Expand Up @@ -601,6 +714,55 @@ fn custom_build_linker_bad_host_with_arch() {
.run();
}

#[cargo_test]
fn custom_build_env_var_rustc_linker_bad_host_with_arch_env_override() {
let target = rustc_host();
let p = project()
.file(
".cargo/config",
&format!(
r#"
[host]
linker = "/path/to/host/linker"
[host.{}]
linker = "/path/to/host/arch/linker"
[target.{}]
linker = "/path/to/target/linker"
"#,
target, target
),
)
.file(
"build.rs",
r#"
use std::env;

fn main() {
assert!(env::var("RUSTC_LINKER").unwrap().ends_with("/path/to/target/linker"));
}
"#,
)
.file("src/lib.rs", "")
.build();

// build.rs should fail due to bad host linker being set
if cargo_test_support::is_nightly() {
p.cargo("build -Z target-applies-to-host -Z host-config --verbose --target")
.env("CARGO_HOST_LINKER", "/path/to/env/host/linker")
.arg(&target)
.masquerade_as_nightly_cargo(&["target-applies-to-host", "host-config"])
.with_status(101)
.with_stderr_contains(
"\
[COMPILING] foo v0.0.1 ([CWD])
[RUNNING] `rustc --crate-name build_script_build build.rs [..]--crate-type bin [..]-C linker=[..]/path/to/env/host/linker [..]`
[ERROR] linker `[..]/path/to/env/host/linker` not found
"
)
.run();
}
}

#[cargo_test]
fn custom_build_env_var_rustc_linker_cross_arch_host() {
let target = rustc_host();
Expand Down