Skip to content
This repository was archived by the owner on Sep 3, 2024. It is now read-only.

Commit db04d15

Browse files
author
William McLendon
committed
TRILFRAME-120-A : Handling non-CACHED cmake vars
1 parent 232202d commit db04d15

File tree

7 files changed

+195
-105
lines changed

7 files changed

+195
-105
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1717
#### Todo (for Unreleased)
1818
-->
1919

20-
## [0.3.1] <!-- YYYY-MM-DD --> [Unreleased]
20+
## [0.4.0] <!-- YYYY-MM-DD --> [Unreleased]
2121
#### Added
2222
#### Changed
23+
- TODO: Add notes for !16/!17 // TRILFRAME-120 + TRILFRAME-136
2324
- Changes to `opt-set-cmake-var` behaviour:
2425
- Trigger a WARNING if `PARENT_SCOPE` and a _TYPE_ is provided since CMake
2526
requires TYPED vars to be CACHE vars and `PARENT_SCOPE` will force the

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[tool.poetry]
22
name = "setprogramoptions"
3-
version = "0.3.1"
3+
version = "0.4.0"
44
description = "Program options helper."
55
authors = [
66
"William McLendon <wcmclen@sandia.gov>"

src/setprogramoptions/SetProgramOptions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,6 @@ class ExpandVarsInText(ExceptionControl):
109109

110110
def __init__(self):
111111
self.exception_control_level = 4
112-
self.exception_control_compact_warnings = True
113112

114113
class VariableFieldData(object):
115114
"""
@@ -526,6 +525,7 @@ def _gen_option_entry(self, option_entry: dict, generator='bash') -> Union[str,
526525

527526
# Update the var formatter's ECL to match the current value.
528527
self._var_formatter.exception_control_level = self.exception_control_level
528+
self._var_formatter.exception_control_compact_warnings = self.exception_control_compact_warnings
529529

530530
# format the value
531531
formatter = self._var_formatter

src/setprogramoptions/SetProgramOptionsCMake.py

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,19 @@
2020
``-DVAR_NAME:BOOL=ON`` using the ``SetProgramOptions`` method ``gen_option_list``
2121
with the ``bash`` generator.
2222
23-
In the case of bash command entries the ``PARENT_SCOPE`` optional parameter is
24-
ignored.
23+
When using the BASH generator to generate command line arguments, CMake
24+
uses the syntax ``-D<VARNAME>[:<TYPE>]=<VALUE>``. The ``TYPE`` field is optional
25+
and if left out CMake will default to a ``STRING`` type. Further, all CMake
26+
varaibles set via the command line using ``-D`` will be CACHE variables and each
27+
``-D`` operation should be considered a FORCE operation too. For example,
28+
``-DFOO:STRING=BAR`` is roughly equivalent to the CMake command:
29+
``set(FOO CACHE STRING "docstring" FORCE)``.
30+
31+
The ``PARENT_SCOPE`` option applies only to non-cache variables and its presence
32+
will instruct CMake to make that variable non-cache. Care should be taken when
33+
using ``PARENT_SCOPE`` as combining it with the usual CACHE operations results
34+
in CMake creating a non-cached variable whose contents are the list containing
35+
``<varname>;CACHE;<type>;doc string``.
2536
2637
See CMake documentation on the `set() <https://cmake.org/cmake/help/latest/command/set.html>`_
2738
command for more information on how fragment file entries are generated.
@@ -73,7 +84,6 @@ class ExpandVarsInTextCMake(ExpandVarsInText):
7384

7485
def __init__(self):
7586
self.exception_control_level = 3
76-
self.exception_control_compact_warnings = True
7787

7888
def _fieldhandler_BASH_CMAKE(self, field):
7989
"""
@@ -183,7 +193,7 @@ def _program_option_handler_opt_set_cmake_fragment(self, params: list, value: st
183193
"""
184194
return None
185195

186-
def _program_option_handler_opt_set_cmake_var_bash(self, params, value) -> str:
196+
def _program_option_handler_opt_set_cmake_var_bash(self, params: list, value: str) -> str:
187197
"""
188198
Line-item generator for ``opt-set-cmake-var`` entries when the *generator*
189199
is set to ``bash``.
@@ -196,30 +206,50 @@ def _program_option_handler_opt_set_cmake_var_bash(self, params, value) -> str:
196206
side-effects since :py:meth:`setprogramoptions.SetProgramOptions._gen_option_entry`
197207
performs a deep-copy of these parameters prior to calling this.
198208
Any changes we make are ephemeral.
209+
210+
Args:
211+
params (list): The parameters of the operation.
212+
value (str): The value of the option that is being assigned.
213+
214+
Raises:
215+
ValueError: This can potentially raise a ``ValueError`` if
216+
``exception_control_level`` is set to 5 if there are
217+
operations that are skipped in Bash generation. If ``ecl``
218+
is less than 5 then warnings are generated to note the
219+
exclusion.
199220
"""
200221
varname = params[0]
201222
params = params[1 : 4]
202223
param_opts = self._helper_opt_set_cmake_var_parse_parameters(params)
203224

225+
# PARENT_SCOPE (i.e., type-1 non-cached operations) should not be
226+
# written to the set of Bash parameters.
227+
if param_opts['VARIANT'] == 1:
228+
msg = f"bash generator - `{varname}={value}` skipped because"
229+
msg += f" it is a non-cached (type-1) operation."
230+
msg += f" To generate a bash arg for this consider adding FORCE or a TYPE."
231+
self.exception_control_event("WARNING", ValueError, message=msg)
232+
return None
233+
234+
# If varname has already been assigned and this assignment
235+
# does not include FORCE then we should skip adding it to the
236+
# set of command line options.
237+
if varname in self._var_formatter_cache and not param_opts['FORCE']:
238+
msg = f"bash generator - `{varname}={value}` skipped because"
239+
msg += f" CACHE var `{varname}` is already set and CMake requires"
240+
msg += f" FORCE to be set to change the value."
241+
self.exception_control_event("WARNING", ValueError, message=msg)
242+
return None
243+
244+
# Prepend `-D` to the parameters
204245
params = ["-D", varname]
205246

206-
if param_opts['VARIANT'] == 1:
207-
# if PARENT_SCOPE was given to something that is typed and forced us to
208-
# be a type-1 variant, then we assign the list "<value>;CACHE;<type>;<docstring>"
209-
if param_opts['TYPE'] != None:
210-
value += f";CACHE;{param_opts['TYPE']};"
211-
212-
if param_opts['VARIANT'] == 2 and param_opts['TYPE'] is not None:
213-
params.append(":" + param_opts['TYPE'])
214-
215-
# Cache 'known' CMake vars here.
216-
try:
217-
if self._var_formatter_cache[varname] is not None and not param_opts['FORCE']:
218-
# Do not add this to the output unless it's a forceful set
219-
return None
220-
except:
221-
pass
247+
# If the type is provided then include the `:<typename>` argument.
248+
# Note: CMake defaults to STRING if not provided.
249+
#if param_opts['VARIANT'] == 2 and param_opts['TYPE'] is not None: # DEADCODE: this can't be false now
250+
params.append(":" + param_opts['TYPE'])
222251

252+
# Save variable to the cache of 'known'/'set' cmake variables
223253
self._var_formatter_cache[varname] = value
224254

225255
return self._generic_program_option_handler_bash(params, value)

src/setprogramoptions/unittests/files_ini/config_test_setprogramoptions.ini

Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ opt-set cmake
1414
opt-set -G : Ninja
1515

1616
[TRILINOS_COMMON]
17-
opt-set-cmake-var Trilinos_ENABLE_COMPLEX BOOL : ON
18-
opt-set-cmake-var Trilinos_ENABLE_THREAD_SAFE BOOL : ON
17+
opt-set-cmake-var Trilinos_ENABLE_COMPLEX BOOL : ON
18+
opt-set-cmake-var Trilinos_ENABLE_THREAD_SAFE BOOL : ON
19+
# These are left UNTYPED so they aren't considered CACHE
20+
# vars. This will cause the bash generator to skip them.
1921
opt-set-cmake-var Trilinos_PARALLEL_COMPILE_JOBS_LIMIT : 20
2022
opt-set-cmake-var Trilinos_PARALLEL_LINK_JOBS_LIMIT : 4
2123

@@ -58,10 +60,10 @@ opt-set-cmake-var CMAKE_VAR_A PARENT_SCOPE FORCE : CMAKE_VAR_A_VAL
5860
opt-set-cmake-var CMAKE_VAR_A FORCE : ON
5961
opt-set-cmake-var CMAKE_VAR_B PARENT_SCOPE : ON
6062
opt-set-cmake-var CMAKE_VAR_C BOOL : ON
61-
opt-set-cmake-var CMAKE_VAR_F BOOL FORCE : ON
62-
opt-set-cmake-var CMAKE_VAR_G FORCE BOOL : ON
63-
opt-set-cmake-var CMAKE_VAR_H BOOL PARENT_SCOPE : ON
64-
opt-set-cmake-var CMAKE_VAR_I PARENT_SCOPE BOOL : ON
63+
opt-set-cmake-var CMAKE_VAR_D BOOL FORCE : ON
64+
opt-set-cmake-var CMAKE_VAR_E FORCE BOOL : ON
65+
opt-set-cmake-var CMAKE_VAR_F BOOL PARENT_SCOPE : ON
66+
opt-set-cmake-var CMAKE_VAR_G PARENT_SCOPE BOOL : ON
6567

6668
[TEST_CMAKE_CACHE_PARAM_TEST_02]
6769
# Validate what happens if there's a bad param.
@@ -136,28 +138,38 @@ opt-set FOO : "${FOOBAR|ENV} -baz"
136138
opt-set FOO : "${FOOBAR} -baz"
137139

138140

141+
#
142+
# Variable expansion tests
143+
#
139144
[TEST_VAR_EXPANSION_COMMON]
145+
# Create the CACHE variable CMAKE_CXX_FLAGS of STRING type.
140146
opt-set-cmake-var CMAKE_CXX_FLAGS STRING : "${LDFLAGS|ENV} -foo"
141147

142-
143148
[TEST_VAR_EXPANSION_UPDATE_01]
149+
# Test an 'update' of CMAKE_CXX_FLAGS that is not FORCE
150+
# - In practice, CMake would not actually update CMAKE_CXX_FLAGS
151+
# here because it's a CACHE var (per TEST_VAR_EXPANSION_COMMON)
152+
# And CMake won't overwrite CACHE vars unless FORCEd.
144153
opt-set cmake
145154
use TEST_VAR_EXPANSION_COMMON
146-
opt-set-cmake-var CMAKE_CXX_FLAGS STRING FORCE : "${CMAKE_CXX_FLAGS|CMAKE} -bar"
155+
opt-set-cmake-var CMAKE_CXX_FLAGS STRING : "${CMAKE_CXX_FLAGS|CMAKE} -bar"
147156

148157
[TEST_VAR_EXPANSION_UPDATE_02]
149-
use TEST_VAR_EXPANSION_UPDATE_01
158+
opt-set cmake
159+
use TEST_VAR_EXPANSION_COMMON
160+
# This should be a problem for Bash generation since the CMake var
161+
# CMAKE_F90_FLAGS would be unknown to bash. Depending on the
162+
# exception_control_level this will either trigger an exception when
163+
# sent to the bash generator or it could resolve the CMAKE var to an
164+
# empty string + warning.
150165
opt-set-cmake-var CMAKE_F90_FLAGS STRING: "${CMAKE_F90_FLAGS|CMAKE} -baz"
151166

152167
[TEST_VAR_EXPANSION_UPDATE_03]
168+
# Updates CMAKE_CXX_FLAGS using a FORCE operation.
169+
# In practice, this would result in CMAKE_CXX_FLAGS = "${LDFLAGS|ENV} -foo -bif"
153170
use TEST_VAR_EXPANSION_UPDATE_01
154171
opt-set-cmake-var CMAKE_CXX_FLAGS STRING FORCE : "${CMAKE_CXX_FLAGS|CMAKE} -bif"
155172

156-
[TEST_VAR_EXPANSION_UPDATE_04]
157-
opt-set cmake
158-
use TEST_VAR_EXPANSION_COMMON
159-
# Test (return None) at SetProgramOptionsCMake.py:_program_option_handler_opt_set_cmake_var_bash:188
160-
opt-set-cmake-var CMAKE_CXX_FLAGS STRING : "${LDFLAGS|ENV} -foobar"
161173

162174
[TEST_STRING_DOUBLE_QUOTES]
163175
opt-set-cmake-var FOO STRING: foo::bar::baz<Type>
@@ -166,9 +178,13 @@ opt-set-cmake-var BAR STRING: "600"
166178

167179
[TEST_CMAKE_VAR_REMOVE]
168180
# Test whether or not opt-remove works for a cmake var
169-
opt-set-cmake-var FOO_TEST: "FOO"
170-
opt-set-cmake-var BAR_TEST: "BAR"
171-
opt-set-cmake-var FOO : "BAZ"
172-
# this should remove `FOO_TEST` from the option list so we will only
173-
# have `BAR_TEST` and `FOO` left.
181+
opt-set-cmake-var FOO_TEST STRING : "FOO"
182+
opt-set-cmake-var BAR_TEST STRING : "BAR"
183+
opt-set-cmake-var BAZ_TEST STRING : "BAZ"
184+
# opt-remove FOO_TEST should remove the FOO_TEST entry
185+
# and leave only the BAR_TEST and BAZ_TEST entries.
174186
opt-remove FOO_TEST
187+
188+
189+
[TEST_CMAKE_VAR_FORCE_ONLY]
190+
opt-set-cmake-var FOO FORCE : "BAR"

0 commit comments

Comments
 (0)