Prevent linker warning about GTest::gtest#294
Prevent linker warning about GTest::gtest#294ClausKlein wants to merge 5 commits intobemanproject:mainfrom
Conversation
Fix duplicative GTest::gtest GTest::gtest_main dependency
ClausKlein
requested review from
bretbrownjr,
camio,
dietmarkuehl,
ednolan,
neatudarius,
steve-downey and
wusatosi
as code owners
camio
left a comment
camio
left a comment
There was a problem hiding this comment.
The BUILD_GMOCK is an implementation detail of some implementations of find_package(GTest). Since it is not part of the basic find_package(GTest) contract, it is subject to change and, consequently, I don't think we should use it.
The other part of this PR looks reasonable to me. The GTest API directly mentions gtest_main as an alternative to gtest as opposed to an addition.
There was a problem hiding this comment.
| set(BUILD_GMOCK OFF) |
tests/beman/exemplar/CMakeLists.txt
Outdated
There was a problem hiding this comment.
| set(BUILD_GMOCK OFF) |
There was a problem hiding this comment.
No, but we need more: set(INSTALL_GTEST OFF) # Disable GoogleTest installation
See #295 for more info
see too https://discourse.cmake.org/t/missing-warning-if-a-public-link-dependencies-is-not-to-be-installed/15475 |
| # NO! EXCLUDE_FROM_ALL | ||
| ) | ||
| set(INSTALL_GTEST OFF) # Disable GoogleTest installation | ||
| # NO! set(INSTALL_GTEST OFF) # Disable GoogleTest installation |
There was a problem hiding this comment.
This is wrong here, it is a general cmake module!
There was a problem hiding this comment.
concur gtest stuff doesn't below here. @nickelpro I assume you concur?
There was a problem hiding this comment.
I agree that it doesn't belong here, but it isn't clear to me where it does belong. We need to set this to off in order to avoid installing gtest as part of our installation package.
There was a problem hiding this comment.
The second one is incorrect, the first I believe to be correct.
Excluding from the all target will still require a target to be built if used, but it will not be part of the install step.
There was a problem hiding this comment.
Than your project can't be installed if you have public used dependencies!
There was a problem hiding this comment.
A project should not install its own dependencies, in general? Unless I've misunderstood something.
I don't expect, or want, say, Google test, or libfmt, to be installed because I use them.
I probably do want to record what the as built software was built against in the CPS data.
I'm afraid I've missed some context, can you link back to the example that is not working with install?
Also, I have thousands of packages that have both gtest and out equivalent of gtest main (where that main also initialized some common infrastructure) and I don't see a linker warning. Can you c/p the warning?
There was a problem hiding this comment.
A project should not install its own dependencies, in general? Unless I've misunderstood something. I don't expect, or want, say, Google test, or libfmt, to be installed because I use them. I probably do want to record what the as built software was built against in the CPS data.
GTest is not the problem, except you build a shared libgtest and want to install your tests.
-- Detecting CXX compile features - done
CMake Warning at /Users/clausklein/Workspace/cpp/boost-git/libs/type_index/CMakeLists.txt:53 (message):
Boost.type_index: `import std;` is not available for C++20
CMake Warning at /Users/clausklein/Workspace/cpp/boost-git/libs/any/CMakeLists.txt:54 (message):
Boost.Any: `import std;` is not available for C++20
-- boost-install-library(boost_config): COMPONENT 'boost'
-- boost_install_library(boost_config): EXPORT_NAME boost_config for TARGET 'boost_config'
-- boost-install-library(boost_mp11): COMPONENT 'boost'
-- boost_install_library(boost_mp11): EXPORT_NAME boost_mp11 for TARGET 'boost_mp11'
-- boost-install-library(boost_mp11): Add find_dependency(Boost::core)
-- boost-install-library(boost_describe): COMPONENT 'boost'
-- boost_install_library(boost_describe): EXPORT_NAME boost_describe for TARGET 'boost_describe'
-- boost-install-library(boost_describe): Add find_dependency(Boost::mp11)
-- boost-install-library(boost_container_hash): COMPONENT 'boost'
-- boost_install_library(boost_container_hash): EXPORT_NAME boost_container_hash for TARGET 'boost_container_hash'
-- boost-install-library(boost_container_hash): Add find_dependency(Boost::describe)
-- boost-install-library(boost_throw_exception): COMPONENT 'boost'
-- boost_install_library(boost_throw_exception): EXPORT_NAME boost_throw_exception for TARGET 'boost_throw_exception'
-- boost-install-library(boost_throw_exception): Add find_dependency(Boost::assert)
-- boost-install-library(boost_type_index): COMPONENT 'boost'
-- boost_install_library(boost_type_index): EXPORT_NAME boost_type_index for TARGET 'boost_type_index'
-- boost-install-library(boost_type_index): 'boost_type_index' has INTERFACE_HEADER_SETS=headers_public
-- boost-install-library(boost_type_index): 'boost_type_index' has CXX_MODULE_SETS=modules_public
-- boost-install-library(boost_type_index): Add find_dependency(Boost::boost_container_hash)
-- boost-install-library(boost_any): COMPONENT 'boost'
-- boost_install_library(boost_any): EXPORT_NAME boost_any for TARGET 'boost_any'
-- boost-install-library(boost_any): 'boost_any' has INTERFACE_HEADER_SETS=headers_public
-- boost-install-library(boost_any): 'boost_any' has CXX_MODULE_SETS=modules_public
-- boost-install-library(boost_any): Add find_dependency(Boost::type_index)
-- boost-install-library(boost_any): Add find_dependency(Boost::config)
-- boost-install-library(boost_any): Add find_dependency(Boost::throw_exception)
-- Configuring done (1.6s)
CMake Error in CMakeLists.txt:
install(EXPORT "boost_throw_exception-targets" ...) includes target
"boost_throw_exception" which requires target "boost_assert" that is not in
any export set.
-- Generating done (0.0s)
CMake Generate step failed. Build files cannot be regenerated correctly.
make: *** [build/compile_commands.json] Error 1
bash-5.3$
There was a problem hiding this comment.
our you get later this:
1/1 Test #6: find-package-test ................***Failed 0.22 sec
Internal cmake changing into directory: /Users/clausklein/Workspace/cpp/beman-project/exemplar/build/tests/beman/exemplar/find-package-test
======== CMake output ======
use ccache
CMake Warning at /Users/clausklein/Workspace/cpp/beman-project/exemplar/cmake/prelude.cmake:46 (message):
$CXX is not set
Call Stack (most recent call first):
CMakeLists.txt:5 (include)
CXXFLAGS=-stdlib=libc++
'brew' '--prefix' 'llvm'
LLVM_DIR=/usr/local/Cellar/llvm/21.1.8_1
CMAKE_CXX_STDLIB_MODULES_JSON=/usr/local/Cellar/llvm/21.1.8_1/lib/c++/libc++.modules.json
CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES=/usr/local/Cellar/llvm/21.1.8_1/include/c++/v1;/usr/local/Cellar/llvm/21.1.8_1/lib/clang/21/include;/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include
CMAKE_CXX_STDLIB_MODULES_JSON=/usr/local/Cellar/llvm/21.1.8_1/lib/c++/libc++.modules.json
CMAKE_CXX_SCAN_FOR_MODULES=ON
CMake Error at /usr/local/share/cmake/Modules/CMakeFindDependencyMacro.cmake:93 (find_package):
By not providing "Findbeman.inplace_vector.cmake" in CMAKE_MODULE_PATH this
project has asked CMake to find a package configuration file provided by
"beman.inplace_vector", but CMake did not find one.
Could not find a package configuration file provided by
"beman.inplace_vector" with any of the following names:
beman.inplace_vectorConfig.cmake
beman.inplace_vector-config.cmake
Add the installation prefix of "beman.inplace_vector" to CMAKE_PREFIX_PATH
or set "beman.inplace_vector_DIR" to a directory containing one of the
above files. If "beman.inplace_vector" provides a separate development
package or SDK, be sure it has been installed.
Call Stack (most recent call first):
/usr/local/share/cmake/Modules/CMakeFindDependencyMacro.cmake:125 (__find_dependency_common)
/Users/clausklein/Workspace/cpp/beman-project/exemplar/build/stagedir/lib/cmake/beman.exemplar/beman.exemplar-config.cmake:3 (find_dependency)
CMakeLists.txt:16 (find_package)
Configuring incomplete, errors occurred!
======== End CMake output ======
Error: cmake execution failed
0% tests passed, 1 tests failed out of 1
Total Test time (real) = 0.26 sec
The following tests FAILED:
6 - find-package-test (Failed)
Errors while running CTest
bash-5.3$
| find_package(GTest REQUIRED) | ||
| set(BUILD_GMOCK OFF) # not needed yet | ||
| set(INSTALL_GTEST OFF) # Disable GoogleTest installation too | ||
| find_package(GTest 1.16 CONFIG REQUIRED) |
There was a problem hiding this comment.
At least the CONFIG option must be used!
Note this too: https://discourse.cmake.org/t/the-findgtest-cmake-module-does-not-respect-the-version-parameter/15476/3
|
I've lost the thread a bit, and I'll have to go back and reread all the links, but gtest showing up in our install sounds like we've tried to simplify install too much and we're not describing precisely what components to install? Exemplar is below tiny so some machinery seems overkill. But it should not be expected that you need to rewrite chunks of exemplar, or the cookiecutter generated project, because you now have a bunch more parts. In my opinion, exemplar should scale smoothly to a few installable components and a couple dozen files. North of 100 files may not fit well, but two orders of magnitude is a change in kind, not just degree. Exemplar is a scaled down small to medium sized project. Larger packages can and will build multiple installable components. In particular being able to install and then test examples is important when dynamic libraries are involved, but should not be part of the "-dev" package, or library package. Making sure those knobs are exposed means someone doesn't have to rewrite our build system or needlessly repackage our packages. Apologies for possibly misapprehending, and channeling Emily Litella, in which case "Nevermind". |
|
I still wait for any decision? |
| # NO! EXCLUDE_FROM_ALL | ||
| ) | ||
| set(INSTALL_GTEST OFF) # Disable GoogleTest installation | ||
| # NO! set(INSTALL_GTEST OFF) # Disable GoogleTest installation |
There was a problem hiding this comment.
The second one is incorrect, the first I believe to be correct.
Excluding from the all target will still require a target to be built if used, but it will not be part of the install step.
| target_link_libraries( | ||
| beman.exemplar.tests.identity | ||
| PRIVATE beman::exemplar GTest::gtest GTest::gtest_main | ||
| PRIVATE beman::exemplar GTest::gtest_main |
There was a problem hiding this comment.
The gtest_main target and library depend on the gtest library, but does not duplicate it. The only thing in libgtest_main.a is gtest_main.cc.o.
Since the use of gtest is not purely transitive, but direct, the project should link what it uses, and not rely on transitive linking of dependencies.
Any error or warning is not from the dependency but an indication of some other problem.
5 participants
Fix duplicative
GTest::gtest GTest::gtest_maindependencyfor #292
dependency_provider cmake module, if needed?