Building without tests-tools #14268
Building without tests-tools #14268
Conversation
PPN-SD
force-pushed
the
cmake-mixxx-main
branch
from
d32ce5d to
2fa3d0c
Compare
|
@daschuer I think it's ready. Bench and testing by default if no def. |
|
Thanks. Before merge, we need you approval: |
PPN-SD
force-pushed
the
cmake-mixxx-main
branch
from
2fa3d0c to
f85cbe1
Compare
Done. Fixed code style. |
PPN-SD
force-pushed
the
cmake-mixxx-main
branch
from
f85cbe1 to
9154a09
Compare
CMakeLists.txt
Outdated
| if(NOT DEFINED BUILD_TESTING) | ||
| find_package(GTest CONFIG) | ||
| if(GTest_FOUND) | ||
| message(STATUS "Found GTest: tests enabled") | ||
| set(BUILD_TESTING ON) | ||
| endif() | ||
| elseif(BUILD_TESTING) | ||
| find_package(GTest CONFIG REQUIRED) | ||
| if(GTest_FOUND) | ||
| message(STATUS "Found GTest") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
What is the reason for not doing:
| if(NOT DEFINED BUILD_TESTING) | |
| find_package(GTest CONFIG) | |
| if(GTest_FOUND) | |
| message(STATUS "Found GTest: tests enabled") | |
| set(BUILD_TESTING ON) | |
| endif() | |
| elseif(BUILD_TESTING) | |
| find_package(GTest CONFIG REQUIRED) | |
| if(GTest_FOUND) | |
| message(STATUS "Found GTest") | |
| endif() | |
| endif() | |
| find_package(GTest CONFIG) | |
| cmake_dependent_option( | |
| BUILD_TESTING | |
| "Build with Unittests" | |
| OFF | |
| "GTest_FOUND" | |
| ON | |
| ) | |
| if(BUILD_TESTING AND NOT GTest_FOUND) | |
| message(FATAL_ERROR "GTest is required for DBUILD_TESTING=ON") | |
| endif() |
same below
There was a problem hiding this comment.
None. But you mean OFF/ON-> ON/OFF no ?
There was a problem hiding this comment.
Oh, just noticed that cmake_dependent_option() does not work, because it forces the value to "OFF" independent form the user value. We want:
default_option(BUILD_TESTING, "Build with Unittests", "GTest_FOUND")
There was a problem hiding this comment.
The commit's logic is clear and CMakeMagic doesn't decrease the number of lines.
There was a problem hiding this comment.
default_option(BUILD_TESTING, "Build with Unittests", "GTest_FOUND")
Reuses a bit of the complexity used elsewhere in our CMakLists.txt.
That's why I prefere this. But I don't insist.
There was a problem hiding this comment.
OK. I'll check that.
PR 2.5 pushed.
|
Grenade on 2.5 now? |
|
PPN-SD
force-pushed
the
cmake-mixxx-main
branch
2 times, most recently
from
695ae64 to
14626ee
Compare
Today, we can't build mixxx without links to gtest/gperftools/benchmark This PR allows building only mixxx and mixxx-lib targets without tests-tools. Two common cmake-definitions are added : * BUILD_TESTING * BUILD_BENCH And one compile definition for test.main.cpp : * USE_BENCH This is helpful for packaging (gentoo here). The negative effect is sorting of tests source files as soon as they are called by mixxx-benchmark target. Signed-off-by: Nicolas PARLANT <[email protected]>
Signed-off-by: Nicolas PARLANT <[email protected]>
PPN-SD
force-pushed
the
cmake-mixxx-main
branch
from
14626ee to
5b81a3c
Compare
|
Does that sound better? |
|
Yes, the main PR was unnecessary. In our case the 2.5 to main merge was conflicting. I have resolved the conflicts, so everything is fine again. |
Today, we can't build mixxx without links to gtest/gperftools/benchmark
This PR allows building only mixxx and mixxx-lib targets without tests-tools.
Two common cmake-definitions are added :
And one compile definition for test.main.cpp :
This is helpful for packaging (gentoo here).
The negative effect is sorting of tests source files as soon as they are called by mixxx-benchmark target.
Duplicate of a previous messy PR : #14264