Closed
Bug 1444274
(gcc-6.1)
Opened 7 years ago
Closed 7 years ago
Require GCC 6.1
Categories
(Firefox Build System :: General, enhancement)
Firefox Build System
General
Tracking
(firefox61 fixed)
RESOLVED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox61 | --- | fixed |
People
(Reporter: jgilbert, Assigned: jgilbert)
References
(Depends on 2 open bugs)
Details
Attachments
(1 file)
Let's track our prereqs for moving to requiring Clang 5.0.
Assignee | ||
Comment 1•7 years ago
|
||
I heard work is being done here, but I can't find it with my searches. Do you know, :ryanvm?
Flags: needinfo?(ryanvm)
Assignee | ||
Comment 3•7 years ago
|
||
So it's actually GCC 5.0 I care about for c++14 features. (specifically relaxed constexpr)
Summary: Require Clang 5.0 → Require GCC 5.0
Comment 4•7 years ago
|
||
302 sfink. When do we get the hazard builds off gcc 4.9?
Flags: needinfo?(mh+mozilla) → needinfo?(sphink)
Updated•7 years ago
|
Component: Platform Support → General
Product: Release Engineering → Firefox Build System
QA Contact: catlee
Comment 5•7 years ago
|
||
Note we'd probably require GCC 6 once we get builds off 4.9.
Updated•7 years ago
|
Flags: needinfo?(sphink)
Assignee | ||
Updated•7 years ago
|
Summary: Require GCC 5.0 → Require at least GCC 5.0+
Updated•7 years ago
|
Summary: Require at least GCC 5.0+ → Require at least GCC 6.0+
Assignee | ||
Comment 6•7 years ago
|
||
With sixgill running on 6.4, maybe that's what we should aim for, absent reason to aim lower. It is the most recent 6.x release. (July 4, 2017)
Comment 7•7 years ago
|
||
There's no reason to reject versions between 6.1 and 6.4. 6.0, yes, because it's the development version of the 6 branch, and the first actual release was 6.1 (gcc now has a pretty unconventional versioning scheme).
Assignee | ||
Updated•7 years ago
|
Summary: Require at least GCC 6.0+ → Require GCC 6.4
Assignee | ||
Comment 8•7 years ago
|
||
There hasn't been any 'nay' sentiment, even from the dev-platform post, so let's just try to move this forward.
Assignee | ||
Updated•7 years ago
|
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review235948
::: taskcluster/ci/toolchain/linux.yml:28
(Diff revision 1)
> - 'build/build-clang/build-clang.py'
> - 'build/build-clang/clang-3.9-linux64.json'
> - 'taskcluster/scripts/misc/tooltool-download.sh'
> toolchain-artifact: public/build/clang.tar.xz
> toolchains:
> - - linux64-gcc-4.9
> + - linux64-gcc-6
Didn't look at the entire patch yet, but let me stop you right there. You can't change the GCC version used for toolchains. I'm pretty sure multiple things break if you do that, starting with clang.
Attachment #8961585 -
Flags: review?(mh+mozilla)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 12•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jgilbert
Summary: Require GCC 6.4 → Require GCC 6.1
Assignee | ||
Comment 13•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review236844
Assignee | ||
Comment 14•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review236846
Attachment #8961585 -
Flags: review?(jgilbert)
Assignee | ||
Updated•7 years ago
|
Attachment #8961585 -
Flags: review?(jgilbert)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(mh+mozilla)
Comment 15•7 years ago
|
||
Have you done a new try showing that this doesn't bust the hazard builds?
Flags: needinfo?(mh+mozilla) → needinfo?(jgilbert)
Assignee | ||
Comment 16•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #15)
> Have you done a new try showing that this doesn't bust the hazard builds?
No, it still has some issue with them. Do you have any pointers there? I'm really out of my comfort zone on this.
Also, is there anything wrong in the proposed patch otherwise?
Flags: needinfo?(jgilbert) → needinfo?(mh+mozilla)
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review236868
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:79
(Diff revision 2)
> '__ORDER_BIG_ENDIAN__': 4321,
> })
>
>
> @memoize
> -def GCC(version):
> +def GCC_V(version):
Unless you change CLANG, VS and CLANG_CL in a similar way, I'd rather leave this alone.
Attachment #8961585 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review236872
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:79
(Diff revision 2)
> '__ORDER_BIG_ENDIAN__': 4321,
> })
>
>
> @memoize
> -def GCC(version):
> +def GCC_V(version):
This is splitting hairs a little fine for a test file like this, and I think the code is clear either way.
The reason it's like this is we assign `GCC = GCC_6` below so we can rely on a default `GCC` instead of having to update versions. If we leave GCC_V as GCC, we shadow that previous assignment, which, while safe here, is frowned upon.
Therefore I would prefer to leave it this way. Let me know if you require it be changed.
Attachment #8961585 -
Flags: review?(jgilbert)
Comment 19•7 years ago
|
||
Assigning GCC = GCC_6 might be convenient, but it's also confusing. If you really want to avoid the version, use a *different* name, like MINIMAL_GCC, BASE_SUPPORTED_GCC, or whatever.
Flags: needinfo?(mh+mozilla)
Comment 20•7 years ago
|
||
And again, if you want to avoid the version for GCC, you should do the same for the others.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 22•7 years ago
|
||
Ok, I don't have any more leads, and this is where I am: (hazard builds fail in assembling some tmpfile from parts unknown)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=58356b2fcedd43b22641686582973bc763edea2a
Who can either point me in the right direction, or just push this through?
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 23•7 years ago
|
||
Worth checking with sfink, probably. I suspect it's just something in the build environment being set up wrong, but I really don't know what's going on with these files. (Including clang+gcc all over?? Toolchain builds still using gcc-4.9??)
It seems close though, and it'll unblock simpler and less risky updates from Google upstream projects. (which use c++14 these days)
Flags: needinfo?(sphink)
Comment 24•7 years ago
|
||
Bug 1449066 is dealing with the hazard builds.
Flags: needinfo?(sphink)
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 25•7 years ago
|
||
Awesome, thanks.
Comment 26•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review238108
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:95
(Diff revision 3)
> -GCC_4_7 = GCC('4.7.3')
> -GXX_4_7 = GXX('4.7.3')
> GCC_4_9 = GCC('4.9.3')
> GXX_4_9 = GXX('4.9.3') + SUPPORTS_DRAFT_CXX14_VERSION
> GCC_5 = GCC('5.2.1') + DEFAULT_C11
> -GXX_5 = GXX('5.2.1') + SUPPORTS_GNUXX14
> +GXX_5 = GXX('5.2.1') + DEFAULT_CXX_14
You shouldn't change this.
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py
(Diff revision 3)
> 'RUST_LIB_SUFFIX': 'a',
> 'OBJ_SUFFIX': 'o',
> },
> }
>
> -
Don't remove this blank line. I expect flake8 to complain about this.
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:439
(Diff revision 3)
> '/usr/bin/clang++-3.6': CLANGXX_3_6 + CLANG_PLATFORM_X86_64_LINUX,
> '/usr/bin/clang-3.3': CLANG_3_3 + CLANG_PLATFORM_X86_64_LINUX,
> '/usr/bin/clang++-3.3': CLANGXX_3_3 + CLANG_PLATFORM_X86_64_LINUX,
> }
> - GCC_4_7_RESULT = ('Only GCC 4.9 or newer is supported '
> - '(found version 4.7.3).')
> +
> + def old_gcc_message(old_ver):
@staticmethod
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:542
(Diff revision 3)
> # When CXX is not set, we guess it from CC.
> self.do_toolchain_test(self.PATHS, {
> - 'c_compiler': self.GCC_5_RESULT,
> - 'cxx_compiler': self.GXX_5_RESULT,
> + 'c_compiler': self.DEFAULT_GCC_RESULT,
> + 'cxx_compiler': self.DEFAULT_GXX_RESULT,
> }, environ={
> - 'CC': 'gcc-5',
> + 'CC': 'gcc',
This changes what the test checks. USe gcc-7.
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:622
(Diff revision 3)
> - },
> - 'cxx_compiler': self.CLANGXX_3_6_RESULT + {
> - 'compiler': '/usr/bin/clang++-3.6',
> - },
> }, environ={
> - 'CC': 'clang-3.6',
> + 'CC': 'clang',
This changes what the test checks. Leave clang-3.6.
::: python/mozbuild/mozbuild/test/configure/test_toolchain_configure.py:842
(Diff revision 3)
> - 'CC': 'gcc-5',
> - 'CXX': 'g++-5',
> + 'CC': 'gcc',
> + 'CXX': 'g++',
This changes what the test checks. Use gcc-7 and g++-7.
::: taskcluster/ci/hazard/kind.yml:45
(Diff revision 3)
> files-changed:
> - js/public/**
> - js/src/**
> toolchains:
> - linux64-clang
> - - linux64-gcc-4.9
> + - linux64-gcc-6
This is handled by bug 1449066
Attachment #8961585 -
Flags: review?(mh+mozilla)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review238128
Attachment #8961585 -
Flags: review?(jgilbert)
Assignee | ||
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review238130
Assignee | ||
Updated•7 years ago
|
Attachment #8961585 -
Flags: review?(jgilbert)
Assignee | ||
Updated•7 years ago
|
Alias: gcc-6.1
Assignee | ||
Comment 31•7 years ago
|
||
Assignee | ||
Comment 32•7 years ago
|
||
@staticmethod seems to then require Classname.Funcname(), which we don't really want to bother with.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f185606f92bba6f4191273ea06f239fad5c6c695
Comment hidden (mozreview-request) |
Comment 34•7 years ago
|
||
mozreview-review |
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -
https://reviewboard.mozilla.org/r/230454/#review238582
Attachment #8961585 -
Flags: review?(mh+mozilla) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 36•7 years ago
|
||
Ok, should have fixed the failures from review fixes:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=03a9698519ded73228ae5f1c08d6da55258a889e
Comment 37•7 years ago
|
||
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f080a35baf6a
Require GCC 6.1+. - r=glandium
Comment 38•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox61:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Comment 39•7 years ago
|
||
Apparently some SpiderMonkey builds still depend on GCC 4.9 (or at least GCC 4.9 headers).
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a4cabfde7cf38a397d0b7600f2b240b048d2f09&selectedJob=171626749
https://treeherder.mozilla.org/logviewer.html#?job_id=171626749&repo=try&lineNumber=901
Should I file a new bug?
Flags: needinfo?(jgilbert)
Comment 40•7 years ago
|
||
Static analysis builds also busted:
https://treeherder.mozilla.org/logviewer.html#?job_id=171626730&repo=try
Comment 41•7 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #39)
> Apparently some SpiderMonkey builds still depend on GCC 4.9 (or at least GCC
> 4.9 headers).
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=1a4cabfde7cf38a397d0b7600f2b240b048d2f09&selectedJob=1
> 71626749
> https://treeherder.mozilla.org/logviewer.
> html#?job_id=171626749&repo=try&lineNumber=901
>
> Should I file a new bug?
(In reply to Masatoshi Kimura [:emk] from comment #40)
> Static analysis builds also busted:
> https://treeherder.mozilla.org/logviewer.html#?job_id=171626730&repo=try
Both of these problems are related: we've upgraded to a new compiler, but the new compiler hasn't been used to compile/package the compilers used for these jobs (at least for the static analysis build; I guess the SM builds use clang, too?). So we're still using the old headers, which causes problems.
Please file a new bug on getting those toolchains rebuilt; perhaps in that bug or another followup we can also figure out how to prevent this from happening again.
Flags: needinfo?(jgilbert)
Comment 42•7 years ago
|
||
The easy short term fix is to make the clang-using jobs use gcc 4.9. Clang toolchains jobs don't trivially build with gcc 6, which is why we left them with 4.9 in the first place.
Comment 43•7 years ago
|
||
The interesting question is why those jobs aren't busted on central?
Comment 44•7 years ago
|
||
They even use the same toolchains:
successful central: https://tools.taskcluster.net/groups/Tasc_JPWQ5Os8YhRvparfw/tasks/UjEDgd5WTV2p3AsAxKZXxg/details
failed try: https://tools.taskcluster.net/groups/c23E4NU8SDi8-x-nt2k5Tg/tasks/ExAPNEHVSCKzOqjZ6AlTtQ/details
Comment 45•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #42)
> The easy short term fix is to make the clang-using jobs use gcc 4.9. Clang
> toolchains jobs don't trivially build with gcc 6, which is why we left them
> with 4.9 in the first place.
Well, we can't change the static analysis builds. And it doesn't do much good to say that we require GCC 6.1 when we have some jobs that don't actually use GCC 6.1 headers, which means that they don't have expected standard library features...
Comment 46•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #42)
> The easy short term fix is to make the clang-using jobs use gcc 4.9. Clang
> toolchains jobs don't trivially build with gcc 6, which is why we left them
> with 4.9 in the first place.
(In reply to Mike Hommey [:glandium] from comment #43)
> The interesting question is why those jobs aren't busted on central?
I'm trying to use C++14-relaxed constexpr in the try run and found that it broke some jobs. Is relaxed constexpr still unusable? :(
Assignee | ||
Updated•7 years ago
|
Assignee | ||
Comment 48•7 years ago
|
||
(In reply to Masatoshi Kimura [:emk] from comment #46)
> (In reply to Mike Hommey [:glandium] from comment #42)
> > The easy short term fix is to make the clang-using jobs use gcc 4.9. Clang
> > toolchains jobs don't trivially build with gcc 6, which is why we left them
> > with 4.9 in the first place.
>
> (In reply to Mike Hommey [:glandium] from comment #43)
> > The interesting question is why those jobs aren't busted on central?
>
> I'm trying to use C++14-relaxed constexpr in the try run and found that it
> broke some jobs. Is relaxed constexpr still unusable? :(
I do have that split out into bug 1444271, since that's the key usecase for me. Sounds like we can't trivially close it yet, even though we have this bug fixed.
Comment 49•7 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #45
> And it doesn't do much
> good to say that we require GCC 6.1 when we have some jobs that don't
> actually use GCC 6.1 headers, which means that they don't have expected
> standard library features...
I thought we were using libc++ for clang builds on automation?
Comment 50•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #49)
> (In reply to Nathan Froyd [:froydnj] from comment #45
> > And it doesn't do much
> > good to say that we require GCC 6.1 when we have some jobs that don't
> > actually use GCC 6.1 headers, which means that they don't have expected
> > standard library features...
>
> I thought we were using libc++ for clang builds on automation?
We are for Android/Mac builds, but not on Linux AFAIK. I guess we could try to wire things up so clang builds on Linux (at least until we ship them to users?) use libc++ instead.
Comment 51•7 years ago
|
||
Things were better than what I thought first. The core language feature, C++14 relaxed constexpr, was working despite of old GCC headers. std::begin/end broke static analysis builds because constexpr std::begin/end are new C++14 *library* features.
I replaced std::begin/end with raw pointers. Now green on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a37c3eab3bebdee2bdc70ec856bbf0069edca0ca
Comment 52•7 years ago
|
||
I'm on Ubuntu 16.04.4 LTS which comes with gcc 5.4.0. After updating to latest m-c, my build stopped working because it now requires gcc 6.1. But mach bootstrap doesn't upgrade my gcc. It downloads a clang tarball into ~/.mozbuild/toolchains but the build doesn't try to use it. What is the expected behaviour here? Am I supposed to manually upgrade gcc (seems like I would need to find the apt source for it) or am I supposed to update my mozconfig to use the clang from ~/.mozbuild?
Assignee | ||
Comment 53•7 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #52)
> I'm on Ubuntu 16.04.4 LTS which comes with gcc 5.4.0. After updating to
> latest m-c, my build stopped working because it now requires gcc 6.1. But
> mach bootstrap doesn't upgrade my gcc. It downloads a clang tarball into
> ~/.mozbuild/toolchains but the build doesn't try to use it. What is the
> expected behaviour here? Am I supposed to manually upgrade gcc (seems like I
> would need to find the apt source for it) or am I supposed to update my
> mozconfig to use the clang from ~/.mozbuild?
Please file a bug against bootstrap.
Assignee | ||
Comment 54•7 years ago
|
||
(In reply to Jeff Gilbert [:jgilbert] from comment #53)
> (In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #52)
> > I'm on Ubuntu 16.04.4 LTS which comes with gcc 5.4.0. After updating to
> > latest m-c, my build stopped working because it now requires gcc 6.1. But
> > mach bootstrap doesn't upgrade my gcc. It downloads a clang tarball into
> > ~/.mozbuild/toolchains but the build doesn't try to use it. What is the
> > expected behaviour here? Am I supposed to manually upgrade gcc (seems like I
> > would need to find the apt source for it) or am I supposed to update my
> > mozconfig to use the clang from ~/.mozbuild?
>
> Please file a bug against bootstrap.
Oops, it's already filed: bug 1451312
You need to log in
before you can comment on or make changes to this bug.
Description
•