Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Revisit fix_is_ci_llvm_available logic #107234

Merged
merged 1 commit into from
Jan 27, 2023
+41 −34
Merged

Revisit fix_is_ci_llvm_available logic #107234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/bootstrap/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,9 @@ impl Config {
config.changelog_seen = toml.changelog_seen;

let build = toml.build.unwrap_or_default();
if let Some(file_build) = build.build {
config.build = TargetSelection::from_user(&file_build);
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB build triple wasn't read from result of get_toml. I don't know whether it was skipped deliberately or just simply forgotten. I'm leaning towards the latter, since build.host and build.target are read below from the same toml.

Copy link
Member

@jyn514 jyn514 Jan 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably accidental; https://github.com/rust-lang/rust/blob/8adbe6e436d7ae03dbb0e82729745e1e514eb508/src/bootstrap/config.rs#L815 gets it right because python sets it for https://github.com/rust-lang/rust/blob/8adbe6e436d7ae03dbb0e82729745e1e514eb508/src/bootstrap/build.rs#L4-L6 in https://github.com/rust-lang/rust/blob/8adbe6e436d7ae03dbb0e82729745e1e514eb508/src/bootstrap/bootstrap.py#L788-L797, so likely no one ever noticed.

I think it shouldn't ever matter in practice; even when we stop using python (#94829) the shell scripts should still respect build.build in config.toml, but changing it to make things testable seems good 👍


set(&mut config.out, flags.build_dir.or_else(|| build.build_dir.map(PathBuf::from)));
// NOTE: Bootstrap spawns various commands with different working directories.
Expand Down
7 changes: 7 additions & 0 deletions src/bootstrap/config/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,13 @@ fn download_ci_llvm() {
assert_eq!(parse_llvm(""), if_available);
assert_eq!(parse_llvm("rust.channel = \"dev\""), if_available);
assert!(!parse_llvm("rust.channel = \"stable\""));
assert!(parse_llvm("build.build = \"x86_64-unknown-linux-gnu\""));
assert!(parse_llvm(
"llvm.assertions = true \r\n build.build = \"x86_64-unknown-linux-gnu\" \r\n llvm.download-ci-llvm = \"if-available\""
));
assert!(!parse_llvm(
"llvm.assertions = true \r\n build.build = \"aarch64-apple-darwin\" \r\n llvm.download-ci-llvm = \"if-available\""
));
}

// FIXME: add test for detecting `src` and `out`
65 changes: 31 additions & 34 deletions src/bootstrap/native.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,43 +180,40 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
// https://doc.rust-lang.org/rustc/platform-support.html#tier-1
let supported_platforms = [
// tier 1
"aarch64-unknown-linux-gnu",
"i686-pc-windows-gnu",
"i686-pc-windows-msvc",
"i686-unknown-linux-gnu",
"x86_64-unknown-linux-gnu",
"x86_64-apple-darwin",
"x86_64-pc-windows-gnu",
"x86_64-pc-windows-msvc",
("aarch64-unknown-linux-gnu", false),
("i686-pc-windows-gnu", false),
("i686-pc-windows-msvc", false),
("i686-unknown-linux-gnu", false),
("x86_64-unknown-linux-gnu", true),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First five are false according to now removed

if (triple == "aarch64-unknown-linux-gnu" || triple.contains("i686")) && asserts {
    return false
}

("x86_64-apple-darwin", true),
("x86_64-pc-windows-gnu", true),
("x86_64-pc-windows-msvc", true),
// tier 2 with host tools
"aarch64-apple-darwin",
"aarch64-pc-windows-msvc",
"aarch64-unknown-linux-musl",
"arm-unknown-linux-gnueabi",
"arm-unknown-linux-gnueabihf",
"armv7-unknown-linux-gnueabihf",
"mips-unknown-linux-gnu",
"mips64-unknown-linux-gnuabi64",
"mips64el-unknown-linux-gnuabi64",
"mipsel-unknown-linux-gnu",
"powerpc-unknown-linux-gnu",
"powerpc64-unknown-linux-gnu",
"powerpc64le-unknown-linux-gnu",
"riscv64gc-unknown-linux-gnu",
"s390x-unknown-linux-gnu",
"x86_64-unknown-freebsd",
"x86_64-unknown-illumos",
"x86_64-unknown-linux-musl",
"x86_64-unknown-netbsd",
("aarch64-apple-darwin", false),
("aarch64-pc-windows-msvc", false),
("aarch64-unknown-linux-musl", false),
("arm-unknown-linux-gnueabi", false),
("arm-unknown-linux-gnueabihf", false),
("armv7-unknown-linux-gnueabihf", false),
("mips-unknown-linux-gnu", false),
("mips64-unknown-linux-gnuabi64", false),
("mips64el-unknown-linux-gnuabi64", false),
("mipsel-unknown-linux-gnu", false),
("powerpc-unknown-linux-gnu", false),
("powerpc64-unknown-linux-gnu", false),
("powerpc64le-unknown-linux-gnu", false),
("riscv64gc-unknown-linux-gnu", false),
("s390x-unknown-linux-gnu", false),
("x86_64-unknown-freebsd", false),
("x86_64-unknown-illumos", false),
("x86_64-unknown-linux-musl", false),
("x86_64-unknown-netbsd", false),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No "llvm with asserts" artifacts are published for tier 2 with host tools.

];
if !supported_platforms.contains(&&*config.build.triple) {
return false;
}

let triple = &*config.build.triple;
if (triple == "aarch64-unknown-linux-gnu" || triple.contains("i686")) && asserts {
// No alt builder for aarch64-unknown-linux-gnu today.
return false;
if !supported_platforms.contains(&(&*config.build.triple, asserts)) {
if asserts == true || !supported_platforms.contains(&(&*config.build.triple, true)) {
return false;
}
}

if CiEnv::is_ci() {
Expand Down