Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Add sample defaults for config.toml
- Allow including defaults in `src/bootstrap/defaults` using `profile = "..."`
- Add default config files
- Combine config files using the merge dependency.
- Add comments to default config files
- Add a README asking to open an issue if the defaults are bad
- Give a loud error if trying to merge `.target`, since it's not
  currently supported
- Use an exhaustive match
- Use `<none>` in config.toml.example to avoid confusion
- Fix bugs in `Merge` derives

Previously, it would completely ignore the profile defaults if there
were any settings in `config.toml`. I sent an email to the `merge` maintainer
asking them to make the behavior in this commit the default.

This introduces a new dependency on `merge` that hasn't yet been vetted.

I want to improve the output when `include = "x"` isn't found:

```
thread 'main' panicked at 'fs::read_to_string(&file) failed with No such file or directory (os error 2) ("configuration file did not exist")', src/bootstrap/config.rs:522:28
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failed to run: /home/joshua/rustc/build/bootstrap/debug/bootstrap test tidy
Build completed unsuccessfully in 0:00:00
```

However that seems like it could be fixed in a follow-up.
  • Loading branch information
jyn514 committed Sep 20, 2020
1 parent 7bdb5de commit c9c8fb8
Show file tree
Hide file tree
Showing 9 changed files with 136 additions and 25 deletions.
23 changes: 23 additions & 0 deletions Cargo.lock
Expand Up @@ -207,6 +207,7 @@ dependencies = [
"ignore",
"lazy_static",
"libc",
"merge",
"num_cpus",
"opener",
"pretty_assertions",
Expand Down Expand Up @@ -1900,6 +1901,28 @@ dependencies = [
"autocfg",
]

[[package]]
name = "merge"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "10bbef93abb1da61525bbc45eeaff6473a41907d19f8f9aa5168d214e10693e9"
dependencies = [
"merge_derive",
"num-traits",
]

[[package]]
name = "merge_derive"
version = "0.1.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "209d075476da2e63b4b29e72a2ef627b840589588e71400a25e3565c4f849d07"
dependencies = [
"proc-macro-error",
"proc-macro2",
"quote",
"syn",
]

[[package]]
name = "minifier"
version = "0.0.33"
Expand Down
10 changes: 10 additions & 0 deletions config.toml.example
Expand Up @@ -9,6 +9,16 @@
# a custom configuration file can also be specified with `--config` to the build
# system.

# =============================================================================
# Global Settings
# =============================================================================

# Use different pre-set defaults than the global defaults.
#
# See `src/bootstrap/defaults` for more information.
# Note that this has no default value (x.py uses the defaults in `config.toml.example`).
#profile = <none>

# =============================================================================
# Tweaking how LLVM is compiled
# =============================================================================
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Expand Up @@ -48,6 +48,7 @@ lazy_static = "1.3.0"
time = "0.1"
ignore = "0.4.10"
opener = "0.4"
merge = "0.1.0"

[target.'cfg(windows)'.dependencies.winapi]
version = "0.3"
Expand Down
76 changes: 51 additions & 25 deletions src/bootstrap/config.rs
Expand Up @@ -16,6 +16,7 @@ use crate::flags::Flags;
pub use crate::flags::Subcommand;
use crate::util::exe;
use build_helper::t;
use merge::Merge;
use serde::Deserialize;

macro_rules! check_ci_llvm {
Expand Down Expand Up @@ -278,10 +279,31 @@ struct TomlConfig {
rust: Option<Rust>,
target: Option<HashMap<String, TomlTarget>>,
dist: Option<Dist>,
profile: Option<String>,
}

impl Merge for TomlConfig {
fn merge(&mut self, TomlConfig { build, install, llvm, rust, dist, target, profile: _ }: Self) {
fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>) {
if let Some(new) = y {
if let Some(original) = x {
original.merge(new);
} 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);
assert!(target.is_none(), "merging target-specific config is not currently supported");
}
}

/// TOML representation of various global build decisions.
#[derive(Deserialize, Default, Clone)]
#[derive(Deserialize, Default, Clone, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Build {
build: Option<String>,
Expand Down Expand Up @@ -321,7 +343,7 @@ struct Build {
}

/// TOML representation of various global install decisions.
#[derive(Deserialize, Default, Clone)]
#[derive(Deserialize, Default, Clone, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Install {
prefix: Option<String>,
Expand All @@ -338,7 +360,7 @@ struct Install {
}

/// TOML representation of how the LLVM build is configured.
#[derive(Deserialize, Default)]
#[derive(Deserialize, Default, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Llvm {
skip_rebuild: Option<bool>,
Expand All @@ -365,7 +387,7 @@ struct Llvm {
download_ci_llvm: Option<bool>,
}

#[derive(Deserialize, Default, Clone)]
#[derive(Deserialize, Default, Clone, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Dist {
sign_folder: Option<String>,
Expand All @@ -389,7 +411,7 @@ impl Default for StringOrBool {
}

/// TOML representation of how the Rust build is configured.
#[derive(Deserialize, Default)]
#[derive(Deserialize, Default, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Rust {
optimize: Option<bool>,
Expand Down Expand Up @@ -434,7 +456,7 @@ struct Rust {
}

/// TOML representation of how each build target is configured.
#[derive(Deserialize, Default)]
#[derive(Deserialize, Default, Merge)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct TomlTarget {
cc: Option<String>,
Expand Down Expand Up @@ -523,27 +545,31 @@ impl Config {
}

#[cfg(test)]
let toml = TomlConfig::default();
let get_toml = |_| TomlConfig::default();
#[cfg(not(test))]
let toml = flags
.config
.map(|file| {
use std::process;

let contents = t!(fs::read_to_string(&file));
match toml::from_str(&contents) {
Ok(table) => table,
Err(err) => {
println!(
"failed to parse TOML configuration '{}': {}",
file.display(),
err
);
process::exit(2);
}
let get_toml = |file: PathBuf| {
use std::process;

let contents = t!(fs::read_to_string(&file), "configuration file did not exist");
match toml::from_str(&contents) {
Ok(table) => table,
Err(err) => {
println!("failed to parse TOML configuration '{}': {}", file.display(), err);
process::exit(2);
}
})
.unwrap_or_else(TomlConfig::default);
}
};

let mut toml = flags.config.map(get_toml).unwrap_or_else(TomlConfig::default);
if let Some(include) = &toml.profile {
let mut include_path = config.src.clone();
include_path.push("src");
include_path.push("bootstrap");
include_path.push("defaults");
include_path.push(format!("config.toml.{}", include));
let included_toml = get_toml(include_path);
toml.merge(included_toml);
}

let build = toml.build.unwrap_or_default();

Expand Down
11 changes: 11 additions & 0 deletions src/bootstrap/defaults/README.md
@@ -0,0 +1,11 @@
# About bootstrap defaults

These defaults are intended to be a good starting point for working with x.py,
with the understanding that no one set of defaults make sense for everyone.

They are still experimental, and we'd appreciate your help improving them!
If you use a setting that's not in these defaults that you think others would benefit from, please [file an issue] or make a PR with the changes.
Similarly, if one of these defaults doesn't match what you use personally,
please open an issue to get it changed.

[file an issue]: https://github.com/rust-lang/rust/issues/new/choose
13 changes: 13 additions & 0 deletions src/bootstrap/defaults/config.toml.codegen
@@ -0,0 +1,13 @@
# These defaults are meant for contributors to the compiler who modify codegen or LLVM
[llvm]
# This enables debug-assertions in LLVM,
# catching logic errors in codegen much earlier in the process.
assertions = true

[rust]
# This enables `RUSTC_LOG=debug`, avoiding confusing situations
# where adding `debug!()` appears to do nothing.
# However, it makes running the compiler slightly slower.
debug-logging = true
# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower.
incremental = true
8 changes: 8 additions & 0 deletions src/bootstrap/defaults/config.toml.compiler
@@ -0,0 +1,8 @@
# These defaults are meant for contributors to the compiler who do not modify codegen or LLVM
[rust]
# This enables `RUSTC_LOG=debug`, avoiding confusing situations
# where adding `debug!()` appears to do nothing.
# However, it makes running the compiler slightly slower.
debug-logging = true
# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower.
incremental = true
10 changes: 10 additions & 0 deletions src/bootstrap/defaults/config.toml.library
@@ -0,0 +1,10 @@
# These defaults are meant for contributors to the standard library and documentation.
[build]
# When building the standard library, you almost never want to build the compiler itself.
build-stage = 0
test-stage = 0
bench-stage = 0

[rust]
# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower.
incremental = true
9 changes: 9 additions & 0 deletions src/bootstrap/defaults/config.toml.user
@@ -0,0 +1,9 @@
# These defaults are meant for users and distro maintainers building from source, without intending to make multiple changes.
[build]
# When compiling from source, you almost always want a full stage 2 build,
# which has all the latest optimizations from nightly.
build-stage = 2
test-stage = 2
doc-stage = 2
# When compiling from source, you usually want all tools.
extended = true

0 comments on commit c9c8fb8

Please sign in to comment.