Skip to content

Commit

Permalink
Extend checked_find_package with VERSION_MIN and VERSION_MAX (#1303)
Browse files Browse the repository at this point in the history 
Upon reading cmake docs mre carefully, it's clear that when you pass a
version to find_package (which does much of the heavy lifting inside
checked_find_package), it DOESN'T MEAN you want that version or
later. It means you want to find a version that is "compatible" with
the named version. The meaning of "compatible" is up to the package.
Some packages define it as being >= the requested version. Others
define it as being the same major release, or same major AND minor
release, or exact.

But generally speaking, we only care about the minimum version (e.g.,
"I want FooBar 8.0 or later"). And it's not guaranteed that is what
the package's compatibility will do. In fact, FooBar may be using
compatibility mode "SameMajorVersion", which will make our find of
FooBar succeed for 8.0, 8.1, 8.2, but if a user encounters FooBar 9.0,
our build will reject it, even though it's probably fine.

So I augmented our checked_find_package wrapper to take optional
VERSION_MIN to give the real minimum version, when that's the semantic
we really want (in which case we would omit the ordinary version
number used for the compatibility check and let VERSION_MIN do all the
work). There's also an optional (and usually omitted) VERSION_MAX that
you can use if you are sure that we must be less than a certain
version (the max is exclusive, you must be <, not <=, whereas the min
is inclusive >=).

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
@lgritz
lgritz committed Nov 27, 2020
1 parent 62d1428 commit 3fc5c85
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 13 deletions.
30 changes: 25 additions & 5 deletions src/cmake/checked_find_package.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ set (OPTIONAL_DEPS "" CACHE STRING
"Additional dependencies to consider optional (semicolon-separated list, or ALL)")


# checked_find_package(pkgname ..) is a wrapper for find_package, with the
# checked_find_package(Pkgname ...) is a wrapper for find_package, with the
# following extra features:
# * If either `USE_pkgname` or the all-uppercase `USE_PKGNAME` (or
# `ENABLE_pkgname` or `ENABLE_PKGNAME`) exists as either a CMake or
# * If either `USE_Pkgname` or the all-uppercase `USE_PKGNAME` (or
# `ENABLE_Pkgname` or `ENABLE_PKGNAME`) exists as either a CMake or
# environment variable, is nonempty by contains a non-true/nonzero
# value, do not search for or use the package. The optional ENABLE <var>
# arguments allow you to override the name of the enabling variable. In
# other words, support for the dependency is presumed to be ON, unless
# turned off explicitly from one of these sources.
# * Print a message if the package is enabled but not found. This is based
# on ${pkgname}_FOUND or $PKGNNAME_FOUND.
# on ${Pkgname}_FOUND or $PKGNAME_FOUND.
# * Optional DEFINITIONS <string> are passed to add_definitions if the
# package is found.
# * Optional PRINT <list> is a list of variables that will be printed
Expand All @@ -30,6 +30,14 @@ set (OPTIONAL_DEPS "" CACHE STRING
# present package is only needed because it's a dependency, and
# therefore if <downstream> is disabled, we don't bother with this
# package either.
# * Optional VERSION_MIN and VERSION_MAX, if supplied, give minimum and
# maximum versions that will be accepted. The min is inclusive, the max
# is exclusive (i.e., check for min <= version < max). Note that this is
# not the same as providing a version number to find_package, which
# checks compatibility, not minimum. Sometimes we really do just want to
# say a minimum or a range. (N.B. When our minimum CMake >= 3.19, the
# built-in way to do this is with version ranges passed to
# find_package.)
# * Optional RECOMMEND_MIN, if supplied, gives a minimum recommended
# version, accepting but warning if it is below this number (even
# if above the true minimum version accepted). The warning message
Expand All @@ -43,7 +51,7 @@ macro (checked_find_package pkgname)
# noValueKeywords:
"REQUIRED"
# singleValueKeywords:
"ENABLE;ISDEPOF;RECOMMEND_MIN;RECOMMEND_MIN_REASON"
"ENABLE;ISDEPOF;VERSION_MIN;VERSION_MAX;RECOMMEND_MIN;RECOMMEND_MIN_REASON"
# multiValueKeywords:
"DEFINITIONS;PRINT;DEPS"
# argsToParse:
Expand Down Expand Up @@ -79,6 +87,18 @@ macro (checked_find_package pkgname)
endif ()
if (_enable OR _pkg_REQUIRED)
find_package (${pkgname} ${_pkg_UNPARSED_ARGUMENTS})
if ((${pkgname}_FOUND OR ${pkgname_upper}_FOUND)
AND ${pkgname}_VERSION
AND (_pkg_VERSION_MIN OR _pkg_VERSION_MAX))
if ((_pkg_VERSION_MIN AND ${pkgname}_VERSION VERSION_LESS _pkg_VERSION_MIN)
OR (_pkg_VERSION_MAX AND ${pkgname}_VERSION VERSION_GREATER _pkg_VERSION_MAX))
message (STATUS "${ColorRed}${pkgname} ${${pkgname}_VERSION} is outside the required range ${_pkg_VERSION_MIN}...${_pkg_VERSION_MAX} ${ColorReset}")
unset (${pkgname}_FOUND)
unset (${pkgname}_VERSION)
unset (${pkgname_upper}_FOUND)
unset (${pkgname_upper}_VERSION)
endif ()
endif ()
if (${pkgname}_FOUND OR ${pkgname_upper}_FOUND)
foreach (_vervar ${pkgname_upper}_VERSION ${pkgname}_VERSION_STRING
${pkgname_upper}_VERSION_STRING)
Expand Down
29 changes: 21 additions & 8 deletions src/cmake/externalpackages.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,11 @@ else ()
# to set the expected variables printed below. So until that's fixed
# force FindBoost.cmake to use the original brute force path.
set (Boost_NO_BOOST_CMAKE ON)
checked_find_package (Boost 1.55 REQUIRED
checked_find_package (Boost REQUIRED
VERSION_MIN 1.55
COMPONENTS ${Boost_COMPONENTS}
RECOMMEND_MIN 1.66
RECOMMEND_MIN_REASON "Boost 1.66 is the oldest version our CI tests against"
PRINT Boost_INCLUDE_DIRS Boost_LIBRARIES
)
endif ()
Expand All @@ -79,8 +82,13 @@ link_directories ("${Boost_LIBRARY_DIRS}")
checked_find_package (ZLIB REQUIRED) # Needed by several packages

# IlmBase & OpenEXR
checked_find_package (OpenEXR 2.0 REQUIRED
PRINT IMATH_INCLUDES)
checked_find_package (OpenEXR REQUIRED
VERSION_MIN 2.0
RECOMMEND_MIN 2.2
RECOMMEND_MIN_REASON
"OpenEXR 2.2 is the oldest version our CI tests against, and the minimum that supports DWA compression"
PRINT IMATH_INCLUDES
)
if (CMAKE_COMPILER_IS_CLANG AND OPENEXR_VERSION VERSION_LESS 2.3)
# clang C++ >= 11 doesn't like 'register' keyword in old exr headers
add_compile_options (-Wno-deprecated-register)
Expand All @@ -93,18 +101,21 @@ endif ()
# OpenImageIO
set (OIIO_LIBNAME_SUFFIX "" CACHE STRING
"Optional name appended to OIIO libraries that are built")
checked_find_package (OpenImageIO 2.0 REQUIRED
checked_find_package (OpenImageIO REQUIRED
VERSION_MIN 2.0
PRINT OIIOTOOL_BIN)
if (OPENIMAGEIO_FOUND)
include_directories ("${OPENIMAGEIO_INCLUDES}")
endif ()


checked_find_package (pugixml 1.8 REQUIRED)
checked_find_package (pugixml REQUIRED
VERSION_MIN 1.8)


# LLVM library setup
checked_find_package (LLVM 7.0 REQUIRED
checked_find_package (LLVM REQUIRED
VERSION_MIN 7.0
PRINT LLVM_SYSTEM_LIBRARIES CLANG_LIBRARIES)
# ensure include directory is added (in case of non-standard locations
include_directories (BEFORE SYSTEM "${LLVM_INCLUDES}")
Expand Down Expand Up @@ -169,7 +180,8 @@ if (USE_CUDA OR USE_OPTIX)
message (STATUS "CUDA_TOOLKIT_ROOT_DIR = ${CUDA_TOOLKIT_ROOT_DIR}")
endif ()

checked_find_package (CUDA 8.0 REQUIRED
checked_find_package (CUDA REQUIRED
VERSION_MIN 8.0
PRINT CUDA_INCLUDES)
set (CUDA_INCLUDES ${CUDA_TOOLKIT_ROOT_DIR}/include)
include_directories (BEFORE "${CUDA_INCLUDES}")
Expand Down Expand Up @@ -202,7 +214,8 @@ if (USE_CUDA OR USE_OPTIX)

# OptiX setup
if (USE_OPTIX)
checked_find_package (OptiX 5.1 REQUIRED)
checked_find_package (OptiX REQUIRED
VERSION_MIN 5.1)
include_directories (BEFORE "${OPTIX_INCLUDES}")
if (NOT USE_LLVM_BITCODE OR NOT USE_FAST_MATH)
message (FATAL_ERROR "Enabling OptiX requires USE_LLVM_BITCODE=1 and USE_FAST_MATH=1")
Expand Down

0 comments on commit 3fc5c85

Please sign in to comment.