From df5821aac9dab8f90699d8718c80ec2f1df57d69 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Tue, 23 Aug 2022 13:11:00 -0600 Subject: [PATCH] tribits_add_option_and_define(): restore optional macro define var arg, refactor (#516) In PR #520, I accidentally put a requirement that the macro var name had to be set. Turns out, there is special logic for it to not be set. The documentation nor any TriBITS test hinted to this. But the file muelu/CMakeLists.txt actually calls tribits_add_option_and_define() with an empty macro var name. To catch this use case, I added unit tests (which will fail if you undo this patch). In order for the unit tests to cover what is needed, I had to factor out the function tribits_pkg_export_cache_var() into its own module TribitsPkgExportCacheVars.cmake so it could be included in the unit test driver. Then, I decided it would be a good idea to factor out a function tribits_pkg_append_set_commands_for_exported_vars() that is called in the module TribitsWriteClientExportFiles.cmake. Lastly, I moved the function tribits_assert_cache_and_local_vars_same_value() into this module as well. This encapsulates all of the logic to export of these package cache vars into a single module (with just a few dependencies). --- test/core/CMakeLists.txt | 2 +- .../core/TestingFunctionMacro_UnitTests.cmake | 32 ++++- .../TribitsAddOptionAndDefine.cmake | 18 ++- .../package_arch/TribitsGeneralMacros.cmake | 24 ---- .../package_arch/TribitsPackageMacros.cmake | 42 +----- .../TribitsPkgExportCacheVars.cmake | 130 ++++++++++++++++++ .../TribitsWriteClientExportFiles.cmake | 14 +- 7 files changed, 179 insertions(+), 83 deletions(-) create mode 100644 tribits/core/package_arch/TribitsPkgExportCacheVars.cmake diff --git a/test/core/CMakeLists.txt b/test/core/CMakeLists.txt index 88e5143c4..a6b4f2aca 100644 --- a/test/core/CMakeLists.txt +++ b/test/core/CMakeLists.txt @@ -49,7 +49,7 @@ tribits_add_advanced_test( TestingFunctionMacro_UnitTests -D${PROJECT_NAME}_TRIBITS_DIR=${${PROJECT_NAME}_TRIBITS_DIR} -P "${CMAKE_CURRENT_SOURCE_DIR}/TestingFunctionMacro_UnitTests.cmake" PASS_REGULAR_EXPRESSION_ALL - "Final UnitTests Result: num_run = 703" + "Final UnitTests Result: num_run = 708" "Final UnitTests Result: PASSED" ) diff --git a/test/core/TestingFunctionMacro_UnitTests.cmake b/test/core/TestingFunctionMacro_UnitTests.cmake index 6994008b3..bb9b66cde 100644 --- a/test/core/TestingFunctionMacro_UnitTests.cmake +++ b/test/core/TestingFunctionMacro_UnitTests.cmake @@ -58,6 +58,7 @@ include(TribitsETISupport) include(TribitsFindPythonInterp) include(TribitsStripQuotesFromStr) include(TribitsStandardizePaths) +include(TribitsAddOptionAndDefine) include(TribitsFilepathHelpers) include(TribitsGetVersionDate) include(TribitsTplFindIncludeDirsAndLibraries) @@ -409,6 +410,34 @@ function(unittest_tribits_strip_quotes_from_str) endfunction() +function(unittest_tribits_add_option_and_define) + + message("\n***") + message("*** Testing tribits_add_option_and_define()") + message("***\n") + + message("\nPass both option and define var, default YES") + tribits_add_option_and_define(optName1 macroOptName1 "doc" YES) + unittest_compare_const(optName1 "YES") + unittest_compare_const(macroOptName1 "ON") + unset(optName1 CACHE) + unset(macroOptName1 CACHE) + + message("\nPass both option and define var, default FALSE") + tribits_add_option_and_define(optName2 macroOptName2 "doc" FALSE) + unittest_compare_const(optName2 "FALSE") + unittest_compare_const(macroOptName2 "OFF") + unset(optName2 CACHE) + unset(macroOptName2 CACHE) + + message("\nPass only option var, not define var, default YES") + tribits_add_option_and_define(optName3 "" "doc" YES) + unittest_compare_const(optName3 "YES") + unset(optName3 CACHE) + +endfunction() + + function(unittest_tribits_get_raw_git_commit_utc_time) message("\n***") @@ -4587,6 +4616,7 @@ unittest_tribits_dir_is_basedir() unittest_tribits_get_dir_array_below_base_dir() unittest_tribits_misc() unittest_tribits_strip_quotes_from_str() +unittest_tribits_add_option_and_define() unittest_tribits_get_version_date_from_raw_git_commit_utc_time() unittest_tribits_get_raw_git_commit_utc_time() unittest_tribits_git_repo_sha1() @@ -4646,4 +4676,4 @@ message("*** Determine final result of all unit tests") message("***\n") # Pass in the number of expected tests that must pass! -unittest_final_result(703) +unittest_final_result(708) diff --git a/tribits/core/package_arch/TribitsAddOptionAndDefine.cmake b/tribits/core/package_arch/TribitsAddOptionAndDefine.cmake index 7ad7eff08..ea812e51c 100644 --- a/tribits/core/package_arch/TribitsAddOptionAndDefine.cmake +++ b/tribits/core/package_arch/TribitsAddOptionAndDefine.cmake @@ -37,12 +37,13 @@ # ************************************************************************ # @HEADER +include(TribitsPkgExportCacheVars) include(GlobalSet) # @MACRO: tribits_add_option_and_define() # -# Add an option and a define variable in one shot. +# Add an option and an optional macro define variable in one shot. # # Usage:: # @@ -59,7 +60,16 @@ include(GlobalSet) # #cmakedefine # # NOTE: This also calls `tribits_pkg_export_cache_var()`_ to export the -# variables ```` and ````. +# variables ```` and ````. This also +# requires that local variables with the same names of these cache variables +# not be assigned with a different value from these cache variables. If they +# are, then an error will occur later when these variables are read. +# +# NOTE: The define var name ```` can be empty "" in which +# case all logic related to ```` is skipped. (But in this +# case, it would be better to just call:: +# +# set( CACHE BOOL "") # macro(tribits_add_option_and_define USER_OPTION_NAME MACRO_DEFINE_NAME DOCSTRING DEFAULT_VALUE @@ -73,8 +83,8 @@ macro(tribits_add_option_and_define USER_OPTION_NAME MACRO_DEFINE_NAME global_set(${MACRO_DEFINE_NAME} OFF) endif() endif() - if (COMMAND tribits_pkg_export_cache_var) - tribits_pkg_export_cache_var(${USER_OPTION_NAME}) + tribits_pkg_export_cache_var(${USER_OPTION_NAME}) + if(NOT ${MACRO_DEFINE_NAME} STREQUAL "") tribits_pkg_export_cache_var(${MACRO_DEFINE_NAME}) endif() endmacro() diff --git a/tribits/core/package_arch/TribitsGeneralMacros.cmake b/tribits/core/package_arch/TribitsGeneralMacros.cmake index 971052abe..cf2a680cb 100644 --- a/tribits/core/package_arch/TribitsGeneralMacros.cmake +++ b/tribits/core/package_arch/TribitsGeneralMacros.cmake @@ -426,27 +426,3 @@ function(tribits_trace_file_processing TYPE_IN PROCESSING_TYPE_IN FILE_PATH) endif() endfunction() - - -# @MACRO: tribits_assert_cache_and_local_vars_same_value() -# -# Asset that a cache variable and a possible local variable (if it exists) -# have the same value. -# -# Usage:: -# -# tribits_assert_cache_and_local_vars_same_value() -# -# If the local var ```` and the cache var ```` -# both exist but have different values, then ``message(SEND_ERROR ...)`` is -# called with an informative error message. -# -macro(tribits_assert_cache_and_local_vars_same_value cacheVarName) - set(cacheVarValue "$CACHE{${cacheVarName}}") - set(localValue "${${cacheVarName}}") - if (NOT localValue STREQUAL cacheVarValue) - message_wrapper(SEND_ERROR "ERROR: The cache variable ${cacheVarName} with the" - " cache var value '${cacheVarValue}' is not the same value as the local" - " variable ${cacheVarName} with value '${localValue}'!") - endif() -endmacro() diff --git a/tribits/core/package_arch/TribitsPackageMacros.cmake b/tribits/core/package_arch/TribitsPackageMacros.cmake index 62a025a12..62d3791e1 100644 --- a/tribits/core/package_arch/TribitsPackageMacros.cmake +++ b/tribits/core/package_arch/TribitsPackageMacros.cmake @@ -52,6 +52,7 @@ include(RemoveGlobalDuplicates) include(TribitsGatherBuildTargets) include(TribitsAddOptionAndDefine) +include(TribitsPkgExportCacheVars) include(TribitsLibraryMacros) include(TribitsAddExecutable) include(TribitsAddExecutableAndTest) @@ -78,47 +79,6 @@ macro(tribits_define_linkage_vars PACKAGE_NAME_IN) endmacro() -# Macro that sets up data-structures for variables to be exported -# -macro(tribits_pkg_init_exported_vars PACKAGE_NAME_IN) - global_set(${PACKAGE_NAME_IN}_PKG_VARS_TO_EXPORT "") -endmacro() - - -# @MACRO: tribits_pkg_export_cache_var() -# -# Macro that registers a package-level cache var to be exported in the -# ``Config.cmake`` file -# -# Usage:: -# -# tribits_pkg_export_cache_var() -# -# where ```` must be the name of a cache variable (or an error -# will occur). -# -# NOTE: This will also export this variable to the -# ``Config.cmake`` file for every enabled subpackage (if this -# is called from a ``CMakeLists.txt`` file of a top-level package that has -# subpackages). That way, any top-level package cache vars are provided by -# any of the subpackages' ``Config.cmake`` files. -# -macro(tribits_pkg_export_cache_var cacheVarName) - if (DEFINED ${PACKAGE_NAME}_PKG_VARS_TO_EXPORT) - # Assert this is a cache var - get_property(cacheVarIsCacheVar CACHE ${cacheVarName} PROPERTY VALUE SET) - if (NOT cacheVarIsCacheVar) - message(SEND_ERROR - "ERROR: The variable ${cacheVarName} is NOT a cache var and cannot" - " be exported!") - endif() - # Add to the list of package cache vars to export - append_global_set(${PACKAGE_NAME}_PKG_VARS_TO_EXPORT - ${cacheVarName}) - endif() -endmacro() - - # Macro that defines variables that create global targets # macro(tribits_define_target_vars PARENT_PACKAGE_NAME_IN) diff --git a/tribits/core/package_arch/TribitsPkgExportCacheVars.cmake b/tribits/core/package_arch/TribitsPkgExportCacheVars.cmake new file mode 100644 index 000000000..6fb85e0ef --- /dev/null +++ b/tribits/core/package_arch/TribitsPkgExportCacheVars.cmake @@ -0,0 +1,130 @@ +# @HEADER +# ************************************************************************ +# +# TriBITS: Tribal Build, Integrate, and Test System +# Copyright 2013 Sandia Corporation +# +# Under the terms of Contract DE-AC04-94AL85000 with Sandia Corporation, +# the U.S. Government retains certain rights in this software. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# 3. Neither the name of the Corporation nor the names of the +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY SANDIA CORPORATION "AS IS" AND ANY +# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL SANDIA CORPORATION OR THE +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +# PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# ************************************************************************ +# @HEADER + + +# @MACRO: tribits_pkg_export_cache_var() +# +# Macro that registers a package-level cache var to be exported in the +# ``Config.cmake`` file +# +# Usage:: +# +# tribits_pkg_export_cache_var() +# +# where ```` must be the name of a cache variable (or an error +# will occur). +# +# NOTE: This will also export this variable to the +# ``Config.cmake`` file for every enabled subpackage (if this +# is called from a ``CMakeLists.txt`` file of a top-level package that has +# subpackages). That way, any top-level package cache vars are provided by +# any of the subpackages' ``Config.cmake`` files. +# +macro(tribits_pkg_export_cache_var cacheVarName) + if (DEFINED ${PACKAGE_NAME}_PKG_VARS_TO_EXPORT) + # Assert this is a cache var + get_property(cacheVarIsCacheVar CACHE ${cacheVarName} PROPERTY VALUE SET) + if (NOT cacheVarIsCacheVar) + message(SEND_ERROR + "ERROR: The variable ${cacheVarName} is NOT a cache var and cannot" + " be exported!") + endif() + # Add to the list of package cache vars to export + append_global_set(${PACKAGE_NAME}_PKG_VARS_TO_EXPORT + ${cacheVarName}) + endif() +endmacro() + + +# @MACRO: tribits_assert_cache_and_local_vars_same_value() +# +# Asset that a cache variable and a possible local variable (if it exists) +# have the same value. +# +# Usage:: +# +# tribits_assert_cache_and_local_vars_same_value() +# +# If the local var ```` and the cache var ```` +# both exist but have different values, then ``message(SEND_ERROR ...)`` is +# called with an informative error message. +# +macro(tribits_assert_cache_and_local_vars_same_value cacheVarName) + set(cacheVarValue "$CACHE{${cacheVarName}}") + set(localValue "${${cacheVarName}}") + if (NOT localValue STREQUAL cacheVarValue) + message_wrapper(SEND_ERROR "ERROR: The cache variable ${cacheVarName} with the" + " cache var value '${cacheVarValue}' is not the same value as the local" + " variable ${cacheVarName} with value '${localValue}'!") + endif() +endmacro() + + +# Function that sets up data-structures for package-level cache var to be +# exported +# +function(tribits_pkg_init_exported_vars PACKAGE_NAME_IN) + global_set(${PACKAGE_NAME_IN}_PKG_VARS_TO_EXPORT "") +endfunction() + + +# Function that injects set() statements for a package's exported cache vars into +# a string. +# +# This is used to create set() statements to be injected into a package's +# ``Config.cmake`` file. +# +function(tribits_pkg_append_set_commands_for_exported_vars packageName + configFileStrInOut + ) + set(configFileStr "${${configFileStrInOut}}") + if (NOT "${${packageName}_PARENT_PACKAGE}" STREQUAL "") + foreach(exportedCacheVar IN LISTS ${${packageName}_PARENT_PACKAGE}_PKG_VARS_TO_EXPORT) + tribits_assert_cache_and_local_vars_same_value(${exportedCacheVar}) + string(APPEND configFileStr + "set(${exportedCacheVar} \"${${exportedCacheVar}}\")\n") + endforeach() + endif() + foreach(exportedCacheVar IN LISTS ${packageName}_PKG_VARS_TO_EXPORT) + tribits_assert_cache_and_local_vars_same_value(${exportedCacheVar}) + string(APPEND configFileStr + "set(${exportedCacheVar} \"${${exportedCacheVar}}\")\n") + endforeach() + set(${configFileStrInOut} "${configFileStr}" PARENT_SCOPE) +endfunction() diff --git a/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake b/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake index 1da48dfce..cf3e9ef2f 100644 --- a/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake +++ b/tribits/core/package_arch/TribitsWriteClientExportFiles.cmake @@ -38,6 +38,7 @@ # @HEADER include(TribitsGeneralMacros) +include(TribitsPkgExportCacheVars) ### ### WARNING: See "NOTES TO DEVELOPERS" at the bottom of the file @@ -579,18 +580,7 @@ function(tribits_append_dependent_package_config_file_includes_and_enables packa # Put in set() statements for exported cache vars string(APPEND configFileStr "\n# Exported cache variables\n") - if (NOT "${${packageName}_PARENT_PACKAGE}" STREQUAL "") - foreach(exportedCacheVar IN LISTS ${${packageName}_PARENT_PACKAGE}_PKG_VARS_TO_EXPORT) - tribits_assert_cache_and_local_vars_same_value(${exportedCacheVar}) - string(APPEND configFileStr - "set(${exportedCacheVar} \"${${exportedCacheVar}}\")\n") - endforeach() - endif() - foreach(exportedCacheVar IN LISTS ${packageName}_PKG_VARS_TO_EXPORT) - tribits_assert_cache_and_local_vars_same_value(${exportedCacheVar}) - string(APPEND configFileStr - "set(${exportedCacheVar} \"${${exportedCacheVar}}\")\n") - endforeach() + tribits_pkg_append_set_commands_for_exported_vars(${packageName} configFileStr) # Include configurations of dependent packages string(APPEND configFileStr