Skip to content

Commit

Permalink
Auto merge of #130529 - onur-ozkan:better-ci-llvm-default, r=Kobzol
Browse files Browse the repository at this point in the history
change `download-ci-llvm` default from `if-unchanged` to `true`

Since #129473 and #130202, using `download-ci-llvm=true` is now the better default and it also fixes #130515.
  • Loading branch information
bors committed Sep 19, 2024
2 parents b0af276 + 05f10f4 commit 13a5097
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 8 deletions.
2 changes: 1 addition & 1 deletion config.example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
#
# Note that many of the LLVM options are not currently supported for
# downloading. Currently only the "assertions" option can be toggled.
#download-ci-llvm = if rust.channel == "dev" || rust.download-rustc != false { "if-unchanged" } else { false }
#download-ci-llvm = true

# Indicates whether the LLVM build is a Release or Debug build
#optimize = true
Expand Down
11 changes: 5 additions & 6 deletions src/bootstrap/src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2738,6 +2738,8 @@ impl Config {
download_ci_llvm: Option<StringOrBool>,
asserts: bool,
) -> bool {
let download_ci_llvm = download_ci_llvm.unwrap_or(StringOrBool::Bool(true));

let if_unchanged = || {
if self.rust_info.is_from_tarball() {
// Git is needed for running "if-unchanged" logic.
Expand All @@ -2761,10 +2763,7 @@ impl Config {
};

match download_ci_llvm {
None => {
(self.channel == "dev" || self.download_rustc_commit.is_some()) && if_unchanged()
}
Some(StringOrBool::Bool(b)) => {
StringOrBool::Bool(b) => {
if !b && self.download_rustc_commit.is_some() {
panic!(
"`llvm.download-ci-llvm` cannot be set to `false` if `rust.download-rustc` is set to `true` or `if-unchanged`."
Expand All @@ -2774,8 +2773,8 @@ impl Config {
// If download-ci-llvm=true we also want to check that CI llvm is available
b && llvm::is_ci_llvm_available(self, asserts)
}
Some(StringOrBool::String(s)) if s == "if-unchanged" => if_unchanged(),
Some(StringOrBool::String(other)) => {
StringOrBool::String(s) if s == "if-unchanged" => if_unchanged(),
StringOrBool::String(other) => {
panic!("unrecognized option for download-ci-llvm: {:?}", other)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn download_ci_llvm() {
assert!(!parse_llvm("llvm.download-ci-llvm = false"));
assert_eq!(parse_llvm(""), if_unchanged);
assert_eq!(parse_llvm("rust.channel = \"dev\""), if_unchanged);
assert!(!parse_llvm("rust.channel = \"stable\""));
assert!(parse_llvm("rust.channel = \"stable\""));
assert_eq!(parse_llvm("build.build = \"x86_64-unknown-linux-gnu\""), if_unchanged);
assert_eq!(
parse_llvm(
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/utils/change_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,9 @@ pub const CONFIG_CHANGE_HISTORY: &[ChangeInfo] = &[
severity: ChangeSeverity::Info,
summary: "New option `dist.vendor` added to control whether bootstrap should vendor dependencies for dist tarball.",
},
ChangeInfo {
change_id: 130529,
severity: ChangeSeverity::Info,
summary: "If `llvm.download-ci-llvm` is not defined, it defaults to `true`.",
},
];

0 comments on commit 13a5097

Please sign in to comment.