559057 - do a release staging run for Linux 64 with updates, unit tests and talos

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Armen [:armenzg]]

Once we have 3.6.4 generated we can do a fake 3.6.5 release that will generate updates for Linux 64 (since we are shipping 3.6.4 linux 64 without updates).

Comment 1

[Armen [:armenzg]]

I forgot to mention that we want unit tests and talos tests for it as well.

Comment 2

[Rail Aliiev [:rail]]

Current status:
* --enable-official-branding used in temporary mozconfigs to produce "Firefox" binaries instead of "Mozilla Developer Preview".
* the previous release (downloaded to staging) is renamed from "mozilladeveloperpreview" to "firefox" to make some builders work
* linux64 and macosx64 builds are OK
* 2 releases have been built (3.7a6 and 3.7a7) for en-US and th locales
* update builder works fine (doesn't produce any update snippets for the new platforms the first time and does produce for the next release)
* l10n_verificaton fails for the first release (en-US only previous release) and works fine for the second release
* ${platform}_update_verify builders work
* final_verificatoin fails but the Bouncer links looks fine

To be tested: Japanese l10n builds

Comment 3

[Rail Aliiev [:rail]]

Attachment: tools changes

Comment 4

[Rail Aliiev [:rail]]

Attachment: buildbotcustom changes

Some notes.

MercurialBuildFactory:
use fix-linux-stack.pl for linux64 and fix-macosx-stack.pl for macosx64

L10nMixin:
use a temporary solution for platforms not listed in shipped-locales, assume linux==linux64 and macosx==macosx64

Comment 5

[Rail Aliiev [:rail]]

Attachment: buildbot-configs changes

Some comments

* values in config.py for linux64 were aligned with linux values
* used the same mozconfig for linux64 and linux release (except CC/CXX)
* Added oldRepoPath variable to release_configs and used in release master for ReleaseUpdatesFactory. Now we can use shipped locales from another repository.
* UPDATE_PACKAGING_R11 tag should be added to the mozilla-central and CVS (see the following patches)

Comment 6

[Rail Aliiev [:rail]]

Attachment: MozAUSLib.pm changes

Comment 7

[Rail Aliiev [:rail]]

Attachment: Bootstrap/Util.pm changes

Comment 8

[[e-mail] (:bhearsum)]

Comment on attachment 447731 [details] [diff] [review]
tools changes

Config.pm is going to need to support a list of platforms being passed in, like we talked about on the phone. Same for patcher-config-bump.pl.

I haven't looked at this in great detail, nor looked for anything missing, but a quick scan looks OK.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 447733 [details] [diff] [review]
buildbotcustom changes

>       # Make sure a supported platform is passed. Allow variations, but make
>       # sure to convert them to the form the locales files ues.
>-      assert platform in ('linux', 'win32', 'macosx', 'osx', 'maemo', 'wince', 'macosx64')
>+      assert platform in ('linux', 'linux64', 'win32', 'macosx', 'osx', 'maemo', 'wince', 'macosx64')
>       self.platform = platform
>       if self.platform.startswith('macosx'):
>         self.platform = 'osx'
>+      if self.platform.startswith('linux'):
>+        self.platform = 'linux'

Need to figure out what we're doing here still, as discussed on the phone.

>-        if config['enable_codecoverage'] and platform in ('linux',):
>+        if config['enable_codecoverage'] and platform in ('linux','linux64'):

You can drop linux64 here, I doubt we're going to run codecoverage on it.


>-    if 'Linux' in builder:
>+    if 'Linux x86-64' in builder:
>+        platform = 'linux64'
>+    elif 'Linux' in builder:
>         platform = 'linux'
>+    elif 'OS X 10.6' in builder:
>+        platform = 'mac64'
>     elif 'OS X' in builder:
>         platform = 'mac'

Can you update the existing OS X check to be more explicit? 'OS X 10.5'.


Again, this looks fine overall, haven't looked in detail.

Comment 10

[[e-mail] (:bhearsum)]

Comment on attachment 447735 [details] [diff] [review]
buildbot-configs changes

>-            'enable_opt_unittests': False,
>+            'enable_opt_unittests': True,

Actually, we want to keep these off on the build machines. We're only ever going to run them on the test machines.
>-            'enable_unittests': False,
>+            'enable_unittests': True,

Same here.

>-
>-# We're using GCC 4.2 which has a bug we can work around.
>-export ac_cv_have_visibility_class_bug=yes

Have you tested this config change? Mozconfig stuff looks fine otherwise.

>+enUSPlatforms       = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')
>+l10nPlatforms       = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')
>+talosTestPlatforms  = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')
>+unittestPlatforms   = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')

You can just use = enUSPlatforms.


>diff --git a/mozilla2-staging/release_master.py b/mozilla2-staging/release_master.py
>--- a/mozilla2-staging/release_master.py
>+++ b/mozilla2-staging/release_master.py
>@@ -377,17 +377,17 @@ for platform in enUSPlatforms:
>             'nextSlave': _nextFastSlave,
>         })
> 
>     if platform in unittestPlatforms:
>         mochitestLeakThreshold = pf.get('mochitest_leak_threshold', None)
>         crashtestLeakThreshold = pf.get('crashtest_leak_threshold', None)
>         for suites_name, suites in branchConfig['unittest_suites']:
>             # Release builds on mac don't have a11y enabled, do disable the mochitest-a11y test
>-            if platform == 'macosx' and 'mochitest-a11y' in suites:
>+            if platform.startswith('macosx') and 'mochitest-a11y' in suites:
>                 suites = suites[:]
>                 suites.remove('mochitest-a11y')
> 
>             test_builders.extend(generateTestBuilder(
>                 branchConfig, 'release', platform, "%s_test" % platform,
>                 "release-%s-unittest" % (platform,),
>                 suites_name, suites, mochitestLeakThreshold,
>                 crashtestLeakThreshold))
>@@ -513,17 +513,17 @@ updates_factory = ReleaseUpdatesFactory(
>     ausHost=branchConfig['aus2_host'],
>     ausServerUrl=ausServerUrl,
>     hgSshKey=hgSshKey,
>     hgUsername=hgUsername,
>     # We disable this on staging, because we don't have a CVS mirror to
>     # commit to
>     commitPatcherConfig=False,
>     clobberURL=branchConfig['base_clobber_url'],
>-    oldRepoPath=sourceRepoPath,
>+    oldRepoPath=oldRepoPath,

Why is this (and the ones below it) changing?

Comment 11

[Rail Aliiev [:rail]]

Attachment: Bootstrap/Util.pm changes

Comment 12

[Rail Aliiev [:rail]]

Attachment: tools changes

(In reply to comment #8)
> (From update of attachment 447731 [details] [diff] [review])
> Config.pm is going to need to support a list of platforms being passed in, like
> we talked about on the phone. Same for patcher-config-bump.pl.

Done. See also Bootstrap::Util

Comment 13

[Rail Aliiev [:rail]]

Attachment: buildbot-configs changes

(In reply to comment #10)
> (From update of attachment 447735 [details] [diff] [review])
> >-            'enable_opt_unittests': False,
> >+            'enable_opt_unittests': True,
> 
> Actually, we want to keep these off on the build machines. We're only ever
> going to run them on the test machines.

OK.

> >-# We're using GCC 4.2 which has a bug we can work around.
> >-export ac_cv_have_visibility_class_bug=yes
> 
> Have you tested this config change? Mozconfig stuff looks fine otherwise.

At least no failures during the build. We use gcc-4.3.3, the work around was for gcc 4.2 if I understand correctly.
 
> >+enUSPlatforms       = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')
> >+l10nPlatforms       = ('linux', 'linux64', 'win32', 'macosx', 'macosx64')
> You can just use = enUSPlatforms.

Done.

> >-    oldRepoPath=sourceRepoPath,
> >+    oldRepoPath=oldRepoPath,
> 
> Why is this (and the ones below it) changing?

Removed. As we talked we don't need to download shipped-locales file from another repository.

L10nVerifyFactory requires l10nPlatforms list to be passed to l10n_verify.sh (see tools diff).

Comment 14

[Rail Aliiev [:rail]]

Attachment: buildbotcustom changes

(In reply to comment #9)
> >+      if self.platform.startswith('linux'):
> >+        self.platform = 'linux'
> 
> Need to figure out what we're doing here still, as discussed on the phone.

I'd prefer to leave it as is, otherwise the logic will be a little bit complex. We would need to keep track of "similar" platforms (macosx64 -> macosx, win32 -> win64) and "similar" locales (ja <-> ja-JP-mac).

Comment 15

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 448291 [details] [diff] [review]
Bootstrap/Util.pm changes

>Index: Bootstrap/Util.pm
> my %PLATFORM_MAP = (# bouncer/shipped-locales platform => patcher2 platform
...
>+my %PLATFORM_FTP_MAP = (# buildbot/ftp directory platform mapping

Looks fine, please just make these two comments in the same sort of style, eg
my %PLATFORM_FTP_MAP = (# buildbot platform => ftp directory

Comment 16

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 448294 [details] [diff] [review]
tools changes

Looks good to me, lots of nice generalisation.

Comment 17

[Rail Aliiev [:rail]]

Attachment: Bootstrap/Util.pm changes

(In reply to comment #15)
> (From update of attachment 448291 [details] [diff] [review])
> >Index: Bootstrap/Util.pm
> > my %PLATFORM_MAP = (# bouncer/shipped-locales platform => patcher2 platform
> ...
> >+my %PLATFORM_FTP_MAP = (# buildbot/ftp directory platform mapping
> 
> Looks fine, please just make these two comments in the same sort of style, eg
> my %PLATFORM_FTP_MAP = (# buildbot platform => ftp directory

Done. Migrating r=nthomas from the previous version.

Comment 18

[Rail Aliiev [:rail]]

Attachment: Bootstrap/Util.pm changes

Runtime fixes, GetBuildbotToFTPPlatformMap should be added to @EXPORT_OK. Tested in staging.

Comment 19

[Rail Aliiev [:rail]]

Attachment: tools changes

Syntax fixes ( :( ). Tested in staging.

Comment 20

[Rail Aliiev [:rail]]

Attachment: buildbotcustom changes

Add missing space (verify_l10n.sh). Tested in staging.

Comment 21

[[e-mail] (:bhearsum)]

Comment on attachment 448474 [details] [diff] [review]
tools changes


> Usage: patcher-config-bump.pl [options]
> This script depends on the MozBuild::Util and Bootstrap::Util modules.
> Options: 
>   -p The product name (eg. firefox, thunderbird, seamonkey, etc.)
>@@ -61,16 +61,18 @@ Options:
>      release. Specifically, this will cause the 'beta' channel snippets to
>      point at the Bouncer server (rather than FTP). When passed, the
>      'release' channel will be considered the channel for final release.
>      This means that 'release' channel snippets will point to bouncer and
>      'beta' channel ones will point to FTP.
>      Generally, Alphas and Betas do not pass this, final and point releases do.
>   -l The path and filename to the shipped-locales file for this release.
>   -n Release notes URL.
>+  --platforms The list of platforms (multiple). Default to:
>+    --platforms linux --platforms macosx --platforms win32

Small nit here: The option should be called --platform, because each instance of it is a single item.

>   -h This usage message.
>   --run-tests will run the (very basic) unit tests included with this script.
> __USAGE__
> 
>         exit(0);
>     }
> 
>     my $error = 0;
>@@ -104,17 +106,19 @@ __USAGE__
>         $config{'brand'} = ucfirst($config{'product'});
>     }
>     if (! defined $config{'app-version'}) {
>         $config{'app-version'} = $config{'version'};
>     }
>     if (! defined $config{'use-beta-channel'}) {
>         $config{'use-beta-channel'} = 0;
>     }
>-
>+    if (! defined $config{'platforms'}) {
>+        $config{'platforms'} = ('linux', 'macosx', 'win32');

Please move this list to a global: eg, DEFAULT_PLATFORMS

>+  --platforms The list of platforms (multiple). Default to:
>+    --platforms linux --platforms macosx --platforms win32

>+    if (! defined $config{'platforms'}) {
>+        $config{'platforms'} = ('linux', 'macosx', 'win32');
>+    }

Same thing here.


Looks fine otherwise, but please fix the nits before landing.

Comment 22

[[e-mail] (:bhearsum)]

Comment on attachment 448475 [details] [diff] [review]
buildbotcustom changes

You'll need to update the arguments after the tools changes I suggested, looks fine otherwise.

Comment 23

[[e-mail] (:bhearsum)]

Comment on attachment 448295 [details] [diff] [review]
buildbot-configs changes

Please update the talos masters to use the linux64/macosx64-perf environments you created in the buildbotcustom patch.

>-patcherToolsTag     = 'UPDATE_PACKAGING_R9'
>+patcherToolsTag     = 'UPDATE_PACKAGING_R11'

Did you update any of the update tools in m-c? I don't see an UPDATE_PACKAGING_R11 tag yet.

Comment 24

[Rail Aliiev [:rail]]

(In reply to comment #23)
> (From update of attachment 448295 [details] [diff] [review])
> Please update the talos masters to use the linux64/macosx64-perf environments
> you created in the buildbotcustom patch.

Probably I can remove them from the buildbotcustom patch, they are just copies...
 
> >-patcherToolsTag     = 'UPDATE_PACKAGING_R9'
> >+patcherToolsTag     = 'UPDATE_PACKAGING_R11'
> 
> Did you update any of the update tools in m-c? I don't see an
> UPDATE_PACKAGING_R11 tag yet.

Actually there is no changes in m-c, so we can just tag UPDATE_PACKAGING_R9 with UPDATE_PACKAGING_R11. We use the same tag for hg and cvs tools, this is the main reason why we should bump the tag in both repos if there is a change in one of them.

Comment 25

[[e-mail] (:bhearsum)]

(In reply to comment #24)
> (In reply to comment #23)
> > (From update of attachment 448295 [details] [diff] [review] [details])
> > Please update the talos masters to use the linux64/macosx64-perf environments
> > you created in the buildbotcustom patch.
> 
> Probably I can remove them from the buildbotcustom patch, they are just
> copies...
> 

Yeah, that's fine too.

> > >-patcherToolsTag     = 'UPDATE_PACKAGING_R9'
> > >+patcherToolsTag     = 'UPDATE_PACKAGING_R11'
> > 
> > Did you update any of the update tools in m-c? I don't see an
> > UPDATE_PACKAGING_R11 tag yet.
> 
> Actually there is no changes in m-c, so we can just tag UPDATE_PACKAGING_R9
> with UPDATE_PACKAGING_R11. We use the same tag for hg and cvs tools, this is
> the main reason why we should bump the tag in both repos if there is a change
> in one of them.

Okay, that sounds good to me.

Comment 26

[Rail Aliiev [:rail]]

Attachment: tools changes

This patch includes fix for bug 569546, so we can handle "MozillaDeveloperPreview" style binary names (and renames) while keeping product set to "firefox" (used for FTP directory names) and brand to "Firefox" (used in update configs).

The interdiff against the previous r+ patch is here: http://diff.pastebin.mozilla.org/731063

Tested in staging for mozilla-central and mozilla-1.9.2.

Comment 27

[Rail Aliiev [:rail]]

Attachment: buildbotcustom changes

This patch includes fix for bug 569546.

The interdiff against the previous r+ patch is here:
http://diff.pastebin.mozilla.org/731071

Tested in staging for mozilla-central and mozilla-1.9.2.

Comment 28

[Rail Aliiev [:rail]]

CCing SeaMonkey, Thunderbird and Calendar folks because the next patch may be interesting to you. :)

Comment 29

[Rail Aliiev [:rail]]

Attachment: buildbot-configs changes

This patch includes fix for bug 569546.

The interdiff against the previous r+ patch is here:
http://diff.pastebin.mozilla.org/731072

Tested in staging for mozilla-central and mozilla-1.9.2.

$ ./test-masters.sh  |grep -v " OK$" |wc -l
0

Some notes:

* We have add UPDATE_PACKAGING_R11 tag to mozilla-central, releases/mozilla-1.9.1 and releases/mozilla-1.9.2, probably using the same changeset we used for UPDATE_PACKAGING_R9 or UPDATE_PACKAGING_R10 (we don't introduce any changes to update packaging here), add the same tag to mozilla/tools/release and mozilla/tools/patcher CVS modules and force to use all the masters this tag. Otherwise tools (tip) will try to use some functions which are not in UPDATE_PACKAGING_R{9,10}.

* Added new required "l10nPlatforms" argument of L10nVerifyFactory. Applied to each release_master.py which uses the factory (including calendar, seamonkey, thunderbird). Nothing to be changed in release_config.py.

* Staging changes ported to production. master2.cfg runs mozilla-1.9.1, so no need to add 64bit slaves there. 

* Introduced 2 optional arguments of ReleaseUpdatesFactory (binaryName, oldBinaryName). Now we can handle "MozillaDeveloperPreview" releases without manual patch-configs and update-configs bumps. They set to None for 1.9.1 and 1.9.2.

Comment 30

[[e-mail] (:bhearsum)]

A couple things about the latest patches raise red flags to me:
* Defaulting binaryName/oldBinaryName to None in the configs. This is pretty opaque to me. Saying that either of them is "None" implies that they don't exist, which isn't accurate, of course. Rail and I chatted about this on IRC and he said that we could default them to productName in the configs without issue. Currently, those defaults are set in the scripts. In my opinion, that would be better than the current patch.

* The use of tolower() and lower() is a bit disconcerting, but I'm not entirely sure why more than "it feels klutzy".

Nick, you usually have good ideas about things like this. Do you mind taking a look at the patches and weighing in?

Comment 31

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 449230 [details] [diff] [review]
tools changes

>diff --git a/clobberer/index.php b/clobberer/index.php

Chris should look at the changes here. Setting r?

>diff --git a/release/l10n/verify_l10n.sh b/release/l10n/verify_l10n.sh

We should probably think about splitting l10n_verification out by platform, as it's up to 60 minutes for 3 platforms (on an xserve). Not asking you to solve that here though.

>diff --git a/release/patcher-config-bump.pl b/release/patcher-config-bump.pl
>+  --marname MAR prefix (firefox, mozilladeveloperpreview) for this release.
>+  --oldmarname MAR prefix (firefox, mozilladeveloperpreview) for the previous
>+    release.

Please extend the comment to mention that this is optional and it defaults to.

>diff --git a/release/update-verify-bump.pl b/release/update-verify-bump.pl
>+  --binary-name           The binary prefix (eg. Firefox,
>+                          MozillaDeveloperPreview) for this version.
>+  --old-binary-name       The binary prefix (eg. Firefox,
>+                            MozillaDeveloperPreview) for the previous 
version.

Put the defaults in the comment again. You should add a test for when the binary name changes.

Comment 32

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 449236 [details] [diff] [review]
buildbot-configs changes

>diff --git a/mozilla2-staging/release-firefox-mozilla-1.9.1.py b/mozilla2-staging/release-firefox-mozilla-1.9.1.py
>+binaryName          = None
>+oldBinaryName       = None

I agree with Ben that this isn't super clear, perhaps '' is slightly better. The alternative is productName.capitalize() ? Some sort of get() would be the best solution after bug 569824.

>diff --git a/mozilla2-staging/release-firefox-mozilla-central.py b/mozilla2-staging/release-firefox-mozilla-central.py
>+# TODO: create this file before 3.7a6
> patcherConfig       = 'moz193-branch-patcher2.cfg'

Nit, this file now exists so the comment can be dropped.

>diff --git a/mozilla2/config.py b/mozilla2/config.py
>+                'DISPLAY': ':2',

Eek, wonder how we got away without this before these tests moved to talos-rev3. Nice catch.

>+BRANCHES['mozilla-central']['l10n_platforms'] = ['linux', 'linux64', 'win32', 'macosx', 'macosx64']

Did we make a policy decision to enable l10n for the 64bit builds ? I've forgotten.

>diff --git a/mozilla2/master1.cfg b/mozilla2/master1.cfg
> L10N_SLAVES = {
>     'linux': LINUX_VMS[:8],
>+    'linux64': SLAVES['linux64'],
>     'win32': WIN32_VMS[:8],
>     'macosx': MAC_MINIS[:6] + XSERVES[:2],
>+    'macosx64': SLAVES['macosx-snow'],
> }

If we are doing l10n I'd prefer to allocate a subset of mac 10.6 slaves. We have lots of them but firing up 10+ l10n jobs at the same time gives hg.m.o a headache.
 
>+verifyConfigs       = {'linux':    'moz193-firefox-linux.cfg',
...
>+                       'win32':   'moz193-firefox-win32.cfg'}

Nit, one more space to align the win32 config.

>diff --git a/seamonkey/release-comm-1.9.1.py b/seamonkey/release-comm-1.9.1.py
>-patcherToolsTag            = 'UPDATE_PACKAGING_R9'
>+patcherToolsTag            = 'UPDATE_PACKAGING_R11'

Could you remind me which change requires this ? AIUI Thunderbird/SeaMonkey/etc aren't doing 64bit builds at present, so there's a balance here between having issues in the future if they add 64bit vs differences between R9 and R11 that may destabilize them now.

Leaving r? to see what Rail says.

Comment 33

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 449232 [details] [diff] [review]
buildbotcustom changes

>diff --git a/process/factory.py b/process/factory.py
>@@ -157,30 +160,22 @@ class MozillaBuildFactory(BuildFactory):
>+    ignore_dirs.extend(['%s_update_verify' % p for p in getSupportedPlatforms()])
>+    ignore_dirs.extend(['%s_build' % p for p in getSupportedPlatforms()])
>+    ignore_dirs.extend(['%s_repack' % p for p in getSupportedPlatforms()])
>+    ignore_dirs.extend(['xulrunner_%s_build' % p for p in getSupportedPlatforms()])

We only need to do this for the current platform I think. Same in BaseRepackFactory and CCBaseRepackFactory.

> class L10nVerifyFactory(ReleaseFactory):
>+                 buildSpace=14, **kwargs):

We'll probably have to revisit 'buildSpace=14' for 5 full platforms worth of locales. Another reason to split it out by platform.

>diff --git a/status/generators.py b/status/generators.py

I would advocate landing this part asap as Armen moved some 64bit builders to try.

Comment 34

[Rail Aliiev [:rail]]

Attachment: buildbot-configs changes

Interdiff against the previous version is here: http://diff.pastebin.mozilla.org/732482

(In reply to comment #32)
> I agree with Ben that this isn't super clear, perhaps '' is slightly better.
> The alternative is productName.capitalize() ? Some sort of get() would be the
> best solution after bug 569824.

Fixed.
 
> >+# TODO: create this file before 3.7a6
> > patcherConfig       = 'moz193-branch-patcher2.cfg'
> 
> Nit, this file now exists so the comment can be dropped.

OK.

> >+BRANCHES['mozilla-central']['l10n_platforms'] = ['linux', 'linux64', 'win32', 'macosx', 'macosx64']
> 
> Did we make a policy decision to enable l10n for the 64bit builds ? I've
> forgotten.

AFAIK, no. Removing the hunk. For releases we use l10nPlatforms.
 
> If we are doing l10n I'd prefer to allocate a subset of mac 10.6 slaves. We
> have lots of them but firing up 10+ l10n jobs at the same time gives hg.m.o a
> headache.

Fixed by using [:8] 

> Nit, one more space to align the win32 config.

Fixed.
 
> Could you remind me which change requires this ? AIUI Thunderbird/SeaMonkey/etc
> aren't doing 64bit builds at present, so there's a balance here between having
> issues in the future if they add 64bit vs differences between R9 and R11 that
> may destabilize them now.

There is some changes in tools that require the new version of files in CVS (patcher-config-bump.pl and Release::Patcher::Config use GetBuildbotToFTPPlatformMap). We don't use UPDATE_PACKAGING_R?? tags for tools repository (we use the tip), so there is no way right now to check out the older versions of tools.

Comment 35

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 449809 [details] [diff] [review]
buildbot-configs changes

>diff --git a/mozilla2-staging/l10nbuilds1.ini b/mozilla2-staging/l10nbuilds1.ini
>diff --git a/mozilla2/l10nbuilds1.ini b/mozilla2/l10nbuilds1.ini

Interdiff looks fine but please remove these changes too. Unneeded and potentially a cause of bustage. Ben should take a look at his patch as well.

Comment 36

[Nick Thomas [:nthomas] (UTC+12)]

(In reply to comment #34)
> There is some changes in tools that require the new version of files in CVS
> (patcher-config-bump.pl and Release::Patcher::Config use
> GetBuildbotToFTPPlatformMap). We don't use UPDATE_PACKAGING_R?? tags for tools
> repository (we use the tip), so there is no way right now to check out the
> older versions of tools.

Urgh, OK. The differences between R9 and R10 are given at bug 514305 comment #15. Tagging style in CVS is describe at bug 420947 comment #17.

Comment 37

[Robert Kaiser]

(In reply to comment #29)
> * Added new required "l10nPlatforms" argument of L10nVerifyFactory. Applied to
> each release_master.py which uses the factory (including calendar, seamonkey,
> thunderbird). Nothing to be changed in release_config.py.

As a side note, thanks for doing this on our side as well, I'm happy with all the changes you're doing in seamonkey/ in that patch.

Comment 38

[Rail Aliiev [:rail]]

Attachment: tools changes

Interdiff against the previous patch is here: http://diff.pastebin.mozilla.org/732626

(In reply to comment #31)
> We should probably think about splitting l10n_verification out by platform, as
> it's up to 60 minutes for 3 platforms (on an xserve). Not asking you to solve
> that here though.

+1. As a separate bug. :)
 
> >diff --git a/release/patcher-config-bump.pl b/release/patcher-config-bump.pl
> >+  --marname MAR prefix (firefox, mozilladeveloperpreview) for this release.
> >+  --oldmarname MAR prefix (firefox, mozilladeveloperpreview) for the previous
> >+    release.
> 
> Please extend the comment to mention that this is optional and it defaults to.

Done.

> >diff --git a/release/update-verify-bump.pl b/release/update-verify-bump.pl
> >+  --binary-name           The binary prefix (eg. Firefox,
> >+                          MozillaDeveloperPreview) for this version.
> >+  --old-binary-name       The binary prefix (eg. Firefox,
> >+                            MozillaDeveloperPreview) for the previous 
> version.
> 
> Put the defaults in the comment again. 

Done.

> You should add a test for when the
> binary name changes.

Added. I also fixed major update unit test which used to use /releases/ directory while we expect it to use candidates directory (see Bug 553052 for the details).

catlee: could you take a look at clobberer/index.php changes (see comment #31).

Assuming that r+ from Nick should be applied for this version. :)

Comment 39

[Chris AtLee [:catlee]]

Comment on attachment 449862 [details] [diff] [review]
tools changes

> // TODO: Figure out if we can use LDAP to do this
> $SPECIAL_PEOPLE = array(
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>   '[e-mail]',
>+  '[e-mail]',
>   '[e-mail]',
>+  '[e-mail]',
> );

I think there are some typos here.  mtylor should be mtaylor, and ralliev should be raliiev?

r+ with those fixed

Comment 40

[Rail Aliiev [:rail]]

(In reply to comment #33)
> (From update of attachment 449232 [details] [diff] [review])
> >diff --git a/process/factory.py b/process/factory.py
> >@@ -157,30 +160,22 @@ class MozillaBuildFactory(BuildFactory):
> >+    ignore_dirs.extend(['%s_update_verify' % p for p in getSupportedPlatforms()])
> >+    ignore_dirs.extend(['%s_build' % p for p in getSupportedPlatforms()])
> >+    ignore_dirs.extend(['%s_repack' % p for p in getSupportedPlatforms()])
> >+    ignore_dirs.extend(['xulrunner_%s_build' % p for p in getSupportedPlatforms()])
> 
> We only need to do this for the current platform I think. Same in
> BaseRepackFactory and CCBaseRepackFactory.


AFAIK, MozillaBuildFactory has no idea about the current platform (at least
there is no platform in __init__). In any case, having non existent directory
in ignore_dirs should affect us. 

> I would advocate landing this part asap as Armen moved some 64bit builders to
> try.

Thanks a lot. :)

Comment 41

[Rail Aliiev [:rail]]

Attachment: tools changes

(In reply to comment #39)
> I think there are some typos here.  mtylor should be mtaylor, and ralliev
> should be raliiev?
> 
> r+ with those fixed

Thanks for the catch. Fixed.

Keeping r+.

Comment 42

[[e-mail] (:bhearsum)]

(In reply to comment #35)
> (From update of attachment 449809 [details] [diff] [review])
> >diff --git a/mozilla2-staging/l10nbuilds1.ini b/mozilla2-staging/l10nbuilds1.ini
> >diff --git a/mozilla2/l10nbuilds1.ini b/mozilla2/l10nbuilds1.ini
> 
> Interdiff looks fine but please remove these changes too. Unneeded and
> potentially a cause of bustage. Ben should take a look at his patch as well.

I'm going to assume you meant "this patch", which looks fine to me.

Comment 43

[Rail Aliiev [:rail]]

Attachment: buildbot-configs changes

(In reply to comment #42)
> (In reply to comment #35)
> > (From update of attachment 449809 [details] [diff] [review] [details])
> > >diff --git a/mozilla2-staging/l10nbuilds1.ini b/mozilla2-staging/l10nbuilds1.ini
> > >diff --git a/mozilla2/l10nbuilds1.ini b/mozilla2/l10nbuilds1.ini
> > 
> > Interdiff looks fine but please remove these changes too. Unneeded and
> > potentially a cause of bustage. Ben should take a look at his patch as well.

Done.

> I'm going to assume you meant "this patch", which looks fine to me.

Keeping r+.

Comment 44

[[e-mail] (:bhearsum)]

Comment on attachment 449232 [details] [diff] [review]
buildbotcustom changes

This looks fine, too.

Comment 45

[Rail Aliiev [:rail]]

So far all 5 patches polished (thanks a lot for the comments!), so we can land them!

Here is my checklist for the landing:

1. Commit CVS patches (release, patcher)
2. Tag the release and patcher directories (read files under the directories)  with UPDATE_PACKAGING_R11
3. Tag the following repos (tip or UPDATE_PACKAGING_R10?) with UPDATE_PACKAGING_R11:
* mozilla-central
* comm-central
* releases/mozilla-1.9.1
* releases/mozilla-1.9.2
* releases/comm-1.9.1
* releases/comm-1.9.2
4. Commit/push hg based patches (buildbot-custom, buildbotcustom, tools)

Anything else?

Comment 46

[[e-mail] (:bhearsum)]

(In reply to comment #45)
> So far all 5 patches polished (thanks a lot for the comments!), so we can land
> them!
> 
> Here is my checklist for the landing:
> 
> 1. Commit CVS patches (release, patcher)
> 2. Tag the release and patcher directories (read files under the directories) 
> with UPDATE_PACKAGING_R11
> 3. Tag the following repos (tip or UPDATE_PACKAGING_R10?) with
> UPDATE_PACKAGING_R11:
> * mozilla-central
> * comm-central
> * releases/mozilla-1.9.1
> * releases/mozilla-1.9.2
> * releases/comm-1.9.1
> * releases/comm-1.9.2
> 4. Commit/push hg based patches (buildbot-custom, buildbotcustom, tools)
> 
> Anything else?

This all sounds fine to me. I looked at the _R9 -> _R11 diff in CVS and there was nothing scary in there. Some Bootstrap changes, bug 507405, a couple platform mapping updates -- nothing that should bust anything AFAICT.

Comment 47

[[e-mail] (:bhearsum)]

Comment on attachment 448473 [details] [diff] [review]
Bootstrap/Util.pm changes

Checking in Bootstrap/Util.pm;
/cvsroot/mozilla/tools/release/Bootstrap/Util.pm,v  <--  Util.pm
new revision: 1.18; previous revision: 1.17
done

Comment 48

[[e-mail] (:bhearsum)]

Comment on attachment 447736 [details] [diff] [review]
MozAUSLib.pm changes

Checking in MozAUSLib.pm;
/cvsroot/mozilla/tools/patcher/MozAUSLib.pm,v  <--  MozAUSLib.pm
new revision: 1.13; previous revision: 1.12
done

Comment 49

[[e-mail] (:bhearsum)]

Updated the tag with:
cvs co -r UPDATE_PACKAGING_R10 mozilla/client.mk
cd mozilla
make -f client.mk MOZ_CO_PROJECT=all checkout
cvs up -A tools/update-packaging
cvs up -AdP tools/release
cvs up -AdP tools/patcher
cvs -q tag UPDATE_PACKAGING_R11 2>&1 | tee ../tag.log
grep -v '^T' ../tag.log
cvs tag -b UPDATE_PACKAGING_R11_minibranch client.mk
cvs up -r UPDATE_PACKAGING_R11_minibranch client.mk

edited client.mk, bumped to UPDATE_PACKAGING_R11
commited client.mk

/cvsroot/mozilla/client.mk,v  <--  client.mk
new revision: 1.314.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1; previous revision: 1.314.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1
done


re-tagged
foo-ix-blah:mozilla bhearsum$ cvs -q tag UPDATE_PACKAGING_R11 client.mk
W client.mk : UPDATE_PACKAGING_R11 already exists on version 1.314.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1 : NOT MOVING tag to version 1.314.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1.2.1
foo-ix-blah:mozilla bhearsum$ cvs -q tag -F UPDATE_PACKAGING_R11 client.mk
T client.mk


Updated hg repos as follows:
m-c: hg tag -r UPDATE_PACKAGING_R10 UPDATE_PACKAGING_R11
1.9.2: hg tag -r UPDATE_PACKAGING_R10 UPDATE_PACKAGING_R11
1.9.1: hg tag -r UPDATE_PACKAGING_R9 UPDATE_PACKAGING_R11

I did not update comm repos, because they have no UPDATE_PACKAGING tags to speak of, since the tools get built out of the mozilla-* version.

Comment 50

[[e-mail] (:bhearsum)]

Comment on attachment 449866 [details] [diff] [review]
tools changes

changeset:   628:6b79373edea6

Comment 51

[[e-mail] (:bhearsum)]

Comment on attachment 449232 [details] [diff] [review]
buildbotcustom changes

changeset:   757:3c0494ee80bb

Comment 52

[[e-mail] (:bhearsum)]

Comment on attachment 449875 [details] [diff] [review]
buildbot-configs changes

changeset:   2491:67c9895ff8c5

Comment 53

[[e-mail] (:bhearsum)]

Had to land a minor bustage fix for a couple of release configs:
http://hg.mozilla.org/build/buildbot-configs/rev/045a8087b64d

Comment 54

[Rail Aliiev [:rail]]

Successfully tested in 3.7a5 release automation. Yay!

Comment 55

[Rail Aliiev [:rail]]

Attachment: Use $marName when build > 1

We hit the problem running patcher-config-bump.pl during second build of 4.0b1. We should use ($oldMarName|$marName) instead of $oldMarName

Comment 56

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 455317 [details] [diff] [review]
Use $marName when build > 1

http://hg.mozilla.org/build/tools/rev/1ce012e51326

Comment 57

[Philippe M. Chiasson (:gozer)]

Comment on attachment 449866 [details] [diff] [review]
tools changes

    if (! defined $config{'platform'}) {
        $config{'platform'} = @DEFAULT_PLATFORMS;
    }

That's a bug, should have been :

$config{'platform'} = \@DEFAULT_PLATFORMS;

Comment 58

[Philippe M. Chiasson (:gozer)]

Attachment: @DEFAULT_PLATFORMS typo fix

This broke update generation for Thunderbird 3.1.1, it's a very simple fix.

Comment 59

[[e-mail] (:bhearsum)]

Comment on attachment 455631 [details] [diff] [review]
@DEFAULT_PLATFORMS typo fix

Rail, you wrote this code. Can you review it?

Comment 60

[Rail Aliiev [:rail]]

Comment on attachment 455631 [details] [diff] [review]
@DEFAULT_PLATFORMS typo fix

Looks fine. \@DEFAULT_PLATFORMS is covered by tests:
http://hg.mozilla.org/build/tools/file/bd9a00fe1472/release/patcher-config-bump.pl#l635

We also should fix the same in patcher-config-creator.pl (patch is coming).

Comment 61

[Rail Aliiev [:rail]]

Attachment: @DEFAULT_PLATFORMS fix fo patcher-config-creator.pl

Comment 62

[[e-mail] (:bhearsum)]

Comment on attachment 455631 [details] [diff] [review]
@DEFAULT_PLATFORMS typo fix

changeset:   677:16e66d05fb24

Landed the patcher-config-creator.pl fix in here too.

Comment 63

[Nick Thomas [:nthomas] (UTC+12)]

Clobberer didn't get updated until today (bug 584620). We have to manually deploy that kind of change.