Skip to content

Commit

Permalink
Upgrade to Pex 1.6.11. (pantsbuild#8287)
Browse files Browse the repository at this point in the history
This brings in a fix to isolate `PYTHONPATH` allowing us to undo some
manual `PYTHONPATH` isolation and fixing other cases where we did not
manually hack up isolation.

The release notes are here:
  https://github.com/pantsbuild/pex/releases/v1.6.11
  • Loading branch information
jsirois authored Sep 22, 2019
1 parent 4d4bb48 commit a1bda4b
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 40 deletions.
2 changes: 1 addition & 1 deletion 3rdparty/python/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Markdown==2.1.1
packaging==16.8
parameterized==0.6.1
pathspec==0.5.9
pex==1.6.10
pex==1.6.11
psutil==5.6.3
Pygments==2.3.1
pyopenssl==17.3.0
Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/backend/python/rules/download_pex_bin.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ class DownloadedPexBin(datatype([('executable', str), ('directory_digest', Diges
@rule(DownloadedPexBin, [])
def download_pex_bin():
# TODO: Inject versions and digests here through some option, rather than hard-coding it.
url = 'https://github.com/pantsbuild/pex/releases/download/v1.6.10/pex'
digest = Digest('f4ac0f61f0c469537f04418e9347658340b0bce4ba61d302c5b805d194dda482', 1868106)
url = 'https://github.com/pantsbuild/pex/releases/download/v1.6.11/pex'
digest = Digest('7a8fdfce2de22d25ba38afaa9df0282c33dd436959b3a5c3f788ded2ccc2cae9', 1867604)
snapshot = yield Get(Snapshot, UrlToFetch(url, digest))
yield DownloadedPexBin(executable=snapshot.files[0], directory_digest=snapshot.directory_digest)

Expand Down
16 changes: 2 additions & 14 deletions src/python/pants/backend/python/tasks/pytest_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,24 +448,12 @@ def _do_run_tests_with_args(self, test_targets, pex, args):
try:
env = dict(os.environ)

# TODO(John Sirois): remove PYTHONPATH hacks when we ingest a pex with fixes for:
# https://github.com/pantsbuild/pex/issues/707
# https://github.com/pantsbuild/pex/issues/764
#
# The right answer here though is probably to use v2 parameterized Get to add in the
# extra_pythonpath to the built pex instead of using PYTHONPATH to hack them into the PEX
# environment.

# Ensure we don't leak source files or undeclared 3rdparty requirements into the pytest PEX
# environment.
pythonpath = env.pop('PYTHONPATH', None)
if pythonpath:
self.context.log.warn('scrubbed PYTHONPATH={} from pytest environment'.format(pythonpath))
# But allow this back door for users who do want to force something onto the test pythonpath,
# Allow this back door for users who do want to force something onto the test pythonpath,
# e.g., modules required during a debugging session.
extra_pythonpath = self.get_options().extra_pythonpath
if extra_pythonpath:
env['PYTHONPATH'] = os.pathsep.join(extra_pythonpath)
env['PEX_INHERIT_PATH'] = 'prefer'

# The pytest runner we use accepts a --pdb argument that will launch an interactive pdb
# session on any test failure. In order to support use of this pass-through flag we must
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,6 @@ def prepare_pex_env(self, env: Optional[Dict[str, str]] = None) -> Dict[str, str
interpreter_search_path_env = ensure_interpreter_search_path_env(interpreter)
env.update(interpreter_search_path_env)

# TODO(John Sirois): remove when we ingest a pex with a fix for:
# https://github.com/pantsbuild/pex/issues/707
# Ensure we don't leak source files or undeclared 3rdparty requirements into the PEX
# environment.
pythonpath = env.pop('PYTHONPATH', None)
if pythonpath:
self.context.log.warn('scrubbed PYTHONPATH={} from environment'.format(pythonpath))

return env

def create_pex(self, pex_info=None):
Expand Down
11 changes: 0 additions & 11 deletions src/python/pants/backend/python/tasks/python_tool_prep_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,24 +56,13 @@ def output(self, args, stdin_payload=None, binary_mode=False, **kwargs):

@contextmanager
def run_with(self, workunit_factory, args, **kwargs):
# TODO(John Sirois): remove when we ingest a pex with a fix for:
# https://github.com/pantsbuild/pex/issues/707
# Ensure we don't leak source files or undeclared 3rdparty requirements into the PEX
# environment.
supplied_env = kwargs.pop('env', None)
env = (supplied_env or os.environ).copy()
pythonpath = env.pop('PYTHONPATH', None)
if pythonpath:
self.logger.warning('scrubbed PYTHONPATH={} from environment'.format(pythonpath))

cmdline = self._pretty_cmdline(args)
with workunit_factory(cmd=cmdline) as workunit:
exit_code = self._pex.run(args,
stdout=workunit.output('stdout'),
stderr=workunit.output('stderr'),
with_chroot=False,
blocking=True,
env=env,
**kwargs)
yield cmdline, exit_code, workunit

Expand Down
11 changes: 11 additions & 0 deletions src/python/pants/bin/pants_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,18 @@ def _should_run_with_pantsd(self, global_bootstrap_options):
not self.will_terminate_pantsd() and \
not global_bootstrap_options.concurrent

@staticmethod
def scrub_pythonpath():
# If PYTHONPATH was used to set up the Pants runtime environment, its entries our now on our
# `sys.path` allowing us to run. Do not propagate any of these Pants-specific sys.path entries
# forward to our subprocesses.
pythonpath = os.environ.pop('PYTHONPATH', None)
if pythonpath:
logger.warning(f'Scrubbed PYTHONPATH={pythonpath} from the environment.')

def run(self):
self.scrub_pythonpath()

options_bootstrapper = OptionsBootstrapper.create(env=self._env, args=self._args)
bootstrap_options = options_bootstrapper.bootstrap_options
global_bootstrap_options = bootstrap_options.for_global_scope()
Expand Down
17 changes: 14 additions & 3 deletions src/python/pants/pantsd/pants_daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,8 @@ def _write_named_sockets(self, socket_map):

def run_sync(self):
"""Synchronously run pantsd."""
os.environ.pop('PYTHONPATH')

# Switch log output to the daemon's log stream from here forward.
# Also, register an exiter using os._exit to ensure we only close stdio streams once.
self._close_stdio()
Expand Down Expand Up @@ -420,11 +422,20 @@ def run_sync(self):

def post_fork_child(self):
"""Post-fork() child callback for ProcessManager.daemon_spawn()."""
entry_point = '{}:launch'.format(__name__)
exec_env = combined_dict(os.environ, dict(PANTS_ENTRYPOINT=entry_point))
spawn_control_env = dict(PANTS_ENTRYPOINT=f'{__name__}:launch',
# The daemon should run under the same sys.path as us; so we ensure
# this. NB: It will scrub PYTHONPATH once started to avoid infecting
# its own unrelated subprocesses.
PYTHONPATH=os.pathsep.join(sys.path))
exec_env = combined_dict(os.environ, spawn_control_env)

# Pass all of sys.argv so that we can proxy arg flags e.g. `-ldebug`.
cmd = [sys.executable] + sys.argv
self._logger.debug('cmd is: PANTS_ENTRYPOINT={} {}'.format(entry_point, ' '.join(cmd)))

spawn_control_env_vars = ' '.join(f'{k}={v}' for k, v in spawn_control_env.items())
cmd_line = ' '.join(cmd)
self._logger.debug(f'cmd is: {spawn_control_env_vars} {cmd_line}')

# TODO: Improve error handling on launch failures.
os.spawnve(os.P_NOWAIT, sys.executable, cmd, env=exec_env)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def test_fails_ctrl_c_on_import(self):
pants_run = self.run_pants_with_workdir(self._lifecycle_stub_cmdline(), workdir=tmpdir)
self.assert_failure(pants_run)

self.assertEqual(dedent("""\
self.assertIn(dedent("""\
Interrupted by user:
ctrl-c during import!
"""), pants_run.stderr_data)
Expand Down

0 comments on commit a1bda4b

Please sign in to comment.