-
Notifications
You must be signed in to change notification settings - Fork 13k
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
test: Add a min-llvm-version directive #36198
Conversation
r? @brson |
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
(note that this should hold off on r+ until we confirm travis is green) |
https://travis-ci.org/rust-lang/rust/builds/156884048#L7287
🎊 |
.expect("Malformed llvm version directive"); | ||
// Ignore if actual version is smaller the minimum required | ||
// version | ||
&actual_version[..] < min_version |
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.
Won't this fail for something like llvm version 3.10?
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.
Looks like it.
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.
But LLVM 3.9 is going to be followed by 4.0 so maybe it doesn't matter.
@bors r+ |
📌 Commit 82b57ed has been approved by |
@@ -148,6 +148,8 @@ pub fn compiletest(build: &Build, | |||
if let Some(ref dir) = build.lldb_python_dir { | |||
cmd.arg("--lldb-python-dir").arg(dir); | |||
} | |||
let llvm_version = output(Command::new(&llvm_config).arg("--version")); |
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.
Should this be build.llvm_config
?
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.
Regardless of what it should be, llvm_config
is undefined at this point:
error[E0425]: unresolved name `llvm_config`
--> src/bootstrap/check.rs:151:45
|
151 | let llvm_version = output(Command::new(&llvm_config).arg("--version"));
| ^^^^^^^^^^^
We've got tests which require a particular version of LLVM to run as they're testing bug fixes. Our build system, however, supports multiple LLVM versions, so we can't run these tests on all LLVM versions. This adds a new `min-llvm-version` directive for tests so they can opt out of being run on older versions of LLVM. This then namely applies that logic to the `issue-36023.rs` test case and... Closes rust-lang#36138
82b57ed
to
96283fc
Compare
@bors: r=brson |
📌 Commit 96283fc has been approved by |
@bors rollup |
test: Add a min-llvm-version directive We've got tests which require a particular version of LLVM to run as they're testing bug fixes. Our build system, however, supports multiple LLVM versions, so we can't run these tests on all LLVM versions. This adds a new `min-llvm-version` directive for tests so they can opt out of being run on older versions of LLVM. This then namely applies that logic to the `issue-36023.rs` test case and... Closes rust-lang#36138
test: Add a min-llvm-version directive We've got tests which require a particular version of LLVM to run as they're testing bug fixes. Our build system, however, supports multiple LLVM versions, so we can't run these tests on all LLVM versions. This adds a new `min-llvm-version` directive for tests so they can opt out of being run on older versions of LLVM. This then namely applies that logic to the `issue-36023.rs` test case and... Closes rust-lang#36138
test: Add a min-llvm-version directive We've got tests which require a particular version of LLVM to run as they're testing bug fixes. Our build system, however, supports multiple LLVM versions, so we can't run these tests on all LLVM versions. This adds a new `min-llvm-version` directive for tests so they can opt out of being run on older versions of LLVM. This then namely applies that logic to the `issue-36023.rs` test case and... Closes rust-lang#36138
LLVM 3.9.0 release did not include r279980, so the test actually fails with LLVM 3.9.0, although it succeeds with Rust's LLVM fork. |
@sanxiyn I'm sorry I didn't get them to backport the patches 😞. |
We've got tests which require a particular version of LLVM to run as they're
testing bug fixes. Our build system, however, supports multiple LLVM versions,
so we can't run these tests on all LLVM versions.
This adds a new
min-llvm-version
directive for tests so they can opt out ofbeing run on older versions of LLVM. This then namely applies that logic to the
issue-36023.rs
test case and...Closes #36138