From 8e18d610f5f38df97dad4247b608c4abe320c7b8 Mon Sep 17 00:00:00 2001
From: Tim Siegel <siegeltr@gmail.com>
Date: Thu, 21 Apr 2022 18:33:53 -0400
Subject: [PATCH] cmake: Add SDL dep for Linux dfhack; deprecate BUILD_TESTS

On Linux, libdfhack.so depends on libSDL.so, but that was not marked
inside CMake. As it's only used via LD_PRELOAD, there was no problem.
But when linking unit tests against it, this becomes necessary. It may
be wise to add a find_package(SDL) to provide the user with more
control, but just a hard-coded "SDL" should work for most installs.

The CTest module creates a BUILD_TESTING option, which clashes
(thematically, not in code) with the existing BUILD_TESTS option.
Resolve it thus:
- Deprecate BUILD_TESTS; it still works, but is marked as an advanced
  option so it doesn't show in the CMake UI by default.
- Add a new BUILD_TEST_SCRIPTS that does what BUILD_TESTS used to do,
  but is a "dependent" option so it goes away if BUILD_TESTING=OFF.

The up-shot is that, by default, the C++ unit tests will be built
(BUILD_TESTING=ON) and the Lua integration tests are not installed
(BUILD_TEST_SCRIPTS=OFF).
---
 CMakeLists.txt         | 32 +++++++++++++++++++++++---------
 library/CMakeLists.txt | 12 +++++++++---
 2 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index e36385c43..caf53c2a0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -527,16 +527,30 @@ if(BUILD_DOCS)
     install(FILES "README.html" DESTINATION "${DFHACK_DATA_DESTINATION}")
 endif()
 
-option(BUILD_TESTS "Build 'test' target, and install integration tests in hack/scripts/test" OFF)
-if(BUILD_TESTS)
-    include(CTest)
-    # Install Lua tests into the scripts folder
-    if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts")
-        message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.")
+# Testing with CTest
+include(CTest)
+
+include(CMakeDependentOption)
+cmake_dependent_option(
+    BUILD_TEST_SCRIPTS "Install integration tests in hack/scripts/test" OFF
+    "BUILD_TESTING" OFF)
+mark_as_advanced(FORCE BUILD_TESTS)
+
+# Handle deprecated BUILD_TESTS option
+option(BUILD_TESTS "Deprecated option; please use BUILD_TEST_SCRIPTS=ON" OFF)
+if(BUILD_TESTING AND BUILD_TESTS)
+    set(BUILD_TEST_SCRIPTS ON FORCE)
+endif()
+
+if(BUILD_TESTING)
+    if(BUILD_TEST_SCRIPTS)
+        if(EXISTS "${dfhack_SOURCE_DIR}/test/scripts")
+            message(SEND_ERROR "test/scripts must not exist in the dfhack repo since it would conflict with the tests installed from the scripts repo.")
+        endif()
+        install(DIRECTORY ${dfhack_SOURCE_DIR}/test
+            DESTINATION ${DFHACK_DATA_DESTINATION}/scripts)
+        install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts)
     endif()
-    install(DIRECTORY ${dfhack_SOURCE_DIR}/test
-        DESTINATION ${DFHACK_DATA_DESTINATION}/scripts)
-    install(FILES ci/test.lua DESTINATION ${DFHACK_DATA_DESTINATION}/scripts)
 else()
     add_custom_target(test
         COMMENT "Nothing to do: CMake option BUILD_TESTS is OFF"
diff --git a/library/CMakeLists.txt b/library/CMakeLists.txt
index dd9582b5b..a62eb2fe0 100644
--- a/library/CMakeLists.txt
+++ b/library/CMakeLists.txt
@@ -82,12 +82,16 @@ set(MAIN_SOURCES
     RemoteTools.cpp
 )
 
-if(BUILD_TESTS)
+if(BUILD_TESTING)
     # TODO Make a function or macro for this
     add_executable(MiscUtils.test MiscUtils.test.cpp)
-    # FIXME Hard-coded "SDL" works on Linux, but probably isn't portable
-    target_link_libraries(MiscUtils.test dfhack SDL)
+    target_link_libraries(MiscUtils.test dfhack)
     add_test(NAME MiscUtils.test COMMAND MiscUtils.test)
+
+    # How to get `test` to ensure everything is up to date before running
+    # tests? This add_dependencies() fails with:
+    #   Cannot add target-level dependencies to non-existent target "test".
+    #add_dependencies(test MiscUtils.test)
 endif()
 
 if(WIN32)
@@ -400,6 +404,8 @@ if(APPLE)
     target_link_libraries(dfhack ncurses)
     set_target_properties(dfhack PROPERTIES VERSION 1.0.0)
     set_target_properties(dfhack PROPERTIES SOVERSION 1.0.0)
+elseif(UNIX)
+    target_link_libraries(dfhack SDL)
 endif()
 
 target_link_libraries(dfhack protobuf-lite clsocket lua jsoncpp_static dfhack-version ${PROJECT_LIBS})