[context] Split "env" values and prefixes.

recipe_engine/recipe_api.py

Index: recipe_engine/recipe_api.py
diff --git a/recipe_engine/recipe_api.py b/recipe_engine/recipe_api.py
index 6ec91c3137bcfc74aa533b73cbc9083ce8df6773..d35fcf291b394e8d3c24f0f89b32db5c00d93fe0 100644
--- a/recipe_engine/recipe_api.py
+++ b/recipe_engine/recipe_api.py
@@ -9,6 +9,7 @@ import contextlib
 import copy
 import json
 import keyword
+import os
 import re
 import types
 
@@ -257,9 +258,9 @@ class TriggerSpec(_TriggerSpec):
 
 
 _StepConfig = collections.namedtuple('_StepConfig',
-    ('name', 'base_name', 'cmd', 'cwd', 'env', 'allow_subannotations',
-     'trigger_specs', 'timeout', 'infra_step', 'stdout', 'stderr', 'stdin',
-     'ok_ret', 'step_test_data', 'nest_level'))
+    ('name', 'base_name', 'cmd', 'cwd', 'env_prefixes', 'env', 'pathsep',

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

pathsep is necessary for simulations I guess?

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

Yeah, for accuracy I think we should use whatever "pathsep" the path module
uses.

+     'allow_subannotations', 'trigger_specs', 'timeout', 'infra_step', 'stdout',
+     'stderr', 'stdin', 'ok_ret', 'step_test_data', 'nest_level'))
 
 class StepConfig(_StepConfig):
   """
@@ -277,6 +278,16 @@ class StepConfig(_StepConfig):
         "delete_this": None,
         "static_value": "something",
     }
+
+  The optional "env_prefix" parameter contains values that, if specified, will
+  transform an environment variable into a "pathsep"-delimited sequence of
+  items:
+    - If an environment variable is also specified for this key, it will be
+      appended as the last element: <prefix0>:...:<prefixN>:ENV
+    - If no enviornment variable is specified, the current environment's value
+      will be appended, unless it's empty: <prefix0>:...:<prefixN>[:ENV]?
+    - If an enviornment variable with a value of None (delete) is specified,
+      nothing will be appeneded: <prefix0>:...:<prefixN>

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

"There is currently no way to remove prefix paths; once they're there, they're
there for good. If you think you need to remove paths from the prefix lists,
please talk to infra-dev@chromium.org."

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

Done.

   """
 
   _RENDER_WHITELIST=frozenset((
@@ -287,14 +298,16 @@ class StepConfig(_StepConfig):
     'base_name',
     'nest_level',
     'ok_ret',
+    'pathsep',
     'step_test_data',
   ))
 
   @classmethod
-  def create(cls, name, base_name=None, cmd=None, cwd=None, env=None,
-             allow_subannotations=None, trigger_specs=None, timeout=None,
-             infra_step=None, stdout=None, stderr=None, stdin=None,
-             ok_ret=None, step_test_data=None, step_nest_level=None):
+  def create(cls, name, base_name=None, cmd=None, cwd=None, env_prefixes=None,
+             pathsep=None, env=None, allow_subannotations=None,
+             trigger_specs=None, timeout=None, infra_step=None, stdout=None,
+             stderr=None, stdin=None, ok_ret=None, step_test_data=None,
+             step_nest_level=None):
     """
     Initializes a new StepConfig step API dictionary.
 
@@ -305,7 +318,9 @@ class StepConfig(_StepConfig):
           name component of just this step. If None, this will be set to "name".
       cmd: command to run. Acceptable types: str, Path, Placeholder, or None.
       cwd (str or None): absolute path to working directory for the command
+      env_prefixes (dict): environment prefix variables, described above.
       env (dict): overrides for environment variables, described above.
+      pathsep (str): the path separator string.
       allow_subannotations (bool): if True, lets the step emit its own
           annotations. NOTE: Enabling this can cause some buggy behavior. Please
           strongly consider using step_result.presentation instead. If you have
@@ -333,7 +348,9 @@ class StepConfig(_StepConfig):
         base_name=(base_name or name),
         cmd=cmd,
         cwd=cwd,
-        env=env,
+        env_prefixes={k: tuple(v) for k, v in (env_prefixes or {}).iteritems()},
+        env=env or {},
+        pathsep=(pathsep or os.pathsep),

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

this should just be required... os.pathsep might accidentally cause a sadness I
think?

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

Yeah, but "StepConfig" is called in a lot of places internally to
"recipe_engine" for testing, nesting, phantom steps, etc., and having each site
supply a "pathsep" is weird. I think defaulting to "os.pathsep" is safe; the
"step" module will do the right thing here.

         allow_subannotations=bool(allow_subannotations),
         trigger_specs=[TriggerSpec._create(**trig)
                        for trig in (trigger_specs or ())],
@@ -349,6 +366,7 @@ class StepConfig(_StepConfig):
 
   def render_to_dict(self):
     self = self._replace(
+        env_prefixes={k: list(v) for k, v in self.env_prefixes.iteritems()},

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

map(str, v)?

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

I don't think that'd work. We need this here b/c comparison against JSON will
project tuples into lists. I think this isn't a bad way to write the
translation.

         trigger_specs=[trig._render_to_dict()
                        for trig in (self.trigger_specs or ())],
     )