Closed Bug 1366404 Opened 7 years ago Closed 7 years ago

Add AArch64 Nightly builds

Categories

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

50 Branch
Unspecified
Android
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(2 files, 1 obsolete file)

      No description provided.
Assignee: nobody → nchen
Status: NEW → ASSIGNED
Comment on attachment 8870100 [details]
Bug 1366404 - Add AArch64 Nightly build;

https://reviewboard.mozilla.org/r/141582/#review145578

OK, this looks fine by me, but let's get Dustin's additonal review (or redirect), since  this touches more things than I am expert in (`taskcluster/taskgraph/*` in particular).

I would like you to explicitly state why this is not `-api-21` as well.  I see that you've cloned `android-x86`, but are you really confident that we'll not want to split `aarch64` by API if and when we do the split?  (It's quite likely we _wouldn't_ split `x86` since it's basically a dead architecture in the hardware device market.)  I'm happy with this for now -- the work to tag by API is roughly the same now as it is later -- but it's worth stating explicitly that you understand this and have a reason for the choice.

Please file follow-up to ensure artifact builds get filled.  Thanks!

::: commit-message-c800b:5
(Diff revision 1)
> +Bug 1366404 - Add AArch64 Nightly build; r?nalexander
> +
> +Add configurations for building and uploading AArch64 Nightly builds.
> +Right now the builds are in tier 1, but I'm not sure if they should be
> +in tier 2.

I think you and your team get the "right of first refusal": if you think that patches that burn the aarch64 build should be backed out with prejudice, you should be able to start with that policy until it proves onerous.

::: commit-message-c800b:7
(Diff revision 1)
> +
> +Add configurations for building and uploading AArch64 Nightly builds.
> +Right now the builds are in tier 1, but I'm not sure if they should be
> +in tier 2.
> +
> +I also didn't want to worry about Artifact builds for this patch, but I

Nah, happy to have it as follow-up.  I reckon you'll just need to bump `artifacts.py` to know about your architecture.
Attachment #8870100 - Flags: review?(nalexander) → review+
Attachment #8870100 - Flags: review?(dustin)
(In reply to Nick Alexander :nalexander from comment #2)
> Comment on attachment 8870100 [details]
> Bug 1366404 - Add AArch64 Nightly build;
> 
> https://reviewboard.mozilla.org/r/141582/#review145578
> 
> I would like you to explicitly state why this is not `-api-21` as well.  I
> see that you've cloned `android-x86`, but are you really confident that
> we'll not want to split `aarch64` by API if and when we do the split?  (It's
> quite likely we _wouldn't_ split `x86` since it's basically a dead
> architecture in the hardware device market.)  I'm happy with this for now --
> the work to tag by API is roughly the same now as it is later -- but it's
> worth stating explicitly that you understand this and have a reason for the
> choice.

Ok. I don't really think we will split AArch64 the way we split ARMv7 before. Originally, we split into API 9 and API 11+ because of lots of "constrained" devices that were stuck with API 9. We made an API 9 APK in order to lower our footprint on those devices. That probably will not be a problem for AArch64, because devices with API 21+ and AArch64 support are usually more than capable for running Fennec.

Secondly, it was a big change for Android going from API 9 to API 11+, so we saved quite a bit of code/resources when we stripped out API 11+. I don't see such drastic changes going from API 21 to upcoming versions, so even if we did split, I don't think it'll get us much benefit.
Comment on attachment 8870100 [details]
Bug 1366404 - Add AArch64 Nightly build;

https://reviewboard.mozilla.org/r/141582/#review146134
Attachment #8870100 - Flags: review?(dustin) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a62c6d515e19
Add AArch64 Nightly build; r=nalexander r=dustin
https://hg.mozilla.org/mozilla-central/rev/a62c6d515e19
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
This is causing beetmover bustage; we probably need to copy this comment [1] above every single artifact constant, as well as land a beetmoverscript patch.

[1] https://hg.mozilla.org/mozilla-central/file/a62c6d515e19/taskcluster/taskgraph/transforms/beetmover.py#l18
Attached patch aarch64_puppet.diff (obsolete) — Splinter Review
Attachment #8871919 - Flags: review?(mtabara)
Comment on attachment 8871919 [details] [diff] [review]
aarch64_puppet.diff

Review of attachment 8871919 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with following comments:

::: modules/beetmover_scriptworker/templates/script_config.json.erb
@@ +18,5 @@
>          "firefox_nightly_repacks": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/firefox_nightly_repacks.yml",
>          "fennec_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennec_nightly.yml",
>          "fennec_nightly_repacks": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennec_nightly_repacks.yml",
>          "fennecx86_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennecx86_nightly.yml"
> +        "fennecaarch64_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennecarch64_nightly.yml"

Might be a syntax error here "fennecarch64" instead of "fennecaarch64". I don't see any "fennecarch64" under https://github.com/mozilla-releng/beetmoverscript/pull/65/files.

@@ +28,5 @@
>              "firefox_nightly_repacks": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/firefox_nightly_repacks.yml",
>              "fennec_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennec_nightly.yml",
>              "fennec_nightly_repacks": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennec_nightly_repacks.yml",
>              "fennecx86_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennecx86_nightly.yml"
> +            "fennecarch64_nightly": "<%= scope.lookupvar("beetmover_scriptworker::settings::root") %>/lib/python3.5/site-packages/beetmoverscript/templates/fennecaarch64_nightly.yml"

Same as above.
Attachment #8871919 - Flags: review?(mtabara) → review+
Attachment #8871919 - Attachment is obsolete: true
Attachment #8871926 - Flags: review?(mtabara)
Comment on attachment 8871926 [details] [diff] [review]
aarch64_puppet.diff with typo fixes

Carrying forward r+
Attachment #8871926 - Flags: review?(mtabara) → review+
Depends on: 1368484
See Also: → 1467868
Blocks: 1368484
No longer depends on: 1368484
Blocks: 1467868
See Also: 1467868
Product: Firefox for Android → Firefox Build System
Target Milestone: Firefox 55 → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: