Closed
Bug 789373
Opened 9 years ago
Closed 9 years ago
use the new --disable-ion flag for specific b2g-related builds
Categories
(Release Engineering :: General, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: joduinn, Assigned: aki)
References
Details
Attachments
(9 files, 2 obsolete files)
4.43 KB,
patch
|
aki
:
review+
bhearsum
:
checked-in+
|
Details | Diff | Splinter Review |
12.23 KB,
patch
|
aki
:
review+
dvander
:
checked-in+
|
Details | Diff | Splinter Review |
402 bytes,
patch
|
dvander
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
4.65 KB,
patch
|
dvander
:
review+
Callek
:
checked-in+
|
Details | Diff | Splinter Review |
2.64 KB,
patch
|
Callek
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
3.17 KB,
patch
|
Callek
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
26.01 KB,
patch
|
aki
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
4.07 KB,
patch
|
aki
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
3.68 KB,
patch
|
Callek
:
review+
aki
:
checked-in+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•9 years ago
|
||
(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.
(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•9 years ago
|
||
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•9 years ago
|
||
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
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•9 years ago
|
||
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•9 years ago
|
||
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.
Attachment #659850 -
Flags: review?(aki)
Comment 8•9 years ago
|
||
Oh, and I removed the now-unused b2g GB configs while I was in there.
Assignee | ||
Updated•9 years ago
|
Attachment #659850 -
Flags: review?(aki) → review+
Comment 9•9 years ago
|
||
Comment on attachment 659850 [details] [diff] [review] disable ionmonkey Landed on m-c: https://hg.mozilla.org/mozilla-central/rev/9a0b706c41d6
Attachment #659850 -
Flags: checked-in+
Comment 10•9 years ago
|
||
(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!
Sweet, thank you! Does this mean TBPL will automatically disable IonMonkey now, and we're free to land?
Comment 12•9 years ago
|
||
(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•9 years ago
|
||
(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.
Disable on the rest of B2G builds, as well as Fennec.
Attachment #659895 -
Flags: review?(aki)
Assignee | ||
Comment 15•9 years ago
|
||
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.
Attachment #659895 -
Flags: review?(aki) → review+
Comment on attachment 659895 [details] [diff] [review] more disabling https://hg.mozilla.org/mozilla-central/rev/7585c9b91877
Attachment #659895 -
Flags: checked-in+
Assignee | ||
Comment 17•9 years ago
|
||
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.
Attachment #660534 -
Flags: review?(dvander)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → aki
Assignee | ||
Comment 18•9 years ago
|
||
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).
Attachment #660535 -
Flags: review?(dvander)
Attachment #660534 -
Flags: review?(dvander) → review+
Attachment #660535 -
Flags: review?(dvander) → review+
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 660534 [details] [diff] [review] android-noion mozconfig http://hg.mozilla.org/mozilla-central/rev/13f933298cf2
Attachment #660534 -
Flags: checked-in+
Assignee | ||
Comment 20•9 years ago
|
||
Attachment #660662 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 21•9 years ago
|
||
Attachment #660663 -
Flags: review?(bugspam.Callek)
Updated•9 years ago
|
Attachment #660662 -
Flags: review?(bugspam.Callek) → review+
Assignee | ||
Comment 22•9 years ago
|
||
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.
Attachment #660666 -
Flags: review?(bugspam.Callek)
Comment 23•9 years ago
|
||
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
Attachment #660663 -
Flags: review?(bugspam.Callek) → review+
Comment 24•9 years ago
|
||
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?
Attachment #660666 -
Flags: review?(bugspam.Callek)
Attachment #660666 -
Flags: review+
Attachment #660666 -
Flags: feedback?(bhearsum)
Assignee | ||
Comment 25•9 years ago
|
||
Attachment #660669 -
Flags: review?(bugspam.Callek)
Updated•9 years ago
|
Attachment #660669 -
Flags: review?(bugspam.Callek) → review+
Comment 26•9 years ago
|
||
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.
Assignee | ||
Comment 27•9 years ago
|
||
(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.
Assignee | ||
Comment 28•9 years ago
|
||
(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.
Assignee | ||
Comment 29•9 years ago
|
||
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+.
Attachment #660666 -
Attachment is obsolete: true
Attachment #660666 -
Flags: feedback?(bhearsum)
Attachment #660963 -
Flags: review+
Assignee | ||
Comment 30•9 years ago
|
||
Reordering the platform list. Passes test-masters with the fixed configs. Carrying forward r+.
Attachment #660663 -
Attachment is obsolete: true
Attachment #660964 -
Flags: review+
Assignee | ||
Comment 31•9 years ago
|
||
Comment on attachment 660964 [details] [diff] [review] [custom] with fixes http://hg.mozilla.org/build/buildbotcustom/rev/0304188eeaf9
Attachment #660964 -
Flags: checked-in+
Assignee | ||
Comment 32•9 years ago
|
||
Comment on attachment 660963 [details] [diff] [review] [configs] with fixes http://hg.mozilla.org/build/buildbot-configs/rev/ee976dc0b77b
Attachment #660963 -
Flags: checked-in+
Assignee | ||
Comment 33•9 years ago
|
||
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.
Attachment #660669 -
Flags: checked-in+
Assignee | ||
Comment 34•9 years ago
|
||
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.
Attachment #660662 -
Flags: checked-in+
Assignee | ||
Comment 35•9 years ago
|
||
Bustage fix: http://hg.mozilla.org/build/tools/rev/81ad26d0844d I wish json allowed trailing commas. And comments.
Assignee | ||
Comment 36•9 years ago
|
||
Attachment #661035 -
Flags: review?(bugspam.Callek)
Updated•9 years ago
|
Attachment #661035 -
Flags: review?(bugspam.Callek) → review+
Assignee | ||
Comment 37•9 years ago
|
||
Comment on attachment 661035 [details] [diff] [review] fix thunderbird http://hg.mozilla.org/build/buildbot-configs/rev/df3cb47aacc7
Attachment #661035 -
Flags: checked-in+
Assignee | ||
Comment 38•9 years ago
|
||
android-noion builds+tests should be live now. Testing...
Assignee | ||
Comment 39•9 years ago
|
||
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•9 years ago
|
||
Comment on attachment 660535 [details] [diff] [review] re-enable ionmonkey on android armv7 https://hg.mozilla.org/integration/mozilla-inbound/rev/4f9d4f98f133
Attachment #660535 -
Flags: checked-in+
Comment 41•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4f9d4f98f133
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Product: mozilla.org → Release Engineering
Updated•3 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•