Closed Bug 1444274 (gcc-6.1) Opened 7 years ago Closed 7 years ago

Require GCC 6.1

Categories

(Firefox Build System :: General, enhancement)

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

Tracking

(firefox61 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla61
Tracking Flags:
Tracking Status
firefox61 --- fixed

People

(Reporter: jgilbert, Assigned: jgilbert)

References

(Depends on 2 open bugs)

Details

Attachments

(1 file)

Bug 1444274 - Require GCC 6.1+. -
(deleted), text/x-review-board-request
glandium
: review+
Details 
Let's track our prereqs for moving to requiring Clang 5.0.
I heard work is being done here, but I can't find it with my searches. Do you know, :ryanvm?
Flags: needinfo?(ryanvm)
302 glandium.
Flags: needinfo?(ryanvm) → needinfo?(mh+mozilla)
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
302 sfink. When do we get the hazard builds off gcc 4.9?
Flags: needinfo?(mh+mozilla) → needinfo?(sphink)
Component: Platform Support → General
Product: Release Engineering → Firefox Build System
QA Contact: catlee
Depends on: 1444543
Note we'd probably require GCC 6 once we get builds off 4.9.
Flags: needinfo?(sphink)
Summary: Require GCC 5.0 → Require at least GCC 5.0+
Summary: Require at least GCC 5.0+ → Require at least GCC 6.0+
No longer blocks: 1443706
Depends on: 1445820
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)
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).
Summary: Require at least GCC 6.0+ → Require GCC 6.4
There hasn't been any 'nay' sentiment, even from the dev-platform post, so let's just try to move this forward.
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)
Assignee: nobody → jgilbert
Summary: Require GCC 6.4 → Require GCC 6.1
Comment on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -

https://reviewboard.mozilla.org/r/230454/#review236846
Attachment #8961585 - Flags: review?(jgilbert)
Attachment #8961585 - Flags: review?(jgilbert)
Flags: needinfo?(mh+mozilla)
Have you done a new try showing that this doesn't bust the hazard builds?
Flags: needinfo?(mh+mozilla) → needinfo?(jgilbert)
(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 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)
Depends on: 1449066
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)
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)
And again, if you want to avoid the version for GCC, you should do the same for the others.
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)
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)
Bug 1449066 is dealing with the hazard builds.
Flags: needinfo?(sphink)
Flags: needinfo?(mh+mozilla)
Awesome, thanks.
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 on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -

https://reviewboard.mozilla.org/r/230454/#review238128
Attachment #8961585 - Flags: review?(jgilbert)
Attachment #8961585 - Flags: review?(jgilbert)
Blocks: 1449094
Alias: gcc-6.1
@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 on attachment 8961585 [details]
Bug 1444274 - Require GCC 6.1+. -

https://reviewboard.mozilla.org/r/230454/#review238582
Attachment #8961585 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/f080a35baf6a
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox61: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
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)
(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)
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.
The interesting question is why those jobs aren't busted on central?
(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...
(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? :(
No longer blocks: 1449094
(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.
Blocks: 1451104
(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?
(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.
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
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?
(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.
(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
Depends on: 1451396
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: