Ableton / link

Ableton Link
Other
1.06k stars 145 forks source link

disable tests if 'catch' is not available #128

Closed umlaeute closed 1 year ago

umlaeute commented 1 year ago

Link comes with a test-suite (great!), that is powered by catch.

in certain situations, it's impossible (or at least: hard) to have catch installed, but you still might want to build the examples.

my specific use-case is, cross-compiling the link Debian packages (which ship the examples as compiled binaries).

it would be great if it was possible to somehow disable building the tests, either via an explicit Cmake option (e.g. LINK_BUILD_TESTS) or implicitly by checking whether catch is available.

the latter would be more comfortable, but i understand the appeal of the former (where not running the testsuite is supposed to only happen intentionally).

i guess this is somewhat related to removing the 'catch' submodule...

umlaeute commented 1 year ago

my fix so far is implementing the 1st option:

--- ableton-link.orig/CMakeLists.txt
+++ ableton-link/CMakeLists.txt
@@ -22,6 +22,8 @@
   option(LINK_BUILD_VLD "Build with VLD support (VLD must be installed separately)" OFF)
 endif()

+option(LINK_BUILD_TESTS "Build unit test binaries" ON)
+
 #  ____       _   _
 # |  _ \ __ _| |_| |__  ___
 # | |_) / _` | __| '_ \/ __|
@@ -36,7 +38,9 @@
 include(extensions/abl_link/abl_link.cmake)

 add_subdirectory(include)
+if(LINK_BUILD_TESTS)
 add_subdirectory(src)
+endif()
 add_subdirectory(examples)
 add_subdirectory(extensions/abl_link)
fgo-ableton commented 1 year ago

TBH I don't understand the issue with Catch being installed. We removed the submodule, but we still have the header only version of Catch 2 in the repo: https://github.com/Ableton/link/blob/master/third_party/catch/catch.hpp

umlaeute commented 1 year ago

i totally understand your concerns. it's a bit... complicated.

It is Debian policy to avoid vendored copies of libraries, and use those versions that are packaged in Debian instead. For Link this means, that we strip away the entire third_party/ folder when importing the sources.

So when building the Debian package (on our build infrastructure), I cannot use the header-only version of Catch from your sources, but have to install the Debian catch package. Now, due to a bug in thecatch package, it is not installable in our (Debian's) cross-building workflow.

The simple fix is, to just not build the test binaries when cross-compiling (as they are mostly useless anyhow, since they are not runnable in a cross-building context).


With your feedback, I see that my suggestion to automatically disable building the tests whenever Catch is not found doesn't make sense at all (as from your POV of view, it is always available).

Nevertheless, it would be great if you could allow for our special case, by providing some means to explicitly not build the test binaries if there is no intention to ever run them.

I think that a solution along the lines of https://github.com/Ableton/link/issues/128#issuecomment-1377955077 might be acceptable for you.

fgo-ableton commented 1 year ago

Thanks for the explanation. I added the option to disable building the tests as you suggested.