From 772ca69f803a8f294d6e04f9e258becc35179233 Mon Sep 17 00:00:00 2001 From: James Johnston Date: Tue, 4 Aug 2015 23:47:38 -0400 Subject: [PATCH 1/3] get_filename_component: Tests now check for proper CACHE usage. The RunCMake.get_filename_component test was improved to assert that each test variable outputted by get_filename_component is or is not a cache variable, as per the particular test. Signed-off-by: James Johnston --- .../get_filename_component/CMakeLists.txt | 2 +- .../KnownComponents.cmake | 33 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Tests/RunCMake/get_filename_component/CMakeLists.txt b/Tests/RunCMake/get_filename_component/CMakeLists.txt index 12cd3c775..74b3ff8de 100644 --- a/Tests/RunCMake/get_filename_component/CMakeLists.txt +++ b/Tests/RunCMake/get_filename_component/CMakeLists.txt @@ -1,3 +1,3 @@ -cmake_minimum_required(VERSION 2.8.4) +cmake_minimum_required(VERSION 3.3) project(${RunCMake_TEST} NONE) include(${RunCMake_TEST}.cmake) diff --git a/Tests/RunCMake/get_filename_component/KnownComponents.cmake b/Tests/RunCMake/get_filename_component/KnownComponents.cmake index 9d7cf9079..3f20e1ac2 100644 --- a/Tests/RunCMake/get_filename_component/KnownComponents.cmake +++ b/Tests/RunCMake/get_filename_component/KnownComponents.cmake @@ -1,9 +1,11 @@ +# Assertion macro macro(check desc actual expect) if(NOT "x${actual}" STREQUAL "x${expect}") message(SEND_ERROR "${desc}: got \"${actual}\", not \"${expect}\"") endif() endmacro() +# General test of all component types given an absolute path. set(filename "/path/to/filename.ext.in") set(expect_DIRECTORY "/path/to") set(expect_NAME "filename.ext.in") @@ -13,14 +15,19 @@ set(expect_PATH "/path/to") foreach(c DIRECTORY NAME EXT NAME_WE PATH) get_filename_component(actual_${c} "${filename}" ${c}) check("${c}" "${actual_${c}}" "${expect_${c}}") + list(APPEND non_cache_vars actual_${c}) endforeach() +# Test Windows paths with DIRECTORY component and an absolute Windows path. get_filename_component(test_slashes "c:\\path\\to\\filename.ext.in" DIRECTORY) check("DIRECTORY from backslashes" "${test_slashes}" "c:/path/to") +list(APPEND non_cache_vars test_slashes) get_filename_component(test_winroot "c:\\filename.ext.in" DIRECTORY) check("DIRECTORY in windows root" "${test_winroot}" "c:/") +list(APPEND non_cache_vars test_winroot) +# Test finding absolute paths. get_filename_component(test_absolute "/path/to/a/../filename.ext.in" ABSOLUTE) check("ABSOLUTE" "${test_absolute}" "/path/to/filename.ext.in") @@ -29,10 +36,36 @@ check("ABSOLUTE .. in root" "${test_absolute}" "/path/to/filename.ext.in") get_filename_component(test_absolute "c:/../path/to/filename.ext.in" ABSOLUTE) check("ABSOLUTE .. in windows root" "${test_absolute}" "c:/path/to/filename.ext.in") +list(APPEND non_cache_vars test_absolute) + +# Test CACHE parameter for most component types. get_filename_component(test_cache "/path/to/filename.ext.in" DIRECTORY CACHE) check("CACHE 1" "${test_cache}" "/path/to") +# Make sure that the existing CACHE entry from previous is honored: get_filename_component(test_cache "/path/to/other/filename.ext.in" DIRECTORY CACHE) check("CACHE 2" "${test_cache}" "/path/to") unset(test_cache CACHE) get_filename_component(test_cache "/path/to/other/filename.ext.in" DIRECTORY CACHE) check("CACHE 3" "${test_cache}" "/path/to/other") + +list(APPEND cache_vars test_cache) + +# Test that ONLY the expected cache variables were created. +get_cmake_property(current_cache_vars CACHE_VARIABLES) +get_cmake_property(current_vars VARIABLES) + +foreach(thisVar ${cache_vars}) + if(NOT thisVar IN_LIST current_cache_vars) + message(SEND_ERROR "${thisVar} expected in cache but was not found.") + endif() +endforeach() + +foreach(thisVar ${non_cache_vars}) + if(thisVar IN_LIST current_cache_vars) + message(SEND_ERROR "${thisVar} unexpectedly found in cache.") + endif() + if(NOT thisVar IN_LIST current_vars) + # Catch likely typo when appending to non_cache_vars: + message(SEND_ERROR "${thisVar} not found in regular variable list.") + endif() +endforeach() From 38ed5866ed212effe0217f193f728ee54ea7378b Mon Sep 17 00:00:00 2001 From: James Johnston Date: Wed, 5 Aug 2015 00:30:58 -0400 Subject: [PATCH 2/3] get_filename_component: Added initial tests for PROGRAM component. The RunCMake.get_filename_component test now tests basic functionality of the PROGRAM component argument of get_filename_component. Signed-off-by: James Johnston --- .../get_filename_component/KnownComponents.cmake | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Tests/RunCMake/get_filename_component/KnownComponents.cmake b/Tests/RunCMake/get_filename_component/KnownComponents.cmake index 3f20e1ac2..1532792e4 100644 --- a/Tests/RunCMake/get_filename_component/KnownComponents.cmake +++ b/Tests/RunCMake/get_filename_component/KnownComponents.cmake @@ -38,6 +38,18 @@ check("ABSOLUTE .. in windows root" "${test_absolute}" "c:/path/to/filename.ext. list(APPEND non_cache_vars test_absolute) +# Test the PROGRAM component type. +get_filename_component(test_program_name "/ arg1 arg2" PROGRAM) +check("PROGRAM with no args output" "${test_program_name}" "/") + +get_filename_component(test_program_name "/ arg1 arg2" PROGRAM + PROGRAM_ARGS test_program_args) +check("PROGRAM with args output: name" "${test_program_name}" "/") +check("PROGRAM with args output: args" "${test_program_args}" " arg1 arg2") + +list(APPEND non_cache_vars test_program_name) +list(APPEND non_cache_vars test_program_args) + # Test CACHE parameter for most component types. get_filename_component(test_cache "/path/to/filename.ext.in" DIRECTORY CACHE) check("CACHE 1" "${test_cache}" "/path/to") From d035e9687a2d28e2f64ec3a316f8abea57e49d63 Mon Sep 17 00:00:00 2001 From: James Johnston Date: Wed, 5 Aug 2015 00:33:59 -0400 Subject: [PATCH 3/3] get_filename_component: Fix bug where CACHE was ignored. If PROGRAM_ARGS is provided to get_filename_component, fix bug where the command failed to honor the CACHE argument. Added test cases to RunCMake.get_filename_component that fail when the bug is not fixed to prevent regressions. Signed-off-by: James Johnston --- Source/cmGetFilenameComponentCommand.cxx | 4 +-- .../KnownComponents.cmake | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Source/cmGetFilenameComponentCommand.cxx b/Source/cmGetFilenameComponentCommand.cxx index 67f9f2dc1..13a9afbe7 100644 --- a/Source/cmGetFilenameComponentCommand.cxx +++ b/Source/cmGetFilenameComponentCommand.cxx @@ -24,7 +24,7 @@ bool cmGetFilenameComponentCommand // Check and see if the value has been stored in the cache // already, if so use that value - if(args.size() == 4 && args[3] == "CACHE") + if(args.size() >= 4 && args[args.size() - 1] == "CACHE") { const char* cacheValue = this->Makefile->GetDefinition(args[0]); if(cacheValue && !cmSystemTools::IsNOTFOUND(cacheValue)) @@ -111,7 +111,7 @@ bool cmGetFilenameComponentCommand return false; } - if(args.size() == 4 && args[3] == "CACHE") + if(args.size() >= 4 && args[args.size() - 1] == "CACHE") { if(!programArgs.empty() && !storeArgs.empty()) { diff --git a/Tests/RunCMake/get_filename_component/KnownComponents.cmake b/Tests/RunCMake/get_filename_component/KnownComponents.cmake index 1532792e4..386109f56 100644 --- a/Tests/RunCMake/get_filename_component/KnownComponents.cmake +++ b/Tests/RunCMake/get_filename_component/KnownComponents.cmake @@ -62,6 +62,38 @@ check("CACHE 3" "${test_cache}" "/path/to/other") list(APPEND cache_vars test_cache) +# Test the PROGRAM component type with CACHE specified. + +# 1. Make sure it makes a cache variable in the first place for basic usage: +get_filename_component(test_cache_program_name_1 "/ arg1 arg2" PROGRAM CACHE) +check("PROGRAM CACHE 1 with no args output" "${test_cache_program_name_1}" "/") +list(APPEND cache_vars test_cache_program_name_1) + +# 2. Set some existing cache variables & make sure the function returns them: +set(test_cache_program_name_2 DummyProgramName CACHE FILEPATH "") +get_filename_component(test_cache_program_name_2 "/ arg1 arg2" PROGRAM CACHE) +check("PROGRAM CACHE 2 with no args output" "${test_cache_program_name_2}" + "DummyProgramName") +list(APPEND cache_vars test_cache_program_name_2) + +# 3. Now test basic usage when PROGRAM_ARGS is used: +get_filename_component(test_cache_program_name_3 "/ arg1 arg2" PROGRAM + PROGRAM_ARGS test_cache_program_args_3 CACHE) +check("PROGRAM CACHE 3 name" "${test_cache_program_name_3}" "/") +check("PROGRAM CACHE 3 args" "${test_cache_program_args_3}" " arg1 arg2") +list(APPEND cache_vars test_cache_program_name_3) +list(APPEND cache_vars test_cache_program_args_3) + +# 4. Test that existing cache variables are returned when PROGRAM_ARGS is used: +set(test_cache_program_name_4 DummyPgm CACHE FILEPATH "") +set(test_cache_program_args_4 DummyArgs CACHE STRING "") +get_filename_component(test_cache_program_name_4 "/ arg1 arg2" PROGRAM + PROGRAM_ARGS test_cache_program_args_4 CACHE) +check("PROGRAM CACHE 4 name" "${test_cache_program_name_4}" "DummyPgm") +check("PROGRAM CACHE 4 args" "${test_cache_program_args_4}" "DummyArgs") +list(APPEND cache_vars test_cache_program_name_4) +list(APPEND cache_vars test_cache_program_name_4) + # Test that ONLY the expected cache variables were created. get_cmake_property(current_cache_vars CACHE_VARIABLES) get_cmake_property(current_vars VARIABLES)