From 1ed1fdbb43fc967678a6bce476deec3dca779edf Mon Sep 17 00:00:00 2001
From: Daniel McClanahan <1305167+cosmicexplorer@users.noreply.github.com>
Date: Wed, 21 Feb 2018 11:45:30 -0800
Subject: [PATCH] refactor out unnecessary interpreter wrapper to use a
 contextmanager

---
 src/python/pants/backend/python/BUILD         |  6 --
 .../backend/python/sandboxed_interpreter.py   | 64 -------------------
 .../tasks/build_local_python_distributions.py | 40 ++++++++++--
 3 files changed, 33 insertions(+), 77 deletions(-)
 delete mode 100644 src/python/pants/backend/python/sandboxed_interpreter.py

diff --git a/src/python/pants/backend/python/BUILD b/src/python/pants/backend/python/BUILD
index 0da3c87ce2e..79bbbb292e5 100644
--- a/src/python/pants/backend/python/BUILD
+++ b/src/python/pants/backend/python/BUILD
@@ -28,7 +28,6 @@ python_library(
     ':python_chroot',
     ':python_requirement',
     ':python_requirements',
-    ':sandboxed_interpreter',
     ':sdist_builder',
     ':thrift_builder',
   ]
@@ -126,11 +125,6 @@ python_library(
   sources = ['python_requirements.py'],
 )
 
-python_library(
-  name = 'sandboxed_interpreter',
-  sources = ['sandboxed_interpreter.py'],
-)
-
 python_library(
   name = 'sdist_builder',
   sources = ['sdist_builder.py'],
diff --git a/src/python/pants/backend/python/sandboxed_interpreter.py b/src/python/pants/backend/python/sandboxed_interpreter.py
deleted file mode 100644
index a8b19622c3a..00000000000
--- a/src/python/pants/backend/python/sandboxed_interpreter.py
+++ /dev/null
@@ -1,64 +0,0 @@
-# coding=utf-8
-# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
-# Licensed under the Apache License, Version 2.0 (see LICENSE).
-
-from __future__ import (absolute_import, division, generators, nested_scopes, print_function,
-                        unicode_literals, with_statement)
-
-import os
-
-from pex.interpreter import PythonInterpreter
-
-from pants.util.memo import memoized_method
-
-
-class SandboxedInterpreter(PythonInterpreter):
-
-  class ToolchainLocationError(Exception):
-    def __init__(self, dir_path):
-      msg = "path '{}' does not exist or is not a directory".format(dir_path)
-      super(ToolchainLocationError, self).__init__(msg)
-
-  class BaseInterpreterError(Exception): pass
-
-  # using another PythonInterpreter to populate the superclass constructor args
-  def __init__(self, llvm_base_dir, base_interp):
-
-    if not os.path.isdir(llvm_base_dir):
-      raise ToolchainLocationError(llvm_base_dir)
-    if not isinstance(base_interp, PythonInterpreter):
-      raise BaseInterpreterError(
-        "invalid PythonInterpreter: '{}'".format(repr(base_interp)))
-
-    self._llvm_base_dir = llvm_base_dir
-
-    # this feels a little hacky -- what if pex's PythonInterpreter later needs
-    # another constructor arg that's not just a property of the class?
-    super(SandboxedInterpreter, self).__init__(
-      base_interp.binary, base_interp.identity, extras=base_interp.extras)
-
-  # made into an instance method here (unlike PythonInterpreter superclass) to
-  # use instance property self._llvm_base_dir
-  @memoized_method
-  def sanitized_environment(self):
-    sanitized_env = super(SandboxedInterpreter, self).sanitized_environment()
-
-    # use our compiler at the front of the path
-    # TODO: when we provide ld and stdlib headers, don't add the original path
-    sanitized_env['PATH'] = ':'.join([
-      os.path.join(self._llvm_base_dir, 'bin'),
-      os.environ.get('PATH'),
-    ])
-
-    # TODO: figure out whether we actually should be compiling fat binaries.
-    # this line tells distutils to only compile for 64-bit archs -- if not, it
-    # will attempt to build a fat binary for 32- and 64-bit archs, which makes
-    # clang invoke "lipo", an osx command which does not appear to be open
-    # source. see Lib/distutils/sysconfig.py and Lib/_osx_support.py in CPython.
-    sanitized_env['ARCHFLAGS'] = '-arch x86_64'
-
-    env_vars_to_scrub = ['CC', 'CXX']
-    for env_var in env_vars_to_scrub:
-      sanitized_env.pop(env_var, None)
-
-    return sanitized_env
diff --git a/src/python/pants/backend/python/tasks/build_local_python_distributions.py b/src/python/pants/backend/python/tasks/build_local_python_distributions.py
index b53e6b3defa..ceb02fb4364 100644
--- a/src/python/pants/backend/python/tasks/build_local_python_distributions.py
+++ b/src/python/pants/backend/python/tasks/build_local_python_distributions.py
@@ -8,12 +8,12 @@
 import glob
 import os
 import shutil
+from contextlib import contextmanager
 
 from pex.interpreter import PythonInterpreter
 
 from pants.backend.native.subsystems.llvm import LLVM
 from pants.backend.python.python_requirement import PythonRequirement
-from pants.backend.python.sandboxed_interpreter import SandboxedInterpreter
 from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary
 from pants.backend.python.tasks.pex_build_util import is_local_python_dist
 from pants.backend.python.tasks.setup_py import SetupPyRunner
@@ -67,7 +67,6 @@ def execute(self):
                             fingerprint_strategy=DefaultFingerprintStrategy(),
                             invalidate_dependents=True) as invalidation_check:
         interpreter = self.context.products.get_data(PythonInterpreter)
-        sandboxed_interpreter = SandboxedInterpreter(self.llvm_base_dir, interpreter)
 
         for vt in invalidation_check.invalid_vts:
           if vt.target.dependencies:
@@ -76,7 +75,7 @@ def execute(self):
                          'List any 3rd party requirements in the install_requirements argument '
                          'of your setup function.'
             )
-          self._create_dist(vt.target, vt.results_dir, sandboxed_interpreter)
+          self._create_dist(vt.target, vt.results_dir, interpreter)
 
         for vt in invalidation_check.all_vts:
           dist = self._get_whl_from_dir(os.path.join(vt.results_dir, 'dist'))
@@ -100,13 +99,40 @@ def _copy_sources(self, dist_tgt, dist_target_dir):
                                   src_relative_to_target_base)
       shutil.copyfile(abs_src_path, src_rel_to_results_dir)
 
-  def _create_dist(self, dist_tgt, dist_target_dir, sandboxed_interpreter):
+  @contextmanager
+  def _sandboxed_setuppy(self):
+    sanitized_env = os.environ.copy()
+
+    # use our compiler at the front of the path
+    # TODO: when we provide ld and stdlib headers, don't add the original path
+    sanitized_env['PATH'] = ':'.join([
+      os.path.join(self.llvm_base_dir, 'bin'),
+      sanitized_env.get('PATH'),
+    ])
+
+    # TODO: figure out whether we actually should be compiling fat binaries.
+    # this line tells distutils to only compile for 64-bit archs -- if not, it
+    # will attempt to build a fat binary for 32- and 64-bit archs, which makes
+    # clang invoke "lipo", an osx command which does not appear to be open
+    # source. see Lib/distutils/sysconfig.py and Lib/_osx_support.py in CPython.
+    sanitized_env['ARCHFLAGS'] = '-arch x86_64'
+
+    env_vars_to_scrub = ['CC', 'CXX']
+    for env_var in env_vars_to_scrub:
+      sanitized_env.pop(env_var, None)
+
+    with environment_as(**sanitized_env):
+      yield
+
+
+  def _create_dist(self, dist_tgt, dist_target_dir, interpreter):
     """Create a .whl file for the specified python_distribution target."""
     self.context.log.debug("dist_target_dir: '{}'".format(dist_target_dir))
     self._copy_sources(dist_tgt, dist_target_dir)
-    # Build a whl using SetupPyRunner and return its absolute path.
-    setup_runner = SetupPyRunner(dist_target_dir, 'bdist_wheel', interpreter=sandboxed_interpreter)
-    setup_runner.run()
+    with self._sandboxed_setuppy():
+      # Build a whl using SetupPyRunner and return its absolute path.
+      setup_runner = SetupPyRunner(dist_target_dir, 'bdist_wheel', interpreter=interpreter)
+      setup_runner.run()
 
   def _inject_synthetic_dist_requirements(self, dist, req_lib_addr):
     """Inject a synthetic requirements library that references a local wheel.