exiv2 0.27.x / C++98 incompatible with current googletest

[StefanBruens]

Googletest 1.10.0 requires C++11:

 In file included from /usr/include/c++/9/type_traits:35,
                  from /usr/include/gtest/gtest.h:59,
                  from /home/abuild/rpmbuild/BUILD/exiv2-0.27.2/unitTests/gtestwrapper.h:10,
                  from /home/abuild/rpmbuild/BUILD/exiv2-0.27.2/unitTests/mainTestRunner.cpp:1:
 /usr/include/c++/9/bits/c++0x_warning.h:32:2: error: #error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.
    32 | #error This file requires compiler and library support \

Changing the CMAKE_CXX_STANDARD from 98 to 11 allows to also build the test suite.

As far as I can see, this has no effect on the ABI, the only drawback of the change are a lot of auto_ptr deprecation warnings, iff the corresponding warning is enabled.

[clanmills]

@StefanBruens We use google test 1.8.0 #575

The Exiv2 v0.27 family of releases uses C++98. The Exiv2 v0.28 product in development uses C++11. I can't comment on the version of gtest used by v0.28 as I am not involved.

Exiv2 v0.27.3 is 93% complete. RC1 is on-track to ship as scheduled on 2020-04-30. I don't intend to change the toolset at this stage of the project. In fact, I built gtest 1.8 this morning on FreeBSD, NetBSD and SunOS as part UNIX support which will be in the release. #1018 (comment)

I'm going to close this issue as I don't intend any action for the 0.27 family of products. I'm happy to continue the discussion if you wish.

[StefanBruens]

Sorry, but I can not understand this mindset ...

Distributions want to update googletest. Other projects start to depend on googletest 1.10.x, so distributions even have to update googletest.

By forcing C++98 (needlessly, ...), you block any googletest updates in the distributions.

I don't say you should require C++11 for exiv2 0.27.x, but you should allow it. 0.27.x compiles and works fine when compiled with C++11.

As long as there is no release which allows C++11, this issue should not be closed. Close it when you have a solution.

[clanmills]

@StefanBruens I'm astonished by your words.

What exactly do you want changed? Is it possible to pass this on the cmake command. Something such as:

cmake .. -DCMAKE_CXX_STANDARD=11

[clanmills]

I've invited @piponazo to join this discussion as he is Team Exiv2's CMake Wizard.

[clanmills]

I've looked further into this. A small change in cmake/mainSetup.cmake enables C++11. Exiv2 builds (with thousands of deprecated warnings, as you mentioned). It built successfully and passes the test suite on macOS 10.15.4 and Xcode 11.4.1 (most recent versions)

As a team, we decided to "modernise" the code for v0.28 and stick with the C++98 for Exiv2 v0.27 (and the subsequent dot releases). The KDE people asked us to adopt C++11 and to avoid the temptation to go for C++14 or C++17. I believe the v0.28 development branch (master) builds in C++11 without warnings.

From your words, I can see you are very displeased by our position. I have explained what we are doing about C++11 and our reasons.

I could add a new option such as -DEXIV2_BUILD_WITH_CXX=11, however that would be equivalent to saying "Exiv2 v0.27 supports building with C++11, 14, 17" and I don't have the testing band-width to open up such a commitment. Remember, this is open source, you are welcome to change your copy and compile it with all manner of tools.

502 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance $ git diff
diff --git a/cmake/mainSetup.cmake b/cmake/mainSetup.cmake
index a4d983d4..edb82ba7 100644
--- a/cmake/mainSetup.cmake
+++ b/cmake/mainSetup.cmake
@@ -18,7 +18,7 @@ if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
     set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)
 endif()
 
-set(CMAKE_CXX_STANDARD 98)
+set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS ON)
 
503 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance $ 

[clanmills]

I've taken a look at this on macOS and ubuntu 18.04. When I build/install googletest 1.10, I'm unable to compile the Exiv2 unit_tests for the reason stated by @StefanBruens. The gtest headers require C++11.

I'm unwilling to change the tool-chain for v0.27.3 so close to RC1. One way to address is to build/install gtest 1.8.0 into /usr/gtest18 then add some directories to the header and library search paths.

On macOS, unit_tests appear to be statically linked:

617 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance/build $ otool -L bin/unit_tests 
bin/unit_tests:
	/usr/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	/usr/local/lib/libexpat.1.dylib (compatibility version 8.0.0, current version 8.0.0)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 902.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)
618 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance/build $ 

On Linux, unit_test is linked to a shared gtest library.

617 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance/build $ otool -L bin/unit_tests 
bin/unit_tests:
	/usr/local/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	/usr/local/lib/libexpat.1.dylib (compatibility version 8.0.0, current version 8.0.0)
	/usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 902.1.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1281.100.1)
618 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance/build $ 618 rmills@rmillsmbp:~/gnu/github/exiv2/0.27-maintenance/build $ 

On Linux, gtest is a shared library. I've manually removed the library, and rebuilt/installed gtest 1.8.0 as a static library. bin/unit_test is statically liked to gtest:

rmills@rmillsmbp-ubuntu:~/gnu/github/exiv2/0.27-maintenance/build$ ldd bin/unit_tests 
	linux-vdso.so.1 (0x00007ffe703c5000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fc6a8b56000)
	libexpat.so.1 => /lib/x86_64-linux-gnu/libexpat.so.1 (0x00007fc6a8924000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fc6a8705000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fc6a837c000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fc6a7fde000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fc6a7bed000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fc6a79d5000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fc6a93ed000)
rmills@rmillsmbp-ubuntu:~/gnu/github/exiv2/0.27-maintenance/build$ 

Next I will investigate how to tell CMake to search there for headers/libraries.

[clanmills]

Here's how to do it:

1. Use -DCMAKE_INSTALL_PREFIX=/usr/gtest18 -DBUILD_SHARED_LIBS=Off when you build/install gtest 1.8.0
2. Use -DCMAKE_PREFIX_PATH=/usr/gtest18 when you build exiv2 to use the gtest 1.8 static libraries.

$ cd rmills@rmillsmbp-ubuntu:~/gnu/gtest/googletest-release-1.8.0/build
$ cmake .. -DCMAKE_INSTALL_PREFIX=/usr/gtest18 -DBUILD_SHARED_LIBS=Off
$ make
-- Installing: /usr/gtest18/include/gtest/gtest-typed-test.h
$ cd /usr/local/lib
dir *gtest*
libgtest.a  libgtest_main.a
$ sudo rm -rf *gtest*
$ cd -/home/rmills/gnu/github/exiv2/0.27-maintenance
$ rm -rf build ; mkdir build ; cd build
$ cmake .. -DEXIV2_BUILD_UNIT_TESTS=On
-- The CXX compiler identification is GNU 7.5.0
.....
-- Performing Test COMPILER_HAS_DEPRECATED_ATTR - Success
CMake Error at /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:137 (message):
  Could NOT find GTest (missing: GTEST_LIBRARY GTEST_MAIN_LIBRARY)
Call Stack (most recent call first):
  /usr/share/cmake-3.10/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake-3.10/Modules/FindGTest.cmake:196 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  unitTests/CMakeLists.txt:1 (find_package)


-- Configuring incomplete, errors occurred!
See also "/home/rmills/gnu/github/exiv2/0.27-maintenance/b/CMakeFiles/CMakeOutput.log".
See also "/home/rmills/gnu/github/exiv2/0.27-maintenance/b/CMakeFiles/CMakeError.log".
$ cmake .. -DCMAKE_PREFIX_PATH=/usr/gtest18 -DEXIV2_BUILD_UNIT_TESTS=On
....
$ make
...
$ bin/unit_tests 2> /dev/null | tail -2 
[  PASSED  ] 126 tests.
Tests finished with return value: 0
$ ldd bin/unit_tests 
	linux-vdso.so.1 (0x00007fff817e9000)
	libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fa30468d000)
	libexpat.so.1 => /lib/x86_64-linux-gnu/libexpat.so.1 (0x00007fa30445b000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa30423c000)
	libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fa303eb3000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa303b15000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa303724000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa30350c000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fa304f24000)
$ 

[piponazo]

Hi all,

@StefanBruens when you say:

Distributions want to update googletest. Other projects start to depend on googletest 1.10.x, so distributions even have to update googletest

I do not think it is a valid point for asking as to change the current configuration in Exiv2. The dependency on GTest from our side is optional and disabled by default, and it is only needed to compile the Unit Tests:

option( EXIV2_BUILD_UNIT_TESTS        "Build unit tests"                                      OFF )

Any distribution could satisfy the mandatory dependencies on Exiv2 to build only the Exiv2 library and main application. Another story would be to be able to collaborate in the project ... but if you really want to collaborate in the project, you have many alternatives to install the dependencies. With conan, you could bring gtest 1.8 into your local user space, without affecting your gtest 1.10 installation in a modern distribution.

[clanmills]

@piponazo What a clever person you are. Of course, with conan you can specify that you wish to link gtest 1.8.0 static. Two questions:

1. Can you edit your post to say how to do that?
2. Can we make the default for v0.27.3?

@StefanBruens said:

Sorry, but I can not understand this mindset ...

Sadly, he has not explained what he really wants.

[piponazo]

After re-reading the post, I think that I see @StefanBruens 's point about the usage of CMAKE_CXX_STANDARD.

What we can do about that is the following:

1. Remove the usage of CMAKE_CXX_STANDARD which forces to use C++98 for all the targets in the project (library, applications and unit tests).
2. Use target_compile_features(exiv2lib PUBLIC cxx_std_98) for the library so that we clearly indicate that the library is using that standard and we propagate this information to library consumers.

It could be that the 2nd point could create conflicts with people trying to consume the project from other project using higher c++ standards, and therefore we would only need to implement the 1st point.

I'll try to spend some time on this topic this evening and solve the situation. @StefanBruens if you have some more insight about the problems you see with the current solution and possible ways to deal with it, please let us know.

[clanmills]

No response from @StefanBruens. I'm going to close this issue.

[clanmills]

@StefanBruens A fix has been put into the 0.27-maintenance branch and will ship in Exiv2 v0.27.3 on 2020-06-30. #1176

Exiv2 v0.27.3 RC1 will be published this week on 2020-04-30 and will include this change. For information about Exiv2 v0.27.3 see: #1018 (comment)

Both @piponazo and I would be very pleased to have your feedback that we have successfully solved this issue to your satisfaction.

[clanmills]

I've submitted a PR #1191 to prevent 2305 warnings message from GCC concerning unique_ptr.

This issue has been a total time-waster created by Mr Bruens who, having very rudely opened this issue, hasn't had the courtesy to engage with Team Exiv2. As soon as this appeared, I though "irrelevant". Exiv2 v0.27.3 is built with C++98 and using gtest 1.8.0 for unit testing.

[StefanBruens]

If you think adding workarounds on every single downstream is a good solution, so be it.

I have opened this issue to make you aware of a real live problem. Do you think it is a good idea distributions ship totally untested packages, because unit tests can't be compiled? Distribution packages use different compilers, different libraries, etc, each a possible source of regressions you are not aware of.

[StefanBruens]

Hi all,

@StefanBruens when you say:

Distributions want to update googletest. Other projects start to depend on googletest 1.10.x, so distributions even have to update googletest

I do not think it is a valid point for asking as to change the current configuration in Exiv2. The dependency on GTest from our side is optional and disabled by default, and it is only needed to compile the Unit Tests:

Because shipping untested software is always a good idea ... If a current GCC starts to optimize the exiv2 code in a different way, this may expose bugs. The default for the unit tests should be enabled, not disabled.

[clanmills]

@StefanBruens I have tried very hard to understand your request and worked to give you solutions.

Regrettably, you haven't bothered to cooperate with me. For sure, you haven't given feedback. And haven't bothered to acknowledge the effort that has been put in on your behalf.

Why don't you ask Google to ensure that GTest 1.10.x can build with C++98?

[StefanBruens]

You have not managed to release a version of exiv2 almost 9 years after the C++11 standards approval, so the right thing for Google is to move GTest back to C++98 ...

I have stated from the very beginning what I want - a released exiv2 version which is compatible with contemporary compilers and frameworks, without adding local hacks and workarounds, and which has been tested on different architectures and with different compilers and compiler versions - which is implicit when one targets a variety of distributions, and not just one golden sample.

Stop blaming me when you don't understand the problem, but consider it exists nevertheless. And show some patience when you don't get an answer immediately, people may be busy doing other things. Everytime I looked at the issue, you had already closed it - why should I even bother with an issue you deny it even exists? During the last 14 days, the issue had been in the Open state for 3 days.