Skip to content

Commit

Permalink
Auto merge of #111566 - clubby789:bootstrap-override-config, r=ozkanonur
Browse files Browse the repository at this point in the history 
Override config.toml options from command line

https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/Running.20tests.20on.20precompiled.20rustc/near/357763280

cc `@jyn514`
  • Loading branch information
@bors
bors committed May 24, 2023
2 parents d69787f + 7a7cbe0 commit 70db836
Show file tree
Hide file tree
Showing 6 changed files with 303 additions and 47 deletions.
150 changes: 124 additions & 26 deletions src/bootstrap/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ impl SplitDebuginfo {
}

/// LTO mode used for compiling rustc itself.
#[derive(Default, Clone, PartialEq)]
#[derive(Default, Clone, PartialEq, Debug)]
pub enum RustcLto {
Off,
#[default]
Expand Down Expand Up @@ -508,29 +508,42 @@ struct TomlConfig {
profile: Option<String>,
}

/// Describes how to handle conflicts in merging two [`TomlConfig`]
#[derive(Copy, Clone, Debug)]
enum ReplaceOpt {
/// Silently ignore a duplicated value
IgnoreDuplicate,
/// Override the current value, even if it's `Some`
Override,
/// Exit with an error on duplicate values
ErrorOnDuplicate,
}

trait Merge {
fn merge(&mut self, other: Self);
fn merge(&mut self, other: Self, replace: ReplaceOpt);
}

impl Merge for TomlConfig {
fn merge(
&mut self,
TomlConfig { build, install, llvm, rust, dist, target, profile: _, changelog_seen: _ }: Self,
TomlConfig { build, install, llvm, rust, dist, target, profile: _, changelog_seen }: Self,
replace: ReplaceOpt,
) {
fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>) {
fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>, replace: ReplaceOpt) {
if let Some(new) = y {
if let Some(original) = x {
original.merge(new);
original.merge(new, replace);
} else {
*x = Some(new);
}
}
}
do_merge(&mut self.build, build);
do_merge(&mut self.install, install);
do_merge(&mut self.llvm, llvm);
do_merge(&mut self.rust, rust);
do_merge(&mut self.dist, dist);
self.changelog_seen.merge(changelog_seen, replace);
do_merge(&mut self.build, build, replace);
do_merge(&mut self.install, install, replace);
do_merge(&mut self.llvm, llvm, replace);
do_merge(&mut self.rust, rust, replace);
do_merge(&mut self.dist, dist, replace);
assert!(target.is_none(), "merging target-specific config is not currently supported");
}
}
Expand All @@ -547,10 +560,33 @@ macro_rules! define_config {
}

impl Merge for $name {
fn merge(&mut self, other: Self) {
fn merge(&mut self, other: Self, replace: ReplaceOpt) {
$(
if !self.$field.is_some() {
self.$field = other.$field;
match replace {
ReplaceOpt::IgnoreDuplicate => {
if self.$field.is_none() {
self.$field = other.$field;
}
},
ReplaceOpt::Override => {
if other.$field.is_some() {
self.$field = other.$field;
}
}
ReplaceOpt::ErrorOnDuplicate => {
if other.$field.is_some() {
if self.$field.is_some() {
if cfg!(test) {
panic!("overriding existing option")
} else {
eprintln!("overriding existing option: `{}`", stringify!($field));
crate::detail_exit(2);
}
} else {
self.$field = other.$field;
}
}
}
}
)*
}
Expand Down Expand Up @@ -623,6 +659,37 @@ macro_rules! define_config {
}
}

impl<T> Merge for Option<T> {
fn merge(&mut self, other: Self, replace: ReplaceOpt) {
match replace {
ReplaceOpt::IgnoreDuplicate => {
if self.is_none() {
*self = other;
}
}
ReplaceOpt::Override => {
if other.is_some() {
*self = other;
}
}
ReplaceOpt::ErrorOnDuplicate => {
if other.is_some() {
if self.is_some() {
if cfg!(test) {
panic!("overriding existing option")
} else {
eprintln!("overriding existing option");
crate::detail_exit(2);
}
} else {
*self = other;
}
}
}
}
}
}

define_config! {
/// TOML representation of various global build decisions.
#[derive(Default)]
Expand Down Expand Up @@ -864,28 +931,27 @@ impl Config {

pub fn parse(args: &[String]) -> Config {
#[cfg(test)]
let get_toml = |_: &_| TomlConfig::default();
fn get_toml(_: &Path) -> TomlConfig {
TomlConfig::default()
}

#[cfg(not(test))]
let get_toml = |file: &Path| {
fn get_toml(file: &Path) -> TomlConfig {
let contents =
t!(fs::read_to_string(file), format!("config file {} not found", file.display()));
// Deserialize to Value and then TomlConfig to prevent the Deserialize impl of
// TomlConfig and sub types to be monomorphized 5x by toml.
match toml::from_str(&contents)
toml::from_str(&contents)
.and_then(|table: toml::Value| TomlConfig::deserialize(table))
{
Ok(table) => table,
Err(err) => {
eprintln!("failed to parse TOML configuration '{}': {}", file.display(), err);
.unwrap_or_else(|err| {
eprintln!("failed to parse TOML configuration '{}': {err}", file.display());
crate::detail_exit(2);
}
}
};

})
}
Self::parse_inner(args, get_toml)
}

fn parse_inner<'a>(args: &[String], get_toml: impl 'a + Fn(&Path) -> TomlConfig) -> Config {
fn parse_inner(args: &[String], get_toml: impl Fn(&Path) -> TomlConfig) -> Config {
let mut flags = Flags::parse(&args);
let mut config = Config::default_opts();

Expand Down Expand Up @@ -998,8 +1064,40 @@ impl Config {
include_path.push("defaults");
include_path.push(format!("config.{}.toml", include));
let included_toml = get_toml(&include_path);
toml.merge(included_toml);
toml.merge(included_toml, ReplaceOpt::IgnoreDuplicate);
}

let mut override_toml = TomlConfig::default();
for option in flags.set.iter() {
fn get_table(option: &str) -> Result<TomlConfig, toml::de::Error> {
toml::from_str(&option)
.and_then(|table: toml::Value| TomlConfig::deserialize(table))
}

let mut err = match get_table(option) {
Ok(v) => {
override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate);
continue;
}
Err(e) => e,
};
// We want to be able to set string values without quotes,
// like in `configure.py`. Try adding quotes around the right hand side
if let Some((key, value)) = option.split_once("=") {
if !value.contains('"') {
match get_table(&format!(r#"{key}="{value}""#)) {
Ok(v) => {
override_toml.merge(v, ReplaceOpt::ErrorOnDuplicate);
continue;
}
Err(e) => err = e,
}
}
}
eprintln!("failed to parse override `{option}`: `{err}");
crate::detail_exit(2)
}
toml.merge(override_toml, ReplaceOpt::Override);

config.changelog_seen = toml.changelog_seen;

Expand Down
77 changes: 71 additions & 6 deletions src/bootstrap/config/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use super::{Config, Flags, TomlConfig};
use super::{Config, Flags};
use clap::CommandFactory;
use std::{env, path::Path};

fn toml(config: &str) -> impl '_ + Fn(&Path) -> TomlConfig {
|&_| toml::from_str(config).unwrap()
}

fn parse(config: &str) -> Config {
Config::parse_inner(&["check".to_owned(), "--config=/does/not/exist".to_owned()], toml(config))
Config::parse_inner(&["check".to_owned(), "--config=/does/not/exist".to_owned()], |&_| {
toml::from_str(config).unwrap()
})
}

#[test]
Expand Down Expand Up @@ -94,3 +92,70 @@ fn detect_src_and_out() {
fn clap_verify() {
Flags::command().debug_assert();
}

#[test]
fn override_toml() {
let config = Config::parse_inner(
&[
"check".to_owned(),
"--config=/does/not/exist".to_owned(),
"--set=changelog-seen=1".to_owned(),
"--set=rust.lto=fat".to_owned(),
"--set=rust.deny-warnings=false".to_owned(),
"--set=build.gdb=\"bar\"".to_owned(),
"--set=build.tools=[\"cargo\"]".to_owned(),
"--set=llvm.build-config={\"foo\" = \"bar\"}".to_owned(),
],
|&_| {
toml::from_str(
r#"
changelog-seen = 0
[rust]
lto = "off"
deny-warnings = true
[build]
gdb = "foo"
tools = []
[llvm]
download-ci-llvm = false
build-config = {}
"#,
)
.unwrap()
},
);
assert_eq!(config.changelog_seen, Some(1), "setting top-level value");
assert_eq!(
config.rust_lto,
crate::config::RustcLto::Fat,
"setting string value without quotes"
);
assert_eq!(config.gdb, Some("bar".into()), "setting string value with quotes");
assert_eq!(config.deny_warnings, false, "setting boolean value");
assert_eq!(
config.tools,
Some(["cargo".to_string()].into_iter().collect()),
"setting list value"
);
assert_eq!(
config.llvm_build_config,
[("foo".to_string(), "bar".to_string())].into_iter().collect(),
"setting dictionary value"
);
}

#[test]
#[should_panic]
fn override_toml_duplicate() {
Config::parse_inner(
&[
"check".to_owned(),
"--config=/does/not/exist".to_owned(),
"--set=changelog-seen=1".to_owned(),
"--set=changelog-seen=2".to_owned(),
],
|&_| toml::from_str("changelog-seen = 0").unwrap(),
);
}
3 changes: 3 additions & 0 deletions src/bootstrap/flags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ pub struct Flags {
#[arg(global(true))]
/// paths for the subcommand
pub paths: Vec<PathBuf>,
/// override options in config.toml
#[arg(global(true), value_hint = clap::ValueHint::Other, long, value_name = "section.option=value")]
pub set: Vec<String>,
/// arguments passed to subcommands
#[arg(global(true), last(true), value_name = "ARGS")]
pub free_args: Vec<String>,
Expand Down
Loading

0 comments on commit 70db836

Please sign in to comment.