Closed
Bug 1321073
Opened 4 years ago
Closed 4 years ago
--enable-rust on Android builds
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox53 fixed)
RESOLVED
FIXED
mozilla53
Tracking | Status | |
---|---|---|
firefox53 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
References
Details
Attachments
(1 file)
7.57 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
It would be nice to have all of our automation builds using Rust, even if it would lower the detection of --disable-rust breakage.
![]() |
Assignee | |
Comment 1•4 years ago
|
||
There's no good, common place to put the mozconfig.rust include. The mobile/android/config/mozconfigs/common file isn't suitable, because that file gets included for artifact builds (tier-2) and as those are --disable-compile-environment, moz.configure complains that we are passing --enable-rust. So we have this monstronsity of a patch, which appears to be passing on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=69832c90d093c1b3bf4f3ca2deaa8e110629e4ea which requires bug 1320960 so we get an x86-android standard library.
Attachment #8815436 -
Flags: review?(mshal)
Comment 2•4 years ago
|
||
Comment on attachment 8815436 [details] [diff] [review] enable Rust on Android builds Does it work if you --enable-rust in the common one and then --disable-rust after including it in the frontend and gradle-dependencies mozconfigs? That's roughly synonymous with how the common file sets HOST_CC and the main mozconfig unsets it, though obviously this isn't an environment variable :)
Attachment #8815436 -
Flags: review?(mshal) → review+
Comment 3•4 years ago
|
||
Why not have mobile/android/config/mozconfigs/common_compile and mobile/android/config/mozconfigs/common_artifact?
![]() |
Assignee | |
Comment 4•4 years ago
|
||
(In reply to :Ms2ger (⌚ UTC+1/+2) from comment #3) > Why not have mobile/android/config/mozconfigs/common_compile and > mobile/android/config/mozconfigs/common_artifact? I thought about this, but didn't go this route because common_compile would only contain an include of mozconfig.rust. It's possible that more things could be lifted into common_compile, but I suspect most of those are already in the "common" file. I think the right way to re-organizing things is to make android{,-x86}/common files and lift things into there; maybe that would show things that could profitably be moved to the toplevel.
![]() |
Assignee | |
Comment 5•4 years ago
|
||
(In reply to Michael Shal [:mshal] from comment #2) > Does it work if you --enable-rust in the common one and then --disable-rust > after including it in the frontend and gradle-dependencies mozconfigs? > That's roughly synonymous with how the common file sets HOST_CC and the main > mozconfig unsets it, though obviously this isn't an environment variable :) For the record, since we discussed this possibility on IRC: I didn't try this, but suspect that it wouldn't work because the gradle-dependencies build would still see the --enable-rust bit on the configure command line and complain about it straightaway, rather than the build configuration that would result from --enable-rust --disable-rust.
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b6a523e34b enable Rust on Android builds; r=mshal
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1309c56ba4cd follow-up - don't --enable-rust for android-api-15-frontend; r=me
Comment 8•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e3b6a523e34b https://hg.mozilla.org/mozilla-central/rev/1309c56ba4cd
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment 9•4 years ago
|
||
as a note, this increased the fennec install size: == Change summary for alert #4401 (as of November 30 2016 15:46 UTC) == Regressions: 1% installer size summary android-4-0-armv7-api15 opt 31851694 -> 32034454.92 For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=4401 there is no specific requirement to fix this, in fact- this is probably already known or expected.
Updated•3 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•