730325 - Integrate Thunderbird build config with Firefox build config

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[John Hopkins (:jhopkins)]

Thunderbird builds should use the same buildbot masters are Firefox builds.
We aren't close to the per-master builder limit yet so this should be doable.

Comment 1

[John Hopkins (:jhopkins)]

Attachment: latest buildbot-configs changes on staging

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 606024 [details] [diff] [review]
latest buildbot-configs changes on staging

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

::: mozilla/staging_thunderbird_config.py
@@ +1,1 @@
> +MAC_SNOW_MINIS = ['moz2-darwin10-slave%02i' % x for x in range(1,30) + range(40,57)]

Hmmm, I think I made a small mistake in my docstring in thunderbird_config.py and setup-master.py. Those places _should_ be referencing thunderbird_{production,staging}_config.py instead of {production,staging}_thunderbird_config.py. Those files import the existing GLOBAL_VARS and allow for overrides. Can you pull against that and move the necessary parts to thunderbird_staging_config.py? Sorry for the confusion :(.

You also shouldn't be redefining slave names anywhere - if you need them you can import them from the plain 'localconfig' file.

Comment 3

[John Hopkins (:jhopkins)]

Attachment: https://hg.mozilla.org/users/john.hopkins_mozillamessaging.com/buildbotcustom-stage/rev/fc1aab2ed027

We've been relying on our mozconfigs to do source checkouts but our l10n dep builds do not use mozconfigs.  As a result, we need to checkout comm-central using another mechanism.  This patch adds hooks in a couple of places to allow "alternate checkout" code to be triggered in the right place.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 607976 [details] [diff] [review]
https://hg.mozilla.org/users/john.hopkins_mozillamessaging.com/buildbotcustom-stage/rev/fc1aab2ed027

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

Can you not append the arguments you need to the configure call done in the repack factory? I'm really really not a fan of this =(.

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 607976 [details] [diff] [review]
https://hg.mozilla.org/users/john.hopkins_mozillamessaging.com/buildbotcustom-stage/rev/fc1aab2ed027

- You said that you only need this additional checkout for l10n, because en-US does it through client.mk and some mozconfig options. Because of that, let's move all this work to BaseRepackFactory.
- The required code should be part of the Factory, and not added in an external function. Put that code in a method on BaseRepackFactory.
- Don't pass the entire 'config' dictionary, most of it is already passed as a kwarg. Pull out the arguments you do need and pass _them_ in a dictionary. Let's call it something like 'clientPyArgs'.
- Use a specific flag like 'callClientPy' instead of 'altCheckout'.

Comment 6

[John Hopkins (:jhopkins)]

Attachment: buildbot-configs patch

Comment 7

[John Hopkins (:jhopkins)]

Attachment: buildbotcustom diff

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 613853 [details] [diff] [review]
buildbot-configs patch

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

feedback+, but the structure issues we talked about a few weeks ago, that i fixed in my github branches, haven't been addressed yet. There's also a ton of what looks to me like unnecessary content in mozilla-tests/thunderbird_config.py around talos and android test stuff that needs to go. more comments inline:

::: mozilla-tests/thunderbird_config.py
@@ +2,5 @@
> +
> +from config import BRANCH_UNITTEST_VARS, TALOS_DIRTY_OPTS, TALOS_TP_OPTS, \
> +                   TALOS_TP4_OPTS, TALOS_ADDON_OPTS, \
> +                   TALOS_BASELINE_ADDON_OPTS, TALOS_REMOTE_FENNEC_OPTS, \
> +                   TALOS_CMD, MOZHARNESS_REPO

Why is all this talos stuff getting imported, we don't run Talos for Thunderbird, do we?

@@ +281,5 @@
> +        BRANCHES[branch]['platforms'][suiteToAdd[0]][suiteToAdd[1]][type] = \
> +            addSuite( suiteGroupName=suiteToAdd[3], newSuiteName=suiteToAdd[4],
> +                      suiteList=BRANCHES[branch]['platforms'][suiteToAdd[0]][suiteToAdd[1]][type])
> +
> +ANDROID_XUL_UNITTEST_DICT = {

Why does this, and the dict below it, exist in this file?

@@ +738,5 @@
> +BRANCHES['comm-central']['platforms']['android']['enable_debug_unittests'] = True
> +BRANCHES['comm-central']['xperf_tests'] = (1, True, {}, WIN7_ONLY)
> +BRANCHES['comm-central']['tprow_tests'] = (1, True, TALOS_TP_OPTS, ALL_PLATFORMS)
> +BRANCHES['comm-central']['trobopan_tests'] = (1, True, TALOS_REMOTE_FENNEC_OPTS, ANDROID_NATIVE)
> +BRANCHES['comm-central']['trobocheck_tests'] = (1, True, TALOS_REMOTE_FENNEC_OPTS, ANDROID_NATIVE)

And these, too?

::: mozilla-tests/thunderbird_production_config.py
@@ +1,1 @@
> +from copy import deepcopy

Doesn't this file need some real contents? The Firefox equivalent has a bunch of stuff: https://github.com/mozilla/buildbot-configs/blob/master/mozilla-tests/production_config.py#L22

::: mozilla-tests/thunderbird_staging_config.py
@@ +1,1 @@
> +from copy import deepcopy

Same for this one.

::: mozilla/build_localconfig.py
@@ +27,5 @@
>      'mozilla-aurora',
>      'mozilla-release',
>      'mozilla-esr10',
>      ])
> +ACTIVE_THUNDERBIRD_BRANCHES = ['mozilla-central']

Is this the only branch we want enabled for Thunderbird? And shouldn't it be 'comm-central' instead, since that's what's used in thunderbird_config.py?

::: mozilla/staging_thunderbird_config.py
@@ +1,1 @@
> +from staging_config import

Everything here should be in thunderbird_staging_config.py.

@@ +27,5 @@
> +TRY_SLAVES['linux64'] += TRY_LINUX64 + TRY_LINUX64_IXS
> +TRY_SLAVES['macosx'] += TRY_MAC
> +TRY_SLAVES['macosx64'] += TRY_MAC64
> +TRY_SLAVES['win32'] += TRY_WIN32_IXS
> +

You don't need to do any slave definitions here. thunderbird_config.py directly imports them from localconfig.py (which ends up being staging_config.py or production_config.py). Please remove these sections.

::: mozilla/thunderbird_production_config.py
@@ +1,1 @@
> +from copy import deepcopy

Needs to get flushed out still.

::: setup-master.py
@@ +279,5 @@
>          "staging-scheduler_master",
>          local_links = [
>              ('staging_scheduler_master_sm01_localconfig.py', 'master_localconfig.py'),
>              ('staging_config.py', 'localconfig.py'),
> +            ('staging_thunderbird_config.py', 'thunderbird_localconfig.py'),

It looks like you didn't grab the latest from my github branch, which we talked about a few weeks ago. You should be using thunderbird_{staging,product}_config.py here.

Comment 9

[John Hopkins (:jhopkins)]

Comment on attachment 613854 [details] [diff] [review]
buildbotcustom diff

Landed in:

http://hg.mozilla.org/build/buildbotcustom/rev/a7eab7ada08b
http://hg.mozilla.org/build/buildbotcustom/rev/7cdcdaca3bed
http://hg.mozilla.org/build/buildbotcustom/rev/edd8b9f2decc

Comment 10

[[e-mail] (:bhearsum)]

This is breaking things:
ERROR - TEST-FAIL bm23-tests1-windows failed to run checkconfig
INFO  - log for "bm23-tests1-windows" is "/builds/buildbot/preproduction/slave/test-masters/buildbot-configs/test-output/bm23-tests1-windows-sc44DQ-checkconfig.log"
INFO  - creating "bm24-tests1-linux" master
INFO  - created  "bm24-tests1-linux" master, running checkconfig
INFO  - starting to print log file '/builds/buildbot/preproduction/slave/test-masters/buildbot-configs/test-output/bm24-tests1-linux-LkfGDz-checkconfig.log'
INFO  - Traceback (most recent call last):
INFO  -   File "/builds/buildbot/preproduction/slave/test-masters/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_a2045101fe7a_production_0.8-py2.6.egg/buildbot/scripts/runner.py", line 1039, in doCheckConfig
INFO  -     ConfigLoader(configFileName=configFileName)
INFO  -   File "/builds/buildbot/preproduction/slave/test-masters/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_a2045101fe7a_production_0.8-py2.6.egg/buildbot/scripts/checkconfig.py", line 31, in __init__
INFO  -     self.loadConfig(configFile, check_synchronously_only=True)
INFO  -   File "/builds/buildbot/preproduction/slave/test-masters/sandbox/lib/python2.6/site-packages/buildbot-0.8.2_hg_a2045101fe7a_production_0.8-py2.6.egg/buildbot/master.py", line 628, in loadConfig
INFO  -     exec f in localDict
INFO  -   File "/builds/buildbot/preproduction/slave/test-masters/buildbot-configs/test-output/bm24-tests1-linux-LkfGDz/master.cfg", line 6, in <module>
INFO  -     import buildbotcustom.misc
INFO  -   File "/builds/buildbot/preproduction/slave/test-masters/buildbotcustom/misc.py", line 1549
INFO  -      if config.get('mozilla_dir'):
INFO  -       ^

Comment 11

[John Hopkins (:jhopkins)]

Attachment: reallyShort() patch + bustage fix

Comment 12

[[e-mail] (:bhearsum)]

Comment on attachment 614148 [details] [diff] [review]
reallyShort() patch + bustage fix

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

Can you summarize what the problem and solution was?

Comment 13

[John Hopkins (:jhopkins)]

bhearsum: the key bit is at @@ -1656,23 +1661,23 where I inserted 'product' into the 'properties' dict.  That property was lost in the original patch, thus causing the assertion failure that we saw.

Comment 14

[John Hopkins (:jhopkins)]

Comment on attachment 614148 [details] [diff] [review]
reallyShort() patch + bustage fix

Landed in http://hg.mozilla.org/build/buildbotcustom/rev/31291093d27d

Comment 15

[John Hopkins (:jhopkins)]

Attachment: updated buildbot-configs patch

bhearsum: Updated patch addressing most of the review comments.
A couple of things not addressed:
- build slave definitions still need to be removed
- rename thunderbird_staging_config.py to staging_thunderbird_config.py (etc)

I would like to address those changes in a future patch if you're okay with that so we can get this baselined and move onto building parallel builds this week.

This patch passes the 'setup-master.py -t' tests.

Comment 16

[[e-mail] (:bhearsum)]

Comment on attachment 615181 [details] [diff] [review]
updated buildbot-configs patch

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

r=me with the noted staging config file name change and fix-up of setup-master.py. As far as I'm concerned, this can land once that's done, assuming you're happy with the output from your dev master.

::: mozilla-tests/thunderbird_config.py
@@ +426,5 @@
> +        BRANCHES['comm-central'][suite + '_tests'] = (0, True) + options
> +    else:
> +        # Suites that are turned on by default
> +        BRANCHES['comm-central'][suite + '_tests'] = (1, True) + options
> +BRANCHES['comm-central']['xperf_tests'] = (1, True, {}, WIN7_ONLY)

Is this xperf_tests entry required? Please remove if it isn't.

::: mozilla/release-thunderbird-comm-beta.py
@@ +1,2 @@
> +releaseConfig = {}
> +releaseConfig['disable_tinderbox_mail'] = True

I'm a little surprised to see release configs here, but it can't hurt to land them ahead of the release integration code.

@@ +1,5 @@
> +releaseConfig = {}
> +releaseConfig['disable_tinderbox_mail'] = True
> +
> +# Release Notification
> +releaseConfig['AllRecipients']       = ['[e-mail]','[e-mail]','[e-mail]']

Can you check with Alex and Callek to see if they really want to be getting these mails?

::: mozilla/staging_thunderbird_config.py
@@ +1,1 @@
> +from staging_config import \

Per IRC, this should be thunderbird_staging_config.py.

::: setup-master.py
@@ +279,5 @@
>          "staging-scheduler_master",
>          local_links = [
>              ('staging_scheduler_master_sm01_localconfig.py', 'master_localconfig.py'),
>              ('staging_config.py', 'localconfig.py'),
> +            ('staging_thunderbird_config.py', 'thunderbird_localconfig.py'),

Please be sure to remove these now-unnecessary symlinks.

Comment 17

[John Hopkins (:jhopkins)]

Patch with renamed files landed in https://hg.mozilla.org/users/john.hopkins_mozillamessaging.com/buildbot-configs-stage-approved/rev/bb260b55320e

Will follow up to address remaining feedback.

Comment 18

[John Hopkins (:jhopkins)]

Attachment: remove xperf_tests from thunderbird config

Comment 19

[[e-mail] (:bhearsum)]

Comment on attachment 617890 [details] [diff] [review]
remove xperf_tests from thunderbird config

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

Can we close this bug now, and track issues individually? The core of this is certainly done!

Comment 20

[John Hopkins (:jhopkins)]

Comment on attachment 617890 [details] [diff] [review]
remove xperf_tests from thunderbird config

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

Landed in https://hg.mozilla.org/build/buildbot-configs/rev/ab2c44b6e1ce