Closed Bug 1403058 Opened 2 years ago Closed 2 years ago

Add MinGW Browser Build

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: tjr, Assigned: tjr)

References

(Depends on 1 open bug)

Details

(Whiteboard: [tor])

Attachments

(1 file)

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 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.
Attachment #8912095 - Flags: review?(mh+mozilla)
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...)
ah, is it that your gcc is not toolchain prefixed, but windres is?
(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 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 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.
I think the mozharness config looks good other than the above comment, which is probably ignorable.
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
(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 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.
Attachment #8912095 - Flags: review?(mh+mozilla)
Okay, confirmed that and removed tooltool!
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.
Attachment #8912095 - Flags: review?(mh+mozilla) → review+
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/0ecbc0a2984c
Add the MinGW32 browser build to Taskcluster r=glandium
https://hg.mozilla.org/mozilla-central/rev/0ecbc0a2984c
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Depends on: 1414016
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.