profiler = true in config.toml no longer works #79124
Labels
C-bug
Category: This is a bug.
T-bootstrap
Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Comments
jyn514
added
the
T-bootstrap
|
FYI, I confirmed the workaround (setting the target-specific |
|
I can't reproduce this issue.
changelog-seen = 2
[llvm]
link-shared = true
[build]
sanitizers = true
profiler = true
[install]
[rust]
[target.x86_64-unknown-linux-gnu]
cc = "clang"
cxx = "clang++"
ar = "/usr/bin/llvm-ar"
ranlib = "/usr/bin/llvm-ranlib"
linker = "clang"
llvm-config = "/usr/bin/llvm-config"
[dist]> LANG=C git status
On branch master
Your branch is up to date with 'origin/master'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: src/bootstrap/config.rs
no changes added to commit (use "git add" and/or "git commit -a")The master branch is at b5c37e8
diff --git a/src/bootstrap/config.rs b/src/bootstrap/config.rs
index 94319a6d1e9..eb0e78bc941 100644
--- a/src/bootstrap/config.rs
+++ b/src/bootstrap/config.rs
@@ -341,7 +341,7 @@ fn do_merge<T: Merge>(x: &mut Option<T>, y: Option<T>) {
}
/// TOML representation of various global build decisions.
-#[derive(Deserialize, Default, Clone, Merge)]
+#[derive(Deserialize, Default, Clone, Merge, Debug)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
struct Build {
build: Option<String>,
@@ -621,8 +621,8 @@ pub fn parse(args: &[String]) -> Config {
config.config = cfg;
}
+ println!("Build: {:?}", toml.build);
let build = toml.build.unwrap_or_default();
-
config.hosts = if let Some(arg_host) = flags.host {
arg_host
} else if let Some(file_host) = build.host {
@@ -899,7 +899,14 @@ pub fn parse(args: &[String]) -> Config {
target.sanitizers = cfg.sanitizers.unwrap_or(build.sanitizers.unwrap_or_default());
target.profiler = cfg.profiler.unwrap_or(build.profiler.unwrap_or_default());
+ println!("`sanitizers` in [build]: {:?}", build.sanitizers);
+ println!("`profiler` in [build]: {:?}", build.profiler);
+ println!("`sanitizers` in [target.{:?}]: {:?}", triple, cfg.sanitizers);
+ println!("`profiler` in [target.{:?}]: {:?}", triple, cfg.profiler);
+ println!("final sanitizers config for {:?} : {:?}", triple, target.sanitizers);
+ println!("final profiler config for {:?} profiler: {:?}", triple, target.profiler);
config.target_config.insert(TargetSelection::from_user(&triple), target);
+
}
}
build log: binary: |
|
Also test on Windows 10 Output: binary:
|
|
Try it on Windows with a clean copy of [build]
profiler = true(I also set Don't change the target section. It should still say It could be that just having the |
Dylan-DPC-zz
pushed a commit
to Dylan-DPC-zz/rust
that referenced
this issue
Nov 19, 2020
…k-Simulacrum fix handling the default config for profiler and sanitizers rust-lang#78354 don't handle the case that user don't add any target-specific config in `[target.*]` of `config.toml`: ```toml changelog-seen = 2 [llvm] link-shared = true [build] sanitizers = true profiler = true [install] [rust] [dist] ``` The previes code handle the default config in `Config::prase()`: ```rust target.sanitizers = cfg.sanitizers.unwrap_or(build.sanitizers.unwrap_or_default()); target.profiler = cfg.profiler.unwrap_or(build.profiler.unwrap_or_default()); config.target_config.insert(TargetSelection::from_user(&triple), target); ``` In this case, `toml.target` don't contain any target, so the above code won't execute. Instead, a default `Target` is insert in https://github.com/rust-lang/rust/blob/c919f490bbcd2b29b74016101f7ec71aaa24bdbb/src/bootstrap/sanity.rs#L162-L166 The default value for `bool` is false, hence the issue in rust-lang#79124 This fix change the type of `sanitizers` and `profiler` to `Option<bool>`, so the default value is `None`, and fallback config is handled in `Config::sanitizers_enabled` and `Config::profiler_enabled` fix rust-lang#79124 cc `@Mark-Simulacrum` `@richkadel`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
PR #78354 broke the build for users of profiler_builtins.
The PR copies the (commented out) #profiler = false line (among other things) from the [build] section to the sample target-specific (linux in config.toml.example) section, and made related changes to bootstrap.
Intuitively (especially because the [build] profiler = false/true option is still present, I believe that if profiler = true in build, then profiler should also be true in the target-specific setting, if not explicitly set for the target.
Since I always build with profiler = true, I did not expect the build to stop working after rebasing with the change, but with this PR, the profiler_builtins library is no longer built.
I think the new target-specific settings, if not set, need to default to the non-specific setting.
Workaround:
I had to add a target-specific setting on each platform I test with (Linux, Windows, and MacOS). On Mac, for example, I added these two lines in my config.toml:
It's still building, but it looks like it finally started building the profiler_builtins library.
cc @Mark-Simulacrum
The text was updated successfully, but these errors were encountered: