Merge pull request #16 from rsps/redesign-safeguard-properties

Redesign safeguard properties

Author: aedart

CHANGELOG.md

@@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * Project's root `CMakeLists.txt`.
 * `dependencies.cmake` and `dev-dependencies.cmake` scripts.
 * `CPM.cmake` script that downloads specified version of [CPM](https://github.com/cpm-cmake/CPM.cmake).
-* `fail_in_source_build()`, `extract_value()`, `requires_arguments()` and `safeguard_properties()` utils functions, in `helpers.cmake`.
+* `fail_in_source_build()`, `extract_value()`, `requires_arguments()` and `safeguard_properties()` utils, in `helpers.cmake`.
 * `dump()`, `dd()` and `var_dump()` in `debug.cmake`.
 * ANSI utils, in `output.cmake`
 * `semver_parse()`, `write_version_file` and `version_from_file()` utils, in `version.cmake`.

cmake/rsp/helpers.cmake

@@ -90,45 +90,28 @@ if (NOT COMMAND "safeguard_properties")
 
     #! safeguard_properties : Invoke a "risky" callback whilst "safeguarding" properties
     #
-    # Function copies the values of the specified properties, invokes the callback, and
-    # restores the properties' values.
+    # Macro copies the values of the specified properties, invokes the callback, and
+    # restores the properties' original values.
     #
-    # Caution: This function does NOT prevent properties from being force-cached.
+    # Caution: This macro does NOT prevent properties from being force-cached.
     # Environment variables are NOT prevented changed.
     #
-    # Alternatively, consider using cmake's `block()`.
+    # Alternatively, consider using cmake's `block()` or a function with its own variable
+    # scope.
     #
     # @see https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#variables
     # @see https://cmake.org/cmake/help/latest/manual/cmake-language.7.html#environment-variables
     # @see https://cmake.org/cmake/help/latest/command/block.html#block
     #
-    # @param [CALLBACK <command>]           Risky command or macro to be invoked.
-    # @param [PROPERTIES <variable>...]     One or more properties to safeguard.
+    # @param <command> callback     Risky command or macro to be invoked.
+    # @param <list> properties      List of variable names - variables to safeguard from
+    #                               undesired value changes.
     #
-    # @return
-    #       [PROPERTIES <variable>...]      Restored properties
-    #
-    function(safeguard_properties)
-        set(options "") # N/A
-        set(oneValueArgs CALLBACK)
-        set(multiValueArgs PROPERTIES)
-
-        cmake_parse_arguments(INPUT "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
-        requires_arguments("CALLBACK;PROPERTIES" INPUT)
-
-        # ---------------------------------------------------------------------------------------------- #
-
-        # Abort if callback not defined
-        if (NOT COMMAND "${INPUT_CALLBACK}")
-            message(FATAL_ERROR "Callback \"${INPUT_CALLBACK}()\" does not exist")
-        endif ()
-
-        # ---------------------------------------------------------------------------------------------- #
-
+    macro(safeguard_properties callback properties)
         set(prefix "original_")
 
         # Copy each provided property
-        foreach (prop ${INPUT_PROPERTIES})
+        foreach (prop ${properties})
             message(VERBOSE "Safeguarding: ${prop}, original value: ${${prop}}")
 
             set("${prefix}${prop}" "${${prop}}")
@@ -137,17 +120,17 @@ if (NOT COMMAND "safeguard_properties")
         # ---------------------------------------------------------------------------------------------- #
 
         # Invoke the risky callback
-        message(VERBOSE "Invoking risky callback: ${INPUT_CALLBACK}")
-        cmake_language(CALL "${INPUT_CALLBACK}")
+        message(VERBOSE "Invoking risky callback: ${callback}")
+        cmake_language(CALL "${callback}")
 
         # ---------------------------------------------------------------------------------------------- #
 
         # Restore each provided property
-        foreach (prop ${INPUT_PROPERTIES})
+        foreach (prop ${properties})
             message(VERBOSE "Restoring: ${prop} from: ${${prop}}, to original value: ${${prefix}${prop}}")
 
-            # Ensure that property is set on parent scope
-            set("${prop}" "${${prefix}${prop}}" PARENT_SCOPE)
+            # Restore property's original value
+            set("${prop}" "${${prefix}${prop}}")
         endforeach ()
-    endfunction()
+    endmacro()
 endif ()

dependencies.cmake

@@ -4,14 +4,13 @@
 
 include_guard()
 
-function(install_dependencies)
+macro(install_dependencies)
     message(VERBOSE "Installing Dependencies for ${PROJECT_NAME}")
 
     # Avoid building tests for dependencies...
     set(BUILD_TESTING off)
 
     # Add dependencies here...
     message(VERBOSE "    N/A")
-
-endfunction()
-safeguard_properties(CALLBACK "install_dependencies" PROPERTIES BUILD_TESTING)
+endmacro()
+safeguard_properties("install_dependencies" "BUILD_TESTING")

dev-dependencies.cmake

@@ -7,14 +7,13 @@ include_guard()
 # Include regular dependencies
 include("dependencies.cmake")
 
-function(install_dev_dependencies)
+macro(install_dev_dependencies)
     message(VERBOSE "Installing Development Dependencies for ${PROJECT_NAME}")
 
     # Avoid building tests for dependencies...
     set(BUILD_TESTING off)
 
     # Add dev-dependencies here...
     message(VERBOSE "    N/A")
-
-endfunction()
-safeguard_properties(CALLBACK "install_dev_dependencies" PROPERTIES BUILD_TESTING)
+endmacro()
+safeguard_properties("install_dev_dependencies" "BUILD_TESTING")

tests/unit/helpers/safeguard_properties_test.cmake

@@ -1,5 +1,6 @@
 include("rsp/testing")
 include("rsp/helpers")
+include("rsp/debug")
 
 define_test_case(
     "Safeguard Properties Test"
@@ -13,18 +14,14 @@ function(can_safeguard_properties)
     set(c "ccc")
 
     function(risky_callback)
+        message(VERBOSE "risky callback invoked (function)")
+
         set(a "1" PARENT_SCOPE)
         set(b "2" PARENT_SCOPE)
         set(c "3" PARENT_SCOPE)
     endfunction()
 
-    safeguard_properties(
-        CALLBACK "risky_callback"
-        PROPERTIES
-            a
-            b
-            c
-    )
+    safeguard_properties("risky_callback" "a;b;c")
 
     # Debug
     #risky_callback()
@@ -33,3 +30,28 @@ function(can_safeguard_properties)
     assert_string_equals("bbb" ${b} MESSAGE "Property b was modified")
     assert_string_equals("ccc" ${c} MESSAGE "Property c was modified")
 endfunction()
+
+define_test("unguarded properties can be changed" "unguarded_properties_can_be_changed")
+function(unguarded_properties_can_be_changed)
+    set(a "aaa")
+    set(b "bbb")
+    set(c "ccc")
+
+    macro(risky_callback)
+        message(NOTICE "risky callback invoked (macro)")
+
+        set(a "1")
+        set(b "2")
+        set(c "3")
+    endmacro()
+
+    # Note: "c" is NOT safeguarded here
+    safeguard_properties("risky_callback" "a;b")
+
+    # Debug
+    #risky_callback()
+
+    assert_string_equals("aaa" ${a} MESSAGE "Property a was modified")
+    assert_string_equals("bbb" ${b} MESSAGE "Property b was modified")
+    assert_string_equals("3" ${c} MESSAGE "Property c SHOULD had been modified")
+endfunction()