Require minimim GCC version (i.e. GCC 13)

[mikekasprzak]

Context

I ran into issues trying to build Mir on Pop!_OS, which is currently still based on Ubuntu 22.04 LTS. The reason was twofold:

• Ubuntu 22.04 LTS ships with GCC 11
• Ubuntu 22.04 LTS ships with an incompatible version of google-mock (googletest), possibly other libraries

For this initial PR I'll only be focused on the GCC issue.

Details

Cmake has a flag CMAKE_CXX_STANDARD that is used to specify the required C++ specification, but incomplete GCC versions will still pass that test even if they don't fully conform to the specification. For example, you can enable C++20 and C++23 support in GCC 11 with g++ -std=c++20 and g++ -std=c++23 respectfully, even if though they aren't feature complete.

The problem I ran into was that GCC 11 was missing certain Standard Library headers, specifically <format>. From what I can gather from the GCC changelog, GCC 13 was the first version that included std::format support.

My workaround

I eventually had success by adding some PPA repositories with a newer GCC and newer library versions.

• GCC 13; https://launchpad.net/~ubuntu-toolchain-r/+archive/ubuntu/ppa
• googletest and other libraries: https://launchpad.net/~savoury1/+archive/ubuntu/build-tools

NOTE: I don't specifically recommend these, I'm just sharing how I got it working.

Once GCC 13 was installed, I configured update-alternatives so I could easily switch GCC and G++ versions with sudo update-alternatives --config gcc.

For testing I installed GCC 9, GCC 11, and GCC 13.

• GCC 9 has no support for -std=c++20
• GCC 11 supports both -std=c++20 and -std=c++23, but is missing key standard libraries
• GCC 13 works

Solution

I added a "minimum GXX version" constant and a check to the CMakeLists.txt file that halts the build script if you're not running at least GCC 13. The check is only done if we know for sure that compiler is GCC (i.e. GNU to cmake).

Sample output:

mike@DevL14:~/Work/mir$ cmake -B build5
-- Could NOT find Lcov (missing: LCOV_EXECUTABLE GENHTML_EXECUTABLE) 
-- Could NOT find gcovr (missing: GCOVR_EXECUTABLE) 
CMake Error at CMakeLists.txt:67 (message):
  Insufficent gcc version.  Found 11.4.0, expected >= 13.0


-- Configuring incomplete, errors occurred!
mike@DevL14:~/Work/mir$ 

Other options explored

Originally I wanted to see if I could impose a requirement in debian/control for a minimum GCC version, but that didn't seem like the best idea as you might want to use Clang instead.

Future work

It's very possible that other compilers like Clang and MSVC may have similar discrepancies (i.e. the C++20 standard can be enabled even if it's not fully supported).

Also, the minimum google-mock version in debian/control should be updated. To what version requires a bit more research.

Feedback is welcome. If I missed a contributions guide or reference, let me know and I'll update this accordingly.

[AlanGriffiths]

I ran into issues trying to build Mir on Pop!_OS, which is currently still based on Ubuntu 22.04 LTS.

The most recent release of Mir (2.17.0) builds on 22.04, it is only the current development main branch that uses language features that go beyond those supported in g++-11.2. Depending on your needs you could checkout/branch the v2.17.0 tag or use the release tarball.

[mikekasprzak]

I can already see that a pair of the check scripts have failed, but that seems to be because they were running GCC 11 (possibly Ubuntu 22.04).

[mikekasprzak]

I ran into issues trying to build Mir on Pop!_OS, which is currently still based on Ubuntu 22.04 LTS.

The most recent release of Mir (2.17.0) builds on 22.04, it is only the current development main branch that uses language features that go beyond those supported in g++-11.2. Depending on your needs you could checkout/branch the v2.17.0 tag or use the release tarball.

Ah, that's good to know! 😅 It hadn't occurred to me that 2.17.0 wouldn't need GCC 13. 🤦

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.35%. Comparing base (96738e0) to head (c295beb).
Report is 442 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3472   +/-   ##
=======================================
  Coverage   77.35%   77.35%           
=======================================
  Files        1076     1076           
  Lines       68922    68922           
=======================================
+ Hits        53317    53318    +1     
+ Misses      15605    15604    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mikekasprzak]

Pushed some potential fixes for the check failures. According to this:

actions/runner-images#9848

The ubuntu-latest runner will be switching to Ubuntu 24.04 sometime in August, so my changes to symbols-check.yaml would have fixed itself then, but the change to .readthedocs.yaml would have still broke as it was hardcoded to 22.04.

The error in the build appears to be the cmake version not liking the AND operator, so I broke it up into separate if's.

[AlanGriffiths]

While this has the potential to offer a better message when building with an old version of g++, I'm not sure that this is a good solution:

1. it doesn't address other compilers; and.
2. the potential benefit is easily lost by failing to update when the requirement changes

I'll wait and see what other opinions exist

[mikekasprzak]

Fair points. For what its worth, this doesn't negatively affect non-GCC compilers. Adding a Clang check is easy enough too, once a desirable Clang version is known, and any unknown compilers just wont see these messages.

On that note, if the goal is to make Ubuntu 24.04 the recommended build environment, best I can tell 24.04 ships with GCC 13 and Clang 18. I don't yet know if Mir builds on Clang 18, but in my quick test it doesn't build on Clang 14 (Ubuntu 22.04) or Clang 17 (a newer version I had handy). For reference Clang 14 dies right away attempting to build googletest, and Clang 17 gets about 50% done before choking on some std::clamp calls in Miral.

the potential benefit is easily lost by failing to update when the requirement changes

I don't think they are. As long as we're not talking about having to clean and re-run cmake after changing the tooling, you still need to decide what compilers to support. In an ideal world you could code against a C++ specification version, but in practice you need to concede to the C++ features supported by (in this case) GCC.

Ideally I'd chosen a subset of C++ language features supported by both GCC 13 and Clang 18. If this is already the case then awesome, I just don't have Clang 18 handy so I can't verify.

Tangentially I do wish there was a better way to do this than looking at compiler versions, especially since in some cases it looks like it's the standard libraries at fault. GCC 11 got to about 70% before it needed <format>.

[AlanGriffiths]

I don't think they are. As long as we're not talking about having to clean and re-run cmake after changing the tooling, you still need to decide what compilers to support. In an ideal world you could code against a C++ specification version, but in practice you need to concede to the C++ features supported by (in this case) GCC.

No, I'm talking about two years time when we switch our development base to 26.04. There's the potential of failing to update MINIMUM_GNUCXX_COMPILER_VERSION.

[mikekasprzak]

Gotcha. 👍

I just stumbled across a "compiler features" feature of cmake. On paper it sounds like this might work better, avoiding the need to specify specific compilers by release version.

https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html#manual:cmake-compile-features(7)

[Conan-Kudo]

Instead, why not use fmt so that we can support the current GCC versions? It provides an equivalent interface.

[RAOF]

So, I'm in favour of giving a warning when we suspect that the toolchain is too old. I don't think it should be a FATAL_ERROR, because people and distros sometimes backport features without bumping the version.

To make it a fatal error, I'd want these to be feature checks, rather than version checks.

But I'm ambivalent on feature checks, as they're (marginally) more effort to maintain and the benefit is somewhat better compile failure messages.

On the other hand, since the goal is just marginally better build failures, maybe we could take the feature checks and just update them when we remember or when someone else hits a missing check?

[AlanGriffiths]

I just stumbled across a "compiler features" feature of cmake.

That does look like a more robust approach, but it isn't immediately clear to me that it can check library features. But, if that isn't the case, checking library features (in this case "is __cpp_lib_format defined?") wouldn't be hard to implement.

However, it is not easy to know which features are used in code without actually building against a toolchain (or multiple toolchains) that doesn't support them. And even then, features arrive in bunches which make it easy to miss some. (Were we to attempt to "do it right" I think we'd create of more work than simply supporting those older toolchains directly.)

In practice, I suspect, feature checks are only going to be added when someone encounters a problem building on an older buildchain than we target. And by the time they report the problem they will have likely identified the cause and resolution.

I remain concerned about the maintenance burden for a small improvement in error reporting. So far as I recall, this is the first time in more than a decade of this project that this has been an issue for anyone, and I suspect that in a few months (after 24.04.1) it will fade in importance.

[mikekasprzak]

Ha, I'm glad that you're for the idea of checking __cpp_lib_format as I was about to propose exactly that. 😉

The hiccup of target_compiler_features() is that, as the name suggests, it only checks for the compiler features from the list here. Notably absent are current features like the "spaceship operator" (<=>), and ALL standard library features. The other issue with target_compile_features() is you must specify a single cmake target, meaning it would need to be duplicated for every library and executable generated by the build scripts, which frankly isn't viable. On the plus side, CMAKE_CXX_COMPILE_FEATURES does contain a semicolon separated string of all features detected by cmake, so we can get the same effect as target_compiler_features() on everything by checking against that.

For checking the standard library, check_cxx_symbol_exists() is almost exactly what we need, I'm just not particularly happy with the syntax. Usage looks like this.

include (CheckCXXSymbolExists)
check_cxx_symbol_exists(__cpp_lib_format "version" LIB_FORMAT_EXISTS)

Arg1 is the define/symbol to look for, arg2 the header file to check, and arg3 a variable to set equal to 0 or 1 if found. If we wanted to do something, say import fmt if std::format wasn't found, this would work. But if we wanted to die and alert the user that their standard library is missing something we need, this code gets unnecessarily verbose the more items we add.

include (CheckCXXSymbolExists)
check_cxx_symbol_exists(__cpp_lib_format "version" LIB_FORMAT_EXISTS)
if (NOT ${LIB_FORMAT_EXISTS})
  message(FATAL_ERROR "std::format isn't available")
endif()

Currently I'm experimenting with writing cmake macros to make writing this less ugly.

[AlanGriffiths]

I'm glad that you're for the idea of checking __cpp_lib_format

I'm not "for" that, just saying that that would be a way to test for the specific case you encountered. But the general case is problematic.

If anything, I'm against adding complexity that needs to be maintained for very little benefit.

[mikekasprzak]

I'm not "for" that

My apologies, poor choice of words.

If anything, I'm against adding complexity that needs to be maintained for very little benefit.

Fair enough. That said, choosing C++20 as a feature cap over say C++17 does invite this complexity due to the lag of compiler versions paired with supported Linux distros like Ubuntu 22.04 LTS. It's frustrating that a Rust developer only has to lag a few months, or a JavaScript developer 1-2 years, but that's the nature of how Linux is so closely tied to its compiler.

Still I agree. If maintaining this requires anything but the tiniest amount of effort, it becomes a burden. I've only ever met one other developer that didn't hate managing build scripts and tooling. When you're not coding you're not coding.

Again this could be avoided entirely if C++17 was declared the feature cap. Still, I think taking the effort to clarify what C++20 features are required would benefit all contributors. Even listing older features like constexpr and consteval would be a handy cue for features contributor should be using, as C++ has no shortage of ways to do the exact same thing. Keeping a written style guide up to date would more effort than adding a line to a required feature list, and saying where to look.

An example of what I think this should (could) look like.

# Required C++ features. Reference: https://en.cppreference.com/w/cpp/feature_test
require_cxx_feature(__cpp_constexpr)
require_cxx_feature(__cpp_consteval)
require_cxx_feature(__cpp_decltype_auto)
require_cxx_feature(__cpp_generic_lambdas)
...

# Required C++ standard libraries. Reference: https://en.cppreference.com/w/cpp/feature_test
require_cxx_standard_lib(__cpp_lib_format)
require_cxx_standard_lib(__cpp_lib_coroutine)
check_cxx_standard_lib(__cpp_lib_modules HAS_LIB_MODULES)
...

[mikekasprzak]

It turns out I was able to use the same syntax for both compiler features and library features. You can find a new separate PR over here: #3479.

[AlanGriffiths]

Closing as #3479 has more buy-in