1319332 - Use the cpu given as --target/--host for rust

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Currently, we have a hardcoded list of rust targets, set depending on cpu family and OS.

The immediate problem is that since bug 1299864, Mozilla builds use a non-default rust target (i586-unknown-linux-gnu) to avoid SSE2 codegen from the normal i686 rust target.

But while doing that, it also enforces anyone building a x86 linux Firefox with rust enabled to have that target available, while a) it's likely unavailable b) it's likely not what the person building wants.

The least worse thing we can do IMHO is to derive the rust target from the C++ target, and make them both use the same "raw" CPU.

Which means to build with the i586-unknown-linux-gnu target on our (Mozilla) builds, we'd switch to --target=i586-linux-gnu instead of --target=i686-linux-gnu. People building with a --target=i686-linux-gnu would then pick the i686-unknown-linux-gnu target for rust.

I tested that on try, comparing the output with the current build, and there is no difference, except for the package file names, which can be worked around (and MOZ_BUILD_DATE).

Comment 1

[Ralph Giles (:rillian)]

Isn't the i585-linux support requirement going away in firefox 53, with bug 1308167? Or is this about maintaining tier-3 build support for that target?

Note that you'll still need a cpu mapping table, unless you fix also our for C++/android arm vs rust armv7.

Comment 3

[Henri Sivonen (:hsivonen)]

(In reply to Ralph Giles (:rillian) needinfo me from comment #1)
> Note that you'll still need a cpu mapping table, unless you fix also our for
> C++/android arm vs rust armv7.

And even GNU/Linux on ARMv7 using Mozilla-shipped Rust. E.g. Debian says armv7l-unknown-linux-gnueabihf bit Mozilla-shipped (rustup.rs) Rust wants armv7-unknown-linux-gnueabihf. (Notice no 'l' after '7'.)

It could be argued that this is a Rust bug, but anyway.

Comment 4

[Mike Hommey [:glandium]]

We "just" need to correlate with the output from rustc --print target-list.

Comment 5

[Dan Horák]

Hi, has been there any progress? We are running a CI system for tier-3 arches (ppc64/ppc64le, s390x) and it requires patching the sources before we can even attempt a build there.

Comment 6

[(not currently active) Ted Mielczarek]

Nobody has done any work here that I'm aware of. Per comment 4, the best way to fix this is probably:
1) add a function in rust.configure that runs `rustc --print target-list` and calls `split_triplet` on each line of output to canonicalize it. This will probably need some fixup because `split_triplet` currently errors on unknown OS/CPU values. Perhaps we could add an additional argument like `allow_unknown=True` that would simply set `canonical_os = os` and `canonical_cpu = cpu` for unknown values:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/build/moz.configure/init.configure#500

2) In `rust_target`, instead of the hardcoded lookup table, make a lookup table from the list of values returned from the previous step, probably still indexed by (cpu, os), like `targets = {(t.cpu, t.os): t.alias for t in rustc_targets}`:
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/build/moz.configure/rust.configure#133

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1319332 - Derive the rust targets from the build host/target.

Review commit: https://reviewboard.mozilla.org/r/186700/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186700/

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

Always fun when something works locally but not on try.

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186700/diff/1-2/

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

https://reviewboard.mozilla.org/r/186700/#review191850

::: build/moz.configure/rust.configure:159
(Diff revision 2)
>          # compiled for the appropriate ABI.  We need to perform appropriate
>          # munging to get the correct option to rustc.
>          #
>          # The canonical list of targets supported can be derived from:
>          #
>          # https://github.com/rust-lang/rust/blob/master/src/librustc_back/target/mod.rs

You can probably drop this reference now that you've added `rust_supported_targets` and just say that we get the list of supported targets from rustc and try to find the closest match to the autoconf-style target triple we're using.

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186700/diff/2-3/

Comment 12

[Dan Horák]

The proposed patch allows to detect Rust on Fedora 27 ppc64le and Fedora 26 s390x systems.

Comment 13

[Nathan Froyd [:froydnj]]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

https://reviewboard.mozilla.org/r/186700/#review192880

This is nice.

::: build/moz.configure/rust.configure:162
(Diff revision 2)
> -        # 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
> +            if building_with_gcc:
> +                host_or_target_raw_os = host_or_target_os = 'windows-gnu'
> +            else:
> +                host_or_target_raw_os = host_or_target_os = 'windows-msvc'

The double assignment here is a little subtle--I was almost going to comment on the absence of assignment to `host_or_target_os`.  Maybe:

```
if building_with_gcc:
  host_or_target_raw_os =
else:
  host_or_target_raw_os =
host_or_target_os = host_or_target_raw_os
```
?

::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:1421
(Diff revision 2)
> +            # Raw list returned by rustc version 1.19, + ios, which somehow
> +            # don't appear https://github.com/rust-lang/rust/issues/36156

Nit: "...which somehow doesn't appear in the default list"?

::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:1537
(Diff revision 2)
> +            'x86_64-unknown-openbsd',
> +            'aarch64-unknown-linux-gnu',
> +            'armv7-unknown-linux-gnueabihf',
> +            'sparc64-unknown-linux-gnu',
> +            'i686-unknown-linux-gnu',
> +            'x86_64-unknown-linux-gnu',

I think Fedora (maybe others?) use `x86_64-pc-linux-gnu`?  Does that get canonicalized by config.sub?

Comment 14

[Mike Hommey [:glandium]]

> I think Fedora (maybe others?) use `x86_64-pc-linux-gnu`?  Does that get canonicalized by config.sub?

No it's not, and indeed, while config.guess used to give x86_64-unknown-linux-gnu, it now returns x86_64-pc-linux-gnu. That said, the unknown/pc part is ignored entirely. I'll replace anyways.

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8915486 [details]
Bug 1319332 - Derive the rust targets from the build host/target.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186700/diff/3-4/

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/035d3125ca2f
Derive the rust targets from the build host/target. r=froydnj

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/035d3125ca2f