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

Build Rust with MinGW GCC 8.1.0 #51989

Closed
wants to merge 3 commits into from
Closed

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented Jul 2, 2018

I've tested it locally on Windows 10 and Wine and it seems to fix #47048.
GDB also should be fixed: #40184 (comment)

This commit is not meant to be merged as is but to run tests with new toolchain.
If it succeeds it can be considered to push toolchain packages to AWS and begin using them.

@Mark-Simulacrum
Copy link
Member

r? @alexcrichton

We can't use try here since that doesn't run AppVeyor... we'll probably want to test locally if we decide to go ahead with this.

@mati865
Copy link
Contributor Author

mati865 commented Jul 2, 2018

I see. I've put [WIP] tags to avoid merging it by mistake because I was hoping try will work the same as r+.

In this case somebody with access to AWS could upload packages and open new PR superseding this one.

@Mark-Simulacrum
Copy link
Member

@bors r+

Uploaded artifacts to S3 mirror and updated the PR

@bors
Copy link
Contributor

bors commented Jul 2, 2018

📋 Looks like this PR is still in progress, ignoring approval.

Hint: Remove [WIP] from this PR's title when it is ready for review.

@Mark-Simulacrum Mark-Simulacrum changed the title [WIP] Build Rust with MinGW GCC 8.1.0 Build Rust with MinGW GCC 8.1.0 Jul 2, 2018
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 2, 2018

📌 Commit c3474d073a3e8a1aefa4bb3ca08e21ed31f6b992 has been approved by Mark-Simulacrum

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 2, 2018
@bors
Copy link
Contributor

bors commented Jul 6, 2018

⌛ Testing commit c3474d073a3e8a1aefa4bb3ca08e21ed31f6b992 with merge 4103f528f03fcd2016e4fdabf4f3203a35d2cfc9...

@bors
Copy link
Contributor

bors commented Jul 6, 2018

💔 Test failed - status-appveyor

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

mati865 commented Jul 6, 2018

Okay, I wrongly assumed it will work on 32bit if it works on 64bit...

I'll prepare 32bit environment over the weekend and see if I can fix it.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 7, 2018
@mati865
Copy link
Contributor Author

mati865 commented Jul 8, 2018

So I ran over dozen of builds locally with i686 GCC 8.1 from mingw-builds with different configure and build options and I was unable to reproduce this error.

I looked into llvm-emscripten and it's using very outdated llvm checkout.
The line in which the error was triggered was later changed to uint8_t instead of char llvm-mirror/llvm@30e9aa6#diff-ed4cb1e2f672825819ef46b07d866eb6L714. This is the only relevant thing I could find.

@alexcrichton
Copy link
Member

@mati865 the Emscripten LLVM is based on LLVM 4 which is pretty old at this point, but if there's a patch to send we can push it to our branch and try to fix the compile?

@mati865
Copy link
Contributor Author

mati865 commented Jul 9, 2018

@alexcrichton the thing is I cannot reproduce this error locally with the same compiler and configure options.

@alexcrichton
Copy link
Member

@mati865 I can reproduce locally if I download the same compiler, enable emscripten, and then ./x.py build src/llvm-emscripten. Could you try that?

@mati865
Copy link
Contributor Author

mati865 commented Jul 9, 2018

So it turns out running ./x.py changes PATH variable at some point which I used to test different compiler:

$ export PATH="/opt/gcc8/mingw32/bin/:$PATH"
$ gcc -v      # to see if everything is ok

When cmake is being ran from src/bootstrap/native.rs variable PATH is already modified to contain:
"D:\\msys64\\mingw32\\bin;D:\\msys64\\opt\\gcc8\\mingw32\\bin\\;D:\\msys64\\mingw32\\bin;...
Second occurrence of D:\\msys64\\mingw32\\bin is where original MSYS2 PATH starts.

I'll test everything again when I fix my PATH.

@alexcrichton
Copy link
Member

Hm odd! I set a custom PATH myself and it worked (via ./x.py), but glad it was found for you at least!

@mati865
Copy link
Contributor Author

mati865 commented Jul 10, 2018

Okay Travis didn't complain, I think it can be tried again.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 10, 2018

📌 Commit c1da82a20b548d5f1d21dff3e8da1f6e28547452 has been approved by alexcrichton

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 10, 2018
@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 25, 2018
@bors
Copy link
Contributor

bors commented Jul 25, 2018

⌛ Testing commit 3fdafc8 with merge 5be69306aa065f8106e91f42d101e88612335cba...

@bors
Copy link
Contributor

bors commented Jul 25, 2018

💔 Test failed - status-travis

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 25, 2018
@rust-highfive
Copy link
Collaborator

The job arm-android of 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.
[00:17:54] [RUSTC-TIMING] proc_macro test:false 10.505
[00:17:54]    Compiling syntax_ext v0.0.0 (file:///checkout/src/libsyntax_ext)
[00:18:19] [RUSTC-TIMING] syntax_ext test:false 25.125

Broadcast message from root@travis-job-bfe1c554-21f1-4290-ae40-a3e7db5a44cc
 (unknown) at 7:57 ...
The system is going down for power off NOW!
[00:21:54] 
[00:21:54] Session terminated, terminating shell... ...terminated.

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 @TimNN. (Feature Requests)

@pietroalbini
Copy link
Member

Oh well, the machine got killed.

@bors retry

@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 Jul 25, 2018
@bors
Copy link
Contributor

bors commented Jul 25, 2018

⌛ Testing commit 3fdafc8 with merge 2b6cd0393bb5c5a3b57e6567beafafb5c1542ac4...

@bors
Copy link
Contributor

bors commented Jul 25, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 25, 2018
@pietroalbini
Copy link
Member

Ok, this consistently runs out of memory.

@mati865
Copy link
Contributor Author

mati865 commented Jul 25, 2018

There might be no easy fix for that, current AppVeyor configuration has 4 cores and 6 GiB memory + 1 GiB swap which should suffice.
Some of possible workarounds:

  • bump llvm hoping unspecified commit reduced compiler memory footprint
  • wait for GCC 8.2 hoping it's more optimised
  • switch to clang which tends to use less memory. It used to be very problematic with MinGW in the past but version 7 (trunk) received many fixes.

@kennytm kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 25, 2018
@pietroalbini
Copy link
Member

Hmm, @rust-lang/infra, what's the best way to handle this?

@alexcrichton
Copy link
Member

Fow now AFAIK we don't have a pressing need to upgrade GCC, so it's unlikely to receive a lot of effort to help push it over the finish line. If no one can think of a solution off the top of their heads we should likely close this for now

@mati865
Copy link
Contributor Author

mati865 commented Jul 30, 2018

While this bug is quite problematic I do agree there is no straightforward fix for now.
Close this PR whenever you want.

I'll reach out mingw-w64 developers to see if something can be done.

@alexcrichton
Copy link
Member

Ok, thanks regardless for pushing on this though @mati865!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
8 participants