From 241f83c32edce990dcc17ab35ac74abd1b38c9a7 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Thu, 9 Jun 2022 18:50:23 -0600 Subject: [PATCH 1/5] Address style, formatting, and typos from PR #485 (#483) --- test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt | 6 +++--- tribits/core/utils/TribitsParseArgumentsHelpers.cmake | 2 +- tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt b/test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt index 15e465a5a..541321c97 100644 --- a/test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt +++ b/test/ctest_driver/TribitsExampleMetaProject/CMakeLists.txt @@ -98,16 +98,16 @@ set(TribitsExampleMetaProject_COMMON_CONFIG_ARGS ########################################################################################## - set(cdash_build_url_expected_regex +set(cdash_build_url_expected_regex "https://testing[.]sandia[.]gov/cdash/index[.]php[?]project=TribitsExampleMetaProject&filtercount=3&showfilters=1&filtercombine=and&field1=site&compare1=61&value1=CustomSite&field2=buildname&compare2=61&value2=CTestDriver_TribitsExMetaProj_clone_default_branch_remote&field3=buildstamp&compare3=61&value3=[0-9][0-9][0-9][0-9][0-9][0-9][0-9][0-9]-[0-9][0-9][0-9][0-9]-Continuous" ) # NOTE: Above, we have to repeat [0-9] 8 times and 4 times for the regex for # the number of digits in the buildstarttime. CMake regex does not support # \d{8}-\d{4} :-( - set(cdash_revisions_builds_url_expected_regex +set(cdash_revisions_builds_url_expected_regex "https://testing[.]sandia[.]gov/cdash/index.php[?]project=TribitsExampleMetaProject&filtercount=1&showfilters=1&field1=revision&compare1=61&value1=863461e3035d24c632e175c087761e83db28bdc3") - set(cdash_revisions_nonpassing_tests_expected_regex +set(cdash_revisions_nonpassing_tests_expected_regex "https://testing[.]sandia[.]gov/cdash/queryTests.php[?]project=TribitsExampleMetaProject&filtercount=2&showfilters=1&filtercombine=and&field1=revision&compare1=61&value1=863461e3035d24c632e175c087761e83db28bdc3&field2=status&compare2=62&value2=passed" ) diff --git a/tribits/core/utils/TribitsParseArgumentsHelpers.cmake b/tribits/core/utils/TribitsParseArgumentsHelpers.cmake index c4cd79fdf..75327668f 100644 --- a/tribits/core/utils/TribitsParseArgumentsHelpers.cmake +++ b/tribits/core/utils/TribitsParseArgumentsHelpers.cmake @@ -84,7 +84,7 @@ endfunction() # @MACRO: tribits_assert_parse_arg_one_or_more_values() # -# Assert that a set of parse argument have at least one value +# Assert that a set of parse arguments have at least one value # # Usage:: # diff --git a/tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake b/tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake index 04166830c..c6904f119 100644 --- a/tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake +++ b/tribits/ctest_driver/TribitsGetCDashUrlsInsideCTestS.cmake @@ -458,8 +458,8 @@ function(tribits_get_cdash_site_from_drop_site_and_location) tribits_assert_parse_arg_one_value(PREFIX CTEST_DROP_LOCATION) tribits_assert_parse_arg_one_value(PREFIX CDASH_SITE_URL_OUT) # Get the full CDash site from parts - string(FIND "${PREFIX_CTEST_DROP_LOCATION}" "?" endOfSubmitPhpIdx) - string(SUBSTRING "${PREFIX_CTEST_DROP_LOCATION}" 0 ${endOfSubmitPhpIdx} submitPhpPart) + string(FIND "${PREFIX_CTEST_DROP_LOCATION}" "?" beginningOfQueryString) + string(SUBSTRING "${PREFIX_CTEST_DROP_LOCATION}" 0 ${beginningOfQueryString} submitPhpPart) string(REPLACE "/submit.php" "" endCDashUrl "${submitPhpPart}") set(cdashSiteUrl "${PREFIX_CTEST_DROP_SITE}${endCDashUrl}") set(${PREFIX_CDASH_SITE_URL_OUT} "https://${cdashSiteUrl}" PARENT_SCOPE) From 4cf60d7ea61821d179abc5896de9dbfdf0b31eb1 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Fri, 10 Jun 2022 09:15:16 -0600 Subject: [PATCH 2/5] Remove quotes from --pretty argument value (#483) In this case, there are no spaces in the --pretty=format: argument so we don't need the quotes and therefore don't need to suffer the complexity of removing them after the fact. But note the case I copied and pasted this from in tribits_generate_single_repo_version_string() still requires them because that format spec has spaces. --- .../core/package_arch/TribitsGitRepoVersionInfo.cmake | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake b/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake index 9d4943dc8..8bc23895b 100644 --- a/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake +++ b/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake @@ -83,24 +83,17 @@ function(tribits_git_repo_sha1 gitRepoDir gitRepoSha1Out) set(gitRepoSha1 "") if (failureMsg STREQUAL "") execute_process( - COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%H" + COMMAND ${GIT_EXECUTABLE} log -1 "--pretty=format:%H" WORKING_DIRECTORY ${gitRepoDir} RESULT_VARIABLE gitCmndRtn OUTPUT_VARIABLE gitCmndOutput ) - # NOTE: Above we have to add quotes '"' or CMake will not accept the - # command. However, git will put those quotes in the output so we have to - # strip them out below! if (NOT gitCmndRtn STREQUAL 0) set(failureMsg "ERROR: ${GIT_EXECUTABLE} command returned ${gitCmndRtn}!=0 for repo ${gitRepoDir}!") else() - # Strip the quotes off :-( - string(LENGTH "${gitCmndOutput}" gitCmndOutputLen) - math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2") - string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep} gitRepoSha1) + set(gitRepoSha1 "${gitCmndOutput}") endif() endif() - # ToDo: Factor above out into its own function! if (NOT failureMsg STREQUAL "" AND PARSE_FAILURE_MESSAGE_OUT STREQUAL "") message(FATAL_ERROR "${failureMsg}") From f601d450511992fc211806ca45b449c9c336ded4 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Fri, 10 Jun 2022 09:37:31 -0600 Subject: [PATCH 3/5] Switch over to use find_program(Git) (#483) This was caught in review by @KyleFromKitware. Turns out that TriBITS was using find_program(Git) everywhere else already. --- test/core/TestingFunctionMacro_UnitTests.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/TestingFunctionMacro_UnitTests.cmake b/test/core/TestingFunctionMacro_UnitTests.cmake index 8673ad8d1..352d79a2a 100644 --- a/test/core/TestingFunctionMacro_UnitTests.cmake +++ b/test/core/TestingFunctionMacro_UnitTests.cmake @@ -494,7 +494,7 @@ function(unittest_tribits_git_repo_sha1) message("*** Testing tribits_git_repo_sha1()") message("***\n") - find_program(GIT_EXECUTABLE ${GIT_NAME}) + find_package(Git REQUIRED) set(tribitsProjDir "${${PROJECT_NAME}_TRIBITS_DIR}/..") From 9c5c186f71cdce60481838c41b89523866cac8d0 Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Fri, 10 Jun 2022 10:18:07 -0600 Subject: [PATCH 4/5] Call cmake_policy() at file-scope and not in functions (#485) It was pointed out by @KyleFromKitware in the review of PR #485 that calling cmake_policy() in a function will change a policy in the calling scope. NOTE: The function() command does not create a new policy scope but does create a new variable scope. Just the opposite, include() creates a new policy scope but not a new variable scope. --- .../core/package_arch/TribitsAddAdvancedTest.cmake | 13 +++++++------ tribits/core/package_arch/TribitsAddTest.cmake | 4 ++-- .../TribitsExternalPackageWriteConfigFile.cmake | 4 ++-- tribits/core/utils/UnitTestHelpers.cmake | 9 ++------- tribits/ctest_driver/TribitsReadTagFile.cmake | 4 +++- 5 files changed, 16 insertions(+), 18 deletions(-) diff --git a/tribits/core/package_arch/TribitsAddAdvancedTest.cmake b/tribits/core/package_arch/TribitsAddAdvancedTest.cmake index 10e564a2c..98500ecde 100644 --- a/tribits/core/package_arch/TribitsAddAdvancedTest.cmake +++ b/tribits/core/package_arch/TribitsAddAdvancedTest.cmake @@ -46,7 +46,13 @@ include(AppendStringVar) include(PrintVar) -# +# Avoid quoted strings lookup variables +cmake_policy(SET CMP0054 NEW) +# NOTE: For some reason, setting this policy at the top level with TriBITS +# in TribitsCMakePolices.cmake does not affect this function. Therefore, I +# have to set it again here. + + # @FUNCTION: tribits_add_advanced_test() # # Function that creates an advanced test defined by stringing together one or @@ -900,11 +906,6 @@ function(tribits_add_advanced_test TEST_NAME_IN) set(TEST_NAME ${TEST_NAME_IN}) endif() - # Avoid quoted strings lookup variables - cmake_policy(SET CMP0054 NEW) - # NOTE: For some reason, setting this policy at the top level with TriBITS - # in TribitsCMakePolices.cmake does not affect this function. Therefore, I - # have to set it again here. # # A) Parse the overall arguments and figure out how many tests diff --git a/tribits/core/package_arch/TribitsAddTest.cmake b/tribits/core/package_arch/TribitsAddTest.cmake index d8355783a..3be4c329d 100644 --- a/tribits/core/package_arch/TribitsAddTest.cmake +++ b/tribits/core/package_arch/TribitsAddTest.cmake @@ -39,6 +39,8 @@ include(TribitsAddTestHelpers) +cmake_policy(SET CMP0007 NEW) # Don't ignore empty list elements + # @FUNCTION: tribits_add_test() # @@ -815,8 +817,6 @@ function(tribits_add_test EXE_NAME) message("TRIBITS_ADD_TEST: ${EXE_NAME} ${ARGN}") endif() - cmake_policy(SET CMP0007 NEW) # Don't ignore empty list elements - global_set(TRIBITS_ADD_TEST_ADD_TEST_INPUT) global_set(TRIBITS_SET_TEST_PROPERTIES_INPUT) global_set(MESSAGE_WRAPPER_INPUT) diff --git a/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake b/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake index d32a0ca5c..0ab3cc48b 100644 --- a/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake +++ b/tribits/core/package_arch/TribitsExternalPackageWriteConfigFile.cmake @@ -41,6 +41,8 @@ include(TribitsGeneralMacros) include(MessageWrapper) +cmake_policy(SET CMP0057 NEW) # Support if ( ... IN_LIST ... ) + # @FUNCTION: tribits_external_package_write_config_file() # @@ -458,7 +460,6 @@ function(tribits_external_package_process_libraries_list_library_entry tplName libentry libEntryType libTargetsInOut lastLibProcessedInOut configFileStrInOut ) - cmake_policy(SET CMP0057 NEW) # Support if ( ... IN_LIST ... ) # Set local vars for inout vars set(libTargets ${${libTargetsInOut}}) set(lastLibProcessed ${${lastLibProcessedInOut}}) @@ -644,7 +645,6 @@ endfunction() function(tribits_external_package_append_upstream_target_link_libraries_get_name_and_vis upstreamTplDepEntry upstreamTplDepNameOut upstreamTplDepVisOut ) - cmake_policy(SET CMP0057 NEW) # Split on ':' to get [:] string(REPLACE ":" ";" upstreamTplAndVisList "${upstreamTplDepEntry}") list(LENGTH upstreamTplAndVisList upstreamTplAndVisListLen) diff --git a/tribits/core/utils/UnitTestHelpers.cmake b/tribits/core/utils/UnitTestHelpers.cmake index 912efb0a5..0fecf4770 100644 --- a/tribits/core/utils/UnitTestHelpers.cmake +++ b/tribits/core/utils/UnitTestHelpers.cmake @@ -40,6 +40,8 @@ include(CMakeParseArguments) include(GlobalSet) +cmake_policy(SET CMP0007 NEW) + # @MACRO: unittest_initialize_vars() # @@ -159,11 +161,6 @@ endfunction() # function(unittest_string_block_compare stringVar stringExpected) - # Don't ignore empty elements in list() operations (they are very important - # here) - cmake_policy(PUSH) - cmake_policy(SET CMP0007 NEW) - message("\nCheck: ${stringVar} equals expected string:") math( EXPR NUMRUN ${UNITTEST_OVERALL_NUMRUN}+1 ) @@ -233,8 +230,6 @@ function(unittest_string_block_compare stringVar stringExpected) global_set(UNITTEST_OVERALL_NUMPASSED ${NUMPASSED}) endif() - cmake_policy(POP) - endfunction() diff --git a/tribits/ctest_driver/TribitsReadTagFile.cmake b/tribits/ctest_driver/TribitsReadTagFile.cmake index 5400b55f1..3073fb520 100644 --- a/tribits/ctest_driver/TribitsReadTagFile.cmake +++ b/tribits/ctest_driver/TribitsReadTagFile.cmake @@ -1,3 +1,6 @@ +cmake_policy(SET CMP0007 NEW) + + # @FUNCTION: tribits_read_ctest_tag_file() # # Read in the /Testing/TAG file contents @@ -10,7 +13,6 @@ function(tribits_read_ctest_tag_file tagFileIn buildStartTimeOut cdashGroupOut cdashModelOut ) - cmake_policy(SET CMP0007 NEW) file(READ "${tagFileIn}" tagFileStr) string(REPLACE "\n" ";" tagFileStrList "${tagFileStr}") list(GET tagFileStrList 0 buildStartTime) From e82c4544fbd50769b85bab45cb98ef009ca4643c Mon Sep 17 00:00:00 2001 From: "Roscoe A. Bartlett" Date: Fri, 10 Jun 2022 11:08:38 -0600 Subject: [PATCH 5/5] Remove internal quotes for git log --pretty=format: strings (#485) With newer CMake version, you can just quote the entire argument: "--pretty=format:" where may have spaces and everything seems to work correctly. --- .../TribitsGitRepoVersionInfo.cmake | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake b/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake index 8bc23895b..8dbe8f519 100644 --- a/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake +++ b/tribits/core/package_arch/TribitsGitRepoVersionInfo.cmake @@ -116,31 +116,24 @@ function(tribits_generate_single_repo_version_string gitRepoDir # A) Get the basic version info. execute_process( - COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%h [%ad] <%ae>" + COMMAND ${GIT_EXECUTABLE} log -1 "--pretty=format:%h [%ad] <%ae>" WORKING_DIRECTORY ${gitRepoDir} RESULT_VARIABLE gitCmndRtn OUTPUT_VARIABLE gitCmndOutput ) - # NOTE: Above we have to add quotes '"' or CMake will not accept the - # command. However, git will put those quotes in the output so we have to - # strip them out later :-( if (NOT gitCmndRtn STREQUAL 0) message(FATAL_ERROR "ERROR, ${GIT_EXECUTABLE} command returned ${gitCmndRtn}!=0" " for repo ${gitRepoDir}!") set(gitVersionLine "Error, could not get version info!") else() - # Strip the quotes off :-( - string(LENGTH "${gitCmndOutput}" gitCmndOutputLen) - math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2") - string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep} - gitVersionLine) + set(gitVersionLine "${gitCmndOutput}") endif() # B) Get the first 80 chars of the summary message for more info execute_process( - COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:"%s" + COMMAND ${GIT_EXECUTABLE} log -1 --pretty=format:%s WORKING_DIRECTORY ${gitRepoDir} RESULT_VARIABLE gitCmndRtn OUTPUT_VARIABLE gitCmndOutput @@ -151,25 +144,16 @@ function(tribits_generate_single_repo_version_string gitRepoDir " for extra repo ${gitRepoDir}!") set(gitSummaryStr "Error, could not get version summary!") else() - # Strip off quotes and quote the 80 char string set(maxSummaryLen 80) - math(EXPR maxSummaryLen_PLUS_2 "${maxSummaryLen}+2") - string(LENGTH "${gitCmndOutput}" gitCmndOutputLen) - math(EXPR outputNumCharsToKeep "${gitCmndOutputLen}-2") - string(SUBSTRING "${gitCmndOutput}" 1 ${outputNumCharsToKeep} - gitCmndOutputStripped) - if (gitCmndOutputLen GREATER ${maxSummaryLen_PLUS_2}) - string(SUBSTRING "${gitCmndOutputStripped}" 0 ${maxSummaryLen} - gitSummaryStr) - else() - set(gitSummaryStr "${gitCmndOutputStripped}") - endif() + string(SUBSTRING "${gitCmndOutput}" 0 ${maxSummaryLen} gitSummaryStr) endif() set(${repoVersionStringOut} "${gitVersionLine}\n${gitSummaryStr}" PARENT_SCOPE) endfunction() +# NOTE: Above, it is fine if ${maxSummaryLen} > len(${gitCmndOutput}) as +# string(SUBSTRING ...) will just shorten this to the lenght of the string. function(tribits_assert_git_executable)