Merge branch 'jgfouca/changes_to_env_mach_spec_env' (PR #701)

Incorporates jedwards' fix on another PR but in such a way
as to not break melvin. Specifically, we need to be able to use
environment variables set by the modules in environment_variables. Also,
if we're going to call get_resolved_value, then we cannot support bash
in this field

Test suite: scripts_regression_tests
Test baseline:
Test namelist changes:
Test status: bit for bit

User interface changes?: Minor. environment_variables setting in config_machines.xml works a bit differently now

Code review: jedwards

* jgfouca/changes_to_env_mach_spec_env:
  Changes to handling of environment_variables

cime_config/acme/machines/config_machines.xml

@@ -381,8 +381,8 @@
       </modules>
     </module_system>
     <environment_variables>
-      <env name="NETCDFROOT">$SEMS_NETCDF_ROOT</env>
-      <env name="PNETCDFROOT" mpilib="!mpi-serial">$SEMS_NETCDF_ROOT</env>
+      <env name="NETCDFROOT">$ENV{SEMS_NETCDF_ROOT}</env>
+      <env name="PNETCDFROOT" mpilib="!mpi-serial">$ENV{SEMS_NETCDF_ROOT}</env>
     </environment_variables>
 </machine>
 
@@ -437,8 +437,8 @@
       </modules>
     </module_system>
     <environment_variables>
-      <env name="NETCDFROOT">$SEMS_NETCDF_ROOT</env>
-      <env name="PNETCDFROOT">$SEMS_NETCDF_ROOT</env>
+      <env name="NETCDFROOT">$ENV{SEMS_NETCDF_ROOT}</env>
+      <env name="PNETCDFROOT">$ENV{SEMS_NETCDF_ROOT}</env>
     </environment_variables>
 </machine>
 
@@ -559,8 +559,8 @@
     </modules>
   </module_system>
   <environment_variables>
-    <env name="NETCDFROOT">$SEMS_NETCDF_ROOT</env>
-    <env name="PNETCDFROOT" mpilib="!mpi-serial">$SEMS_NETCDF_ROOT</env>
+    <env name="NETCDFROOT">$ENV{SEMS_NETCDF_ROOT}</env>
+    <env name="PNETCDFROOT" mpilib="!mpi-serial">$ENV{SEMS_NETCDF_ROOT}</env>
     <env name="OMP_STACKSIZE">64M</env>
   </environment_variables>
 </machine>
@@ -629,8 +629,8 @@
     </modules>
   </module_system>
   <environment_variables>
-    <env name="NETCDFROOT">$SEMS_NETCDF_ROOT</env>
-    <env name="PNETCDFROOT" mpilib="!mpi-serial">$SEMS_NETCDF_ROOT</env>
+    <env name="NETCDFROOT">$ENV{SEMS_NETCDF_ROOT}</env>
+    <env name="PNETCDFROOT" mpilib="!mpi-serial">$ENV{SEMS_NETCDF_ROOT}</env>
     <env name="OMP_STACKSIZE">64M</env>
   </environment_variables>
 </machine>

utils/python/CIME/XML/env_mach_specific.py

@@ -51,7 +51,6 @@ def populate(self, machobj):
                 for node in nodes:
                     self.add_child(node)
 
-
     def get_full_records(self, item, attribute=None, resolved=True, subgroup=None):
         """Returns the value as a string of the first xml element with item as attribute value.
         <element_name attribute='attribute_value>value</element_name>"""
@@ -88,27 +87,34 @@ def get_full_records(self, item, attribute=None, resolved=True, subgroup=None):
 
         return results
 
-    def _get_env_for_case(self, compiler, debug, mpilib):
+    def _get_modules_for_case(self, compiler, debug, mpilib):
         module_nodes = self.get_nodes("modules")
-        env_nodes    = self.get_nodes("environment_variables")
         modules_to_load = None
         if module_nodes is not None:
             modules_to_load = self._compute_module_actions(module_nodes, compiler, debug, mpilib)
 
+        return modules_to_load
+
+    def _get_envs_for_case(self, compiler, debug, mpilib):
+        env_nodes = self.get_nodes("environment_variables")
+
         envs_to_set = None
         if env_nodes is not None:
             envs_to_set = self._compute_env_actions(env_nodes, compiler, debug, mpilib)
 
-        return modules_to_load, envs_to_set
+        return envs_to_set
 
     def load_env(self, compiler, debug, mpilib):
         """
         Should only be called by case.load_env
         """
-        modules_to_load, envs_to_set = self._get_env_for_case(compiler, debug, mpilib)
-
+        # Do the modules so we can refer to env vars set by the modules
+        # in the environment_variables block
+        modules_to_load = self._get_modules_for_case(compiler, debug, mpilib)
         if (modules_to_load is not None):
             self.load_modules(modules_to_load)
+
+        envs_to_set = self._get_envs_for_case(compiler, debug, mpilib)
         if (envs_to_set is not None):
             self.load_envs(envs_to_set)
 
@@ -150,15 +156,16 @@ def list_modules(self):
             expect(False, "Unhandled module system '%s'" % module_system)
 
     def make_env_mach_specific_file(self, compiler, debug, mpilib, shell):
-        modules_to_load, envs_to_set = self._get_env_for_case(compiler, debug, mpilib)
+        modules_to_load = self._get_modules_for_case(compiler, debug, mpilib)
+        envs_to_set = self._get_envs_for_case(compiler, debug, mpilib)
+
         filename = ".env_mach_specific.%s" % shell
         lines = []
         if modules_to_load is not None:
             lines.extend(self._get_module_commands(modules_to_load, shell))
 
         if envs_to_set is not None:
             for env_name, env_value in envs_to_set:
-                # Let bash do the work on evaluating and resolving env_value
                 if shell == "sh":
                     lines.append("export %s=%s" % (env_name, env_value))
                 elif shell == "csh":
@@ -171,8 +178,7 @@ def make_env_mach_specific_file(self, compiler, debug, mpilib, shell):
 
     def load_envs(self, envs_to_set):
         for env_name, env_value in envs_to_set:
-            # Let bash do the work on evaluating and resolving env_value
-            os.environ[env_name] = run_cmd_no_fail("echo %s" % env_value)
+            os.environ[env_name] = env_value
 
     # Private API
 
@@ -191,16 +197,15 @@ def _compute_actions(self, nodes, child_tag, compiler, debug, mpilib):
                     expect(child.tag == child_tag, "Expected %s element" % child_tag)
                     if (self._match_attribs(child.attrib, compiler, debug, mpilib)):
                         val = child.text
-                        if val is not None and val.startswith('$'):
-                            if val == "$COMPILER":
-                                val = compiler
-                            elif val == "$MPILIB":
-                                val = mpilib
-                            else:
-                                val = self.get_resolved_value(val)
-                                expect(not val.startswith('$'), "Could not resolve value for val=%s"%val)
-
-                        result.append( (child.get("name"), val) )
+                        if val is not None:
+                            # We allow a couple special substitutions for these fields
+                            for repl_this, repl_with in [("$COMPILER", compiler), ("$MPILIB", mpilib)]:
+                                val = val.replace(repl_this, repl_with)
+
+                            val = self.get_resolved_value(val)
+                            expect("$" not in val, "Not safe to leave unresolved items in env var value: '%s'" % val)
+
+                            result.append( (child.get("name"), val) )
 
         return result
 

utils/python/CIME/case_setup.py

@@ -249,6 +249,7 @@ def _case_setup_impl(case, caseroot, casebaseid, clean=False, test_mode=False, r
 
         # Create needed directories for case
         create_dirs(case)
+        case.load_env()
 
         logger.info("If an old case build already exists, might want to run \'case.build --clean\' before building")