[context] Add path prefix/suffix manipulation.

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 c1119ea275e2477c788f0706b32a2076d5092287..aef0add477d7e45c5ba8b74b75fd713bda18a20c 100644
--- a/recipe_modules/context/api.py
+++ b/recipe_modules/context/api.py
@@ -26,6 +26,7 @@ Example:
 
 
 import collections
+import os
 
 from contextlib import contextmanager
 
@@ -41,6 +42,7 @@ def check_type(name, var, expect):
 
 
 class ContextApi(RecipeApi):
+
   # TODO(iannucci): move implementation of these data directly into this class.
   def __init__(self, **kwargs):
     super(RecipeApi, self).__init__(**kwargs)
@@ -51,10 +53,13 @@ class ContextApi(RecipeApi):
     self._name_prefix = ['']
     # this could be a number, but it makes the logic easier to use a stack.
     self._nest_level = [0]
+    self._prefix_paths = [{}]
+    self._suffix_paths = [{}]
 
   @contextmanager
   def __call__(self, cwd=None, env=None, increment_nest_level=None,
-               infra_steps=None, name_prefix=None):
+               infra_steps=None, name_prefix=None, path_prefix=None,
+               path_suffix=None):
     """Allows adjustment of multiple context values in a single call.
 
     Contextual data:
@@ -71,6 +76,12 @@ class ContextApi(RecipeApi):
         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.

[iannucci, 2017/06/05 19:23:04]

wdyt about something like:

  with api.context(env={"PATH": api.context.Prefix(a, b, c)}):

Then we don't need to additional kwargs, just Prefix and Suffix. I would also
give Prefix and Suffix an __add__ so you could do `Prefix(...)+Suffix(...)`. I
would strongly consider banning mixed use (i.e. an envvar is either
Prefix/Suffix, or it's a string, not both).

[dnj, 2017/06/07 02:59:29]

That seems fine. For now I think we need mixed-use to transition to the new
notation, but I'll add a TODO to forbid mixed-use once that's done.

+      * path_prefix (dict) - Environmental variable prefix path overrides.
+        Consider using the "path_prefix" and "path_remove" helper functions
+        instead of directly manipulating this value.
+      * path_suffix (dict) - Environmental variable suffix path overrides.
+        Consider using the "path_suffix" and "path_remove" helper functions
+        instead of directly manipulating this value.

[iannucci, 2017/06/05 19:23:04]

I don't like the helper function idea; we should keep __call__ as the only way
to set stuff in the context module. I have a plan to clean up the api.step.nest
thing later too, in case that's what you're modeling this after.

[dnj, 2017/06/07 02:59:29]

SGTM

 
     Name prefixes:
 
@@ -99,8 +110,15 @@ class ContextApi(RecipeApi):
         Which, at the time the step executes, will inject the current value of
         $PATH.
 
-    TODO(iannucci): implement env_paths which allows for easier manipulation of
-    `pathsep` environment variables like $PATH, $PYTHONPATH, etc.
+    path_prefix and path_suffix are mappings of environment variables to lists
+    of Path elements, meant to be applied to "pathsep"-separated environment
+    variables such as PATH and PYTHONPATH. These values are prefixed and/or
+    appended to their environment variable key, separated by the OS-specific
+    path separator.
+      * If key begins with a "!" (e.g., "!PATH"), the named element will be
+        removed from its environment instead of added.
+      * Consider using helper functions instead of directly manipulating this
+        value.
 
     TODO(iannucci): combine nest_level and name_prefix
 
@@ -174,6 +192,42 @@ class ContextApi(RecipeApi):
       self._env.append(new)
       to_pop.append(self._env)
 
+    for var, collection, name, is_prefix in (
+        (path_prefix, self._prefix_paths, 'path_prefix', True),
+        (path_suffix, self._suffix_paths, 'path_suffix', False)):
+      if var is None or var == {}:
+        continue
+      check_type(name, var, dict)
+      new = dict(collection[-1])
+      for k, paths in var.iteritems():
+        check_type(k, paths, collections.Iterable)

[iannucci, 2017/06/05 19:23:04]

check_type("KEY %s[%r]" % (name, k), ...)

so then the error looks like:

  TypeError('KEY path_prefix["foobar"] is not ....')

[dnj, 2017/06/07 02:59:28]

No longer needed.

+        remove = k.startswith('!')
+        if remove:
+          k = k[1:]

[iannucci, 2017/06/05 19:23:04]

let's remove the removal functionality for now

[dnj, 2017/06/07 02:59:29]

Done.

+
+        # Remove any occurrences of the current value and any duplicates from
+        # "paths". This will keep the prefix/suffix lists clean.
+        nv = new.get(k, ())
+        seen = set()
+        unique_paths = []
+        for i, path in enumerate(paths):
+          check_type('%s element #%d' % (k, i), path, Path)

[iannucci, 2017/06/05 19:23:04]

'%s[%r]' % (name, k)

[dnj, 2017/06/07 02:59:28]

Also no longer needed.

+          nv = tuple(x for x in nv if x != path)
+          if path not in seen:
+            unique_paths.append(path)
+            seen.add(path)
+        if not remove:
+          if is_prefix:
+            nv = tuple(unique_paths) + nv
+          else:
+            nv += tuple(unique_paths)
+        if nv:
+          new[k] = nv
+        else:
+          new.pop(k, None)
+      collection.append(new)
+      to_pop.append(collection)
+
     try:
       yield
     finally:
@@ -203,7 +257,19 @@ class ContextApi(RecipeApi):
     """
     # TODO(iannucci): store env in an immutable way to avoid excessive copies.
     # TODO(iannucci): handle case-insensitive keys on windows
-    return dict(self._env[-1])
+    env = dict(self._env[-1])
+    for collection, is_prefix in (
+        (self._prefix_paths, True),
+        (self._suffix_paths, False)):
+      for key, paths in collection[-1].iteritems():
+        cur = env.get(key, '%%(%s)s' % (key,))
+        cur = cur.split(os.pathsep) if cur else []

[iannucci, 2017/06/05 19:24:04]

this will fail in production/simulation; use self.m.path.pathsep.

[dnj, 2017/06/07 02:59:29]

Done.

+        if is_prefix:
+          cur = list(str(p) for p in paths) + cur
+        else:
+          cur += list(str(p) for p in paths)
+        env[key] = os.pathsep.join(cur)
+    return env

[iannucci, 2017/06/05 19:23:04]

I'm not sure how I feel about this; I think that step.__call__ should be the
only thing assembling this, and that all other uses of env should just deal with
env. In fact, I would go so far as to make it an error to have the same envvar
key manipulated via env and also the prefix/suffix methods.

Then we should add a prefix_paths and suffix_paths @property to keep this
class's API uniform.

[dnj, 2017/06/07 02:59:29]

Done.

 
   @property
   def infra_step(self):
@@ -232,3 +298,65 @@ class ContextApi(RecipeApi):
     Returns (int) - The current nesting level.
     """
     return self._nest_level[-1]
+
+  @contextmanager
+  def path_prefix(self, key, *paths):
+    """Adds specified paths to the beginning of an environment variable.
+
+    Each path in paths is added, in order, as a prefix to the environment
+    variable "key", delimited by the OS path separator. This can be used for
+    easy manipulation of path environment variables such as PATH and PYTHONPATH.
+
+    If "key" begins with an "!", the specified paths will be removed from the
+    prefix. The "path_remove" helper function may be a more elegant way to
+    remove a path.
+
+    Args:
+      key (str): The environment variable key.
+      paths (...Path): The list of paths to prefix.
+    """
+    check_type('key', key, str)
+    with self(path_prefix={key: paths}):
+      yield
+
+  @contextmanager
+  def path_suffix(self, key, *paths):
+    """Adds specified paths to the end of an environment variable.
+
+    Each path in paths is added, in order, as a suffix to the environment
+    variable "key", delimited by the OS path separator. This can be used for
+    easy manipulation of path environment variables such as PATH and PYTHONPATH.
+
+    If "key" begins with an "!", the specified paths will be removed from the
+    prefix. The "path_remove" helper function may be a more elegant way to
+    remove a path.
+
+    Args:
+      key (str): The environment variable key.
+      paths (...Path): The list of paths to suffix.
+    """
+    check_type('key', key, str)
+    with self(path_suffix={key: paths}):
+      yield
+
+  @contextmanager
+  def path_remove(self, key, *paths):

[iannucci, 2017/06/05 19:23:04]

let's remove this functionality for now. I can't think of any use that wouldn't
be a horrible encapsulation violation. It would be easy to add later if
something needed it. The additive-only API is easier to reason about.

[dnj, 2017/06/07 02:59:29]

Done.

+    """Removes specified paths from an environment variable's prefix/suffix.
+
+    Any path in "paths" will be removed from the environment's prefix and suffix
+    paths. If the path is not present, it will be ignored.
+
+    WARNING: this will NOT remove a path that was not, itself, added as a
+    prefix or suffix. For example, if a path is added to the environment as a
+    string, rather than through "path_prefix" or "path_suffix", it will not be
+    removed.
+
+    Args:
+      key (str): The environment variable key.
+      paths (...Path): The list of paths to suffix.
+    """
+    check_type('key', key, str)
+    if not key.startswith('!'):
+      key = '!'+key
+    with self(path_prefix={key: paths}, path_suffix={key: paths}):
+      yield