Use latest commit of GoogleTest.

[antonsviridenko]

Fixes #1726 Using latest commit from the master branch is recommended by Google.

[antonsviridenko]

D:\a\rnp\rnp\src\tests\streams.cpp(403,42): error C7555: use of designated initializers requires at least '/std:c++20' [D:\a\rnp\rnp\build\src\tests\rnp_tests.vcxproj]

What C++ standards do we support in our codebase?

[codecov[bot]]

Codecov Report

Merging #1885 (90f349d) into master (48eaa1d) will increase coverage by 0.01%. The diff coverage is n/a.

:exclamation: Current head 90f349d differs from pull request most recent head ed88de3. Consider uploading reports for the commit ed88de3 to get more accurate results

@@            Coverage Diff             @@
##           master    #1885      +/-   ##
==========================================
+ Coverage   81.69%   81.70%   +0.01%     
==========================================
  Files         140      138       -2     
  Lines       28923    28944      +21     
==========================================
+ Hits        23628    23650      +22     
+ Misses       5295     5294       -1     

Impacted Files	Coverage Δ
src/tests/streams.cpp	93.25% <0.00%> (ø)
src/tests/load-g10.cpp	96.29% <0.00%> (ø)
src/tests/key-protect.cpp	92.53% <0.00%> (ø)
src/tests/key-add-userid.cpp	100.00% <0.00%> (ø)
src/lib/pass-provider.h
src/lib/key-provider.h
src/tests/key-validate.cpp	99.54% <0.00%> (+<0.01%)	:arrow_up:
src/tests/key-store-search.cpp	97.40% <0.00%> (+0.03%)	:arrow_up:
src/lib/rnp.cpp	79.84% <0.00%> (+0.04%)	:arrow_up:
src/librepgp/stream-parse.cpp	76.46% <0.00%> (+0.05%)	:arrow_up:
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[ronaldtse]

What C++ standards do we support in our codebase?

We probably should define that!

[ni4]

Those are already defined, just didn't get reflection in development docs:

set(CMAKE_C_STANDARD 99)
set(CMAKE_CXX_STANDARD 11)

[ni4]

...however latest GTest main commit seems to require C++14: The 1.12.x branch will be the last to support C++11. Future releases will require at least C++14.. Could we use C++14 while building gtest from sources?

[antonsviridenko]

Ok, let's try to use v1.12.0 then, looks like idea of using latest commit was bit optimistic :)

[antonsviridenko]

v1.12.0 fails on Centos 7 tests again :( Looks like GCC in Centos 7 is too old

 /__w/rnp/rnp/builds/rnp-build/_deps/googletest-src/googletest/include/gtest/gtest-matchers.h:436:3: error: body of constexpr function ‘static constexpr bool testing::internal::MatcherBase<T>::IsInlined() [with M = testing::internal::EqMatcher<std::basic_string<char> >; T = std::basic_string<char>]’ not a return-statement
   }
   ^
/__w/rnp/rnp/builds/rnp-build/_deps/googletest-src/googletest/include/gtest/gtest-matchers.h: In instantiation of ‘void testing::internal::MatcherBase<T>::Init(M&&) [with M = testing::internal::EqMatcher<std::basic_string<char> >; T = std::basic_string<char>]’:
/__w/rnp/rnp/builds/rnp-build/_deps/googletest-src/googletest/include/gtest/gtest-matchers.h:318:28:   required from ‘testing::internal::MatcherBase<T>::MatcherBase(M&&) [with M = testing::internal::EqMatcher<std::basic_string<char> >; <template-parameter-2-2> = void; T = std::basic_string<char>]’
/__w/rnp/rnp/builds/rnp-build/_deps/googletest-src/googletest/include/gtest/gtest-matchers.h:565:62:   required from ‘testing::Matcher<std::basic_string<char> >::Matcher(M&&) [with M = testing::internal::EqMatcher<std::basic_string<char> >; <template-parameter-1-2> = void]’
/__w/rnp/rnp/builds/rnp-build/_deps/googletest-src/googletest/src/gtest-matchers.cc:56:61:   required from here

[ni4]

Yeah, it ships 4.8, which: GCC provides experimental support for the 2011 ISO C++ standard.

[ni4]

...as a solution we may use some CMake stuff to detect whether compiler supports C++14 (via compiler version check or via approaches like here: https://stackoverflow.com/questions/10984442/how-to-detect-c11-support-of-a-compiler-with-cmake )

[antonsviridenko]

Or set GTest version depending on GCC version detected...

[antonsviridenko]

Strange, Fedora 36 has GCC 12.1.1 but compiled GTest fine before...

-- The C compiler identification is GNU 12.1.1 -- The CXX compiler identification is GNU 12.1.1

[ni4]

Strange, Fedora 36 has GCC 12.1.1 but compiled GTest fine before...

Could it be that Fedora 36 runners used pre-built GTest from the Fedora packages?

[ronaldtse]

Or set GTest version depending on GCC version detected...

This is likely the better solution. However, we should consider whether we should drop support for CentOS7/RHEL7 as others are already doing so. Perhaps we should do that in Release 17.

[ni4]

@antonsviridenko Do you need some help with this? It appears to be showstopper for the Fedora 36 support, and should be included into the v0.16.1 release.

[antonsviridenko]

@ni4 Looks like it was a bug in GCC 11, to be more precise, -Wmaybe-uninitialized was enabled by default in GCC 11, but it gave too many false positives https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95006

[ni4]

@antonsviridenko Yeah, it seems so. Anyway, we'll need to workaround it. I'd suggest following solution:

• use latest release of GTest for most solutions.
• except old GCC (pre-C++14), use v1.11.0 of GTest Does this sound good?

[antonsviridenko]

yes, sounds good

[ni4]

@antonsviridenko Don't you mind me pushing fixes to this branch?

[antonsviridenko]

@ni4 no problem, go ahead

but looks like MSVC windows builds fail with

D:\a\rnp\rnp\src\tests\streams.cpp(403,42): error C7555: use of designated initializers requires at least '/std:c++20' [D:\a\rnp\rnp\build\src\tests\rnp_tests.vcxproj]

need to make special condition for MSVC compiler

[ni4]

@antonsviridenko It looks like something changed in latest CMake update in the runner: while MSVC version is the same, compared to 20220801 runner, cmake was updated, and now cl.exe is called with /std:c++14 parameter (why? as we set c++11). Will dig into it.

[ni4]

@antonsviridenko If you are interested in the details, the reason for this is the following commit in Googletest codebase: https://github.com/google/googletest/commit/131878ce9e18e3fcc5fdd690c93ee9d36f0a18d3 This option due to some CMake internals overrides top-level's add_compile_options("/std:c++latest") for the MSVC. And root reason for this is that designated initializers are part of the C++20 standard, however all used by CI compilers (starting from GCC 4.8) seem to have this as extension.

[ni4]

@antonsviridenko does this look good to you now? Cannot assign you as reviewer as you created this PR :)

[antonsviridenko]

yeah, looks good now

[ronaldtse]

Two approvals => merged. Thanks!