Do Android builds in a 64-bits mock environment

RESOLVED FIXED

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: glandium, Assigned: glandium)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 8 obsolete attachments)

4.64 KB, patch
Callek
: review+
Details | Diff | Splinter Review
1.78 KB, patch
Callek
: review+
Details | Diff | Splinter Review
2.22 KB, patch
aki
: review+
Details | Diff | Splinter Review
6.81 KB, patch
Callek
: review+
Details | Diff | Splinter Review
bug 859341 shows that OOM in the 32-bits environment are likely to start coming in the near future. It would be better to preemptively switch to 64-bits environments than to wait for the tree to burn and have to do it in a rush, blocking development in the meantime. As long as a few low-level i686 packages are installed in such a x86-64 environment (libc, libz, libstdc++, libm, libgcc) the current x86 NDK should work (it doesn't depend on too many x86 libraries). In fact, the 64-bits mock environments we have might already have all that is needed.
I needed to add these packages for b2g emulator builds (which also require a 64-bit build environment):

"glibc-devel.i686", "libstdc++.i686", "zlib-devel.i686", "ncurses-devel.i686", "libX11-devel.i686", "mesa-libGL-devel.i686", "mesa-libGL-devel", "libX11-devel"

I suspect the list would be similar for android builds. Maybe we should switch all builds over to 64-bit?
(In reply to Chris AtLee [:catlee] from comment #1)
> I needed to add these packages for b2g emulator builds (which also require a
> 64-bit build environment):
> 
> "glibc-devel.i686", "libstdc++.i686", "zlib-devel.i686",
> "ncurses-devel.i686", "libX11-devel.i686", "mesa-libGL-devel.i686",
> "mesa-libGL-devel", "libX11-devel"

I don't think any of those -devel packages is needed for android. They are likely only required to build the emulator, which we don't build for android.

> I suspect the list would be similar for android builds. Maybe we should
> switch all builds over to 64-bit?

That's the plan for Linux builds, which needs some testing first (and bug 857697).
bug 857697 will almost certainly be enough it what's installed now isn't.  Its also everything from comment 1 accept ncurses-devel.
Depends on: 857697
(In reply to Trevor Saunders (:tbsaunde) from comment #3)
> bug 857697 will almost certainly be enough it what's installed now isn't. 
> Its also everything from comment 1 accept ncurses-devel.

Well, technically, android builds don't use the same environment, so bug 857697 is actually not going to help.
No longer depends on: 857697
Attachment #736692 - Flags: review?(catlee)
Assignee: nobody → mh+mozilla
Maybe it would be desirable to use some project branch instead of try?
Comment on attachment 736692 [details] [diff] [review]
Try x86_64 mock environment for android builds on try

We'll need some packages too.
Attachment #736692 - Flags: review?(catlee)
Assuming the date project branch is really free to use.
Attachment #736799 - Flags: review?(bugspam.Callek)
Attachment #736692 - Attachment is obsolete: true
Attachment #736799 - Attachment is obsolete: true
Attachment #736799 - Flags: review?(bugspam.Callek)
Depends on: 861245
Comment on attachment 736857 [details] [diff] [review]
Try x86_64 mock environment for android builds on the 'date' branch

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

this looks good at a glance, I don't forsee any problems.
Attachment #736857 - Flags: review?(bugspam.Callek) → review+
landed for glandium since he seems to be away and I believe wnated this in today
https://hg.mozilla.org/build/buildbot-configs/rev/932719dcdd15
This is in production.
Posted patch Fixup (obsolete) — Splinter Review
I forgot this when i changed the branch name.
Attachment #737046 - Flags: review?(bugspam.Callek)
Somehow, the following *doesn't* install the android* packages or java:

DEBUG: ['/usr/bin/yum', '--installroot', '/builds/mock_mozilla/mozilla-centos6-x86_64/root/', 'install', 'autoconf213', 'mozilla-python27-mercurial', 'ccache', 'android-sdk15', 'android-sdk16', 'android-ndk5', 'android-ndk8', 'zip', 'java-1.6.0-openjdk-devel', 'zlib-devel', 'glibc-static', 'openssh-clients', 'mpfr', 'wget', 'glibc.i686', 'libstdc++.i686', 'zlib.i686']

https://tbpl.mozilla.org/php/getParsedLog.php?id=21766840&full=1&branch=date

Also, B2G builds are still triggered for the date branch.
Oh, and android-debug builds are in a x86 mock env.
Trevor suggested that the lack of installation of the SDK and NDK may be due to the fact that the packages are for i686 only in the yum repo, and I can totally buy this. This add .i686 to the android-* packages.
Attachment #737114 - Flags: review?(bugspam.Callek)
Attachment #737046 - Attachment is obsolete: true
Attachment #737046 - Flags: review?(bugspam.Callek)
https://hg.mozilla.org/build/buildbot-configs/rev/b2a168ab6058
https://hg.mozilla.org/build/buildbot-configs/rev/2934d299a89e

Backout out for breaking try. e.g. see https://tbpl.mozilla.org/?tree=Try&rev=6ae475a235da as an example inbound push with no modifications that broke

The backout is live, when we revisit (early next week) we'll use the reserved branch that was taken for android, rather than try
(In reply to Justin Wood (:Callek) from comment #17)
> The backout is live, when we revisit (early next week) we'll use the
> reserved branch that was taken for android, rather than try

I was able to test and hit a wall anyways. The tinderbox script was trying to find a x86_64 tarball after the build. And even if that worked, that would have failed on tests.

I'm mystified that it broke try.
> I'm mystified that it broke try.

I was too, the best theory I can think of which I mentioned to callac on irc is that we don't explicitly ask yum to install glib2-devel.x86_64 (if I read the log right)  and it presumably gets pulled in as a dep of gtk2-devel but now we explicitly install glib2-devel.i686 and maybe yum somehow thinks that satisfies the dep and so nolonger installs glib2-devel.x86_64.
Refreshed against current trunk.
Attachment #737581 - Flags: review?(bugspam.Callek)
Attachment #737114 - Attachment is obsolete: true
Attachment #737114 - Flags: review?(bugspam.Callek)
Comment on attachment 737581 [details] [diff] [review]
Fixup, and install .i686 android-* packages

iirc some packages are .noarch not all have .i686 alternatives...
Attachment #737581 - Flags: review?(bugspam.Callek) → review-
(In reply to Justin Wood (:Callek) from comment #21)
> Comment on attachment 737581 [details] [diff] [review]
> Fixup, and install .i686 android-* packages
> 
> iirc some packages are .noarch not all have .i686 alternatives...

afaics, there is no noarch android-* package.
Posted patch Fixups (obsolete) — Splinter Review
Since we're also switching to tooltool, we don't need to fiddle with the android-* packages. This adds missing android-debug and fixes up the try thing.
Attachment #738412 - Flags: review?(bugspam.Callek)
Attachment #737581 - Attachment is obsolete: true
Comment on attachment 738412 [details] [diff] [review]
Fixups

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

::: mozilla/config.py
@@ +1737,2 @@
>          'glibc.i686', 'libstdc++.i686', 'zlib.i686',
>          ]

nit: lets make this:

        list(BRANCHES['date']['platforms'][platform]['mock_packages']) + \
        ['glibc.i686', 'libstdc++.i686', 'zlib.i686', ]

rather than what you have here, to resolve PEP8 violation:
E122 continuation line missing indentation or outdented.

Unless you have a different non PEP8 violation suggestion
Attachment #738412 - Flags: review?(bugspam.Callek) → review+
Posted patch FixupsSplinter Review
How about this?
Attachment #738693 - Flags: review?(bugspam.Callek)
Attachment #738412 - Attachment is obsolete: true
Comment on attachment 738693 [details] [diff] [review]
Fixups

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

WFM, no pep8 issues
Attachment #738693 - Flags: review?(bugspam.Callek) → review+
This does the final roundup here.

I thought there was another bug with the outstanding items where this patch belongs but I can't find it atm, the 'date' specific stuff for linux is new and I reviewed it, but when adding this in I realized we need to map from linux64 not linux32 if we're changing the mock target env here.

All passes test masters and pep8

f? to glandium as a "this is what I want" ack. (this is as I understood his prod to me in IRC)
Attachment #738805 - Flags: review?(nthomas)
Attachment #738805 - Flags: feedback?(mh+mozilla)
Comment on attachment 738805 [details] [diff] [review]
[configs] do linux32 on date and no b2g

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

err actually I lied, apparantly I forgot my test master output here logs to files and not directly to console, this fails because linux64 is not in branch-specific platform list.
Attachment #738805 - Flags: review?(nthomas)
Attachment #738805 - Flags: review-
Attachment #738805 - Flags: feedback?(mh+mozilla)
Comment on attachment 738805 [details] [diff] [review]
[configs] do linux32 on date and no b2g

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

::: mozilla/config.py
@@ +1738,4 @@
>          ]
>  
>  for platform in ['linux', 'linux-debug']:
> +    new_plat = {'linux': 'linux64', 'linux-debug': 'linux64-debug'}

You don't need this trick, the list of packages is the same for linux and linux64.
Attachment #738901 - Flags: review?(nthomas)
Attachment #738805 - Attachment is obsolete: true
Attachment #738901 - Flags: review?(nthomas) → review+
This is in production now.
No longer blocks: 859984
Depends on: 859984
Attachment #747299 - Flags: review?(bugspam.Callek)
This is the same as Mike's patch above, but adjusted for contextual bitrot.

I also slightly edited for s/branch/b/ and s/platform/plat/ to fit the assignment of mock_target onto 1 line in the pep8 80-char limit.

Of note, this can't land until we move the packages into place (myself or buildduty can do that):
* android* from the releng/ i686 repo need to move to the releng/ x64 repo.
** run createrepo after that
** wait for puppetmasters to update
* write a puppet patch to force yum cache's to flush
** land and wait for propagation to puppetmasters
** inform :dustin to also merge it to puppet320

The process is doc'd at https://wiki.mozilla.org/ReleaseEngineering/PuppetAgain/Packages#Landing_Custom_Repository_Changes

In theory we should also move all i*86 packages to the x64 folder I believe now as well, but that should be coordinated elsewhere so that we can watch for fallout explicitly.

Once the packages are in place this can go live in a reconfig at buildduties discretion
Attachment #747299 - Attachment is obsolete: true
Attachment #747299 - Flags: review?(bugspam.Callek)
Attachment #749227 - Flags: review+
(In reply to Justin Wood (:Callek) from comment #33)
> Created attachment 749227 [details] [diff] [review]
> Switch android builds to x86_64 mock environments - updated for bitrot
> 
> This is the same as Mike's patch above, but adjusted for contextual bitrot.
> 
> I also slightly edited for s/branch/b/ and s/platform/plat/ to fit the
> assignment of mock_target onto 1 line in the pep8 80-char limit.
> 
> Of note, this can't land until we move the packages into place (myself or
> buildduty can do that):
> * android* from the releng/ i686 repo need to move to the releng/ x64 repo.

You probably just want to copy them, not move them.

> In theory we should also move all i*86 packages to the x64 folder I believe
> now as well, but that should be coordinated elsewhere so that we can watch
> for fallout explicitly.

I don't think we should.
(In reply to Mike Hommey [:glandium] from comment #34)
> (In reply to Justin Wood (:Callek) from comment #33)
> > Created attachment 749227 [details] [diff] [review]
> > Switch android builds to x86_64 mock environments - updated for bitrot
> > 
> > This is the same as Mike's patch above, but adjusted for contextual bitrot.
> > 
> > I also slightly edited for s/branch/b/ and s/platform/plat/ to fit the
> > assignment of mock_target onto 1 line in the pep8 80-char limit.
> > 
> > Of note, this can't land until we move the packages into place (myself or
> > buildduty can do that):
> > * android* from the releng/ i686 repo need to move to the releng/ x64 repo.
> 
> You probably just want to copy them, not move them.

err indeed!

> > In theory we should also move all i*86 packages to the x64 folder I believe
> > now as well, but that should be coordinated elsewhere so that we can watch
> > for fallout explicitly.
> 
> I don't think we should.

well the yum repo setup tends to have all i686 variants of its x64 varients in the x64 repo as well. In fact it expects them to be present there as well. We can do this in a deduped way if need be. I should note that I meant s/move/copy/ as well here.

Either way *that* is not for this bug
Per #releng chat with glandium, plan is:
 - hwine to do yum repo updates from comment #33 today
 - when done, those will be verified okay from date twig by glandium (Wed?)
 - assuming all okay, can land m-i/m-c after that from bug 860246 and bug 857697

Contingency plan: if concerns arise, can land to m-i first and let soak for a day or so.
Flags: needinfo?(hwine)
(In reply to Hal Wine [:hwine] from comment #36)
>  - when done, those will be verified okay from date twig by glandium (Wed?)

It worked:
DEBUG: Installing:
DEBUG:  android-ndk5       i686   r5c-0moz3                releng-centos6-x86_64  47 M
DEBUG:  android-ndk8       i686   r8c-0moz1                releng-centos6-x86_64 157 M
DEBUG:  android-sdk15      i686   r15-0moz1                releng-centos6-x86_64 385 M
DEBUG:  android-sdk16      i686   r16-0moz2                releng-centos6-x86_64 492 M
in production
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(hwine)
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.