Bug 199108 – [cmake] Switch to built-in handling of C++ standard instead of hardcoding -std=c++17

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Konstantin Tokarev]

Rationale:

1. It provides an abstraction over exact flags of particular compilers - we just specify required version of C++ standard
2. External libraries (like Qt used in WPEQt) may require particular C++ standard or even particular C++ features to be availabe (e.g. Qt requires INTERFACE_COMPILE_FEATURES cxx_decltype, which causes cmake to add -std=gnu++11 unless proper CXX_STANDARD is defined)

[Konstantin Tokarev]

Created attachment 372630 [details]
Patch

[Konstantin Tokarev]

It seems like CXX_STANDARD 17 is nonly available in CMake >= 3.8 :(

[Michael Catanzaro]

You can use CMake version checks to do it this way with new CMake and do it manually in old CMake, and trust us to remove the old way next time CMake version is bumped.

Or you can give up and close this.

[Konstantin Tokarev]

Debian Stretch is shipping 3.7 in their main repository[1], however up-to-date  versions of WebKitGTK are only available in backports[2], where cmake 3.13 is availble. Ubuntu 18.04 ships cmake 3.10. I wonder if we could raise cmake requirement to 3.8 right now.

OTOH, this particular change is not that important, I can even keep it in my fork

[1] https://packages.debian.org/stretch/cmake
[2] https://packages.debian.org/stretch-backports/libwebkit2gtk-4.0-37
[3] https://packages.debian.org/stretch-backports/cmake
[4] https://packages.ubuntu.com/bionic/cmake

[Konstantin Tokarev]

Created attachment 372679 [details]
Patch

[Konstantin Tokarev]

Comment on attachment 372679 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=372679&action=review

> Source/WebKit/PlatformWPE.cmake:-400
> -        CXX_STANDARD 17

This code is removed unconditionally because will result in error with cmake < 3.8

[Fujii Hironori]

(In reply to Konstantin Tokarev from comment #4)
> Debian Stretch is shipping 3.7 in their main repository[1], however
> up-to-date  versions of WebKitGTK are only available in backports[2], where
> cmake 3.13 is availble. Ubuntu 18.04 ships cmake 3.10. I wonder if we could
> raise cmake requirement to 3.8 right now.

Supporting Debian 9 (stretch) has been dropped.
https://lists.webkit.org/pipermail/webkit-dev/2019-May/030619.html
https://lists.webkit.org/pipermail/webkit-dev/2019-May/030625.html

We can bump cmake_minimum_required version to 3.10 now.

[Michael Catanzaro]

(In reply to Fujii Hironori from comment #7)
> We can bump cmake_minimum_required version to 3.10 now.

Yeah that's true. Go for it. If any EWS complain, go for it anyway and we'll get them fixed.

[Konstantin Tokarev]

It's already known that EWS will compalin - if it had cmake >= 3.8 it wouldn't complain about original patch

[Don Olmstead]

This is good. As long as Michael is happy I’m fine with the contents. We use 3.14 cmake so our platforms have no problem with this.

[Konstantin Tokarev]

Created attachment 372938 [details]
Patch

[EWS Watchlist]

Comment on attachment 372938 [details]
Patch

Attachment 372938 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12583556

New failing tests:
svg/text/textpath-reference-update.html

[EWS Watchlist]

Created attachment 372956 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Don Olmstead]

Comment on attachment 372938 [details]
Patch

r=me but you forgot to remove the following in OptionsMSVC.cmake

# Enable C++17
# https://docs.microsoft.com/en-us/cpp/build/reference/std-specify-language-standard-version
add_compile_options(/std:c++17)

[Konstantin Tokarev]

Committed r246873: <https://trac.webkit.org/changeset/246873>