Skip to content

Commit

Permalink
Remove os_ from MacroMaker constructor.
Browse files Browse the repository at this point in the history
The constructor already requires a `Machines` object, instantiated for a
particular machine, to be added. The operating system can be looked up
using this object, so there's no need to store it as a separate string.
(If the lookup ever becomes a performance bottleneck, we can always
cache the string.)
  • Loading branch information
quantheory committed Aug 25, 2016
1 parent 66506ba commit 0e7ca89
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 20 deletions.
6 changes: 3 additions & 3 deletions utils/python/CIME/case.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,12 +715,12 @@ def _create_caseroot_tools(self):
# Create Macros file.
machine = self.get_value("MACH")
if os.getenv("CIME_USE_CONFIG_BUILD") == "TRUE":
os_ = self.get_value("OS")
files = Files()
build_file = files.get_value("BUILD_SPEC_FILE")
machobj = Machines(machine=machine, files=files)
macro_maker = MacroMaker(os_, machobj)
with open(os.path.join(self._caseroot, "Macros"), "w") as macros_file:
macro_maker = MacroMaker(machobj)
macros_path = os.path.join(self._caseroot, "Macros")
with open(macros_path, "w") as macros_file:
macro_maker.write_macros('Makefile', build_file, macros_file)

# Copy any system or compiler Depends files to the case.
Expand Down
17 changes: 6 additions & 11 deletions utils/python/CIME/macros.py
Original file line number Diff line number Diff line change
Expand Up @@ -623,14 +623,11 @@ def add_settings_to_lists(self, flag_vars, value_lists):
else:
self._add_elem_to_lists(elem.tag, elem, value_lists)

def matches_machine(self, os_):
def matches_machine(self):
"""Check whether this block matches a machine/os.
This also sets the specificity of the block, so this must be called
before add_settings_to_lists if machine-specific output is needed.
Arguments:
os_ - Operating system to match.
"""
self._specificity = 0
if "MACH" in self._compiler_elem.keys():
Expand All @@ -640,7 +637,7 @@ def matches_machine(self, os_):
else:
return False
if "OS" in self._compiler_elem.keys():
if os_ == self._compiler_elem.get("OS"):
if self._machobj.get_value("OS") == self._compiler_elem.get("OS"):
self._specificity += 1
else:
return False
Expand All @@ -665,23 +662,21 @@ class MacroMaker(object):
write_macros
"""

def __init__(self, os_, machobj, schema_path=None):
def __init__(self, machobj, schema_path=None):
"""Construct a MacroMaker given machine-specific information.
In the process some information about possible variables is read in
from the schema file.
Arguments:
os_ - Name of a machine's operating system.
machobj - A Machines object for this machine.
schema_path (optional) - Path to config_build.xsd within CIME.
>>> "CFLAGS" in MacroMaker('FakeOS', 'MyMach').flag_vars
>>> "CFLAGS" in MacroMaker('MyMach').flag_vars
True
>>> "MPICC" in MacroMaker('FakeOS', 'MyMach').flag_vars
>>> "MPICC" in MacroMaker('MyMach').flag_vars
False
"""
self.os = os_
self.machobj = machobj

# The schema is used to figure out which variables contain
Expand Down Expand Up @@ -724,7 +719,7 @@ def write_macros(self, build_system, xml_file, output):
for compiler_elem in ET.parse(xml_file).findall("compiler"):
block = CompilerBlock(writer, compiler_elem, self.machobj)
# If this block matches machine settings, use it.
if block.matches_machine(self.os):
if block.matches_machine():
block.add_settings_to_lists(self.flag_vars, value_lists)

# Now that we've scanned through the input, output the variable
Expand Down
19 changes: 13 additions & 6 deletions utils/python/tests/scripts_regression_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1107,15 +1107,22 @@ class MockMachines(object):

"""A mock version of the Machines object to simplify testing."""

def __init__(self, name):
def __init__(self, name, os_):
"""Store the name."""
self.name = name
self.os = os_

def get_machine_name(self):
"""Return the name we were given."""
return self.name

def is_valid_compiler(self, _):
def get_value(self, var_name):
"""Allow the operating system to be queried."""
assert var_name == "OS", "MacrosMaker asked for a value not " \
"implemented in the testing infrastructure."
return self.os

def is_valid_compiler(self, _): # pylint:disable=no-self-use
"""Assume all compilers are valid."""
return True

Expand Down Expand Up @@ -1351,19 +1358,19 @@ class G_TestMacrosBasic(unittest.TestCase):
def test_script_is_callable(self):
"""The test script can be called on valid output without dying."""
# This is really more a smoke test of this script than anything else.
maker = MacroMaker("SomeOS", MockMachines("mymachine"))
maker = MacroMaker(MockMachines("mymachine", "SomeOS"))
test_xml = _wrap_config_build_xml("<compiler><SUPPORTS_CXX>FALSE</SUPPORTS_CXX></compiler>")
get_macros(maker, test_xml, "Makefile")

def test_script_rejects_bad_xml(self):
"""The macro writer rejects input that's not valid XML."""
maker = MacroMaker("SomeOS", MockMachines("mymachine"))
maker = MacroMaker(MockMachines("mymachine", "SomeOS"))
with self.assertRaises(ParseError):
get_macros(maker, "This is not valid XML.", "Makefile")

def test_script_rejects_bad_build_system(self):
"""The macro writer rejects a bad build system string."""
maker = MacroMaker("SomeOS", MockMachines("mymachine"))
maker = MacroMaker(MockMachines("mymachine", "SomeOS"))
bad_string = "argle-bargle."
with self.assertRaisesRegexp(
SystemExit,
Expand All @@ -1387,7 +1394,7 @@ class H_TestMakeMacros(unittest.TestCase):
test_machine = "mymachine"

def setUp(self):
self._maker = MacroMaker(self.test_os, MockMachines(self.test_machine))
self._maker = MacroMaker(MockMachines(self.test_machine, self.test_os))

def xml_to_tester(self, xml_string):
"""Helper that directly converts an XML string to a MakefileTester."""
Expand Down

0 comments on commit 0e7ca89

Please sign in to comment.