Closed Bug 1434316 Opened 7 years ago Closed 6 years ago

Integrate x64 MinGW Build into Task Cluster

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox-esr60 fixed, firefox60 wontfix)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox-esr60 --- fixed
firefox60 --- wontfix

People

(Reporter: tjr, Assigned: tjr)

References

(Blocks 1 open bug)

Details

(Whiteboard: [tor])

Attachments

(3 files, 7 obsolete files)

Bug 1434316 Add 64 Bit Windows Build Targets r=glandium
7.27 KB, patch
tjr
: review+
RyanVM
: approval-mozilla-esr60+
Details | Diff | Splinter Review
Bug 1434316 Bump MinGW version r=glandium
959 bytes, patch
tjr
: review+
RyanVM
: approval-mozilla-esr60+
Details | Diff | Splinter Review
Bug 1434316 Add 64 Bit Windows Build Targets r=glandium
9.48 KB, patch
tjr
: review+
RyanVM
: approval-mozilla-esr60+
Details | Diff | Splinter Review 
We have 32 bit, now we need 64 bit.



Random item: we should investigate -fno-keep-inline-dllexport from win32.
Blocks: 1330608
No longer blocks: 1230910
Depends on: 1440013
Product: Core → Firefox Build System
Depends on: 1443823
Depends on: 1448453
Depends on: 1448461
Depends on: 1448483
Depends on: 1448746
Depends on: 1448748
Depends on: 1448749
Depends on: 1449834
Depends on: 1449835
Update: I got a successful build. I need to investigate whether or not it runs (the opt build segfaulted; and I need to up the timeout for the Debug build to have it complete, so more investigation to do - but a successful build is still something exciting!)
So a non-stripped debug build (either x86 or x64) won't run because Windows won't recognize the xul.dll (which is nearing 3GB)

Example: https://treeherder.mozilla.org/#/jobs?repo=try&revision=db83210956ffdd1936a63709fa86b09fdef1aefe&selectedJob=171109048
[task 2018-03-29T20:18:24.442Z] 20:18:24     INFO -  ac_add_options --enable-debug
[task 2018-03-29T20:18:24.442Z] 20:18:24     INFO -  ac_add_options --disable-optimize

These two together might just be too much. We might have to bump up to at least -O1? Is -Og supported?
(In reply to David Major [:dmajor] from comment #6)
> [task 2018-03-29T20:18:24.442Z] 20:18:24     INFO -  ac_add_options
> --enable-debug
> [task 2018-03-29T20:18:24.442Z] 20:18:24     INFO -  ac_add_options
> --disable-optimize
> 
> These two together might just be too much. We might have to bump up to at
> least -O1? Is -Og supported?

That was a good idea. I tried -O3, -Og, and -Os all with --disable-install-strip.  They were 2,683 MB, 2,768 MB, and 2,599 MB.  All exhibited the same issue =/
You can try the half-measure of setting `STRIP_FLAGS="--strip-debug"`, which is what our `--enable-profiling` builds (which are most of our builds on Nightly) do:
https://dxr.mozilla.org/mozilla-central/rev/a1fb8ffae378963b128deaaf3a76eff9dbb6be21/old-configure.in#675
FWIW I debugged and NtCreateSection is failing with 0xc000007b. And since that's a syscall I couldn't step in any deeper. "Show loader snaps" didn't provide anything useful either.
Depends on: 1456593
Depends on: 1460352
Depends on: 1460620
Depends on: 1456552
Depends on: 1460357
Depends on: 1460645
Depends on: 1460647
Depends on: 1389967
Depends on: 1411401
Depends on: 1460668
Depends on: 1460796
Depends on: 1461421
No longer depends on: 1460352
Expect Bug 1460777 to require this to be rebased for -central (currently focused on esr60 though)
See Also: → 1460777
Assignee: nobody → tom
Blocks: 1472304
Attachment #8952789 - Flags: review?(nfroyd)
Attachment #8952790 - Flags: review?(nfroyd)
Attachment #8952788 - Flags: review?(mh+mozilla)
Attachment #8952788 - Attachment is obsolete: true
Attachment #8952788 - Flags: review?(mh+mozilla)
Attachment #8952789 - Attachment is obsolete: true
Attachment #8952789 - Flags: review?(nfroyd)
Attachment #8973001 - Attachment is obsolete: true
Attachment #8988873 - Flags: review?(mh+mozilla)
Attachment #8988874 - Flags: review?(mh+mozilla)
Comment on attachment 8952789 [details]
Bug 1434316 Add 64 bit MinGW gcc toolchains

https://reviewboard.mozilla.org/r/222014/#review260770

::: build/unix/build-gcc/build-gcc.sh:129
(Diff revision 7)
>  }
>  
>  build_gcc_and_mingw() {
>    mkdir gcc-objdir
>    pushd gcc-objdir
> -  ../gcc-$gcc_version/configure --prefix=$install_dir --target=i686-w64-mingw32 --with-gnu-ld --with-gnu-as --disable-multilib --enable-threads=posix
> +  ../gcc-$gcc_version/configure --prefix=$install_dir --target=$gcc_target --with-gnu-ld --with-gnu-as --disable-multilib --enable-threads=posix

Communicating things through globals is just about as gross in shell as it is in other languages, but OK.
Attachment #8952789 - Flags: review?(nfroyd)
Comment on attachment 8952790 [details]
Bug 1434316 Add x64 to the MinGW rust target

https://reviewboard.mozilla.org/r/222016/#review260772
Attachment #8952790 - Flags: review?(nfroyd) → review+
Attachment #8952789 - Flags: review?(nfroyd)
Comment on attachment 8988873 [details]
Bug 1434316 Add 64 Bit Windows Build Targets

https://reviewboard.mozilla.org/r/254030/#review261520

All these files that are obvious copies of files we already have in the tree should be marked as such. Are you using mercurial or git?
Attachment #8988873 - Flags: review?(mh+mozilla)
Comment on attachment 8988874 [details]
Bug 1434316 Add 64 bit MinGW gcc toolchains

https://reviewboard.mozilla.org/r/254032/#review261524

::: taskcluster/scripts/misc/build-gcc-mingw32.sh:22
(Diff revision 1)
> +gcc_target=$1
>  gcc_ext=xz
>  binutils_version=2.27
>  binutils_ext=bz2
> -binutils_configure_flags="--target=i686-w64-mingw32"
> -mingw_version=bcf1f29d6dc80b6025b416bef104d2314fa9be57
> +binutils_configure_flags="--target=$gcc_target"
> +mingw_version=ee9fc3d0b8c8868280e38384edd968067b8e71fc

Is it possible to not change the version of mingw at the same time?
Attachment #8988874 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #40)
> Comment on attachment 8988873 [details]
> Bug 1434316 Add 64 Bit Windows Build Targets
> 
> https://reviewboard.mozilla.org/r/254030/#review261520
> 
> All these files that are obvious copies of files we already have in the tree
> should be marked as such. Are you using mercurial or git?

Fixed this and broke the MinGW version bump into a separate commit.
Comment on attachment 8988873 [details]
Bug 1434316 Add 64 Bit Windows Build Targets

https://reviewboard.mozilla.org/r/254030/#review262714

::: taskcluster/ci/build/windows.yml:656
(Diff revision 2)
>      worker-type: aws-provisioner-v1/gecko-{level}-b-linux
>      worker:
>          docker-image: {in-tree: mingw32-build}
>          max-run-time: 7200
>          env:
> -            PERFHERDER_EXTRA_OPTIONS: "opt"
> +            PERFHERDER_EXTRA_OPTIONS: "opt 32"

legitimate question: is it better to make the difference between 32 and 64 bits more obvious or to keep the history of the perfherder data?
Attachment #8988873 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8990794 [details]
Bug 1434316 Bump MinGW version

https://reviewboard.mozilla.org/r/255840/#review262716
Attachment #8990794 - Flags: review?(mh+mozilla) → review+
(In reply to Mike Hommey [:glandium] from comment #47)
> Comment on attachment 8988873 [details]
> Bug 1434316 Add 64 Bit Windows Build Targets
> 
> https://reviewboard.mozilla.org/r/254030/#review262714
> 
> ::: taskcluster/ci/build/windows.yml:656
> (Diff revision 2)
> >      worker-type: aws-provisioner-v1/gecko-{level}-b-linux
> >      worker:
> >          docker-image: {in-tree: mingw32-build}
> >          max-run-time: 7200
> >          env:
> > -            PERFHERDER_EXTRA_OPTIONS: "opt"
> > +            PERFHERDER_EXTRA_OPTIONS: "opt 32"
> 
> legitimate question: is it better to make the difference between 32 and 64
> bits more obvious or to keep the history of the perfherder data?

I don't have a strong opinion; but given that we only have build metrics - and that they're screwy regardless because of Bug 1411212 that wehaven't figured out yet - I'm inclined to erase the history for better clarity.
Now that we've landed the NSS bugs we needed; we can finally land this in ESR60. It's been a bit, but all I needed to do was rebase the patches slightly and bump the mingw version and it built and ran fine.  

I just sent in this cleaned up try run which will hopefully go green: https://treeherder.mozilla.org/#/jobs?repo=try&revision=77f786ddd5ab3f8f31037cbeaa92981b5841bda5
Attachment #8952790 - Attachment is obsolete: true
Carrying forward r+ from Comment 41
Attachment #8988874 - Attachment is obsolete: true
Attachment #9030043 - Flags: review+
Carrying forward r+ from Comment 48
Attachment #8990794 - Attachment is obsolete: true
Attachment #9030044 - Flags: review+
Carrying forward r+ from Comment 47
Attachment #8988873 - Attachment is obsolete: true
Attachment #9030045 - Flags: review+
Comment on attachment 9030043 [details] [diff] [review]
Bug 1434316 Add 64 Bit Windows Build Targets r=glandium

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Toolchain-only addition to esr60 to add the x64 version of the mingw-gcc toolchain. This toolchain is used by Tor and will help us avoid breaking the build for them.

User impact if declined: We might break the x64 mingw-gcc build and not know it.

Fix Landed on Version: 

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Toolchain only patch; the worst that would happen is the builds would break and we'd back it out. And they're Tier 2.

String or UUID changes made by this patch:
Attachment #9030043 - Flags: approval-mozilla-esr60?
Comment on attachment 9030043 [details] [diff] [review]
Bug 1434316 Add 64 Bit Windows Build Targets r=glandium

Sure, let's get these running on ESR60 to make life easier for Tor. Approved for ESR60.
Attachment #9030043 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Attachment #9030044 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Attachment #9030045 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: