-
Notifications
You must be signed in to change notification settings - Fork 102
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
Support testing of cg_clif in rust's CI #1357
Conversation
@hexagonal-sun I think this will help with building on NixOS too. Setting |
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.
Thanks! This broadly looks fine to me - I suspect it might need a couple more changes when you write the test code in bootstrap, but I don't have any concerns :)
Yeah, I needed a lot more changes. I got a branch over at https://github.com/bjorn3/rust/tree/test_cg_clif_in_ci. I'm going to call it a day for now though. I will sync the changes to this PR tomorrow and after this PR is merged I will first sync all changes to the rust repo before opening a PR with the actual CI integration. |
ab3e907
to
0ecaca7
Compare
@bjorn3 I've only read the commit descriptions but this looks broadly fine to me still :) let me know if you want a full review. |
There are a couple of things left to implement for me. After that I would appreciate a full review. I will let you know when it is ready. |
490dcbf
to
f20bffc
Compare
f20bffc
to
caec60b
Compare
Landed part of the changes directly on master: dfaeab8...4ece6d0 |
caec60b
to
d262780
Compare
8acf0c2
to
c252660
Compare
Split out https://github.com/bjorn3/rustc_codegen_cranelift/pull/1373 and landed 8acf0c2 and 316d04b directly on master. |
c252660
to
eceb0df
Compare
eceb0df
to
08b1ec4
Compare
e3fe724
to
9002f8f
Compare
9ca3e12
to
0f4c8dc
Compare
17e8c04
to
4088952
Compare
https://github.com/bjorn3/rustc_codegen_cranelift/pull/1378 made it possible to test entirely with an LLVM compiled standard library. This avoids the need to vendor the standard library dependencies. |
@jyn514 This PR is ready for review. The rust side (rust-lang/rust@master...bjorn3:rust:test_cg_clif_in_ci#diff-8479eab02701e686aedb15b567dc8fc31220c6e4efb9565ccc9d662b7fee2214) is still missing vendoring, but that shouldn't be too hard to wire up now. The download dir created by |
49be594
to
b4e4611
Compare
Rust's build system only handles cargo, not rustc.
This will make it easier to skip patching if unnecessary in the future
When building as part of rust, the sysroot source dir is symlinked to the main source dir, which contains the build dir to which we are likely copying.
This makes it easier for ./x.py to vendor all dependencies
b4e4611
to
485d7e1
Compare
i'm not sure when i'll get a chance to look at this, sorry. you could ask someone on bootstrap to review or just open a PR directly to rust-lang/rust. |
I'm going to skip implementing vendoring support on the rust side for now and instead skip the tests that need external crates. I intend to implement it later, but for now even running part of the tests is better than nothing. @jyn514 no worries. I intend to merge this PR and put up a PR for rust-lang/rust today. Most of the complexity of this PR is vendoring support which isn't relevant for now given the above. |
This will need a change on the rust side too to run
./y.rs prepare --download-dir some/dir
(if not building from a source tarball), vendor all deps as necessary and run./y.rs test --out-dir some/other/dir --download-dir some/dir
.r? @jyn514
Part of #1290