-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Allow enabling incremental via config.toml #51317
Conversation
I mean, i'm all for this (we talked about this on IRC) but i'd still want someone like @Mark-Simulacrum to take a look, since i've barely touched rustbuild. |
src/bootstrap/config.rs
Outdated
@@ -529,6 +530,7 @@ impl Config { | |||
set(&mut config.rust_dist_src, rust.dist_src); | |||
set(&mut config.quiet_tests, rust.quiet_tests); | |||
set(&mut config.test_miri, rust.test_miri); | |||
set(&mut config.incremental, rust.incremental); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't quite right, since this means that the configuration, if present, will override the command line flags. Instead we should have the command line flags override the configuration.
(I can provide instructions how best to do this if you need help, just let me know).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am aware of that, but there's no flag for disabling incremental. Either the command is there, or it's not. Or are you talking about the case where the config explicitly says "false" but -i is passed?
@bors r+ |
📌 Commit 3b02376 has been approved by |
…Mark-Simulacrum Allow enabling incremental via config.toml r? @QuietMisdreavus
Rollup of 6 pull requests Successful merges: - #51288 (Remove rustdoc-specific is_import field from HIR) - #51299 (const fn integer operations) - #51317 (Allow enabling incremental via config.toml) - #51323 (Generate br for all two target SwitchInts) - #51326 (Various minor slice iterator cleanups) - #51329 (Remove the unused `-Z trans-time-graph` flag.) Failed merges:
r? @QuietMisdreavus