Closed Bug 1077549 Opened 5 years ago Closed 5 years ago

Bump our minimum supported GCC version up from 4.4 to 4.6

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla37

People

(Reporter: ted, Assigned: tbsaunde)

References

Details

Attachments

(2 files)

Once bug 1056337 gets finished we don't have to support GCC 4.4 any longer. We could bump our minimum supported GCC version up as far as 4.7. glandium suggests we might want to only go to 4.6 right now, as that's what Ubuntu 12.04 (and possibly other distros) shipped with.
Blocks: 1056407
Bumping the minimum gcc to 4.7 will give us the following C++11 features on top of 4.6:

 * member initializers
 * template aliases
 * delegated constructors
 * override and final as C++ keywords (we already support them as MOZ_OVERRIDE and MOZ_FINAL)

Given that the override and final keywords are not crucial to us, and the other 3 features are nice to have but I don't think super important, I think glandium's suggestion to bump to only 4.6 is the right thing to do.
It's worth noting that the member initializers, template aliases, and delegated constructors also require MSVC 2013. When we can bump up to MSVC 2013, bumping to gcc 4.7 makes a lot of sense as well.

gcc 4.6 is the minimum I'll countenance--that's when we get real nullptr support, and our nullptr emulation causes minor issues in MFBT land. We also get forward-declared enums in 4.6 as well as range-based for.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #0)
> glandium suggests we might want to only go to 4.6 right now, as that's what Ubuntu
> 12.04 (and possibly other distros) shipped with.

Especially, distros don't upgrade the compiler they use to build packages, so bumping to something they don't have in a LTS essentially means they can't update their Firefox package.
Sounds good. We can talk about bumping to 4.7 when we talk about bumping our minimum MSVC to 2013 since that will actually let us use the rest of these C++ features.
Summary: Bump our minimum supported GCC version up from 4.4 → Bump our minimum supported GCC version up from 4.4 to 4.6
FWIW, Chrome is actually much more aggressive, and distros needing to build it for security updates may be forced to provide newer toolchains in updates to their stable releases.
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=763278
(In reply to Mike Hommey [:glandium] from comment #5)
> FWIW, Chrome is actually much more aggressive, and distros needing to build
> it for security updates may be forced to provide newer toolchains in updates
> to their stable releases.
> See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=763278

Interesting.  I'd be curious to know if they actually get into any trouble in terms of distro adoption, and if not, to see if we can do the same!
FWIW, the Firefox for Android build machine currently uses gcc 4.7 (bug 899405), but could be updated to use gcc 4.8 (bug 900797) with a little work.
Depends on: 899405
See Also: → 1084056
Would this also update the 'B2G Desktop Linux' builds? I.e. can I mark bug 1075183 a duplicate, or do we still need a separate bug for porting those targets to tooltool?
(In reply to Ralph Giles (:rillian) from comment #8)
> Would this also update the 'B2G Desktop Linux' builds? I.e. can I mark bug
> 1075183 a duplicate, or do we still need a separate bug for porting those
> targets to tooltool?

No, those are different things.
No longer depends on: 1090632
That is, this bug is about making configure barf when building with gcc < 4.6.
As we have bumped to MSVC 2013, I guess we probably can reconsider bumping to 4.7 directly now?
It seems that Ubuntu 12.04 and Debian Wheezy stop upgrading Chromium at 37.0, ok...

But Ubuntu 12.04 LTS is supported until 2017, will we stay on GCC 4.6 until then?
(In reply to Xidorn Quan [:xidorn] (UTC+11) from comment #13)
> It seems that Ubuntu 12.04 and Debian Wheezy stop upgrading Chromium at
> 37.0, ok...
> 
> But Ubuntu 12.04 LTS is supported until 2017, will we stay on GCC 4.6 until
> then?

Lots of people are eager to see the minimum version of GCC increased even past 4.7. But, this bug isn't the place to discuss that. I suggest starting a thread on in the mozilla.dev.platform mailing list.

Good work everyvody!
Attachment #8546300 - Flags: review?(mh+mozilla)
Comment on attachment 8546300 [details] [diff] [review]
only support gcc 4.6+

Review of attachment 8546300 [details] [diff] [review]:
-----------------------------------------------------------------

Please cleanup b2g/confvars.sh too.
Attachment #8546300 - Flags: review?(mh+mozilla) → review+
Attachment #8546908 - Flags: review?(mh+mozilla)
https://hg.mozilla.org/mozilla-central/rev/286e1f883fdb
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
This should have been left open for the second patch.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8546908 [details] [diff] [review]
remove useless gcc version checks

Review of attachment 8546908 [details] [diff] [review]:
-----------------------------------------------------------------

::: configure.in
@@ +2625,1 @@
>        VISIBILITY_FLAGS='-I$(DIST)/system_wrappers -include $(MOZILLA_DIR)/config/gcc_hidden_dso_handle.h'

Note, we could make a case for dropping support for 4.6 on b2g and android.

::: mfbt/Compiler.h
@@ -22,5 @@
>      */
>  #  define MOZ_GCC_VERSION_AT_LEAST(major, minor, patchlevel)          \
>       ((__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__) \
>        >= ((major) * 10000 + (minor) * 100 + (patchlevel)))
> -#  if !MOZ_GCC_VERSION_AT_LEAST(4, 4, 0)

Keep it and make it 4.6.

::: mfbt/tests/TestTypedEnum.cpp
@@ +27,5 @@
>  #    if __has_extension(is_literal) && __has_include(<type_traits>)
>  #      define MOZ_HAVE_IS_LITERAL
>  #    endif
>  #  elif defined(__GNUC__)
>  #    if defined(__GXX_EXPERIMENTAL_CXX0X__)

note this is under a #if __cplusplus >= 201103L, so this is more or less implied to be true. Which essentially means this can become
#  elif defined(__GNUC__) || defined(_MSC_VER)
#    define MOZ_HAVE_IS_LITERAL
#  endif
Attachment #8546908 - Flags: review?(mh+mozilla) → review+
> ::: configure.in
> @@ +2625,1 @@
> >        VISIBILITY_FLAGS='-I$(DIST)/system_wrappers -include $(MOZILLA_DIR)/config/gcc_hidden_dso_handle.h'
> 
> Note, we could make a case for dropping support for 4.6 on b2g and android.

Unless there's other stuff that'd help with I doubt I'll bother, but I don't see much point in supporting building for android with 4.6 either.

> ::: mfbt/tests/TestTypedEnum.cpp
> @@ +27,5 @@
> >  #    if __has_extension(is_literal) && __has_include(<type_traits>)
> >  #      define MOZ_HAVE_IS_LITERAL
> >  #    endif
> >  #  elif defined(__GNUC__)
> >  #    if defined(__GXX_EXPERIMENTAL_CXX0X__)
> 
> note this is under a #if __cplusplus >= 201103L, so this is more or less
> implied to be true. Which essentially means this can become
> #  elif defined(__GNUC__) || defined(_MSC_VER)
> #    define MOZ_HAVE_IS_LITERAL
> #  endif

true, I doubt that's ever compiled with __cplusplus <, and I wonder if we care about clang without it, but work for another day.
https://hg.mozilla.org/mozilla-central/rev/cfe66af4d3c8
Status: REOPENED → RESOLVED
Closed: 5 years ago5 years ago
Resolution: --- → FIXED
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.