Make access to PEX_* environment variables consistent, fixing pex -v …

…issues and #102.

pex/bin/pex.py

@@ -32,8 +32,8 @@
 from pex.resolvable import Resolvable
 from pex.resolver import CachingResolver, Resolver, Unsatisfiable
 from pex.resolver_options import ResolverOptionsBuilder
-from pex.tracer import TRACER, TraceLogger
-from pex.variables import Variables
+from pex.tracer import TRACER
+from pex.variables import ENV, Variables
 from pex.version import SETUPTOOLS_REQUIREMENT, WHEEL_REQUIREMENT, __version__
 
 CANNOT_DISTILL = 101
@@ -504,7 +504,7 @@ def main():
 
   options, reqs = parser.parse_args(args=args)
 
-  with TraceLogger.env_override(PEX_VERBOSE=options.verbosity):
+  with ENV.patch(PEX_VERBOSE=str(options.verbosity)):
     with TRACER.timed('Building pex'):
       pex_builder = build_pex(reqs, options, resolver_options_builder)
 

pex/pex.py

@@ -44,7 +44,9 @@ def clean_environment(cls):
       del os.environ['MACOSX_DEPLOYMENT_TARGET']
     except KeyError:
       pass
-    for key in filter(lambda key: key.startswith('PEX_'), os.environ):
+    # Cannot change dictionary size during __iter__
+    filter_keys = [key for key in os.environ if key.startswith('PEX_')]
+    for key in filter_keys:
       del os.environ[key]
 
   def __init__(self, pex=sys.argv[0], interpreter=None, env=ENV):
@@ -247,9 +249,8 @@ def patch_all(path, path_importer_cache, modules):
     finally:
       patch_all(old_sys_path, old_sys_path_importer_cache, old_sys_modules)
 
-  @classmethod
-  def _wrap_coverage(cls, runner, *args):
-    if 'PEX_COVERAGE' not in os.environ and 'PEX_COVERAGE_FILENAME' not in os.environ:
+  def _wrap_coverage(self, runner, *args):
+    if not self._vars.PEX_COVERAGE and self._vars.PEX_COVERAGE_FILENAME is None:
       runner(*args)
       return
 
@@ -258,8 +259,9 @@ def _wrap_coverage(cls, runner, *args):
     except ImportError:
       die('Could not bootstrap coverage module, aborting.')
 
-    if 'PEX_COVERAGE_FILENAME' in os.environ:
-      cov = coverage.coverage(data_file=os.environ['PEX_COVERAGE_FILENAME'])
+    pex_coverage_filename = self._vars.PEX_COVERAGE_FILENAME
+    if pex_coverage_filename is not None:
+      cov = coverage.coverage(data_file=pex_coverage_filename)
     else:
       cov = coverage.coverage(data_suffix=True)
 
@@ -274,14 +276,13 @@ def _wrap_coverage(cls, runner, *args):
 
       # TODO(wickman) Post-process coverage to elide $PEX_ROOT and make
       # the report more useful/less noisy.  #89
-      if 'PEX_COVERAGE_FILENAME' in os.environ:
+      if pex_coverage_filename:
         cov.save()
       else:
         cov.report(show_missing=False, ignore_errors=True, file=sys.stdout)
 
-  @classmethod
-  def _wrap_profiling(cls, runner, *args):
-    if 'PEX_PROFILE' not in os.environ and 'PEX_PROFILE_FILENAME' not in os.environ:
+  def _wrap_profiling(self, runner, *args):
+    if not self._vars.PEX_PROFILE and self._vars.PEX_PROFILE_FILENAME is None:
       runner(*args)
       return
 
@@ -295,17 +296,18 @@ def _wrap_profiling(cls, runner, *args):
     try:
       return profiler.runcall(runner, *args)
     finally:
-      if 'PEX_PROFILE_FILENAME' in os.environ:
-        profiler.dump_stats(os.environ['PEX_PROFILE_FILENAME'])
+      if self._vars.PEX_PROFILE_FILENAME is not None:
+        profiler.dump_stats(self._vars.PEX_PROFILE_FILENAME)
       else:
-        profiler.print_stats(sort=os.environ.get('PEX_PROFILE_SORT', 'cumulative'))
+        profiler.print_stats(sort=self._vars.PEX_PROFILE_SORT)
 
   def execute(self):
     """Execute the PEX.
 
     This function makes assumptions that it is the last function called by
     the interpreter.
     """
+    teardown_verbosity = self._vars.PEX_TEARDOWN_VERBOSE
     try:
       with self.patch_sys():
         working_set = self._activate()
@@ -330,13 +332,18 @@ def execute(self):
       # squash all exceptions on interpreter teardown -- the primary type here are
       # atexit handlers failing to run because of things such as:
       #   http://stackoverflow.com/questions/2572172/referencing-other-modules-in-atexit
-      if not self._vars.PEX_TEARDOWN_VERBOSE:
+      if not teardown_verbosity:
         sys.stderr.flush()
         sys.stderr = DevNull()
         sys.excepthook = lambda *a, **kw: None
 
   def _execute(self):
-    if self._vars.PEX_INTERPRETER:
+    force_interpreter = self._vars.PEX_INTERPRETER
+
+    self.clean_environment()
+
+    if force_interpreter:
+      TRACER.log('PEX_INTERPRETER specified, dropping into interpreter')
       return self.execute_interpreter()
 
     if self._pex_info_overrides.script and self._pex_info_overrides.entry_point:
@@ -354,15 +361,10 @@ def _execute(self):
     elif self._pex_info.entry_point:
       return self.execute_entry(self._pex_info.entry_point)
     else:
+      TRACER.log('No entry point specified, dropping into interpreter')
       return self.execute_interpreter()
 
   def execute_interpreter(self):
-    force_interpreter = self._vars.PEX_INTERPRETER
-    # TODO(wickman) Is this necessary?
-    if force_interpreter:
-      self._vars.delete('PEX_INTERPRETER')
-    TRACER.log('%s, dropping into interpreter' % (
-        'PEX_INTERPRETER specified' if force_interpreter else 'No entry point specified'))
     if sys.argv[1:]:
       try:
         with open(sys.argv[1]) as fp:
@@ -376,13 +378,6 @@ def execute_interpreter(self):
       code.interact()
 
   def execute_script(self, script_name):
-    # TODO(wickman) This should be acheived by running clean_environment
-    # prior to invocation.  #90
-    if 'PEX_SCRIPT' in os.environ:
-      del os.environ['PEX_SCRIPT']
-
-    # TODO(wickman) PEXEnvironment should probably have a working_set property
-    # or possibly just __iter__.
     dists = list(self._activate())
 
     entry_point = get_entry_point_from_console_script(script_name, dists)
@@ -404,8 +399,7 @@ def execute_content(cls, name, content, argv0=None):
       die('Unable to parse %s.  PEX script support only supports Python scripts.')
     old_name, old_file = globals().get('__name__'), globals().get('__file__')
     try:
-      old_argv0 = sys.argv[0]
-      sys.argv[0] = argv0
+      old_argv0, sys.argv[0] = sys.argv[0], argv0
       globals()['__name__'] = '__main__'
       globals()['__file__'] = name
       exec_function(ast, globals())
@@ -470,6 +464,9 @@ def run(self, args=(), with_chroot=False, blocking=True, setsid=False, **kw):
 
     cmdline = self.cmdline(args)
     TRACER.log('PEX.run invoking %s' % ' '.join(cmdline))
-    process = subprocess.Popen(cmdline, cwd=self._pex if with_chroot else os.getcwd(),
-                               preexec_fn=os.setsid if setsid else None, **kw)
+    process = subprocess.Popen(
+        cmdline,
+        cwd=self._pex if with_chroot else os.getcwd(),
+        preexec_fn=os.setsid if setsid else None,
+        **kw)
     return process.wait() if blocking else process

pex/tracer.py

@@ -1,7 +1,6 @@
 # Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import os
 import sys
 import threading
 import time
@@ -38,25 +37,6 @@ class TraceLogger(object):
   """
     A multi-threaded tracer.
   """
-  _ENV_OVERRIDES = {}
-
-  @classmethod
-  @contextmanager
-  def env_override(cls, **override_dict):
-    OLD_ENV = cls._ENV_OVERRIDES.copy()
-    cls._ENV_OVERRIDES.update(**override_dict)
-    yield
-    cls._ENV_OVERRIDES = OLD_ENV
-
-  @classmethod
-  def env_filter(cls, env_variable):
-    def predicate(verbosity):
-      try:
-        env_verbosity = int(os.environ.get(env_variable, cls._ENV_OVERRIDES.get(env_variable, -1)))
-      except ValueError:
-        env_verbosity = -1
-      return verbosity <= env_verbosity
-    return predicate
 
   def __init__(self, predicate=None, output=sys.stderr, clock=time, prefix=''):
     """

pex/variables.py

@@ -5,6 +5,7 @@
 # checkstyle: noqa
 
 import os
+from contextlib import contextmanager
 
 from .common import die
 
@@ -32,8 +33,7 @@ def iter_help(cls):
       yield variable_name, variable_type, variable_text
 
   def __init__(self, environ=None):
-    # TODO(wickman) Should this be an os.environ.copy()?
-    self._environ = environ if environ is not None else os.environ
+    self._environ = environ.copy() if environ is not None else os.environ
 
   def copy(self):
     return self._environ.copy()
@@ -72,6 +72,15 @@ def _get_int(self, variable, default=None):
     except KeyError:
       return default
 
+  @contextmanager
+  def patch(self, **kw):
+    """Update the environment for the duration of a context."""
+    old_environ = self._environ
+    self._environ = self._environ.copy()
+    self._environ.update(kw)
+    yield
+    self._environ = old_environ
+
   @property
   def PEX_ALWAYS_CACHE(self):
     """Boolean

tox.ini

@@ -111,10 +111,10 @@ commands = pex {posargs:}
 commands = pex {posargs:}
 
 [testenv:py27-package]
-commands = pex --cache-dir {envtmpdir}/buildcache setuptools wheel requests . -o dist/pex -e pex.bin.pex:main -v
+commands = pex --cache-dir {envtmpdir}/buildcache wheel requests . -o dist/pex -e pex.bin.pex:main -v
 
 [testenv:py34-package]
-commands = pex --cache-dir {envtmpdir}/buildcache setuptools wheel requests . -o dist/pex -e pex.bin.pex:main -v
+commands = pex --cache-dir {envtmpdir}/buildcache wheel requests . -o dist/pex -e pex.bin.pex:main -v
 
 # Would love if you didn't have to enumerate environments here :-\
 [testenv:py26]