-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Cross-compile builder to Windows for PRs on Travis #49866
Conversation
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Nice!
Looking at Travis it looks like this took ~6m, although I don't think it's checking librustc yet. I wonder if to solve the miniz-sys issue we could create a shim shell script C compiler which basically does nothing? Or otherwise provide overrides for the relevant build scripts?
Additionally, would it be possible to skip the submodule updates in this PR? I think most of them shouldn't be necessary other than maybe libcompiler_builtins, right?
src/libstd/build.rs
Outdated
@@ -17,6 +17,10 @@ use std::process::Command; | |||
use build_helper::{run, native_lib_boilerplate}; | |||
|
|||
fn main() { | |||
if *"1" == *env::var_os("CHECK").unwrap_or_default() { |
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.
Could this be called RUSTBUILD_CHECK
and that way I think we could skip the == *"1"
and instead just check for its presence
We need submodule updates for at least |
Ah good point yeah, it may be that the submodules are too entangled to remove them from being updated |
45e747c
to
fe63344
Compare
Note: Making all build scripts no-ops doesn't work as winapi (at least) needs it's build script to run in order to compile (or so I assume). That means we'll probably need a whitelist -- how do you feel about that, @alexcrichton? Is a whitelist fine? I'm not really happy with that solution/answer... |
Hm ok we probably don't want to bend over backwards too much as this may run the risk of causing more bugs than it discovers, so maybe we could use a docker image with a MinGW toolchain and go from there? The main thing we want to skip is LLVM mostly |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
fe63344
to
a9c4618
Compare
@alexcrichton Okay, so I've gone with the docker approach now and it compiles std, test, and rustc. I think this is pretty much ready to land from my perspective pending any concerns from you; there are effectively no rustbuild changes to speak of: just making rustc get built on check for --target (not --host) builds. |
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.
This looks great! Just a few minor nits
.travis.yml
Outdated
@@ -176,6 +176,8 @@ matrix: | |||
if: branch = auto | |||
- env: IMAGE=x86_64-gnu-distcheck | |||
if: branch = auto | |||
- env: IMAGE=mingw-check | |||
if: type = pull_request |
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.
I'd actually run this on the auto
branch as well just to be extra sure it's always passing
src/ci/docker/mingw-check/Dockerfile
Outdated
COPY scripts/sccache.sh /scripts/ | ||
RUN sh /scripts/sccache.sh | ||
|
||
ENV SCRIPT python2.7 ../x.py check --target=i686-pc-windows-gnu -vv |
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.
The -vv
here can probably be removed
src/bootstrap/check.rs
Outdated
@@ -64,7 +64,6 @@ pub struct Rustc { | |||
|
|||
impl Step for Rustc { | |||
type Output = (); | |||
const ONLY_HOSTS: bool = true; |
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.
Instead of removing this, would --host=i686-pc-windows-gnu
work below?
a9c4618
to
de34533
Compare
Nits fixed. |
@bors: r+ |
📌 Commit de34533 has been approved by |
…r=alexcrichton Cross-compile builder to Windows for PRs on Travis I chose a completely arbitrary windows target here (I have no idea what's best, we could do multiple -- they are relatively fast). r? @alexcrichton
…r=alexcrichton Cross-compile builder to Windows for PRs on Travis I chose a completely arbitrary windows target here (I have no idea what's best, we could do multiple -- they are relatively fast).
I chose a completely arbitrary windows target here (I have no idea what's best, we could do multiple -- they are relatively fast).
r? @alexcrichton