Closed
Bug 1353541
Opened 8 years ago
Closed 8 years ago
MinGW build broken by rustc
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jdm, Assigned: tjr)
References
(Blocks 1 open bug)
Details
(Whiteboard: [tor])
Attachments
(2 files)
I heard from a contributor with a mingw build that they were stuck on the configure step. The log shows that the Rust compiler can't find the standard library since it's looking for a different target than the one that's installed.
Assignee | ||
Comment 1•8 years ago
|
||
I think this aligns with a bug I have not formalized but am vaguely aware of. A patch that may fix it is:
--- a/build/moz.configure/rust.configure
+++ b/build/moz.configure/rust.configure
@@ -165,8 +165,8 @@ def rust_triple_alias(host_or_target):
# Windows
# XXX better detection of CXX needed here, to figure out whether
# we need i686-pc-windows-gnu instead, since mingw32 builds work.
- ('x86', 'WINNT'): 'i686-pc-windows-msvc',
- ('x86_64', 'WINNT'): 'x86_64-pc-windows-msvc',
+ ('x86', 'WINNT'): 'i686-pc-windows-gnu',
+ ('x86_64', 'WINNT'): 'x86_64-pc-windows-gnu',
}.get((host_or_target.cpu, os_or_kernel), None)
if rustc_target is None:
But I don't know if this is a complete fix, I had a different rust error after applying it:
make[5]: *** No rule to make target '../../toolkit/library/rust/../i686-pc-windows-gnu/release/libgkrust.a', needed by 'xul.dll'. Stop.
/home/tom/Documents/moz/mingw-work/just-build/config/recurse.mk:71: recipe for target 'toolkit/library/target' failed
I haven't investigated any of this very deeply.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131668/#review134430
::: build/moz.configure/rust.configure:168
(Diff revision 1)
> ('arm', 'Android'): 'armv7-linux-androideabi',
> ('x86', 'Android'): 'i686-linux-android',
> # Windows
> # XXX better detection of CXX needed here, to figure out whether
> # we need i686-pc-windows-gnu instead, since mingw32 builds work.
> - ('x86', 'WINNT'): 'i686-pc-windows-msvc',
> + ('x86', 'WINNT'): 'i686-pc-windows-gnu',
This is temporary, need to fix in next rev.
Assignee | ||
Comment 4•8 years ago
|
||
Besides the target being incorrect, rustc generates .lib files, when MinGW expects .a. See https://github.com/rust-lang/rust/pull/29520 for the rustc discussion.
Summary: Mingw build passes msvc target to Rust compiler → MinGW build broken by rustc
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → tom
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8859653 -
Flags: review?(ted)
Attachment #8859653 -
Flags: review?(nfroyd)
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131670/#review134520
Everything else seems fine, but the comment below is a blocker, I think. Or are we just ignoring the possibility of things like clang-cl cross compilation in other mingw patches that you're writing?
::: build/moz.configure/rust.configure:133
(Diff revision 2)
> + # If we are targetting Windows but the host is Linux, we need to
> + # use a different rustc target
> + os_or_kernel = 'WINNT-MINGW' if host.kernel == "Linux" and host_or_target.kernel == "WINNT" else os_or_kernel
This seems like the wrong check. What about the (admittedly hypothetical at the moment) world where somebody wants to cross-compile to Windows using clang-cl? That's going to want to use the windows-msvc target, rather than the windows-gnu target.
I think you want to check `c_compiler` or similar here, if that's at all possible.
Attachment #8859653 -
Flags: review?(nfroyd) → review-
Assignee | ||
Comment 7•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131670/#review134522
::: build/moz.configure/rust.configure:133
(Diff revision 2)
> + # If we are targetting Windows but the host is Linux, we need to
> + # use a different rustc target
> + os_or_kernel = 'WINNT-MINGW' if host.kernel == "Linux" and host_or_target.kernel == "WINNT" else os_or_kernel
We do not support nor intend to support compiling with clang-cl on Linux for Windows, no. I believe this statement is true not just for 'Tor/MinGW stuff' but all of Mozilla Windows builds.
I've talked with the clang-cl team at Google and they are treating 'building for Windows on Linux for clang-cl' as a hobby project and it's not really doable right now.
Down the road we might try to do that but it would be a whole new project. Again, I believe this is true for all of Mozilla.
Comment 8•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131670/#review134640
Attachment #8859653 -
Flags: review- → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 11•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131670/#review135854
This needs a small change, but otherwise looks good.
::: build/moz.configure/rust.configure:133
(Diff revision 4)
> # https://github.com/rust-lang/rust/blob/master/src/librustc_back/target/mod.rs
>
> # Avoid having to write out os+kernel for all the platforms where
> # they don't differ.
> os_or_kernel = host_or_target.kernel if host_or_target.kernel == 'Linux' and host_or_target.os != 'Android' else host_or_target.os
> + # If we are targetting Windows but the host is Linux, we need to
I'm going to concur with froydnj here--this isn't the right check. You'd hit this same issue if you were building on Windows using a mingw toolchain. The right thing to check is whether we're targeting Windows but building with GCC. We already have a `building_with_gcc`:
https://dxr.mozilla.org/mozilla-central/rev/e17cbb839dd225a2da7e5d5bec43cf94e11749d8/build/moz.configure/toolchain.configure#835
...so you can put that in `@depends` and instead write this as:
```
os_or_kernel = 'WINNT-MINGW' if building_with_gcc and host_or_target.kernel == "WINNT" else os_or_kernel
```
Attachment #8859653 -
Flags: review?(ted) → review-
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8859653 [details]
Bug 1353541 Fix rustc in MinGW build
https://reviewboard.mozilla.org/r/131670/#review136860
Thanks, this looks sensible.
::: build/moz.configure/rust.configure:135
(Diff revision 5)
> # Avoid having to write out os+kernel for all the platforms where
> # they don't differ.
> os_or_kernel = host_or_target.kernel if host_or_target.kernel == 'Linux' and host_or_target.os != 'Android' else host_or_target.os
> + # If we are targetting Windows but the compiler is gcc, we need to
> + # use a different rustc target
> + os_or_kernel = 'WINNT-MINGW' if building_with_gcc and host_or_target.kernel == "WINNT" else os_or_kernel
nit: we use single quotes for strings in these files.
Attachment #8859653 -
Flags: review?(ted) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 15•8 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/51edd4c0ae9f
Fix rustc in MinGW build r=froydnj,ted
Keywords: checkin-needed
Comment 16•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•