--enable-rust on Android builds

RESOLVED FIXED in Firefox 53

Status

RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: froydnj, Assigned: froydnj)

Tracking

Trunk
mozilla53
All
Android
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
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

2 years ago
Created attachment 8815436 [details] [diff] [review]
enable Rust on Android builds

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 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+
Why not have mobile/android/config/mozconfigs/common_compile and mobile/android/config/mozconfigs/common_artifact?
(Assignee)

Comment 4

2 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

2 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.

Comment 6

2 years ago
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e3b6a523e34b
enable Rust on Android builds; r=mshal

Comment 7

2 years ago
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

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e3b6a523e34b
https://hg.mozilla.org/mozilla-central/rev/1309c56ba4cd
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(Assignee)

Updated

2 years ago
Depends on: 1322323
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

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.