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

Remove Hacks and Fixmes from PR CI's LLVM-9 Container #79068

Merged
merged 2 commits into from
Nov 19, 2020

Conversation

DevJPM
Copy link
Contributor

@DevJPM DevJPM commented Nov 15, 2020

Now with LLVM 9 being the minimum supported version (thanks to #78848 ), we can
finally remove the hacks in the dockerfile.

This wasn't in the main PR bumping the version as I didn't quite
understand what's going on and needed here.

Relevant issues and PRs:

I hope I actually adressed things correctly here?

@rust-highfive
Copy link
Collaborator

r? @pietroalbini

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2020
python2.7 ../x.py --stage 2 test src/test/mir-opt --pass=build \
--host='' --target=armv5te-unknown-linux-gnueabi && \
python2.7 ../x.py --stage 2 test src/test/mir-opt \
--host='' --target=i686-unknown-linux-gnueabi && \
Copy link
Contributor

Choose a reason for hiding this comment

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

The target is called i686-unknown-linux-gnu (without the eabi, that's an ARM specific thing).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was a bit over-eager with the copy and paste there, it's fixed now (though with a force-push instead of a commit because I didn't see your review in time, sorry)

Now with LLVM 9 being the minimum supported version, we can
finally remove the hacks in the dockerfile.

This wasn't in the main PR bumping the version as I didn't quite
understand what's going on and needed here.
It seems that by default the 32-bit headers are not actually installed
when installing development tooling. As we're using gcc headers,
we need to install them as an extra package.

See for reference:
- https://stackoverflow.com/a/54082790
- https://askubuntu.com/a/106092

Also removed the now unused arm tooling
@jyn514 jyn514 added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Nov 16, 2020
@pietroalbini
Copy link
Member

@bors r+ rollup=iffy

Thanks!

@bors
Copy link
Contributor

bors commented Nov 16, 2020

📌 Commit b8f682b has been approved by pietroalbini

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2020
@bors
Copy link
Contributor

bors commented Nov 19, 2020

⌛ Testing commit b8f682b with merge 5a549d3...

@bors
Copy link
Contributor

bors commented Nov 19, 2020

☀️ Test successful - checks-actions
Approved by: pietroalbini
Pushing 5a549d3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 19, 2020
@bors bors merged commit 5a549d3 into rust-lang:master Nov 19, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants