1479540 - mozilla-central fails to build with latest rust nightly

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

The latest nightly rustc emits "aarch64-fuchsia" as the very first target from `rustc --print target-list` and this blows up at https://searchfox.org/mozilla-central/rev/033d45ca70ff32acf04286244644d19308c359d5/build/moz.configure/init.configure#595

 0:08.08 Traceback (most recent call last):
 0:08.08   File "/index/mozilla-central/gecko-dev/configure.py", line 123, in <module>
 0:08.08     sys.exit(main(sys.argv))
 0:08.08   File "/index/mozilla-central/gecko-dev/configure.py", line 29, in main
 0:08.08     sandbox.run(os.path.join(os.path.dirname(__file__), 'moz.configure'))
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 428, in run
 0:08.08     func(*args)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 474, in _value_for
 0:08.08     return self._value_for_depends(obj, need_help_dependency)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/util.py", line 944, in method_call
 0:08.08     cache[args] = self.func(instance, *args)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 483, in _value_for_depends
 0:08.08     return obj.result(need_help_dependency)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/util.py", line 944, in method_call
 0:08.08     cache[args] = self.func(instance, *args)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 123, in result
 0:08.08     return self._func(*resolved_args)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 1003, in wrapped
 0:08.08     return new_func(*args, **kwargs)
 0:08.08   File "/index/mozilla-central/gecko-dev/build/moz.configure/rust.configure", line 122, in rust_supported_targets
 0:08.08     t = split_triplet(t, allow_unknown=True)
 0:08.08   File "/index/mozilla-central/gecko-dev/python/mozbuild/mozbuild/configure/__init__.py", line 1003, in wrapped
 0:08.08     return new_func(*args, **kwargs)
 0:08.08   File "/index/mozilla-central/gecko-dev/build/moz.configure/init.configure", line 595, in split_triplet
 0:08.09     cpu, manufacturer, os = triplet.split('-', 2)
 0:08.09 ValueError: need more than 2 values to unpack

Comment 1

[Chris Manchester (limited bugmail, email directly)]

I have a workaround for this locally...

Comment 2

[Mike Hommey [:glandium]]

This should actually be fixed in rust. There is no reason for this one target not to follow the pattern that *all* other targets follow. That is, it should be aarch64-unknown-fuchsia.

Comment 3

[Mike Hommey [:glandium]]

Same with x86_64-fuchsia.

Comment 4

[Mike Hommey [:glandium]]

Sigh. Thank you Google. https://github.com/rust-lang/rust/issues/52763

Comment 5

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

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

Comment 6

[Nathan Froyd [:froydnj]]

Comment on attachment 8996147 [details]
Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

https://reviewboard.mozilla.org/r/260378/#review267530

It's really unfortunate that we have to hack around this.

::: build/moz.configure/init.configure:600
(Diff revision 1)
> -    cpu, manufacturer, os = triplet.split('-', 2)
> +    # Additionally, some may omit "unknown" when the manufacturer
> +    # is not specified and emit
> +    #   CPU_TYPE-OPERATING_SYSTEM
> +    parts = triplet.split('-', 2)
> +    if len(parts) == 3:
> +        cpu, manufacturer, os = parts

Maybe replace `manufacturer` here with `_`, so it's obvious that we don't care about it below?

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8996147 [details]
Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36f4ba2fb6f5
Accept "triplet" strings with only two parts in moz.configure. r=froydnj

Comment 9

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/36f4ba2fb6f5

Comment 10

[Masatoshi Kimura [:emk]]

Please consider uplifting because dxr's m-b tree still stops updating.

Comment 11

[Chris Manchester (limited bugmail, email directly)]

No problem, although if it's building the beta tree with a nightly rustc that's not guaranteed to work in general.

Comment 12

[Nick Thomas [:nthomas] (UTC+12)]

mozilla-release and mozilla-esr are also busted configure without this fix (bug 1484554). Is there any risk in uplifting it ?

In the long-term, would it be better to leverage the rust toolchains that taskcluster supports on a per-branch basis ?

Comment 13

[Nick Thomas [:nthomas] (UTC+12)]

Is there any way if we can test if jenkins jobs of mozilla-beta/release/esr60 + this bug will fix builds of dxr ?

Comment 14

[Masatoshi Kimura [:emk]]

(In reply to Nick Thomas [:nthomas] (UTC+12) from comment #12)
> In the long-term, would it be better to leverage the rust toolchains that
> taskcluster supports on a per-branch basis ?

Why dxr is not using them at the moment?

Comment 15

[Chris Manchester (limited bugmail, email directly)]

I'm not sure. This will probably fix things if comment 0 is the error, but it really seems these jobs should be using the rustc versions from the branches they're indexing to avoid something like this in the future.

Comment 16

[(not currently active) Ted Mielczarek]

Does dxr need nightly Rust for a particular reason? If not, it would be better to use the Rust toolchain that our other CI builds use.

Comment 17

[Nick Thomas [:nthomas] (UTC+12)]

I can't see the config for jenkins jobs or workers but suspect it's the same for all dxr branches, so central tends to drive changes across the board. The taskcluster approach, where rust comes in via the MOZ_TOOLCHAINS envvar and all the generation behind that, might be hard to adapt for DXR ? tooltool would probably have fairly easy, but c'est la vie. Perhaps there is something else in-tree DXR can use ?

Comment 18

[Nick Thomas [:nthomas] (UTC+12)]

As noted in bug 1484554, klibby migrated dxr to mdc1 today and swapped to text-only indexing on beta, release, and esr60, which means a matching rust install is not required, and no uplift of the change here.

Comment 19

[Hussam Al-Tayeb]

At the moment, firefox esr60 doesn't compile using the recently released rust 1.29 stable version. 
It would be nice to merge this patch into esr60. Since I compile my own packages, I would like to stay with esr60 on my computers but still have the latest rust 1.29 installation.
I already added it in my local esr60 tree.
Thank you.

Comment 20

[Landry Breuil (:gaston)]

62 doesnt pass configure too with rust 1.29, chokes on x86_64-fuchsia.

Comment 21

[Nathan Froyd [:froydnj]]

We generally do not support building older versions of firefox with newer versions of Rust; the expectation is that older versions of firefox are built with the version of Rust that was current (see build/moz.configure/rust.configure for the minimum Rust version).  We might make an exception for esr60; Chris, WDYT about uplifting this patch to esr60 so newer stable Rust would work?

Comment 22

[Landry Breuil (:gaston)]

(In reply to Nathan Froyd [:froydnj] from comment #21)
> We generally do not support building older versions of firefox with newer
> versions of Rust; 

Well, technically, 62 is *the current firefox version*, and you have no influence on the fact that a distributor might update its rust to *the current rust version*......

Comment 23

[Chris Manchester (limited bugmail, email directly)]

(In reply to Nathan Froyd [:froydnj] from comment #21)
> We generally do not support building older versions of firefox with newer
> versions of Rust; the expectation is that older versions of firefox are
> built with the version of Rust that was current (see
> build/moz.configure/rust.configure for the minimum Rust version).  We might
> make an exception for esr60; Chris, WDYT about uplifting this patch to esr60
> so newer stable Rust would work?

This patch is small enough that there's no harm in uplifting it, but is there a specific reason we want to be able to build older trees with newer rustcs? Would we do the same for a clang or gcc upgrade?

Comment 24

[Nathan Froyd [:froydnj]]

(In reply to Chris Manchester (:chmanchester) from comment #23)
> (In reply to Nathan Froyd [:froydnj] from comment #21)
> > We generally do not support building older versions of firefox with newer
> > versions of Rust; the expectation is that older versions of firefox are
> > built with the version of Rust that was current (see
> > build/moz.configure/rust.configure for the minimum Rust version).  We might
> > make an exception for esr60; Chris, WDYT about uplifting this patch to esr60
> > so newer stable Rust would work?
> 
> This patch is small enough that there's no harm in uplifting it, but is
> there a specific reason we want to be able to build older trees with newer
> rustcs? Would we do the same for a clang or gcc upgrade?

Reducing third-party pain is the only good reason here.

I think patches that touched Rust/C/C++ to permit building with a newer version of the compiler would have to be evaluated on a case-by-case basis.  We wouldn't take a Rust patch that modified code under third_party, I think, but we might take a C/C++ patch that fixed up something really obvious.

Comment 25

[Hussam Al-Tayeb]

(In reply to Chris Manchester (:chmanchester) from comment #23)
 Would we do the same for a clang or gcc upgrade?

Arguably no, but gcc support timeframe is much longer than rust. GCC 6 (released in April 2016) is still supported even though the latest stable release is 8.
I run gcc7.3.1 branch and I don't plan on upgrading my machines to gcc 8 till gcc7 support ends.
Rust, on the other hand, receives no support when the next stable release is shipped. That means I can lag on upgrading to latest gcc (since older releases still get bug fixes) but I have to upgrade to latest Rust to get a supported release.

Comment 26

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8996147 [details]
Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration: Convenience of third parties building esr 60
User impact if declined: None
Fix Landed on Version: 63
Risk to taking this patch (and alternatives if risky): Low risk. Configure will succeed with more versions of rustc as a result of this patch.
String or UUID changes made by this patch: None

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Comment 27

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8996147 [details]
Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

Approval Request Comment
[Feature/Bug causing the regression]: rustc 1.29
[User impact if declined]: None
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Configure will succeed with more versions of rustc as a result of this patch. Builds from mozilla infra will not be impacted.
[String changes made/needed]: None

Comment 28

[Julien Cristau [:jcristau]]

Comment on attachment 8996147 [details]
Bug 1479540 - Accept "triplet" strings with only two parts in moz.configure.

deal with rustc 1.29 weirdness, to help downstreams; approved for m-r and esr60.

Comment 29

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/bc651d3d910c

Comment 31

[Ryan VanderMeulen [:RyanVM]]

Sorry for missing this last week before we shipped 60.2.1. Landed for whatever ESR60 comes next.
https://hg.mozilla.org/releases/mozilla-esr60/rev/381ae910e9b2712699fdc6a9bb9702c04bccb8f5