Skip to content

Commit

Permalink
TRILFRAME-121: Partial revert of changes from MR-12
Browse files Browse the repository at this point in the history
  • Loading branch information
William McLendon committed Sep 10, 2021
1 parent ec84080 commit 6b0d58c
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 8 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [0.3.1] <!-- YYYY-MM-DD --> [Unreleased]
#### Added
#### Changed
- 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
from `configparserenhanced.ExceptionControl` and will raise a "MINOR"
`exception_control_event` (i.e., throws if `exception_control_level` is 4 or greater).
We also tie this _ecl_ to the _ecl_ of the `SetProgramOptions` class so changing this
will affect whether or not the exception is raised or if the output returned is an
empty string.
#### Deprecated
#### Removed
#### Fixed
Expand Down
3 changes: 3 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ Merge Request Process
4. Update unit tests to fully test your additions. We aim for 100% coverage on this project.
5. Ensure unit tests and documentation build cleanly by running `exec-tests.sh`, `exec-makedoc.sh`
and the _example_ applications in the repository.
6. Run the `exec-reformat.py` script in the repository's root directory to update all formatting
using the **[YAPF][2]** tool. This is optional but STRONGLY encouraged, we reserve the right to
reject a merge request if this hasn't been run.

Code of Conduct
===============
Expand Down
15 changes: 14 additions & 1 deletion src/setprogramoptions/SetProgramOptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
sys.exit("Python %s.%s or later is required.\n" % (MIN_PYTHON))

from configparserenhanced import *
import configparserenhanced.ExceptionControl

from .common import *

Expand All @@ -92,7 +93,7 @@ class _VARTYPE_UNKNOWN(object):



class ExpandVarsInText(object):
class ExpandVarsInText(ExceptionControl):
"""
Utility to identify and format variables that are found in a text string.
Expand All @@ -106,7 +107,16 @@ class ExpandVarsInText(object):
they're declaring. These are a variables declared in a *pseudo-language* not bash.
"""

def __init__(self):
self.exception_control_level = 4
self.exception_control_compact_warnings = True

class VariableFieldData(object):
"""
This is essentially a dataclass that is used to pass field data around within
the generators, etc. This captures the relevant fields from a given action
entry.
"""
varfield = typed_property('varfield', expected_type=str, req_assign_before_use=True)
varname = typed_property('varname', expected_type=str, req_assign_before_use=True)
vartype = typed_property('vartype', expected_type=str, req_assign_before_use=True)
Expand Down Expand Up @@ -514,6 +524,9 @@ def _gen_option_entry(self, option_entry: dict, generator='bash') -> Union[str,
if " " in value:
value = '"' + value + '"'

# Update the var formatter's ECL to match the current value.
self._var_formatter.exception_control_level = self.exception_control_level

# format the value
formatter = self._var_formatter
formatter.generator = generator
Expand Down
28 changes: 26 additions & 2 deletions src/setprogramoptions/SetProgramOptionsCMake.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@
import shlex

from configparserenhanced import *

from .SetProgramOptions import SetProgramOptions
from .SetProgramOptions import ExpandVarsInText


# ==============================
# F R E E F U N C T I O N S
# ==============================
Expand All @@ -71,12 +71,36 @@ class ExpandVarsInTextCMake(ExpandVarsInText):
Extends ``ExpandVarsInText`` class to add in support for a ``CMAKE`` generator.
"""

def __init__(self):
self.exception_control_level = 3
self.exception_control_compact_warnings = True

def _fieldhandler_BASH_CMAKE(self, field):
"""Format CMAKE fields for the BASH generator."""
"""
Format CMAKE fields for the BASH generator.
Args:
field (setprogramoptions.VariableFieldData): Stores the action entry's
parameters and field information.
Raises:
ValueError: A ``ValueError`` can be raised if we have an unresolved
CMake variable in the ``.ini`` file (i.e., ``${FIELDNAME|CMAKE}``).
By "unresolved" we mean that the CMake does not have some entry in
the cache that resolves it down to either text or a BASH environment
variable.
This exception is skipped if ``self.exception_control_level`` is set
to 3 or lower. If it is 4 or higher then the exception is raised.
"""
output = field.varfield
if field.varname in self.owner._var_formatter_cache.keys():
output = self.owner._var_formatter_cache[field.varname].strip('"')
else:
# If self.exception_control_level is >= 4 then we'll raise the error
# instead of sending a warning.
msg = f"Unhandled variable expansion for `{field.varname}` in a BASH file."
msg += " CMake variables are only valid in a CMake fragment file."
self.exception_control_event("MINOR", ValueError, msg)
output = ""
return output

Expand Down
45 changes: 40 additions & 5 deletions src/setprogramoptions/unittests/test_SetProgramOptionsCMake.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,26 +224,61 @@ def test_SetProgramOptionsCMake_gen_option_list_bash_expandvars(self):
print("OK")
return 0

def test_SetProgramOptionsCMake_gen_option_list_bash_expandvars_with_unknown_cmake_var(self):
def test_SetProgramOptionsCMake_gen_option_list_bash_expandvars_with_unknown_cmake_var_ecl3(self):
"""
Test the ``gen_option_list`` method using the ``bash`` generator.
Test the ``gen_option_list`` method using the ``bash`` generator when the ECL for
ExpandVarsInTextCMake is set to 3 or lower. This should generate a WARNING.
"""
parser = self._create_standard_parser()
parser.exception_control_level = 3

print("-----[ TEST BEGIN ]----------------------------------------")
section = "TEST_VAR_EXPANSION_UPDATE_02"
print("Section : {}".format(section))

# parse a section
# parse the section
self._execute_parser(parser, section)

option_list_actual = parser.gen_option_list(section, generator="bash")
# Generate a BASH script representing the instructions in the section.

# what answer do we EXPECT:
option_list_expect = [
'cmake',
'-DCMAKE_CXX_FLAGS:STRING="${LDFLAGS} -foo"',
'-DCMAKE_CXX_FLAGS:STRING="${LDFLAGS} -foo -bar"', '-DCMAKE_F90_FLAGS:STRING=" -baz"'
'-DCMAKE_CXX_FLAGS:STRING="${LDFLAGS} -foo -bar"',
'-DCMAKE_F90_FLAGS:STRING=" -baz"'
]

# Generate the BASH entries:
option_list_actual = parser.gen_option_list(section, generator="bash")

# Verify the results:
self.assertListEqual(option_list_actual, option_list_expect)

print("-----[ TEST END ]------------------------------------------")

print("OK")
return 0

def test_SetProgramOptionsCMake_gen_option_list_bash_expandvars_with_unknown_cmake_var_ecl4(self):
"""
Test the ``gen_option_list`` method using the ``bash`` generator when the ECL
for ExpandVarsInTextCMake is set to 4 or higher. This should raise a ``ValueError``.
"""
parser = self._create_standard_parser()
parser.exception_control_level = 5

print("-----[ TEST BEGIN ]----------------------------------------")
section = "TEST_VAR_EXPANSION_UPDATE_02"
print("Section : {}".format(section))

# parse the section
self._execute_parser(parser, section)

# Generate a BASH script representing the instructions in the section.
with self.assertRaises(ValueError):
option_list_actual = parser.gen_option_list(section, generator="bash")

print("-----[ TEST END ]------------------------------------------")

print("OK")
Expand Down

0 comments on commit 6b0d58c

Please sign in to comment.