Fix build with GoogleTest 1.14.0

[andreasstieger]

GoogleTest 1.14.0 requires C++14

Fixes:

[   26s] /usr/include/gtest/internal/gtest-port.h:279:2: error: #error C++ versions less than C++14 are not supported.
[   26s]   279 | #error C++ versions less than C++14 are not supported.
[   26s]       |  ^~~~~

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.29%. Comparing base (86c0cfd) to head (8403ce0).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2209      +/-   ##
==========================================
- Coverage   77.32%   77.29%   -0.03%     
==========================================
  Files         194      194              
  Lines       37742    37746       +4     
==========================================
- Hits        29184    29177       -7     
- Misses       8558     8569      +11     

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

[ni4]

@andreasstieger Thanks for opening this issue! Would not it be enough to build only tests with C++14, or that would not work? Also we should update our CI to use latest Google tests...

[andreasstieger]

I think it is more than fair to apply a 10 year old language standard globally. Doing so certainly does not break anything in the CI.

The more important question is why the CI did not show this? Some builders seem to download and build googletest, the tag is from 1.14.0 and this was not triggered?

[ni4]

I think it is more than fair to apply a 10 year old language standard globally. Doing so certainly does not break anything in the CI.

I believe it is of because Centos 7 (EOLing this June, 2024) has only GCC 4.8.5, which doesn't have 100% support of C++14, but could be wrong. Anyway, we didn't have a major need for any features from C++14 yet.

The more important question is why the CI did not show this? Some builders seem to download and build googletest, the tag is from 1.14.0 and this was not triggered?

That's surprising for me as well, I'll check it out. From the CMakeLists it should use HEAD in almost all cases (except Centos 7/GCC 4.8.5)

[ni4]

@andreasstieger Got it, in CI we download Google Test and build from sources, which sets standard to c++14. However while building with system GoogleTest this doesn't work.

@ronaldtse Should we set standard to c++14 with this release, or wait for the June and v0.18.0 release?
cc @maxirmx

[ni4]

@andreasstieger Could you please specify on which system/which CMake call did you use? I tried to reproduce on clean Fedora 40 and system-installed gtest, but it still sets standard to 14 automatically (and I was unable to track down where it comes from).

[andreasstieger]

cmake invocation:

[   13s] + cd rnp-v0.17.0
[   13s] + find . -type f -name CMakeLists.txt -exec sed -i -re '/^[[:blank:]]*[sS][eE][tT][[:blank:]]*\([[:blank:]]*(CMAKE_BUILD_TYPE|CMAKE_COLOR_MAKEFILE|CMAKE_INSTALL_PREFIX|CMAKE_VERBOSE_MAKEFILE).*\)/{s/^/#IGNORE /}' '{}' +
[   13s] + mkdir -p build
[   13s] + cd build
[   13s] + /usr/bin/cmake /home/abuild/rpmbuild/BUILD/rnp-v0.17.0/. '-GUnix Makefiles' -DCMAKE_INSTALL_PREFIX:PATH=/usr -DCMAKE_INSTALL_BINDIR:PATH=bin -DCMAKE_INSTALL_SBINDIR:PATH=sbin -DCMAKE_INSTALL_LIBEXECDIR:PATH=libexec -DCMAKE_INSTALL_SYSCONFDIR:PATH=etc -DCMAKE_INSTALL_SHAREDSTATEDIR:PATH=/var/lib -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=var -DCMAKE_INSTALL_RUNSTATEDIR:PATH=run -DCMAKE_INSTALL_LIBDIR:PATH=lib64 -DCMAKE_INSTALL_INCLUDEDIR:PATH=include -DCMAKE_INSTALL_DATAROOTDIR:PATH=share -DCMAKE_INSTALL_DOCDIR:PATH=share/doc/packages/rnp -DCMAKE_INSTALL_MANDIR:PATH=share/man -DCMAKE_INSTALL_INFODIR:PATH=share/info -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DCMAKE_BUILD_TYPE=RelWithDebInfo '-DCMAKE_C_FLAGS=-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g' '-DCMAKE_CXX_FLAGS=-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g' '-DCMAKE_Fortran_FLAGS=-O2 -Wall -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g' '-DCMAKE_EXE_LINKER_FLAGS=-flto=auto -Wl,--as-needed -Wl,-z,now' '-DCMAKE_MODULE_LINKER_FLAGS=-flto=auto -Wl,--as-needed' '-DCMAKE_SHARED_LINKER_FLAGS=-flto=auto -Wl,--as-needed -Wl,-z,now' -DLIB_SUFFIX=64 -DCMAKE_VERBOSE_MAKEFILE:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=ON -DBUILD_STATIC_LIBS:BOOL=OFF -DCMAKE_COLOR_MAKEFILE:BOOL=OFF -DCMAKE_INSTALL_DO_STRIP:BOOL=OFF -DCMAKE_MODULES_INSTALL_DIR=/usr/lib64/cmake/rnp -DDOWNLOAD_GTEST:BOOL=OFF -DBUILD_TESTING:BOOL=ON -DSYSTEM_LIBSEXPP:BOOL=ON

Full build:

https://build.opensuse.org/package/show/security:privacy/rnp
https://build.opensuse.org/package/live_build_log/security:privacy/rnp/openSUSE_Tumbleweed/x86_64

[ronaldtse]

@ronaldtse Should we set standard to c++14 with this release, or wait for the June and v0.18.0 release? cc @maxirmx

I think it's safer to set C++14 globally with 0.18.0. I hope this doesn't block 0.17.1.

[andreasstieger]

I do not see this related to 0.17.x or any RNP release at all. Instead this proposed change is related to a googletest v1.14.0 release. So this should not block anything, and can be perfectly worked around by distribution package maintainers, if they want to run tests during their builds. Still a bit concerned why CI did not flag this.

[ni4]

I do not see this related to 0.17.x or any RNP release at all. Instead this proposed change is related to a googletest v1.14.0 release. So this should not block anything, and can be perfectly worked around by distribution package maintainers, if they want to run tests during their builds. Still a bit concerned why CI did not flag this.

Thanks for the input. Finally was able to reproduce the problem. Looks like we introduced some changes since v0.17.0 which as a side effect enable c++14 during rnp_tests building. Would investigate further, to make sure that v0.17.1 compiles/builds as expected.

[ni4]

Closing this as issues was fixed via #2212 . Thanks again for reporting!