[context] Split "env" values and prefixes.

recipe_modules/context/api.py

Index: recipe_modules/context/api.py
diff --git a/recipe_modules/context/api.py b/recipe_modules/context/api.py
index 4f9b9d4beaebfc1f2250b3e76d99c333d1f07eb4..cfa832ff8cb1547f04336492d8f18a5c3c6adc03 100644
--- a/recipe_modules/context/api.py
+++ b/recipe_modules/context/api.py
@@ -42,15 +42,6 @@ def check_type(name, var, expect):
       name, expect.__name__, var, type(var).__name__))
 
 
-_EnvPathComponent = collections.namedtuple('_EnvPathComponent', (
-    'paths',))
-
-# prefixes is a list of strings
-# value is either a string or None
-_EnvValue = collections.namedtuple('_EnvValue', (
-    'prefixes', 'value'))
-
-
 class ContextApi(RecipeApi):
 
   # TODO(iannucci): move implementation of these data directly into this class.
@@ -58,6 +49,7 @@ class ContextApi(RecipeApi):
     super(RecipeApi, self).__init__(**kwargs)
 
     self._cwd = [None]
+    self._env_prefixes = [{}]
     self._env = [{}]
     self._infra_step = [False]
     self._name_prefix = ['']
@@ -65,24 +57,26 @@ class ContextApi(RecipeApi):
     self._nest_level = [0]
 
   @contextmanager
-  def __call__(self, cwd=None, env=None, increment_nest_level=None,
-               infra_steps=None, name_prefix=None):
+  def __call__(self, cwd=None, env_prefixes=None, env=None,
+               increment_nest_level=None, infra_steps=None, name_prefix=None):
     """Allows adjustment of multiple context values in a single call.
 
     Contextual data:
       * cwd (Path) - the current working directory to use for all steps.
         To 'reset' to the original cwd at the time recipes started, pass
         `api.path['start_dir']`.
-      * infra_steps (bool) - if steps in this context should be considered
-        infrastructure steps. On failure, these will raise InfraFailure
-        exceptions instead of StepFailure exceptions.
+      * env_prefixes (dict) - Environmental variable prefix augmentations. See
+          below for more info.
+      * env (dict) - Environmental variable overrides. See below for more info.
       * increment_nest_level (True) - increment the nest level by 1 in this
         context. Typically you won't directly interact with this, but should
         use api.step.nest instead.
+      * infra_steps (bool) - if steps in this context should be considered
+        infrastructure steps. On failure, these will raise InfraFailure
+        exceptions instead of StepFailure exceptions.
       * name_prefix (str) - A string to prepend to the names of all steps in
         this context. These compose with '.' characters if multiple name prefix
         contexts occur. See below for more info.
-      * env (dict) - Environmental variable overrides. See below for more info.
 
     Name prefixes:
 
@@ -109,76 +103,78 @@ class ContextApi(RecipeApi):
             "/path/to/my/stuff:%(PATH)s"
         Which, at the time the step executes, will inject the current value of
         $PATH.
-      * A sentinel value such as Prefix to attach a specific component to a
-        pathsep-delimited list variable.
 
-    TODO(iannucci,dnj): Disallow "env" values to be mixes of string or
-    Prefix/Suffix.
+    "env_prefix" is a list of Path or strings that get prefixed to their
+    respective environment variables, delimited with the system's path
+    separator. This can be used to add entries to environment variables such
+    as "PATH" and "PYTHONPATH". If prefixes are specified and a value is also
+    defined in "env", it will be installed as the last path component if it is
+    not empty.
 
     TODO(iannucci): combine nest_level and name_prefix
 
     Look at the examples in "examples/" for examples of context module usage.
     """
     to_pop = []
+    def _push(st, val):
+      st.append(val)
+      to_pop.append(st)
 
     if cwd is not None:
       check_type('cwd', cwd, Path)
-      self._cwd.append(cwd)
-      to_pop.append(self._cwd)
+      _push(self._cwd, cwd)
 
     if infra_steps is not None:
       check_type('infra_steps', infra_steps, bool)
-      self._infra_step.append(infra_steps)
-      to_pop.append(self._infra_step)
+      _push(self._infra_step, infra_steps)
 
     if increment_nest_level is not None:
       check_type('increment_nest_level', increment_nest_level, bool)
       if not increment_nest_level:
         raise ValueError('increment_nest_level=False makes no sense')
-      self._nest_level.append(self.nest_level+1)
-      to_pop.append(self._nest_level)
+      _push(self._nest_level, self.nest_level+1)
 
     if name_prefix is not None:
       check_type('name_prefix', name_prefix, str)
       cur = self.name_prefix
       if cur:
-        self._name_prefix.append('%s.%s' % (cur, name_prefix))
-      else:
-        self._name_prefix.append(name_prefix)
-      to_pop.append(self._name_prefix)
+        name_prefix = '%s.%s' % (cur, name_prefix)
+      _push(self._name_prefix, name_prefix)
+
+    if env_prefixes is not None and len(env_prefixes) > 0:
+      check_type('env_prefixes', env_prefixes, dict)
+      new = dict(self._env_prefixes[-1])
+      for k, v in env_prefixes.iteritems():
+        if not v:
+          continue
+        k = str(k)
+        new[k] = tuple(v) + new.get(k, ())
+      _push(self._env_prefixes, new)
 
-    if env is not None and env != {}:
+    if env is not None and len(env) > 0:
       check_type('env', env, dict)
       # we hit _env directly to avoid an extra copy.
       new = dict(self._env[-1])
       for k, v in env.iteritems():
         k = str(k)
-        if v is None:
-          ev = _EnvValue(prefixes=(), value=None)
-        else:
-          ev = new.get(k, _EnvValue(prefixes=(), value=''))
-          if isinstance(v, _EnvPathComponent):
-            ev = ev._replace(prefixes=v.paths+ev.prefixes)
-          else:
-            v = str(v)
-            try:
-              # This odd little piece of code does the following:
-              #   * add a bogus dictionary format %(foo)s to v. This forces %
-              #     into 'dictionary lookup' mode
-              #   * format the result with a defaultdict. This allows all
-              #     `%(key)s` format lookups to succeed, but any sequential `%s`
-              #     lookups to fail.
-              # If the string contains any accidental sequential lookups, this
-              # will raise an exception. If not, then this is a pluasible format
-              # string.
-              ('%(foo)s'+v) % collections.defaultdict(str)
-            except Exception:
-              raise ValueError(('Invalid %%-formatting parameter in envvar, '
-                                'only %%(ENVVAR)s allowed: %r') % (v,))
-            ev = ev._replace(value=v)
-        new[k] = ev
-      self._env.append(new)
-      to_pop.append(self._env)
+        if v is not None:
+          v = str(v)
+          try:
+            # This odd little piece of code does the following:
+            #   * add a bogus dictionary format %(foo)s to v. This forces %
+            #     into 'dictionary lookup' mode
+            #   * format the result with a defaultdict. This allows all
+            #     `%(key)s` format lookups to succeed, but any sequential `%s`
+            #     lookups to fail.
+            # If the string contains any accidental sequential lookups, this
+            # will raise an exception. If not, then this is a pluasible format
+            # string.
+            ('%(foo)s'+v) % collections.defaultdict(str)
+          except Exception:
+            raise ValueError(('Invalid %%-formatting parameter in envvar, '
+                              'only %%(ENVVAR)s allowed: %r') % (v,))
+        new[k] = v
+      _push(self._env, new)
 
     try:
       yield
@@ -209,20 +205,21 @@ class ContextApi(RecipeApi):
     """
     # TODO(iannucci): store env in an immutable way to avoid excessive copies.
     # TODO(iannucci): handle case-insensitive keys on windows
-    def parts(ev):
-      for p in ev.prefixes:
-        yield str(p)
-      if ev.value:
-        yield ev.value
+    return dict(self._env[-1])
+
+  @property
+  def env_prefixes(self):
+    """Returns Path prefix modifications to the environment.
 
-    ret = {}
-    for k, ev in self._env[-1].iteritems():
-      if ev.value is None:
-        ret[k] = None
-      else:
-        ret[k] = self.m.path.pathsep.join(parts(ev))
+    This will return a mapping of environment key to Path tuple for Path
+    prefixes registered with the environment.
 
-    return ret
+    Returns (dict) - The env-key -> value(Path) mapping of current environment
+      prefix modifications.
+    """
+    # TODO(iannucci): store env in an immutable way to avoid excessive copies.
+    # TODO(iannucci): handle case-insensitive keys on windows
+    return dict(self._env_prefixes[-1])
 
   @property
   def infra_step(self):
@@ -251,18 +248,3 @@ class ContextApi(RecipeApi):
     Returns (int) - The current nesting level.
     """
     return self._nest_level[-1]
-
-  def Prefix(self, *paths):
-    """Returns: an assignable "env" value that prefixes the specified paths to
-    the beginning of an environment variable.
-
-    Each path in paths is added, in order, as a prefix to the environment
-    variable, delimited by the OS path separator. This can be used for
-    easy manipulation of path environment variables such as PATH and PYTHONPATH.
-
-    Args:
-      paths (...Path): The list of paths to prefix.
-    """
-    for i, path in enumerate(paths):
-        check_type('path element %d' % (i,), path, Path)
-    return _EnvPathComponent(paths=paths)