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.

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

Travis is failing #36138

Closed
TimNN opened this issue Aug 30, 2016 · 10 comments · Fixed by #36198
Closed

Travis is failing #36138

TimNN opened this issue Aug 30, 2016 · 10 comments · Fixed by #36198

Comments

@TimNN
Copy link
Contributor

TimNN commented Aug 30, 2016

I believe this is due to travis using system llvm which misses the fix backported in #36117.

---- [run-pass] run-pass/issue-36023.rs stdout ----

error: test run failed!
status: signal: 11
command: x86_64-unknown-linux-gnu/test/run-pass/issue-36023.stage2-x86_64-unknown-linux-gnu 
stdout:
------------------------------------------

------------------------------------------
stderr:
------------------------------------------

------------------------------------------

thread '[run-pass] run-pass/issue-36023.rs' panicked at 'explicit panic', /build/src/tools/compiletest/src/runtest.rs:2356
note: Run with `RUST_BACKTRACE=1` for a backtrace.

cc @eddyb

@eddyb
Copy link
Member

eddyb commented Aug 30, 2016

@rust-lang/tools Can we do anything about this?

@nagisa
Copy link
Member

nagisa commented Aug 30, 2016

Our best bet is to add a // ignore-travis for tests, I think (alternative is to drop support for LLVM pre 3.9, which would be a shame, even if its buggy in certain cases).

Implementation of such ignore directive could be implemented by injecting an environment variable COMPILETEST_IS_TRAVIS or some such (done this way) and checking for that in src/tools/compiletest/src/header.rs

@jonas-schievink
Copy link
Contributor

There's no need to set an extra environment variable, just check if HAS_JOSH_K_SEAL_OF_APPROVAL is set (or TRAVIS if you're boring 😉 )

@nagisa
Copy link
Member

nagisa commented Aug 30, 2016

@jonas-schievink but… but… what if people have HAS_JOSH_K_SEAL_OF_APPROVAL on their own systems too?

@aturon
Copy link
Member

aturon commented Aug 30, 2016

cc @alexcrichton

@alexcrichton
Copy link
Member

@eddyb no we do not have existing infrastructure for this. I'd prefer to avoid // ignore-travis as that's not the relevant piece here, but rather something like // ignore-llvm-3.7 or // min-llvm-version 3.7. Distros or anyone else building with an older LLVM are likely to hit this and we'd need a solution for them as well.

That being said I think it's fine to temporarily disable the test until we get that. There's basically a 0% chance of us regressing that test as it's just an LLVM bug, and they already have a regression test upstream as well.

@frewsxcv
Copy link
Member

frewsxcv commented Sep 1, 2016

Another option would be to create a base Docker image (publishing it to Docker Hub) that is just FROM ubuntu:xenial and the appropriate steps to build whatever version of LLVM we want.

@alexcrichton
Copy link
Member

We currently advertise ourselves as supporting LLVM 3.7+, and Travis is kinda our test for that, so I'd prefer to not pull that forward just yet (but instead just fix this)

@arielb1
Copy link
Contributor

arielb1 commented Sep 1, 2016

There's basically a 0% chance of us regressing that test as it's just an LLVM bug

For the record, I downgraded our LLVM version in a botched rebase and only this test stopped it at bors. Hopefully, someone would have noticed the absence of a fix, but you can't count of that.

@alexcrichton
Copy link
Member

@arielb1 touche!

alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 1, 2016
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
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
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
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
…rner

Fixed E0529's label and unit test

Fixes rust-lang#36195 part of rust-lang#35233.

This is ready for review, but will likely fail Travis due to rust-lang#36138. I changed the wording of the label, so feedback on that would be appreciated.

r? @jonathandturner
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
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
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
…rner

Fixed E0529's label and unit test

Fixes rust-lang#36195 part of rust-lang#35233.

This is ready for review, but will likely fail Travis due to rust-lang#36138. I changed the wording of the label, so feedback on that would be appreciated.

r? @jonathandturner
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
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
sophiajt pushed a commit to sophiajt/rust that referenced this issue Sep 2, 2016
…rner

Fixed E0529's label and unit test

Fixes rust-lang#36195 part of rust-lang#35233.

This is ready for review, but will likely fail Travis due to rust-lang#36138. I changed the wording of the label, so feedback on that would be appreciated.

r? @jonathandturner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants