980120 - Add tooltool support to Windows builds

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Mike Hommey [:glandium]]

In order for sccache to be deployed on windows, we need windows builds to be using tooltool.

Mike, do you think you can look into this?

Comment 1

[Michael Shal [:mshal]]

Sure thing.

Comment 2

[Michael Shal [:mshal]]

Attachment: windows-tools

I'm not sure if this is the best way to fix it, but retry.py can't call Popen on tooltool.py directly, so we need to prefix the command with python.exe. See also: http://stackoverflow.com/questions/912830/using-subprocess-to-run-python-script-on-windows

Comment 3

[Michael Shal [:mshal]]

Attachment: buildbotcustom.patch

The patch to misc.py passes the tooltool_script argument through so we can specify that in the windows config.

The patch to factory.py is similar to the tools patch - Windows can't execute fetch_and_unpack.sh directly, so we prefix the command with sh.exe. If there's a better solution here just let me know!

Comment 4

[Michael Shal [:mshal]]

Still needed is the config.py patch to pass in tooltool_script and tooltool_manifest_src. The location of tooltool_script depends on bug 980964, and the manifest src needs to be checked into m-c after we get the actual setup.sh uploaded to tooltool.

glandium, did you have any additional requirements aside from 'rm -rf sccache; tar -Jxf sccache.tar.xz'? Also FYI, it seems the tar on the Windows machines doesn't support -J or --xz...

Comment 5

[[e-mail] (:bhearsum)]

Comment on attachment 8387656 [details] [diff] [review]
windows-tools

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

I think the right way to do this is to adjust the commands to include 'python' and 'bash'. We do this in a bunch of places for other things, eg:
https://github.com/mozilla/build-buildbotcustom/blob/master/process/factory.py#L499
https://github.com/mozilla/build-buildbotcustom/blob/master/process/factory.py#L703

Comment 6

[Michael Shal [:mshal]]

Attachment: windows-tools

Specifying 'python .../tooltool.py' for the Windows config is a bit tricky since it gets passed into fetch_and_unpack.sh on the command-line. This moves the script to the end of the command-line, so we can slurp multiple arguments.

Comment 7

[Michael Shal [:mshal]]

Attachment: buildbotcustom.patch

Add 'sh' to fetch_and_unpack.sh, and handle tooltool_script as a list.

Comment 8

[Michael Shal [:mshal]]

Attachment: buildbot-configs.patch

Change buildbot-configs to use a list for tooltool_script. Note the Windows values aren't correct yet - we'll need the actual script path from bug 980964, and obviously a win64/releng.manifest (I was just testing with linux32/releng.manifest). I'm just posting this here now for reference.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 8387953 [details] [diff] [review]
windows-tools

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

These look much better, thanks for removing the hacks :).

Landing this will probably be tricky though. Any jobs that start prior to the reconfig will use the old buildbotcustom code. I don't think it's possible to find a time to land this that doesn't end up with a bunch of jobs getting a version of fetch_and_unpack.sh that doesn't have the order of args that they expect. You may want to consider renaming this script so you can land it without bustage. If you can find a way to support both invocations in one version of the script, that would work too. I'd be fine with something hacky there (eg, some hardcoded thing that uniquely identifies TT_BOOTSTRAP), as long as you remove it after landing.

Comment 10

[Michael Shal [:mshal]]

Attachment: windows-tools.patch

This just adds the script as tooltool_wrapper.sh as discussed in IRC. We can then remove fetch_and_unpack.sh after the dust settles.

Comment 11

[Michael Shal [:mshal]]

Attachment: buildbot-configs.patch

This doesn't have the Windows config yet, it just converts tooltool_script to a list.

Comment 12

[Michael Shal [:mshal]]

Attachment: buildbotcustom.patch

Comment 13

[Michael Shal [:mshal]]

Comment on attachment 8389350 [details] [diff] [review]
windows-tools.patch

tools: https://hg.mozilla.org/build/tools/rev/36d255dfcab5

Comment 14

[Michael Shal [:mshal]]

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

buildbot-configs: https://hg.mozilla.org/build/buildbot-configs/rev/8c57b9d04ad6

Comment 15

[Michael Shal [:mshal]]

Comment on attachment 8389352 [details] [diff] [review]
buildbotcustom.patch

buildbotcustom: https://hg.mozilla.org/build/buildbotcustom/rev/9c95333cc33f

Comment 16

[Michael Shal [:mshal]]

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

Backed out since Linux apparently shouldn't be using /builds/tooltool.py, even with the mock environment. Still investigating.

Comment 17

[Michael Shal [:mshal]]

Attachment: buildbotcustom.patch

Rebased buildbotcustom.patch, and made it work with l10n repack builds. (The buildbot-configs patch is the same as before)

Comment 18

[Michael Shal [:mshal]]

Attachment: tools-l10n

tools patch to handle multiple --tooltool-script arguments.

Comment 19

[Michael Shal [:mshal]]

Attachment: setup.sh

setup.sh script to be uploaded to tooltool for Windows to unpack sccache

Comment 20

[Michael Shal [:mshal]]

Attachment: 0001-Bug-980120-Add-releng.manifest-for-sccache-on-win64.patch

m-c patch to add the win64 releng.manifest for tooltool.

Comment 21

[[e-mail] (:bhearsum)]

Comment on attachment 8394829 [details]
setup.sh

I'm not actually sure how this works...I think rail is probably a better reviewer for this.

Comment 22

[[e-mail] (:bhearsum)]

Comment on attachment 8394826 [details] [diff] [review]
buildbotcustom.patch

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

::: process/release.py
@@ +793,5 @@
>                  if pf.get('tooltool_manifest_src'):
>                      extra_args.extend(['--tooltool-manifest', pf.get('tooltool_manifest_src')])
>                  if pf.get('tooltool_script'):
> +                    for script in pf['tooltool_script']:
> +                        extra_args.extend(['--tooltool-script', script])

This looks OK. Can you confirm that this test run on your master was using this code? http://dev-master1.srv.releng.scl3.mozilla.com:8064/builders/release-mozilla-beta-linux_repack_1%2F6/builds/15

Comment 23

[Michael Shal [:mshal]]

Comment on attachment 8394828 [details] [diff] [review]
tools-l10n

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

Comment 24

[Mike Hommey [:glandium]]

Comment on attachment 8394829 [details]
setup.sh

try 7z x -so sccache.tar.xz | tar -xf -

But this patch should go in bug 980117 anyways.

Comment 25

[Michael Shal [:mshal]]

Comment on attachment 8394826 [details] [diff] [review]
buildbotcustom.patch

buildbotcustom: https://hg.mozilla.org/build/buildbotcustom/rev/f7ad8167f2fc

Comment 26

[Michael Shal [:mshal]]

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

buildbot-configs: https://hg.mozilla.org/build/buildbot-configs/rev/81a2764b35cb

Comment 27

[Michael Shal [:mshal]]

Comment on attachment 8394829 [details]
setup.sh

Moving setup.sh to bug 980117

Comment 28

[[e-mail] (:bhearsum)]

in production

Comment 29

[Mike Hommey [:glandium]]

As far as i can tell, tooltool doesn't run on windows builds yet. I guess there is still a piece needed in buildbot config.

Comment 30

[Mike Hommey [:glandium]]

Attachment: Add tooltool manifests for Windows builds

Presumably, this should work without adding files in the tree. I tested removing the releng.manifest file on linux on try, and the tooltool step says:

browser/config/tooltool-manifests/linux32/releng.manifest doesn't exist, skipping...
program finished with exit code 0

And the build continues as expected.

Without this, we can't test windows tooltool on try.

Comment 31

[Mike Hommey [:glandium]]

Please land this for me once it's reviewed, and put in production asap. Thanks.

Comment 32

[[e-mail] (:bhearsum)]

Comment on attachment 8397762 [details] [diff] [review]
Add tooltool manifests for Windows builds

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

This will go in with the next reconfig - either later today on tomorrow.

Comment 33

[John Hopkins (:jhopkins)]

in production

Comment 34

[Michael Shal [:mshal]]

Attachment: buildbot-configs-tooltool-script.patch

Sorry, I thought the tooltool_manifest_src wouldn't work until it was in tree. In any case, for the Windows manifests, we'll need tooltool_script to pass in 'python'.

Comment 35

[Michael Shal [:mshal]]

Comment on attachment 8397905 [details] [diff] [review]
buildbot-configs-tooltool-script.patch

https://hg.mozilla.org/build/buildbot-configs/rev/b472d70340d2

Comment 36

[John Hopkins (:jhopkins)]

in production

Comment 37

[Michael Shal [:mshal]]

Attachment: buildbot-configs-b2g-thunderbird.patch

I should have added the python tooltool.py lines in b2g_config.py and thunderbird_config.py as well.

Having the tooltool_manifest_src without tooltool_script works for now because those tooltool_manifest_src files don't exist in the tree yet, so tooltool_wrapper.sh helpfully avoids trying to run the default tooltool_script.

Comment 38

[Mike Hommey [:glandium]]

Comment on attachment 8389350 [details] [diff] [review]
windows-tools.patch

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

::: lib/python/build/l10n.py
@@ +72,5 @@
>  
>      run_cmd(["mkdir", "-p", "l10n"])
>  
>      if tooltoolManifest:
> +        cmd = ['scripts/scripts/tooltool/tooltool_wrapper.sh',

Missing 'sh' here. This broke l10n repacks:
http://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/29.0b3-candidates/build1/logs/release-mozilla-beta-win32_repack_1-bm86-build1-build8.txt.gz

Comment 39

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

Landed as https://hg.mozilla.org/build/tools/rev/507a0f0eef46 & moved tags for 29.0b3

Comment 40

[Michael Shal [:mshal]]

Comment on attachment 8398194 [details] [diff] [review]
buildbot-configs-b2g-thunderbird.patch

https://hg.mozilla.org/build/buildbot-configs/rev/ebccd9e955bb

Comment 41

[John Hopkins (:jhopkins)]

in production.

Comment 42

[Mike Hommey [:glandium]]

I think we're in a state where we can say this is fixed.

Comment 43

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

We needed a followup fix for release automation builds:
  http://hg.mozilla.org/build/buildbotcustom/rev/5ac1a6ec2688

The windows 31.0b1 build failed when it tried to run /tools/tooltool.py. This wasn't a problem in 30.0 or older because there was no manifest file in the repo, but one merged this time round.