785392 - Use pre-built mar/bsdiff for l10n repacks

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P2)

Description

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

Right now, we try to build the mar and bsdiff tools *every time* we do a new l10n nightly, because we create the partial mar as part of that process. We do check whether the tools already exist, i.e. built on a previous run, and since we don't reboot or clobber these builders very often, we often don't actually rebuild. Still, the mar and bsdiff tools themselves don't change very often (if at all...maybe yearly), so even just the checking is adding unnecessary time, process, and complexity to our repack process. 

Worse still, unrelated changes have the potential to derail the l10n nightly process. Witness bug 785066, caused by bug 579517.

Now that we have tooltool in place, we should bless a version of mar and bsdiff for each platform and deploy them with tooltool. We should change the current nightly l10n process to use those versions of the tools instead, and clean up the repack process to remove the steps that build the tools in place.

Comment 1

[Rail Aliiev [:rail]]

I'd suggest to package needed binaries as a part of en-US build and upload the package to the same directory with en-US installers. This would eliminate dependency on keeping the tooltool entry up to date (manually).

Comment 2

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

+1 to Rail's suggestion, since we'll have to scramble to update tooltool whenever a mar/mbsdiff change comes along (surprise!). Bonus points for not including a version or product in the filename so it's easy to download without polling.

Comment 3

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

Attachment: Upload mar and mbsdiff when present.

These files will only get uploaded when present, i.e. --enable-update-packaging is specified (nightlies), and will be silently ignored otherwise.

Comment 4

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

Attachment: Copy mar tools to latest-$branch

This has been running on dev-stage01 for me for a few days.

It seems a little hacky to me to rely on the tinderbox_builds_dir to determine the platform in ReleaseToLatest, but it happens to match up exactly with the platforms we use in buildbot-configs and we don't currently pass the platform elsewhere AFAICT.

Willing to try to find a better way (e.g. actually pass a platform to post_upload.py) if you think this is gross.

Comment 5

[Aki Sasaki (not active)]

Comment on attachment 713252 [details] [diff] [review]
Copy mar tools to latest-$branch

It is pretty hacky.

I'm thinking changing

+        elif filename.startswith('mar') or filename.startswith('mbsdiff'):

to

        elif filename in ('mar', 'mar.exe', 'mbsdiff', 'mbsdiff.exe'):

or

        elif filename in ('mar', 'mar.exe') or filename.startswith('mbsdiff'):

might avoid unexpected behavior if we ever upload anything with a name that starts with 'mar' that isn't 'mar' or 'mar.exe'.

Do we ever fall off the end of if/elif chain?

Comment 6

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

Comment on attachment 713252 [details] [diff] [review]
Copy mar tools to latest-$branch

You could maybe write this as:
        elif filename in ('mar', 'mar.exe', 'mbsdiff', 'mbsdiff.exe'):
            if options.tinderbox_builds_dir:
                platform = options.tinderbox_builds_dir.split('-')[-1]
                if platform in ('win32', 'macosx64', 'linux', 'linux64'):
                    CopyFileToDir(f, upload_dir, '%s/%s' % (marToolsPath, platform))

Comment 7

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

Comment on attachment 713252 [details] [diff] [review]
Copy mar tools to latest-$branch

Checked in using Nick's pattern:

https://hg.mozilla.org/build/tools/rev/ade10274495c

(In reply to Aki Sasaki [:aki] from comment #5) 
> Do we ever fall off the end of if/elif chain?

There's nothing I can find that produces a nightly without also populating tinderbox-builds.

Comment 8

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

Comment on attachment 713251 [details] [diff] [review]
Upload mar and mbsdiff when present.

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/c706a4235ae4

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=72c85779e0f5

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c706a4235ae4

Comment 10

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

Comment on attachment 713251 [details] [diff] [review]
Upload mar and mbsdiff when present.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): none
User impact if declined: none
Testing completed (on m-c, etc.): m-i, uplifted to m-c yesterday 
Risk to taking this patch (and alternatives if risky): very low
String or UUID changes made by this patch: none

Uploads two new tools that are only present when --enable-update-packaging is specified (nightlies). These tools help short-circuit the l10n repack process and allow us to start using the much larger pool of win64 builders for win32 l10n nightly repacks on m-c and aurora (bug 784848).

Comment 11

[Lukas Blakk [:lsblakk] use ?needinfo]

same as bug 774947 comment 10.