789373 - use the new --disable-ion flag for specific b2g-related builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

Per meeting w/dvander, dmandelin:

We need IonMonkey landed on mozilla-inbound, and then mozilla-central for the FF18 release. We also need to make sure that IonMonkey does not disrupt B2G work also now happening on mozilla-central. This means we need IonMonkey enabled and used by default on all builds *except* a specifically chosen list of builds. To do this:

1) dvander will create new "--disable-ion" flag and land it. Bug#789319 has details.

2) RelEng will ignore this new flag for Firefox builds, so FF18 users will see IonMonkey goodness once it lands. RelEng will update mozconfigs to use this new flag for these specific Fennec, B2G desktop builds and B2G builds in https://hg.mozilla.org/mozilla-central:
config/mozconfigs/ics_armv7a_gecko/*
(not sure if this is complete list of config files)

3) dvander will land IonMonkey on mozilla-inbound, and migrate to mozilla-central.

4) Until RelEng have B2G tests running on B2G builds, RelEng will generate some android builds with, and also without IonMonkey. This will require us to create a new type of build, for example: "android-no-ion". Each of these android builds then needs the usual Android tests run on it. This will also require changes to tbpl. 
Note: The "android-armv6" and "android-x86" builds should only be created with "--disable-ion", as IonMonkey is not relevant there. 

5) After RelEng has B2G tests running on B2G builds, we will disable the new "android-no-ion" builds.
Note: to be explicit, android-armv6 and android-x86 should continue to be run with "--disable-ion", as IonMonkey is not relevant there.

Comment 1

[Aki Sasaki (not active)]

(In reply to John O'Duinn [:joduinn] from comment #0)
> 2) RelEng will ignore this new flag for Firefox builds, so FF18 users will
> see IonMonkey goodness once it lands. RelEng will update mozconfigs to use
> this new flag for these specific Fennec, B2G desktop builds and B2G builds
> in https://hg.mozilla.org/mozilla-central:
> config/mozconfigs/ics_armv7a_gecko/*
> (not sure if this is complete list of config files)

b2g/config/mozconfigs/
mobile/android/config/mozconfigs/

> 4) Until RelEng have B2G tests running on B2G builds, RelEng will generate
> some android builds with, and also without IonMonkey.

I think it would be simpler to enable android tests on the IonMonkey branch, and keep ionmonkey on for android there and off on m-c.  Then the android tests on the ionmonkey branch would be an indicator of android testing status with ionmonkey on.

However, if we're looking to get per-checkin test results on m-c for *non*-ionmonkey checkins (but with ionmonkey on), this might be the best solution.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Aki Sasaki [:aki] from comment #1)
> I think it would be simpler to enable android tests on the IonMonkey branch,
> and keep ionmonkey on for android there and off on m-c.  Then the android
> tests on the ionmonkey branch would be an indicator of android testing
> status with ionmonkey on.
> 
> However, if we're looking to get per-checkin test results on m-c for
> *non*-ionmonkey checkins (but with ionmonkey on), this might be the best
> solution.

We really want to land IonMonkey on mozilla-central pref'd on so it can ship with desktop and mobile Firefox 18. Our concern is not conflicting with B2G's development. I don't think we could ship IonMonkey if it's pref'd off on m-c.

Comment 3

[Phil Ringnalda (:philor)]

Tree rules in a nutshell: if it's visible on m-c, you cannot break it, so it must also run on try and on everything that merges to m-c. If there's visible android-no-ion on m-c, then it has to be on inbound, and fx-team, and services-central, and build-system, and try, and whatever else I'm forgetting. Do we actually have the tegra capacity to basically double the test load?

A fun alternative: run them hidden and on inbound only, for the retrospective pleasure of the b2g folks to look back at what broke them, the same system that jetpack uses.

Comment 4

[Ed Morley [:emorley]]

If we do go ahead with this idea (I personally not liking the sound of doubling our tegra load and also sheriff android manual starring time, but oh well), then we'll need a bug to add TBPL support for the new android-no-ion builds, once the name has been confirmed:
https://bugzilla.mozilla.org/enter_bug.cgi?product=Webtools&component=Tinderboxpushlog

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

What is the ETA for completing step 2 of comment #1?

We've completed step #1 on our end, and are still blocked on landing IonMonkey.

Comment 6

[Phil Ringnalda (:philor)]

Isn't step 4, the misnumbered one that needs to be 3, the killer? Step 2 doesn't even require someone who knows anything about b2g, just someone with a mozilla-central tree and the knowledge (which can be gained by opening logs off tbpl and looking for the "got mozconfig" buildstep) that they are in /b2g/config/mozconfigs/ics_armv7a_gecko/* rather than in /config/.

Comment 7

[[e-mail] (:bhearsum)]

Attachment: disable ionmonkey

This disables IonMonkey for the ICS-based B2G builds as well as the Armv6 Android builds - I believe that covers all of the places we want them off.

Comment 8

[[e-mail] (:bhearsum)]

Oh, and I removed the now-unused b2g GB configs while I was in there.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 659850 [details] [diff] [review]
disable ionmonkey

Landed on m-c: https://hg.mozilla.org/mozilla-central/rev/9a0b706c41d6

Comment 10

[[e-mail] (:bhearsum)]

(In reply to Phil Ringnalda (:philor) from comment #6)
> Isn't step 4, the misnumbered one that needs to be 3, the killer? Step 2
> doesn't even require someone who knows anything about b2g, just someone with
> a mozilla-central tree and the knowledge (which can be gained by opening
> logs off tbpl and looking for the "got mozconfig" buildstep) that they are
> in /b2g/config/mozconfigs/ics_armv7a_gecko/* rather than in /config/.

You are correct, anyone can do such modifications now. You can even push them to try!

Comment 11

[David Anderson [:dvander] - inactive, e-mail if emergency]

Sweet, thank you! Does this mean TBPL will automatically disable IonMonkey now, and we're free to land?

Comment 12

[[e-mail] (:bhearsum)]

(In reply to David Anderson [:dvander] from comment #11)
> Sweet, thank you! Does this mean TBPL will automatically disable IonMonkey
> now, and we're free to land?

Any pushes that come after my push to m-c will have ionmonkey disabled for ICS b2g builds & Android armv6 builds - if that's what you mean.

Comment 13

[Ed Morley [:emorley]]

(In reply to David Anderson [:dvander] from comment #11)
> Sweet, thank you! Does this mean TBPL will automatically disable IonMonkey
> now, and we're free to land?

TBPL is just a dashboard, it doesn't compile anything or run any tests. Buildbot is what schedules the builds/tests; 'tinderbox (builds)' is/are the (albeit slightly out of date) name typically given to what is produced on each push.

All that has landed is the part that deals with:
(from comment 0) "Note: The "android-armv6" and "android-x86" builds should only be created with "--disable-ion", as IonMonkey is not relevant there." 

The rest of comment 0 step 4 (ie: Android on armv7) still needs buildbot-config patches to create both 'android' and 'android-no-ion' builds. The bug to add TBPL parser support for this new build name (mentioned in comment 4) hasn't been filed yet either.

However, I'm still not convinced the current plan is the best way forwards - given as yet, no one has responded to the capacity concerns in comment 3, comment 4 & comment 6.

Comment 14

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: more disabling

Disable on the rest of B2G builds, as well as Fennec.

Comment 15

[Aki Sasaki (not active)]

Comment on attachment 659895 [details] [diff] [review]
more disabling

To be clear, this will disable ionmonkey across the board for b2g (adding desktop) and android fennec (adding x86 + armv7).

Per dvander, we want to turn it back on for armv7 fennec but we need to either solve the no-ionmonkey builds/tests or the b2g testing first.

Comment 16

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 659895 [details] [diff] [review]
more disabling

https://hg.mozilla.org/mozilla-central/rev/7585c9b91877

Comment 17

[Aki Sasaki (not active)]

Attachment: android-noion mozconfig

This simply copies mobile/android/config/mozconfigs/android/nightly to mobile/android/config/mozconfigs/android-noion/nightly .  Since the former currently has --disable-ionmonkey, that should be sufficient.

The sooner this lands, the sooner this will merge to project branches; it'll also make staging the builds easier.

Comment 18

[Aki Sasaki (not active)]

Attachment: re-enable ionmonkey on android armv7

This needs to land after:

a) attachment 660534 [details] [diff] [review] lands
b) android-noion builds are running
c) android-noion tests are running (potentially only jsreftests).

Comment 19

[Aki Sasaki (not active)]

Comment on attachment 660534 [details] [diff] [review]
android-noion mozconfig

http://hg.mozilla.org/mozilla-central/rev/13f933298cf2

Comment 20

[Aki Sasaki (not active)]

Attachment: android-noion puppet

Comment 21

[Aki Sasaki (not active)]

Attachment: [custom] android-noion

Comment 22

[Aki Sasaki (not active)]

Attachment: [configs] android-noion builds and tests

I ran a couple staging android-noion builds which:
* are named correctly
* uploading to the android-noion dir
* using the proper mozconfig
* are only on m-c and m-i per email thread
* sendchanged

Then I brought up the staging test master, and verified
* they're named correctly
* respond to the corresponding sendchange
* only on m-c and m-i per email thread
* not running reftests or crashtests per email thread (jsreftests are on though; is that right?)

I didn't actually run any tegra tests; I can run those tests if you think there might be reason to.

Comment 23

[Justin Wood (:Callek)]

Comment on attachment 660663 [details] [diff] [review]
[custom] android-noion

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

::: process/factory.py
@@ +5644,5 @@
>              #32 bit (includes mac browsers)
>              if self.OS in ('xp', 'vista', 'win7', 'fedora', 'tegra_android', \
>                             'tegra_android-armv6', 'leopard', 'snowleopard', \
> +                           'leopard-o', 'lion', 'mountainlion',
> +                           'tegra_android-noion'):

nit: I'd prefer if the tegra* were grouped together, but I'm *ok* with how you listed it here

Comment 24

[Justin Wood (:Callek)]

Comment on attachment 660666 [details] [diff] [review]
[configs] android-noion builds and tests

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

r+ with my questions addressed.

::: mozilla-tests/config.py
@@ +139,5 @@
>  
> +PLATFORMS['android-noion']['slave_platforms'] = ['tegra_android-noion']
> +PLATFORMS['android-noion']['env_name'] = 'android-perf'
> +PLATFORMS['android-noion']['is_mobile'] = True
> +PLATFORMS['android-noion']['tegra_android-noion'] = {'name': "Android no-ionmonkey Tegra 250"}

Just as a note, we'll need a TBPL patch as well to account for this

@@ +638,5 @@
> +}
> +for suite in ANDROID_UNITTEST_DICT['opt_unittest_suites']:
> +    if suite[0].startswith('reftest') or suite[0].startswith('crashtest'):
> +        continue
> +    ANDROID_NOION_UNITTEST_DICT['opt_unittest_suites'].append(suite)

iirc bhearsum won't like this, but I'm ok with it. (note to ben, who i'm f?-ing due to this comment, these noion tests are to go away entirely when B2G tests are properly stood up)

@@ +1120,5 @@
>  
> +# XXX Bug 789373 hack - add android-noion until we have b2g testing
> +# Delete all references to android-noion once we have b2g testing
> +for branch in BRANCHES:
> +    if branch not in ('mozilla-central', 'mozilla-inbound'):

Are you sure we don't want try supported here?

::: mozilla/config.py
@@ +793,5 @@
>              'create_snippet': False,
>              'create_partial': False,
>              'slaves': SLAVES['mock'],
>              'platform_objdir': OBJDIR,
> +            'update_platform': 'Android_arm-eabi-gcc3-noion',

Why are we swapping to a different update_platform for Android-debug? If we do this, imo, we need to fix the debug builds on aus to continue getting updates.

@@ +1485,5 @@
> +
> +# XXX bug 789373 hack until we get b2g testing going.
> +# Delete all references to android-noion once we have b2g tests.
> +for b in BRANCHES.keys():
> +    if b not in ('mozilla-central', 'mozilla-inbound'):

again, do we need/want this on try?

Comment 25

[Aki Sasaki (not active)]

Attachment: [tools] update pm.json

Comment 26

[[e-mail] (:bhearsum)]

Comment on attachment 660666 [details] [diff] [review]
[configs] android-noion builds and tests

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

I'm horrified that we need to do this at all. That's not your fault of course though, I have a few questions inline.

::: mozilla-tests/BuildSlaves.py.template
@@ +14,5 @@
>      "w764": "pass",
>      "vista": "pass",
>      "tegra_android": "pass",
> +    "tegra_android-armv6": "pass",
> +    "tegra_android-noion": "pass",

So, we're only doing noion for armv7?

::: mozilla-tests/config.py
@@ +1120,5 @@
>  
> +# XXX Bug 789373 hack - add android-noion until we have b2g testing
> +# Delete all references to android-noion once we have b2g testing
> +for branch in BRANCHES:
> +    if branch not in ('mozilla-central', 'mozilla-inbound'):

Indeed, if these are going to be builders that are intended to be unhidden we should have try support for them or else sheriffs will hide them whenever they break.

::: mozilla/config.py
@@ +736,5 @@
> +            'enable_codesighs': False,
> +            'create_partial': False,
> +            'slaves': SLAVES['mock'],
> +            'platform_objdir': OBJDIR,
> +            'update_platform': 'Android_arm-eabi-gcc3-noion',

Is there support in the build system for this? Unfortunately, this isn't a source of truth for update platform names, it merely reflects what the build system gives us. If we want nightly builds there's probably a few other things we need to do too.

Comment 27

[Aki Sasaki (not active)]

(In reply to Ben Hearsum [:bhearsum] from comment #26)
> ::: mozilla-tests/BuildSlaves.py.template
> @@ +14,5 @@
> >      "w764": "pass",
> >      "vista": "pass",
> >      "tegra_android": "pass",
> > +    "tegra_android-armv6": "pass",
> > +    "tegra_android-noion": "pass",
> 
> So, we're only doing noion for armv7?

Correct.

> ::: mozilla-tests/config.py
> @@ +1120,5 @@
> >  
> > +# XXX Bug 789373 hack - add android-noion until we have b2g testing
> > +# Delete all references to android-noion once we have b2g testing
> > +for branch in BRANCHES:
> > +    if branch not in ('mozilla-central', 'mozilla-inbound'):
> 
> Indeed, if these are going to be builders that are intended to be unhidden
> we should have try support for them or else sheriffs will hide them whenever
> they break.

Easy enough to add.

> ::: mozilla/config.py
> @@ +736,5 @@
> > +            'enable_codesighs': False,
> > +            'create_partial': False,
> > +            'slaves': SLAVES['mock'],
> > +            'platform_objdir': OBJDIR,
> > +            'update_platform': 'Android_arm-eabi-gcc3-noion',
> 
> Is there support in the build system for this? Unfortunately, this isn't a
> source of truth for update platform names, it merely reflects what the build
> system gives us. If we want nightly builds there's probably a few other
> things we need to do too.

We already did this with android-xul, and I think we're still doing it with android-armv6.  I'm less concerned with having a good location for noion updates as I am not polluting android armv7 updates.

Comment 28

[Aki Sasaki (not active)]

(In reply to Justin Wood (:Callek) from comment #24)
> ::: mozilla-tests/config.py
> @@ +139,5 @@
> >  
> > +PLATFORMS['android-noion']['slave_platforms'] = ['tegra_android-noion']
> > +PLATFORMS['android-noion']['env_name'] = 'android-perf'
> > +PLATFORMS['android-noion']['is_mobile'] = True
> > +PLATFORMS['android-noion']['tegra_android-noion'] = {'name': "Android no-ionmonkey Tegra 250"}
> 
> Just as a note, we'll need a TBPL patch as well to account for this

bug 790848

> @@ +638,5 @@
> > +}
> > +for suite in ANDROID_UNITTEST_DICT['opt_unittest_suites']:
> > +    if suite[0].startswith('reftest') or suite[0].startswith('crashtest'):
> > +        continue
> > +    ANDROID_NOION_UNITTEST_DICT['opt_unittest_suites'].append(suite)
> 
> iirc bhearsum won't like this, but I'm ok with it. (note to ben, who i'm
> f?-ing due to this comment, these noion tests are to go away entirely when
> B2G tests are properly stood up)

On IRC, Ben said he noted all of his concerns.
I don't like this either, but it's not any less programmatic than the rest of mozilla-tests/config.py and it's certainly a faster way to get where we want to [temporarily] go.

> > +# XXX Bug 789373 hack - add android-noion until we have b2g testing
> > +# Delete all references to android-noion once we have b2g testing
> > +for branch in BRANCHES:
> > +    if branch not in ('mozilla-central', 'mozilla-inbound'):
> 
> Are you sure we don't want try supported here?

Added.

> ::: mozilla/config.py
> @@ +793,5 @@
> >              'create_snippet': False,
> >              'create_partial': False,
> >              'slaves': SLAVES['mock'],
> >              'platform_objdir': OBJDIR,
> > +            'update_platform': 'Android_arm-eabi-gcc3-noion',
> 
> Why are we swapping to a different update_platform for Android-debug? If we
> do this, imo, we need to fix the debug builds on aus to continue getting
> updates.

android-debug was a mistake, good catch.

> @@ +1485,5 @@
> > +
> > +# XXX bug 789373 hack until we get b2g testing going.
> > +# Delete all references to android-noion once we have b2g tests.
> > +for b in BRANCHES.keys():
> > +    if b not in ('mozilla-central', 'mozilla-inbound'):
> 
> again, do we need/want this on try?

Done.

Comment 29

[Aki Sasaki (not active)]

Attachment: [configs] with fixes

With fixes.  Besides the nits/fixes listed in this bug, I also turned off nightly updates for android-noion per Ben in IRC.

Carrying forward r+.

Comment 30

[Aki Sasaki (not active)]

Attachment: [custom] with fixes

Reordering the platform list.  Passes test-masters with the fixed configs.
Carrying forward r+.

Comment 31

[Aki Sasaki (not active)]

Comment on attachment 660964 [details] [diff] [review]
[custom] with fixes

http://hg.mozilla.org/build/buildbotcustom/rev/0304188eeaf9

Comment 32

[Aki Sasaki (not active)]

Comment on attachment 660963 [details] [diff] [review]
[configs] with fixes

http://hg.mozilla.org/build/buildbot-configs/rev/ee976dc0b77b

Comment 33

[Aki Sasaki (not active)]

Comment on attachment 660669 [details] [diff] [review]
[tools] update pm.json

http://hg.mozilla.org/build/puppet-manifests/rev/d6a05685baf1

I think this will propagate to the masters in the next 30min; I can push the tools patch, merge the configs/custom patches, and reconfig at that point.

Afterwards, a test build, land the android re-enable ionmonkey mozconfig patch, and send a dev.platform post.

Comment 34

[Aki Sasaki (not active)]

Comment on attachment 660662 [details] [diff] [review]
android-noion puppet

http://hg.mozilla.org/build/tools/rev/ba922e3fdac6

Oops, the previous comment should have marked this patch as checked-in, and this comment should have been on the tools patch.

Comment 35

[Aki Sasaki (not active)]

Bustage fix: http://hg.mozilla.org/build/tools/rev/81ad26d0844d
I wish json allowed trailing commas. And comments.

Comment 36

[Aki Sasaki (not active)]

Attachment: fix thunderbird

Comment 37

[Aki Sasaki (not active)]

Comment on attachment 661035 [details] [diff] [review]
fix thunderbird

http://hg.mozilla.org/build/buildbot-configs/rev/df3cb47aacc7

Comment 38

[Aki Sasaki (not active)]

android-noion builds+tests should be live now.  Testing...

Comment 39

[Aki Sasaki (not active)]

Build was successful; I see tests running on tbpl.
I'm going to enable ionmonkey on armv7 and write a dev.planning post.

Comment 40

[Justin Wood (:Callek)]

Comment on attachment 660535 [details] [diff] [review]
re-enable ionmonkey on android armv7

https://hg.mozilla.org/integration/mozilla-inbound/rev/4f9d4f98f133

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4f9d4f98f133