From 21c9db1861825cdc7f89e90bf166115a2581b663 Mon Sep 17 00:00:00 2001
From: "Justin M. Keyes" <justinkz@gmail.com>
Date: Fri, 1 Jun 2018 20:17:24 +0200
Subject: build/CMake: find_package(… REQUIRED)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

"Always use `find_package` with `REQUIRED`."

- We make an exception for LuaJit (not REQUIRED): the `nvim-test` target
  is included only if we can find LuaJit.

This is partially a cargo-cult (reference below), but it uncovered at
least one problem: `find_package(LibIntl REQUIRED)` fails on my vanilla
ubuntu 16.04 system.

ref: https://schneide.blog/2017/11/06/4-tips-for-better-cmake/

> optional dependencies is nice, but skipping on REQUIRED is not the way
> you want to do it. In the worst case, some of your features will just
> not work if those packages are not found, with no explanation
> whatsoever. Instead, use explicit feature-toggles (e.g. using option())
> that either skip the find_package call or use it with REQUIRED, so the
> user will know that another lib is needed for this feature.
---
 .travis.yml                |  1 +
 CMakeLists.txt             | 34 +++++++++++++++++++++-------------
 src/nvim/CMakeLists.txt    |  4 +++-
 src/nvim/po/CMakeLists.txt |  2 +-
 4 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index b275a5262d..fa884bd021 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -53,6 +53,7 @@ jobs:
       compiler: clang
       env: >
         CLANG_SANITIZER=ASAN_UBSAN
+        # Use Lua so that ASAN can test our embedded Lua support. 8fec4d53d0f6
         CMAKE_FLAGS="$CMAKE_FLAGS -DPREFER_LUA=ON"
       sudo: true
     - os: linux
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4cafdef73f..96d88c7469 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -13,9 +13,9 @@ include(PreventInTreeBuilds)
 
 # Prefer our bundled versions of dependencies.
 if(DEFINED ENV{DEPS_BUILD_DIR})
-set(DEPS_PREFIX "$ENV{DEPS_BUILD_DIR}/usr" CACHE PATH "Path prefix for finding dependencies")
+  set(DEPS_PREFIX "$ENV{DEPS_BUILD_DIR}/usr" CACHE PATH "Path prefix for finding dependencies")
 else()
-set(DEPS_PREFIX "${CMAKE_CURRENT_SOURCE_DIR}/.deps/usr" CACHE PATH "Path prefix for finding dependencies")
+  set(DEPS_PREFIX "${CMAKE_CURRENT_SOURCE_DIR}/.deps/usr" CACHE PATH "Path prefix for finding dependencies")
 endif()
 if(CMAKE_CROSSCOMPILING AND NOT UNIX)
   list(INSERT CMAKE_FIND_ROOT_PATH 0 ${DEPS_PREFIX})
@@ -53,6 +53,14 @@ if(WIN32 OR CMAKE_SYSTEM_NAME STREQUAL "Darwin")
   set(USE_FNAME_CASE TRUE)
 endif()
 
+option(ENABLE_LIBINTL "enable libintl" ON)
+if(MSVC)
+  # TODO(justinmk): need bundled iconv for MSVC.
+  option(ENABLE_LIBICONV "enable libiconv" OFF)
+else()
+  option(ENABLE_LIBICONV "enable libiconv" ON)
+endif()
+
 # Set default build type.
 if(NOT CMAKE_BUILD_TYPE)
   message(STATUS "CMAKE_BUILD_TYPE not given, defaulting to 'Debug'.")
@@ -331,6 +339,7 @@ if(PREFER_LUA)
   find_package(Lua REQUIRED)
   set(LUA_PREFERRED_INCLUDE_DIRS ${LUA_INCLUDE_DIR})
   set(LUA_PREFERRED_LIBRARIES ${LUA_LIBRARIES})
+  # Passive (not REQUIRED): if LUAJIT_FOUND is not set, nvim-test is skipped.
   find_package(LuaJit)
 else()
   find_package(LuaJit REQUIRED)
@@ -399,31 +408,30 @@ if((CLANG_ASAN_UBSAN OR CLANG_MSAN OR CLANG_TSAN) AND NOT CMAKE_C_COMPILER_ID MA
   message(FATAL_ERROR "Sanitizers are only supported for Clang.")
 endif()
 
-if(CMAKE_SYSTEM_NAME MATCHES "OpenBSD|FreeBSD")
-  message(STATUS "detected OpenBSD/FreeBSD; disabled jemalloc. #5318")
+if(CMAKE_SYSTEM_NAME MATCHES "OpenBSD|FreeBSD|Windows")  # see #5318
+  message(STATUS "skipping jemalloc on this system: ${CMAKE_SYSTEM_NAME}")
   option(ENABLE_JEMALLOC "enable jemalloc" OFF)
 else()
   option(ENABLE_JEMALLOC "enable jemalloc" ON)
 endif()
 
-if (ENABLE_JEMALLOC)
+if(ENABLE_JEMALLOC)
   if(CLANG_ASAN_UBSAN OR CLANG_MSAN OR CLANG_TSAN)
     message(STATUS "Sanitizers have been enabled; don't use jemalloc.")
   else()
-    find_package(JeMalloc)
-    if(JEMALLOC_FOUND)
-      include_directories(SYSTEM ${JEMALLOC_INCLUDE_DIRS})
-    endif()
+    find_package(JeMalloc REQUIRED)
+    include_directories(SYSTEM ${JEMALLOC_INCLUDE_DIRS})
   endif()
 endif()
 
-find_package(LibIntl)
-if(LibIntl_FOUND)
+if(ENABLE_LIBINTL)
+  # LibIntl (not Intl) selects our FindLibIntl.cmake script. #8464
+  find_package(LibIntl REQUIRED)
   include_directories(SYSTEM ${LibIntl_INCLUDE_DIRS})
 endif()
 
-find_package(Iconv)
-if(Iconv_FOUND)
+if(ENABLE_LIBICONV)
+  find_package(Iconv REQUIRED)
   include_directories(SYSTEM ${Iconv_INCLUDE_DIRS})
 endif()
 
diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt
index 2d803792c8..bdedce8076 100644
--- a/src/nvim/CMakeLists.txt
+++ b/src/nvim/CMakeLists.txt
@@ -484,7 +484,9 @@ set_property(
   APPEND_STRING PROPERTY COMPILE_FLAGS " -DMAKE_LIB "
 )
 
-if(LUAJIT_FOUND)
+if(NOT LUAJIT_FOUND)
+  message(STATUS "luajit not found, skipping nvim-test (unit tests) target")
+else()
   set(NVIM_TEST_LINK_LIBRARIES ${NVIM_LINK_LIBRARIES} ${LUAJIT_LIBRARIES})
   add_library(
     nvim-test
diff --git a/src/nvim/po/CMakeLists.txt b/src/nvim/po/CMakeLists.txt
index 94cc63baea..a7b910f0eb 100644
--- a/src/nvim/po/CMakeLists.txt
+++ b/src/nvim/po/CMakeLists.txt
@@ -1,4 +1,4 @@
-find_package(Gettext)
+find_package(Gettext REQUIRED)
 find_program(XGETTEXT_PRG xgettext)
 find_program(ICONV_PRG iconv)
 
-- 
cgit