1403058 - Add MinGW Browser Build

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

This bug is created from Bug 1330608. It encompasses adding the browser build component while that bug has been scoped to just the toolchain.  

The dependencies will stay attached to that bug though, because it would generate an obscene amount of spam to move them.

Comment 1

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review commit: https://reviewboard.mozilla.org/r/183474/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183474/

Comment 2

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/1-2/

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review189602

::: browser/config/mozconfigs/win32/mingw32:38
(Diff revision 2)
> +#    CARGO
> +. "$topsrcdir/browser/config/mozconfigs/common"
> +
> +# MinGW Stuff
> +ac_add_options --target=i686-w64-mingw32
> +ac_add_options --with-toolchain-prefix=i686-w64-mingw32-

this shouldn't be necessary, cross compilations default to have the target as a toolchain prefix.

::: browser/config/mozconfigs/win32/mingw32:63
(Diff revision 2)
> +if [ -z "$no_tooltool" ]
> +then

I doubt we'll ever have builds with no_tooltool set for mingw, so you can skip this test.

::: browser/config/tooltool-manifests/mingw32/releng.manifest:2
(Diff revision 2)
> +  {
> +    "algorithm": "sha512",
> +    "visibility": "public",
> +    "filename": "rustc-x86_64-unknown-linux-gnu-mingw32-cross-repack.tar.xz",
> +    "version": "rustc 1.20.0 (f3d6973f4 2017-08-27) repack",
> +    "unpack": true,
> +    "digest": "9f1af5cf4febda716f5d89e9de2482814d62c456637fbe235469225f2cfdb3401cfc1b955a7225d17600332c88b093bccbef6f702f106d117bbb23786ea3fcbd",
> +    "size": 211950648
> +  }

Now that rust toolchains are set in tree, please use that.

::: taskcluster/ci/build/windows.yml:486
(Diff revision 2)
> +        tier: 2
> +    worker-type: aws-provisioner-v1/gecko-{level}-b-linux
> +    worker:
> +        max-run-time: 7200
> +        env:
> +            TOOLTOOL_MANIFEST: "browser/config/tooltool-manifests/mingw32/releng.manifest"

and you won't need this :)

::: taskcluster/ci/build/windows.yml:492
(Diff revision 2)
> +    run:
> +        using: mozharness
> +        script: mozharness/scripts/fx_desktop_build.py
> +        config:
> +            - builds/releng_base_windows_32_mingw_builds.py
> +        tooltool-downloads: public

nor that.

::: testing/mozharness/configs/builds/releng_base_windows_32_mingw_builds.py:106
(Diff revision 2)
> +    'mock_packages': [
> +        'autoconf213', 'python', 'mozilla-python27', 'zip', 'mozilla-python27-mercurial',
> +        'git', 'ccache', 'perl-Test-Simple', 'perl-Config-General',
> +        'yasm', 'wget',
> +        'mpfr',  # required for system compiler
> +        'xorg-x11-font*',  # fonts required for PGO
> +        'imake',  # required for makedepend!?!
> +        ### <-- from releng repo
> +        'yasm', 'ccache',
> +        ###
> +        'valgrind',
> +    ],

Actually, we probably don't use mock anymore, you probably can get away with not cargo culting this. Please flag someone from releng for this file, as mentioned in the other bug.

Comment 4

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review189614

::: browser/config/mozconfigs/win32/mingw32:38
(Diff revision 2)
> +#    CARGO
> +. "$topsrcdir/browser/config/mozconfigs/common"
> +
> +# MinGW Stuff
> +ac_add_options --target=i686-w64-mingw32
> +ac_add_options --with-toolchain-prefix=i686-w64-mingw32-

This is actually needed to locate windres (and one other tool I forgot as it was quite long ago...)

Comment 5

[Mike Hommey [:glandium]]

ah, is it that your gcc is not toolchain prefixed, but windres is?

Comment 6

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

(In reply to Mike Hommey [:glandium] from comment #5)
> ah, is it that your gcc is not toolchain prefixed, but windres is?

The opposite. I have the correct prefix on the gcc executable, and the build system finds that find. The windres executable is also prefixed; but the build system only looks for a prefixed executable if 'with-toolchain-prefix' is set.

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review189832

::: taskcluster/ci/build/windows.yml:486
(Diff revision 2)
> +        tier: 2
> +    worker-type: aws-provisioner-v1/gecko-{level}-b-linux
> +    worker:
> +        max-run-time: 7200
> +        env:
> +            TOOLTOOL_MANIFEST: "browser/config/tooltool-manifests/mingw32/releng.manifest"

It turns out I do, at least temporarily. I opened #1403997 for this issue.

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/2-3/

Comment 9

[Aki Sasaki (not active)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review189928

::: testing/mozharness/configs/builds/releng_base_windows_32_mingw_builds.py:24
(Diff revision 3)
> +        'check-test',
> +        'update',  # decided by query_is_nightly()
> +    ],
> +    "buildbot_json_path": "buildprops.json",
> +    'exes': {
> +        "buildbot": "/tools/buildbot/bin/buildbot",

I'm not 100% sure here, but the `buildbot` exe, the `sendchange` action, and the `enable_unittest_sendchange` may break from Taskcluster. This is something to keep an eye on in the try runs. It's possible that the tc config skips it anyway, which will make it harmless.

Comment 10

[Aki Sasaki (not active)]

I think the mozharness config looks good other than the above comment, which is probably ignorable.

Comment 11

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/3-4/

Comment 12

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Okay, cleaned up the stuff aki mentioned. Two outstanding issues:

- I still have --with-toolchain-prefix because it's required for winres (and one other executable I forget)
- Removing the tooltool references entirely is blocked by Bug 1403997

Comment 13

[Mike Hommey [:glandium]]

(In reply to Tom Ritter [:tjr] from comment #6)
> (In reply to Mike Hommey [:glandium] from comment #5)
> > ah, is it that your gcc is not toolchain prefixed, but windres is?
> 
> The opposite. I have the correct prefix on the gcc executable, and the build
> system finds that find. The windres executable is also prefixed; but the
> build system only looks for a prefixed executable if 'with-toolchain-prefix'
> is set.

That's actually not true. What it might be doing, though, is that it looks with the "wrong" prefix.

windres is searched from old-configure with https://dxr.mozilla.org/mozilla-central/source/build/autoconf/toolchain.m4#83

TOOLCHAIN_PREFIX, from that expression, is passed down from python configure, with the value inherited from https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#247

where first_toolchain_prefix is defined a few lines earlier https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#239 as the first of what's returned by https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#232 which, by default, returns *two* values, the first of which is derived from target.toolchain, which comes from https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/init.configure#608, and which, for the given target of i686-w64-mingw32, would be i686-mingw32.

Long story short, this would be fixed by moving those AC_CHECK_PROGS to python configure, making them enumerate all the toolchain prefixes. But that's out of scope here.

Comment 14

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/4-5/

Comment 15

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/5-6/

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review190946

We're getting there.

::: taskcluster/ci/build/windows.yml:486
(Diff revision 6)
> +        tier: 2
> +    worker-type: aws-provisioner-v1/gecko-{level}-b-linux
> +    worker:
> +        max-run-time: 7200
> +        env:
> +            TOOLTOOL_MANIFEST: "browser/config/tooltool-manifests/mingw32/releng.manifest"

You can remove this and the file now.

::: taskcluster/ci/toolchain/windows.yml:164
(Diff revision 6)
> +        using: toolchain-script
> +        script: repack_rust.py
> +        arguments: [
> +          '--channel', '1.19.0',
> +          '--host', 'i686-unknown-linux-gnu',
> +          '--target', 'i686-pc-windows-msvc',

probably don't need this

::: taskcluster/scripts/misc/build-gcc-mingw32.sh:17
(Diff revision 6)
>  data_dir=$HOME_DIR/src/build/unix/build-gcc
>  
>  . $data_dir/build-gcc.sh
>  
> -gcc_version=5.4.0
> -gcc_ext=bz2
> +gcc_version=6.4.0
> +gcc_ext=gz

Please use the same as build-gcc-6-linux.sh.

::: taskcluster/scripts/misc/build-gcc-mingw32.sh:37
(Diff revision 6)
>  # GPG key used to sign MPC
>  $GPG --import $data_dir/AD17A21EF8AED8F1CC02DBD9F7D5C9BF765C61E3.key
>  
>  cat > $root_dir/checksums <<EOF
>  369737ce51587f92466041a97ab7d2358c6d9e1b6490b3940eb09fb0a9a6ac88  binutils-2.27.tar.bz2
> -608df76dec2d34de6558249d8af4cbee21eceddbcb580d666f7a5a583ca3303a  gcc-5.4.0.tar.bz2
> +4715f02413f8a91d02d967521c084990c99ce1a671b8a450a80fbd4245f4b728  gcc-6.4.0.tar.gz

Likewise.

::: testing/mozharness/configs/builds/releng_base_windows_32_mingw_builds.py:62
(Diff revision 6)
> +        'CCACHE_DIR': '/builds/ccache',
> +        'CCACHE_COMPRESS': '1',
> +        'CCACHE_UMASK': '002',

Since you're depending on sccache, you don't need this or adding ccache to the $PATH.

Comment 17

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/6-7/

Comment 18

[Mike Hommey [:glandium]]

Additional patch in bug 1403997 fixes the tooltool problem for good:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=fb1282f09aea868e6d4cec7af3e836bb96602ec4

Comment 19

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/183474/diff/7-8/

Comment 20

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Okay, confirmed that and removed tooltool!

Comment 21

[Mike Hommey [:glandium]]

Comment on attachment 8912095 [details]
Bug 1403058 Add the MinGW32 browser build to Taskcluster

https://reviewboard.mozilla.org/r/183474/#review192034

::: browser/config/mozconfigs/win32/mingw32:64
(Diff revision 8)
> +ac_add_options --disable-crashreporter # Bug 1391685
> +ac_add_options --disable-maintenance-service
> +
> +# Find our toolchain
> +CC="$TOOLTOOL_DIR/gcc/bin/i686-w64-mingw32-gcc"
> +CXX="$TOOLTOOL_DIR/gcc/bin/i686-w64-mingw32-g++"

Note, not setting CXX/HOST_CXX should also work.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ecbc0a2984c
Add the MinGW32 browser build to Taskcluster r=glandium

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/0ecbc0a2984c