Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Fixes #445 - Ship Fenix x86_64 builds #2856

Merged
12 commits merged into from
May 29, 2019
Merged

Fixes #445 - Ship Fenix x86_64 builds #2856

12 commits merged into from
May 29, 2019

Conversation

st3fan
Copy link
Contributor

@st3fan st3fan commented May 25, 2019

This patch add support for building x86_64 builds. This is an experiment and I would like to get this working and tested on this PR branch first. This has low priority but ideally starts working before the end of July because of a Google Play Store deadline regarding 64-bit builds.

@st3fan st3fan requested review from a team as code owners May 25, 2019 02:32
@boek
Copy link
Contributor

boek commented May 25, 2019

Looks like the builds aren’t happy with it though

@st3fan
Copy link
Contributor Author

st3fan commented May 25, 2019

@boek sorry i should have marked this as Do Not Merge or WIP - This has no priority, just something I'd like to get going.

@st3fan st3fan removed the request for review from a team May 25, 2019 12:30
@ghost
Copy link

ghost commented May 25, 2019

Askin for help from @mozilla-mobile/releng - TaskCluster tells me

HTTP 424 Artifact "public/actions.json" not found at "/opt/fenix/actions.json"

And does not show any more details for these builds. Did I break things completely with my changes or is there something else going on?

@JohanLorenzo
Copy link
Contributor

@st3fan hi! This Taskcluster error is some red herring[1]. In order to see the real error:

  1. Copy the "Task Group ID" to the "Task ID" field and click on "Inspect"
  2. Access the logs of the task.

In this case, it's a gradle error: No such property: geckoview_beta_x86 for class: Gecko

https://tools.taskcluster.net/groups/W7lE52rJSfaSQRSJNBeQvQ/tasks/W7lE52rJSfaSQRSJNBeQvQ/runs/0/logs/public%2Flogs%2Flive_backing.log#L392

[1] It shows up because the Taskcluster WebUI looks into this artifact to enable/disable commands under the "Actions" drop down. I've seen it for long, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1554599 to track this.

Copy link
Contributor

@pocmo pocmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will at least need an update here too:
https://github.com/mozilla-mobile/fenix/blob/master/app/build.gradle#L136-L140
.. to generate unique version codes (x86_64 > x86 > arm64 > arm)

@ghost
Copy link

ghost commented May 27, 2019

This now produces x86_64 APKs. @JohanLorenzo or @mitchhentges can you take a peek at the suggestions that @pocmo made in the previous comment? I am not sure what to do with the version code.

Right now x86 and x86_64 are treated the same way (if (variant.flavorName.contains("x86"))) and follow the same logic. Do we need x86_64 builds to have a different code?

The comment above that code says:

Our x86 builds need a higher version code to avoid installing ARM builds on an x86 device with ARM compatibility mode.

Without changes that still happens for x86_64. Or does x86_64 need special treatment?

@pocmo
Copy link
Contributor

pocmo commented May 27, 2019

Without changes that still happens for x86_64. Or does x86_64 need special treatment?

They still need to be unique for every APK. And since the x86 AND x86_64 build can be installed on 64 bit x86 devices we would want to use a higher version code for the 64 bit build so that those devices get that build.

TL;DR: Adding a case that does +3 for x86_64 should be enough here.

@ghost
Copy link

ghost commented May 28, 2019

I am just not having a lot of luck ...

> Task :architecture:compileDebugKotlin
Could not perform incremental compilation: Could not connect to Kotlin compile daemon
Could not connect to kotlin daemon. Using fallback strategy.

OpenJDK 64-Bit Server VM warning: INFO: os::commit_memory(0x00000000b2150000, 82509824, 0) failed; error='Cannot allocate memory' (errno=12)

> Task :architecture:compileDebugKotlin FAILED
#
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 82509824 bytes for committing reserved memory.
# An error report file with more information is saved as:
# /opt/fenix/hs_err_pid338.log

FAILURE: Build failed with an exception.

I'll run this again - but running out of memory is not something I've seen before.

@ghost ghost requested review from a team and pocmo May 28, 2019 11:33
app/build.gradle Outdated Show resolved Hide resolved
app/build.gradle Show resolved Hide resolved
@ghost
Copy link

ghost commented May 28, 2019

@mitchhentges i see you editing in the decision_task.py too - do these changes still make sense? For example I see that I don't have to specify the architectures manually anymore - will it just discover by itself what flavors we have now?

@mitchhentges
Copy link
Contributor

Looks great, yeah - it'll automatically detect all build types now for release

@ghost
Copy link

ghost commented May 28, 2019

@mitchhentges great - lets see if the last small changes result in a green build and then we can land it I hope.

@ghost ghost added eng:build Build system, gradle, configuration eng:automation Build automation, Continuous integration, .. and removed pr:do-not-land work in progress labels May 28, 2019
@ghost ghost merged commit ff5eca1 into master May 29, 2019
@pocmo pocmo deleted the BuildX8664 branch May 29, 2019 19:23
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:automation Build automation, Continuous integration, .. eng:build Build system, gradle, configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants