568298 - support l10n-merge on firefox release factories

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

[Axel Hecht [:Pike]]

Given the sprint of betas, we'd like to have l10n-merge as an option for Firefox releases, as we do for Fennec already.

Should be an option, just, really.

Blocks Firefox 4 Beta program.

Comment 1

[Chris Cooper [:coop] (he/him)]

(In reply to comment #0)
> Given the sprint of betas, we'd like to have l10n-merge as an option for
> Firefox releases, as we do for Fennec already.
> 
> Should be an option, just, really.

I'm fine with this being an option for alphas and betas, but I don't think we'd ever want to use l10n-merge for an RC/release. Is that the consensus? Putting out a potentially half-translated release is unacceptable IMO.

Comment 2

[Axel Hecht [:Pike]]

Me, myself, and I all agree with you there. Team of four :-)

Comment 3

[Axel Hecht [:Pike]]

Can we have an ETA on this for this today's delivery meeting? This is a blocker for an aggressive beta program dance.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

What's the progress here?

Comment 5

[[e-mail] (:bhearsum)]

Removing priority to get this back into the triage queue.

Comment 6

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

(In reply to comment #1)
> (In reply to comment #0)
> > Given the sprint of betas, we'd like to have l10n-merge as an option for
> > Firefox releases, as we do for Fennec already.
> > 
> > Should be an option, just, really.
> 
> I'm fine with this being an option for alphas and betas, but I don't think we'd
> ever want to use l10n-merge for an RC/release. Is that the consensus? Putting
> out a potentially half-translated release is unacceptable IMO.

(In reply to comment #2)
> Me, myself, and I all agree with you there. Team of four :-)

After some discussion within RelEng, we're proposing doing the following:

1) l10n-merge will be used for nightly, alpha builds
2) l10n-merge will be used for betas *only* when we know there will be another beta. However, l10n-merge will not be used for the last beta of a release, in order to reduce risk of surprises when doing RC.
3) l10n-merge will not used for RCs or any release builds

Let us know if we're missing something?

Comment 7

[Axel Hecht [:Pike]]

Nothing new.

The interesting piece right now is that we have fx4 beta 2 code-freezing on Monday, and we'd love to put that out to l10n with l10n-merge on. What we do in later betas is something we should discuss in .planning, with the fx drivers and l10n drivers on board.

Comment 8

[Chris Cooper [:coop] (he/him)]

I have a patch written. I'll try to grab a staging env tonight to get it tested.

Comment 9

[Chris Cooper [:coop] (he/him)]

Attachment: Remove custom compareLocales from ReleaseRepackFactory

Falls back on the step from BaseRepackFactory which allows mergeLocale.

Tested via staging runs of 4.0b2 with mergeLocales both on and off on moz2-master, which is still running on staging-master:9016 if you want to see the logs.

Config changes upcoming.

Comment 10

[Chris Cooper [:coop] (he/him)]

Attachment: Add mergeLocales to release configs

checkconfig fails if mergeLocales isn't specified (good thing), so we need to add it to all the release configs.

I've opted to make it open-ended so we always have the option of putting out a release with mergeLocales on, but the default is off.

Comment 11

[Chris Cooper [:coop] (he/him)]

(In reply to comment #10)
> I've opted to make it open-ended so we always have the option of putting out a
> release with mergeLocales on, but the default is off.

Except for m-c in this case, or course, which we want to be running with mergeLocales for 4.0b2. Just saving a step here.

Comment 12

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

Comment on attachment 458139 [details] [diff] [review]
Remove custom compareLocales from ReleaseRepackFactory

>diff --git a/process/factory.py b/process/factory.py
>-    def compareLocales(self):
>-        self.addStep(ShellCommand,
...
>-         flunkOnWarnings=True,

This is lost when we fall back to the copy in BackRepackFactory. I don't know if it makes sense to preserve it or not, but it would be worth checking if our expectations for nightlies are too loose when it comes to beta releases.

r- because I think the patch is incomplete by not passing 'LOCALE_MERGEDIR=$PWD/merged' to 'make installers-%(locale)'. At least, that's the best I can figure out from the consistent failures in the make_installers_locale step, in your staging runs between Jul 17 13:59 and 14:39 where compare locales was run. And there's some problem with sync not being merged over, despite the include
  http://hg.mozilla.org/mozilla-central/file/default/browser/locales/l10n.ini

Comment 13

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

Comment on attachment 458140 [details] [diff] [review]
Add mergeLocales to release configs

>diff --git a/mozilla2-staging/release-fennec-1.0.py b/mozilla2-staging/release-fennec-1.0.py
>+# mergeLocales allows missing localized strings to be filled in by their en-US
>+# equivalent string. This is on (True) by default for nightly builds, but 
>+# should be False for releases *EXCEPT* alphas and early betas. If in doubt, 
>+# ask release-drivers.
>+mergeLocales        = False
> productName         = 'fennec'
> appName             = 'mobile'
> mergeLocales        = True

You're changing an existing declaration of mergeLocales here and in the staging release-fennec-1.1.py. Please preserve the existing value.

>diff --git a/mozilla2-staging/release-firefox-mozilla-1.9.1.py b/mozilla2-staging/release-firefox-mozilla-1.9.1.py
>+mergeLocales       = False
> l10nRepoClonePath   = 'releases/l10n-mozilla-1.9.1'

Comment block & declaration got pasted twice into this file.
Nit: align the ='s; same for mozilla2-staging/release-firefox-mozilla-1.9.2.py

>diff --git a/mozilla2-staging/release-firefox-mozilla-central.py b/mozilla2-staging/release-firefox-mozilla-central.py
>+binaryName         = 'Firefox'
>+oldBinaryName      = 'MozillaDeveloperPreview'

You should set |oldBinaryName      = binaryName| for 4.0b2. Thanks for updating this to something more current.

>diff --git a/mozilla2/release-fennec-1.0.py b/mozilla2/release-fennec-1.0.py
>+# mergeLocales allows missing localized strings to be filled in by their en-US
>+# equivalent string. This is on (True) by default for nightly builds, but
>+# should be False for releases *EXCEPT* alphas and early betas. If in doubt,
>+# ask release-drivers.
>+mergeLocales       = False
> productName         = 'fennec'
> appName             = 'mobile'
> mergeLocales        = False

There's just duplication here, rather than the value changing in staging equvalent. Same for release-fennec-1.1.py.

r+ with the comments fixed.

Comment 14

[Mike Beltzner [:beltzner, not reading bugmail]]

Does this need to be checked in before we roll builds?

Comment 15

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

Yes, but to build repo's rather than mozilla-central.

Comment 16

[Chris Cooper [:coop] (he/him)]

Attachment: Remove custom compareLocales from ReleaseRepackFactory, v2

(In reply to comment #12)
> This is lost when we fall back to the copy in BackRepackFactory. I don't know
> if it makes sense to preserve it or not, but it would be worth checking if our
> expectations for nightlies are too loose when it comes to beta releases.

I don't think so. The only other thing compareLocales does in BaseRepackFactory is remove an existing merged dir.

> r- because I think the patch is incomplete by not passing
> 'LOCALE_MERGEDIR=$PWD/merged' to 'make installers-%(locale)'.

Fixed.

Also, because I'm pedantic and have been staring at these steps for 4 days, I've changed the name/description to repack_installers across the board to be more consistent.

Comment 17

[[e-mail] (:bhearsum)]

Comment on attachment 458668 [details] [diff] [review]
Remove custom compareLocales from ReleaseRepackFactory, v2

Looks fine to me, based on Nick's prior review comment and your follow-up. r=me as long as your staging run passes.

Comment 18

[Chris Cooper [:coop] (he/him)]

Attachment: Add mergeLocales to release configs, v3

Fixes the nits from nthomas in comment 13, and carrying forward the review.

Comment 19

[Chris Cooper [:coop] (he/him)]

Comment on attachment 458678 [details] [diff] [review]
Add mergeLocales to release configs, v3

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

Comment 20

[Chris Cooper [:coop] (he/him)]

Comment on attachment 458668 [details] [diff] [review]
Remove custom compareLocales from ReleaseRepackFactory, v2

http://hg.mozilla.org/build/buildbotcustom/rev/f406be13afb6