Merge branch 'TRILFRAME-120-A' into 'master'

Trilframe-136: Merging in TRILFRAME-120

See merge request trilinos-devops-consolidation/code/SetProgramOptions!17

Author: William McLendon

CHANGELOG.md

@@ -17,22 +17,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 #### Todo (for Unreleased)
 -->
 
-## [0.3.1] <!-- YYYY-MM-DD --> [Unreleased]
+## [0.4.0] <!-- YYYY-MM-DD --> [Unreleased]
 #### Added
 #### Changed
 - Changes to `opt-set-cmake-var` behaviour:
   - Trigger a WARNING if `PARENT_SCOPE` and a _TYPE_ is provided since CMake
     requires TYPED vars to be CACHE vars and `PARENT_SCOPE` will force the
     var to be considered a non CACHE var that would store the parameters
-    before `PARENT_SCOPE` in the call as _list_ entries.
+    before `PARENT_SCOPE` in the call as _list_ entries. (TRILFRAME-128)
   - Trigger an exception if `PARENT_SCOPE` and `FORCE` are both provided since
     `FORCE` is only valid for CACHE variables but `PARENT_SCOPE` is only valid
     for non-CACHE vars (and as noted above will _force_ the `set()` operation
     to be a non-CACHE var which then triggers a CMake error because `FORCE` is
-    invalid to that form of `set()`.
+    invalid to that form of `set()`. (TRILFRAME-128)
   - **bash** generated options that have a TYPE and `PARENT_SCOPE` are processed
     to generate the same _list_ string that would be generated by the matching
-    `set()` operation (with the warning).
+    `set()` operation (with the warning). (TRILFRAME-128)
+  - For **bash** generation, trigger a WARNING and DO NOT generate the `-D` CLI option 
+    if `opt-set-cmake-var` without `FORCE` is called on a CMake Cache variable that has 
+    already been set. (TRILFRAME-120).
+  - For **bash** generation, trigger a WARNING and DO NOT generate the `-D` CLI option 
+    if `opt-set-cmake-var` is called on a non-cache CMake variable. (TRILFRAME-136)
+  - For **bash** generation, always add a _TYPE_ to the `-D` CLI option. (TRILFRAME-136)
 - Revert(ish) the modifications from MR12 which removed the error when generating
   BASH output if there is an unhandled CMake variable left hanging around.
   Rather than make this an unavoidable error though, ExpandVarsInText now inherits
@@ -44,6 +50,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 #### Deprecated
 #### Removed
 #### Fixed
+- Bash generation handling of the FORCE flag. See TRILFRAME-120 for details.
 #### Security
 #### Internal
 - Add test to verify that `opt-remove` also works to remove variables added
@@ -91,5 +98,3 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 #### Added
 - Initial version release. From now on changes should be noted in the
   CHANGELOG.
-
-

SetProgramOptions.wpr

@@ -90,8 +90,7 @@ proj.launch-config = {loc('../../coding/pyExampleLib/configparser/configparser-e
         'ustom',
         (u'',
          'launch-5Gyw4cvA0PytPLLy'))}
-proj.main-file = loc('examples/example-01.py')
+proj.main-file = loc('exec-example-SetProgramOptions.py')
 testing.auto-test-file-specs = (('regex',
-                                 'setprogramoptions\\\\unittests\\\\test_.*'\
-                                 '\\.py'),)
+                                 'unittests\\\\test_.*\\.py'),)
 testing.test-framework = {None: ':internal pytest'}

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "setprogramoptions"
-version = "0.3.1"
+version = "0.4.0"
 description = "Program options helper."
 authors = [
     "William McLendon <wcmclen@sandia.gov>"

src/setprogramoptions/SetProgramOptions.py

@@ -109,7 +109,6 @@ class ExpandVarsInText(ExceptionControl):
 
     def __init__(self):
         self.exception_control_level = 4
-        self.exception_control_compact_warnings = True
 
     class VariableFieldData(object):
         """
@@ -526,6 +525,7 @@ def _gen_option_entry(self, option_entry: dict, generator='bash') -> Union[str,
 
                 # Update the var formatter's ECL to match the current value.
                 self._var_formatter.exception_control_level = self.exception_control_level
+                self._var_formatter.exception_control_compact_warnings = self.exception_control_compact_warnings
 
                 # format the value
                 formatter = self._var_formatter

src/setprogramoptions/SetProgramOptionsCMake.py

@@ -20,8 +20,21 @@
 ``-DVAR_NAME:BOOL=ON`` using the ``SetProgramOptions`` method ``gen_option_list``
 with the ``bash`` generator.
 
-In the case of bash command entries the ``FORCE`` and ``PARENT_SCOPE`` optional
-parameters are ignored.
+When using the BASH generator to generate command line arguments, CMake
+uses the syntax ``-D<VARNAME>[:<TYPE>]=<VALUE>``. The ``TYPE`` field is optional
+and if left out CMake will default to a ``STRING`` type. Further, all CMake
+variables set via the command line using ``-D`` will be CACHE variables and each
+``-D`` operation should be considered a FORCE operation too. For example,
+``-DFOO:STRING=BAR`` is roughly equivalent to the CMake command:
+``set(FOO CACHE STRING "docstring" FORCE)``.
+
+The ``PARENT_SCOPE`` option applies only to non-cache variables and its presence
+will instruct CMake to make that variable non-cache. Care should be taken when
+using ``PARENT_SCOPE`` as combining it with the usual CACHE operations results
+in CMake creating a non-cached variable whose contents are the list containing
+``<varname>;CACHE;<type>;doc string``. As a result, the BASH generator issues
+warnings with no generated command line arguments when either 1. ``PARENT_SCOPE``
+OR 2. solely a variable name AND variable value are passed in to `opt-set-cmake-var`.
 
 See CMake documentation on the `set() <https://cmake.org/cmake/help/latest/command/set.html>`_
 command for more information on how fragment file entries are generated.
@@ -31,8 +44,10 @@
 
 :Authors:
     - William C. McLendon III <wcmclen@sandia.gov>
+    - Evan Harvey <eharvey@sandia.gov>
 """
 from __future__ import print_function
+from enum import Enum
 
 #import inspect
 #from pathlib import Path
@@ -73,7 +88,6 @@ class ExpandVarsInTextCMake(ExpandVarsInText):
 
     def __init__(self):
         self.exception_control_level = 3
-        self.exception_control_compact_warnings = True
 
     def _fieldhandler_BASH_CMAKE(self, field):
         """
@@ -120,7 +134,10 @@ def _fieldhandler_CMAKE_FRAGMENT_CMAKE(self, field):
 #   M A I N   C L A S S
 # ===============================
 
-
+class VarType(Enum):
+    """Enumeration used to check for CMake variable types in SPOC."""
+    CACHE = 1
+    NON_CACHE = 2
 
 class SetProgramOptionsCMake(SetProgramOptions):
     """Extends SetProgramOptions to add in CMake option support.
@@ -183,7 +200,7 @@ def _program_option_handler_opt_set_cmake_fragment(self, params: list, value: st
         """
         return None
 
-    def _program_option_handler_opt_set_cmake_var_bash(self, params, value) -> str:
+    def _program_option_handler_opt_set_cmake_var_bash(self, params: list, value: str) -> str:
         """
         Line-item generator for ``opt-set-cmake-var`` entries when the *generator*
         is set to ``bash``.
@@ -196,23 +213,50 @@ def _program_option_handler_opt_set_cmake_var_bash(self, params, value) -> str:
             side-effects since :py:meth:`setprogramoptions.SetProgramOptions._gen_option_entry`
             performs a deep-copy of these parameters prior to calling this.
             Any changes we make are ephemeral.
+
+        Args:
+            params (list): The parameters of the operation.
+            value (str): The value of the option that is being assigned.
+
+        Raises:
+            ValueError: This can potentially raise a ``ValueError`` if
+                ``exception_control_level`` is set to 5 if there are
+                operations that are skipped in Bash generation. If ``ecl``
+                is less than 5 then warnings are generated to note the
+                exclusion.
         """
         varname = params[0]
         params = params[1 : 4]
         param_opts = self._helper_opt_set_cmake_var_parse_parameters(params)
 
-        params = ["-D", varname]
+        # Type-1 (non-cached / PARENT_SCOPE / non-typed) entries should not be
+        # written to the set of Bash parameters.
+        if param_opts['VARIANT'] == VarType.NON_CACHE:
+            msg = f"bash generator - `{varname}={value}` skipped because"
+            msg += f" it is a non-cached (type-1) operation."
+            msg += f" To generate a bash arg for this consider adding FORCE or a TYPE"
+            msg += f" and remove PARENT_SCOPE if it exists."
+            self.exception_control_event("WARNING", ValueError, message=msg)
+            return None
+
+        # If varname has already been assigned and this assignment
+        # does not include FORCE then we should skip adding it to the
+        # set of command line options.
+        if varname in self._var_formatter_cache and not param_opts['FORCE']:
+            msg = f"bash generator - `{varname}={value}` skipped because"
+            msg += f" CACHE var `{varname}` is already set and CMake requires"
+            msg += f" FORCE to be set to change the value."
+            self.exception_control_event("WARNING", ValueError, message=msg)
+            return None
 
-        if param_opts['VARIANT'] == 1:
-            # if PARENT_SCOPE was given to something that is typed and forced us to
-            # be a type-1 variant, then we assign the list "<value>;CACHE;<type>;<docstring>"
-            if param_opts['TYPE'] != None:
-                value += f";CACHE;{param_opts['TYPE']};"
+        # Prepend `-D` to the parameters
+        params = ["-D", varname]
 
-        if param_opts['VARIANT'] == 2 and param_opts['TYPE'] is not None:
-            params.append(":" + param_opts['TYPE'])
+        # If the type is provided then include the `:<typename>` argument.
+        # Note: CMake defaults to STRING if not provided.
+        params.append(":" + param_opts['TYPE'])
 
-        # Cache 'known' CMake vars here.
+        # Save variable to the cache of 'known'/'set' cmake variables
         self._var_formatter_cache[varname] = value
 
         return self._generic_program_option_handler_bash(params, value)
@@ -339,12 +383,15 @@ def _helper_opt_set_cmake_var_parse_parameters(self, params: list):
         """
         default_cache_var_type = "STRING"
 
-        output = {'FORCE': False, 'PARENT_SCOPE': False, 'TYPE': None}
+        output = {'FORCE': False, 'PARENT_SCOPE': False, 'TYPE': None, 'VARIANT': None}
 
         for option in params[: 4]:
             if option == "FORCE":
                 output['FORCE'] = True
                 # If FORCE is found but we have no TYPE yet, set to the default.
+                # TODO: Should we be setting the default to STRING here when FORCE
+                #       is provided with no explicit type? Future CMake versions might
+                #       someday change the default which would possibly break this?
                 if output['TYPE'] is None:
                     output['TYPE'] = default_cache_var_type
             elif option == "PARENT_SCOPE":
@@ -358,6 +405,7 @@ def _helper_opt_set_cmake_var_parse_parameters(self, params: list):
         if output['FORCE'] and output['PARENT_SCOPE']:
             msg = "ERROR: CMake does not allow `FORCE` and `PARENT_SCOPE` to both be used."
             self.exception_control_event("CATASTROPHIC", ValueError, message=msg)
+
         # Case 2: PARENT_SCOPE and CACHE will cause a CMake warning
         #         and the value will include the cache entries as a list:
         #             `set(FOO "VAL" CACHE STRING "docstring" PARENT_SCOPE)`
@@ -381,18 +429,18 @@ def _helper_opt_set_cmake_var_parse_parameters(self, params: list):
         # PARENT_SCOPE forces Type-1 (i.e., non-cache var)
         # - This will override CACHE, at least as of CMake 3.21.x
         if output['PARENT_SCOPE']:
-            output['VARIANT'] = 1
+            output['VARIANT'] = VarType.NON_CACHE
 
         # FORCE implies CACHE. If type wasn't provided then it's a STRING
         elif output['FORCE']:
-            output['VARIANT'] = 2
+            output['VARIANT'] = VarType.CACHE
 
         # If a TYPE is provided then it's a type-2 (CACHE) assignment.
         elif output['TYPE'] is not None:
-            output['VARIANT'] = 2
+            output['VARIANT'] = VarType.CACHE
 
         # Otherwise, a simple set is a type-1
         else:
-            output['VARIANT'] = 1
+            output['VARIANT'] = VarType.NON_CACHE
 
         return output

src/setprogramoptions/unittests/files_ini/config_test_setprogramoptions.ini

@@ -14,8 +14,10 @@ opt-set cmake
 opt-set -G : Ninja
 
 [TRILINOS_COMMON]
-opt-set-cmake-var  Trilinos_ENABLE_COMPLEX       BOOL   : ON
-opt-set-cmake-var  Trilinos_ENABLE_THREAD_SAFE   BOOL   : ON
+opt-set-cmake-var  Trilinos_ENABLE_COMPLEX              BOOL   : ON
+opt-set-cmake-var  Trilinos_ENABLE_THREAD_SAFE          BOOL   : ON
+# These are left UNTYPED so they aren't considered CACHE
+# vars. This will cause the bash generator to skip them.
 opt-set-cmake-var  Trilinos_PARALLEL_COMPILE_JOBS_LIMIT : 20
 opt-set-cmake-var  Trilinos_PARALLEL_LINK_JOBS_LIMIT    : 4
 
@@ -58,17 +60,28 @@ opt-set-cmake-var CMAKE_VAR_A PARENT_SCOPE FORCE : CMAKE_VAR_A_VAL
 opt-set-cmake-var CMAKE_VAR_A FORCE        : ON
 opt-set-cmake-var CMAKE_VAR_B PARENT_SCOPE : ON
 opt-set-cmake-var CMAKE_VAR_C BOOL         : ON
-opt-set-cmake-var CMAKE_VAR_F BOOL FORCE   : ON
-opt-set-cmake-var CMAKE_VAR_G FORCE BOOL   : ON
-opt-set-cmake-var CMAKE_VAR_H BOOL PARENT_SCOPE : ON
-opt-set-cmake-var CMAKE_VAR_I PARENT_SCOPE BOOL : ON
+opt-set-cmake-var CMAKE_VAR_D BOOL FORCE   : ON
+opt-set-cmake-var CMAKE_VAR_E FORCE BOOL   : ON
+opt-set-cmake-var CMAKE_VAR_F BOOL PARENT_SCOPE : ON
+opt-set-cmake-var CMAKE_VAR_G PARENT_SCOPE BOOL : ON
+
 
 [TEST_CMAKE_CACHE_PARAM_TEST_02]
 # Validate what happens if there's a bad param.
 # Note: FORCE option will make this a CACHE var of type STRING
 opt-set-cmake-var CMAKE_VAR_A FORCE FOOBAR: ON
 
 
+# This section is to be used to ensure that PARENT_SCOPE
+# will force a type-1 (non-cache) var.
+# The entries with PARENT_SCOPE enabled should not appear
+# in generated BASH output.
+[TEST_CMAKE_PARENT_SCOPE_NOT_BASH]
+opt-set-cmake-var FOO_VAR_A PARENT_SCOPE        : FOO_VAL A
+opt-set-cmake-var FOO_VAR_B STRING PARENT_SCOPE : FOO_VAL B
+
+
+
 #
 # Sample Test Configurations
 #
@@ -136,22 +149,37 @@ opt-set FOO : "${FOOBAR|ENV} -baz"
 opt-set FOO : "${FOOBAR} -baz"
 
 
+#
+# Variable expansion tests
+#
 [TEST_VAR_EXPANSION_COMMON]
+# Create the CACHE variable CMAKE_CXX_FLAGS of STRING type.
 opt-set-cmake-var CMAKE_CXX_FLAGS STRING : "${LDFLAGS|ENV} -foo"
 
-
 [TEST_VAR_EXPANSION_UPDATE_01]
+# Test an 'update' of CMAKE_CXX_FLAGS that is not FORCE
+# - In practice, CMake would not actually update CMAKE_CXX_FLAGS
+#   here because it's a CACHE var (per TEST_VAR_EXPANSION_COMMON)
+#   And CMake won't overwrite CACHE vars unless FORCEd.
 opt-set cmake
 use TEST_VAR_EXPANSION_COMMON
-opt-set-cmake-var CMAKE_CXX_FLAGS STRING: "${CMAKE_CXX_FLAGS|CMAKE} -bar"
+opt-set-cmake-var CMAKE_CXX_FLAGS STRING : "${CMAKE_CXX_FLAGS|CMAKE} -bar"
 
 [TEST_VAR_EXPANSION_UPDATE_02]
-use TEST_VAR_EXPANSION_UPDATE_01
+opt-set cmake
+use TEST_VAR_EXPANSION_COMMON
+# This should be a problem for Bash generation since the CMake var
+# CMAKE_F90_FLAGS would be unknown to bash. Depending on the
+# exception_control_level this will either trigger an exception when
+# sent to the bash generator or it could resolve the CMAKE var to an
+# empty string + warning.
 opt-set-cmake-var CMAKE_F90_FLAGS STRING: "${CMAKE_F90_FLAGS|CMAKE} -baz"
 
 [TEST_VAR_EXPANSION_UPDATE_03]
+# Updates CMAKE_CXX_FLAGS using a FORCE operation.
+# In practice, this would result in CMAKE_CXX_FLAGS = "${LDFLAGS|ENV} -foo -bif"
 use TEST_VAR_EXPANSION_UPDATE_01
-opt-set-cmake-var CMAKE_CXX_FLAGS STRING: "${CMAKE_CXX_FLAGS|CMAKE} -bif"
+opt-set-cmake-var CMAKE_CXX_FLAGS STRING FORCE : "${CMAKE_CXX_FLAGS|CMAKE} -bif"
 
 
 [TEST_STRING_DOUBLE_QUOTES]
@@ -161,9 +189,28 @@ opt-set-cmake-var BAR STRING: "600"
 
 [TEST_CMAKE_VAR_REMOVE]
 # Test whether or not opt-remove works for a cmake var
-opt-set-cmake-var FOO_TEST: "FOO"
-opt-set-cmake-var BAR_TEST: "BAR"
-opt-set-cmake-var FOO     : "BAZ"
-# this should remove `FOO_TEST` from the option list so we will only
-# have `BAR_TEST` and `FOO` left.
+opt-set-cmake-var FOO_TEST STRING : "FOO"
+opt-set-cmake-var BAR_TEST STRING : "BAR"
+opt-set-cmake-var BAZ_TEST STRING : "BAZ"
+# opt-remove FOO_TEST should remove the FOO_TEST entry
+# and leave only the BAR_TEST and BAZ_TEST entries.
 opt-remove FOO_TEST
+
+
+[TEST_CMAKE_VAR_FORCE_ONLY]
+opt-set-cmake-var FOO FORCE : "BAR"
+
+
+[TEST_CMAKE_VAR_IN_BASH_GENERATOR]
+# The purpose of this section is to see what we do if someone
+# has an _update_ operation (i.e., append/prepend to an existing var)
+# and that cmake var does not exist already. In this case we emulate
+# this by simulating a typo but it could occur in other cases where
+# a cmake generator would expect a fragment to update something that was
+# defined elsewhere. Should the bash generator throw an error or replace
+# "FOO_VAE" with an empty string?
+#
+# Set FOO_VAR to something concrete
+opt-set-cmake-var FOO_VAR STRING : "FOO"
+# Simulated typo in FOO_VAR update
+opt-set-cmake-var FOO_VAR FORCE  : "BAR ${FOO_VAE|CMAKE}"