[context] Split "env" values and prefixes.

recipe_engine/step_runner.py

Index: recipe_engine/step_runner.py
diff --git a/recipe_engine/step_runner.py b/recipe_engine/step_runner.py
index 421072e31e9884a3867486abd3e55116fb6b6e4e..6e7d632e0ccb1cadc35bb1fe9ee9525e52d519c3 100644
--- a/recipe_engine/step_runner.py
+++ b/recipe_engine/step_runner.py
@@ -178,7 +178,9 @@ class SubprocessStepRunner(StepRunner):
       )
       step_config = None  # Make sure we use rendered step config.
 
-      step_env = _merge_envs(os.environ, (rendered_step.config.env or {}))
+      step_env = _merge_envs(os.environ,
+          rendered_step.config.env, rendered_step.config.env_prefixes,
+          rendered_step.config.pathsep)
       # Now that the step's environment is all sorted, evaluate PATH on windows
       # to find the actual intended executable.
       rendered_step = _hunt_path(rendered_step, step_env)
@@ -425,9 +427,17 @@ class fakeEnviron(object):
   def __getitem__(self, key):
     return '<%s>' % key
 
+  def get(self, key, default=None):
+    return self[key]

[iannucci, 2017/06/12 19:58:40]

oof, this is starting to get weird; this and __getitem__ don't agree

[dnj, 2017/06/13 19:31:21]

How don't they? This returns __getitem__.

+
   def keys(self):
     return self.data.keys()
 
+  def pop(self, key, default=None):
+    result = self.data.get(key, default)
+    self.data[key] = None
+    return result
+
   def __delitem__(self, key):
     self.data[key] = None
 
@@ -465,7 +475,10 @@ class SimulationStepRunner(StepRunner):
       step_test = self._test_data.pop_step_test_data(step_config.name,
                                                      test_data_fn)
       rendered_step = render_step(step_config, step_test)
-      step_env = _merge_envs(fakeEnviron(), (rendered_step.config.env or {}))
+
+      # Merge our environment. Note that do NOT apply prefixes when rendering
+      # expectations, as they are rendered independently.
+      step_env = _merge_envs(fakeEnviron(), rendered_step.config.env, {}, None)
       rendered_step = rendered_step._replace(
         config=rendered_step.config._replace(env=step_env.data))
       step_config = None  # Make sure we use rendered step config.
@@ -688,7 +701,11 @@ def construct_step_result(rendered_step, retcode):
   return step_result
 
 
-def _merge_envs(original, override):
+# Sentinel used by "_merge_envs" to indicate a missing value.
+_MISSING = object()
+
+
+def _merge_envs(original, overrides, prefixes, pathsep):
   """Merges two environments.
 
   Returns a new environment dict with entries from |override| overwriting
@@ -696,18 +713,41 @@ def _merge_envs(original, override):
   remove the environment variable. Values can contain %(KEY)s strings, which
   will be substituted with the values from the original (useful for amending, as
   opposed to overwriting, variables like PATH).
+
+  See recipe_api.StepConfig for environment construction rules.
   """
   result = original.copy()
   subst = (original if isinstance(original, fakeEnviron)
            else collections.defaultdict(lambda: '', **original))
-  if not override:
+
+  if not any((prefixes, overrides)):
     return result
-  for k, v in override.items():
+
+  merged = set()
+  for k, path_tuple in prefixes.iteritems():
+    if not path_tuple:
+      continue
+    merged.add(k)
+
+    # If the same key is defined in "overrides", we need to interact with it.
+    # We'll do so here, and skip it in the "overrides" construction.
+    val = overrides.get(k, _MISSING)
+    if val is _MISSING:

[iannucci, 2017/06/12 19:58:40]

this might be clearer

  val = overrides.get(k, original.get(k, ''))

?

[dnj, 2017/06/13 19:31:21]

Can't do that b/c we need to apply subst only if it's in "overrides". I
reorganized a bit though, I think it's more readable.

+      # Not defined. Append "val" iff it is defined in "original" and not empty.
+      val = original.get(k, '')
+    elif val is not None:
+      val = str(val) % subst
+    if val:
+      path_tuple += (val,)
+    result[k] = pathsep.join(str(v) for v in path_tuple)
+
+  for k, v in overrides.iteritems():
+    if k in merged:
+      continue
     if v is None:
-      if k in result:
-        del result[k]
+      result.pop(k, None)
     else:
-      result[str(k)] = str(v) % subst
+      result[k] = str(v) % subst
   return result