D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
This revision was automatically updated to reflect the committed changes.
Closed by commit R240:b99d2d2c5ded: RFC: Make ECMAddTests respect BUILD_TESTING 
(authored by kfunk).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D7187?vs=17835=17858#toc

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7187?vs=17835=17858

REVISION DETAIL
  https://phabricator.kde.org/D7187

AFFECTED FILES
  modules/ECMAddTests.cmake

To: kfunk, vkrause
Cc: kossebau, vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, 
#build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Volker Krause 
vkrause accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk, vkrause
Cc: kossebau, vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, 
#build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk added a comment.


  In https://phabricator.kde.org/D7187#133497, @kossebau wrote:
  
  > +1 as well. Please also add a note in the API dox what BUILD_TESTING will 
do on this macro, so this is not magic behaviour and people can plan with this 
(like making sure to not set `target_link_libraries(... Qt5::Tests)` 
separately, but only pass as args in the macro *cough*kdevelop*cough*)
  
  
  Will do.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: kossebau, vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, 
#build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Friedrich W . H . Kossebau 
kossebau added a comment.


  +1 as well. Please also add a note in the API dox what BUILD_TESTING will do 
on this macro, so this is not magic behaviour and people can plan with this 
(like making sure to not `set target_link_libraries(... Qt5::Tests)` 
separately, but only pass as args in the macro *cough*kdevelop*cough*)

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: kossebau, vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, 
#build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk added a comment.


  Guys... no one gave me an "Accepted' yet :)

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Volker Krause 
vkrause added a comment.


  Another +1, would also help the KDE Yocto recipes.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: vkrause, elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Andreas Sturmlechner 
asturmlechner added a comment.


  In https://phabricator.kde.org/D7187#11, @kfunk wrote:
  
  > Anyhow, this patch tries to make it easy to build KDevelop without tests; 
trying to avoid hacky approaches like those being done in Gentoo:
  
  
  Much appreciated if we can drop those.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Elvis Angelaccio 
elvisangelaccio added a comment.


  Oh, I see. +1 then!

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk added a comment.


  In https://phabricator.kde.org/D7187#133358, @elvisangelaccio wrote:
  
  > In `ecm_mark_as_test` (which is used in `ecm_add_test`) we already disable 
the target if `BUILD_TESTING` is false, why is that not enough?
  
  
  Good question. I didn't explain that in the commit message:
  
  `ecm_add_test` also invokes `target_link_libraries(...)` which may pull in 
additionally dependencies if `BUILD_TESTING=ON`.
  
  Example:
  
ecm_add_test(test_svnrecursiveadd.cpp LINK_LIBRARIES
Qt5::Test Qt5::Gui
)
  
  
  
  - If `BUILD_TESTING=OFF` & without this patch: We still need Qt5::Test around.
  - If `BUILD_TESTING=OFF` & with this patch: We no longer need Qt5::Test at 
all.
  
  This patch actually makes it pretty easy to get rid off the Qt5::Test 
dependency in a whole KDE project without touching lots of CMake code.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Elvis Angelaccio 
elvisangelaccio added a comment.


  In `ecm_mark_as_test` (which is used in `ecm_add_test`) we already disable 
the target if `BUILD_TESTING` is false, why is that not enough?

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: elvisangelaccio, asturmlechner, apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk updated this revision to Diff 17835.
kfunk added a comment.


  Fix typo. "no-top" is something else.

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7187?vs=17830=17835

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7187

AFFECTED FILES
  modules/ECMAddTests.cmake

To: kfunk
Cc: apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Aleix Pol Gonzalez 
apol added a comment.


  +1

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: apol, #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk added a comment.


  It's not possible to easily disable tests at the moment, is it? I might be 
missing something.
  
  Anyhow, this patch tries to make it easy to build KDevelop without tests; 
trying to avoid hacky approaches like those being done in Gentoo:
  
https://bugs.gentoo.org/show_bug.cgi?id=627242

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D7187

To: kfunk
Cc: #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk updated this revision to Diff 17830.
kfunk added a comment.


  Remove unrelated hunks

REPOSITORY
  R240 Extra CMake Modules

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7187?vs=17829=17830

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7187

AFFECTED FILES
  modules/ECMAddTests.cmake

To: kfunk
Cc: #frameworks, #build_system

D7187: RFC: Make ECMAddTests respect BUILD_TESTING

2017-08-07 Thread Kevin Funk 
kfunk created this revision.
Restricted Application added projects: Frameworks, Build System.
Restricted Application added subscribers: Build System, Frameworks.

REVISION SUMMARY
  Use-case: Make building unit tests optional, by just following the CMake
  BUILD_TESTING option.
  
  The usual approach to conditionally build tests is to do:
  
if (BUILD_TESTING)
add_executable(TestOne TestOne.cpp)
target_link_libraries(TestOne my_library)
endif()
  
  or:
  
if (BUILD_TESTING)
add_subdirectory(tests)
endif()
  
  This patch just turns all calls to ecm_add_test(...) into no-ops if
  BUILD_TESTING=OFF.
  
  See:
  
https://cmake.org/cmake/help/v3.6/module/CTest.html

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D7187

AFFECTED FILES
  modules/ECMAddTests.cmake
  modules/ECMEnableSanitizers.cmake

To: kfunk
Cc: #frameworks, #build_system