From 56b364b9cc59f100e32d4575b0a94cd87eb9f3d9 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Fri, 15 Feb 2019 13:52:59 +0000 Subject: [PATCH 01/11] Always honor interprerter constraints --- pex/pex_bootstrapper.py | 4 ++++ tests/test_integration.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/pex/pex_bootstrapper.py b/pex/pex_bootstrapper.py index 3c5bc8e35..b002dee4f 100644 --- a/pex/pex_bootstrapper.py +++ b/pex/pex_bootstrapper.py @@ -116,6 +116,10 @@ def maybe_reexec_pex(compatibility_constraints): elif ENV.PEX_PYTHON_PATH: target = _select_interpreter(ENV.PEX_PYTHON_PATH, compatibility_constraints) + else: + # Apply constraints to target using regular PATH + target = _select_interpreter(pex_python_path=None, compatibility_constraints=compatibility_constraints) + if target and os.path.realpath(target) != os.path.realpath(sys.executable): cmdline = [target] + sys.argv TRACER.log('Re-executing: cmdline="%s", sys.executable="%s", PEX_PYTHON="%s", ' diff --git a/tests/test_integration.py b/tests/test_integration.py index 9db98c64d..2225eae39 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -415,6 +415,36 @@ def test_interpreter_resolution_with_pex_python_path(): else: assert str(pex_python_path.split(':')[0]).encode() in stdout +@pytest.mark.skipif(IS_PYPY) +def test_interpreter_constraints_honored_without_ppp_or_pp(): + + # Create a pex with interpreter constraints, but for not the default interpreter in the path. + with temporary_dir() as td: + + py36_path = ensure_python_interpreter(PY36) + + pex_out_path = os.path.join(td, 'pex.pex') + env = make_env( + PEX_IGNORE_RCFILES="1", + PATH=os.path.dirname(py36_path) + ) + res = run_pex_command(['--disable-cache', + '--interpreter-constraint===%s' % PY36, + '-o', pex_out_path], + env=env + ) + res.assert_success() + + # We want to try to run that pex with no environment variables set + stdin_payload = b'import sys; print(sys.executable); sys.exit(0)' + + stdout, rc = run_simple_pex(pex_out_path, stdin=stdin_payload, env=env) + print("BL: stdout = {}".format(stdout)) + assert rc == 0 + + # If the constraints are honored, it will have run python3.6 and not python3.5 + if sys.version_info[0] == 3: + assert ("Python %s" % PY36) in str(stdout).split("\n")[0] @pytest.mark.skipif(NOT_CPYTHON36) def test_interpreter_resolution_pex_python_path_precedence_over_pex_python(): From ca0afe7724a847a409c3e8d7f57d7deeaf9031fe Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Fri, 15 Feb 2019 15:24:42 +0000 Subject: [PATCH 02/11] Style --- pex/pex_bootstrapper.py | 5 ++++- tests/test_integration.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pex/pex_bootstrapper.py b/pex/pex_bootstrapper.py index b002dee4f..8d7b1017b 100644 --- a/pex/pex_bootstrapper.py +++ b/pex/pex_bootstrapper.py @@ -118,7 +118,10 @@ def maybe_reexec_pex(compatibility_constraints): else: # Apply constraints to target using regular PATH - target = _select_interpreter(pex_python_path=None, compatibility_constraints=compatibility_constraints) + target = _select_interpreter( + pex_python_path=None, + compatibility_constraints=compatibility_constraints + ) if target and os.path.realpath(target) != os.path.realpath(sys.executable): cmdline = [target] + sys.argv diff --git a/tests/test_integration.py b/tests/test_integration.py index 2225eae39..2e11a506b 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -415,6 +415,7 @@ def test_interpreter_resolution_with_pex_python_path(): else: assert str(pex_python_path.split(':')[0]).encode() in stdout + @pytest.mark.skipif(IS_PYPY) def test_interpreter_constraints_honored_without_ppp_or_pp(): @@ -446,6 +447,7 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): if sys.version_info[0] == 3: assert ("Python %s" % PY36) in str(stdout).split("\n")[0] + @pytest.mark.skipif(NOT_CPYTHON36) def test_interpreter_resolution_pex_python_path_precedence_over_pex_python(): with temporary_dir() as td: From d76ead972fc8892b7bed46ef2ea44c1cb9f2a24f Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Mon, 18 Mar 2019 15:13:51 +0000 Subject: [PATCH 03/11] Fix test --- tests/test_integration.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 2e11a506b..84431bbd3 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -423,14 +423,19 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): with temporary_dir() as td: py36_path = ensure_python_interpreter(PY36) + py35_path = ensure_python_interpreter(PY35) pex_out_path = os.path.join(td, 'pex.pex') env = make_env( PEX_IGNORE_RCFILES="1", - PATH=os.path.dirname(py36_path) + PATH=":".join([ + os.path.dirname(py36_path), + os.path.dirname(py35_path) + ]), + PEX_VERBOSE="1" ) res = run_pex_command(['--disable-cache', - '--interpreter-constraint===%s' % PY36, + '--interpreter-constraint===%s' % PY35, '-o', pex_out_path], env=env ) @@ -443,9 +448,9 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): print("BL: stdout = {}".format(stdout)) assert rc == 0 - # If the constraints are honored, it will have run python3.6 and not python3.5 + # If the constraints are honored, it will have run python3.5 and not python3.6 if sys.version_info[0] == 3: - assert ("Python %s" % PY36) in str(stdout).split("\n")[0] + assert ("Python %s" % PY35) in str(stdout).split("\n")[0] @pytest.mark.skipif(NOT_CPYTHON36) From e98343687749ad85e146736ed2976daf030f1d81 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 10:04:45 +0000 Subject: [PATCH 04/11] Commit missing import to tests --- tests/test_integration.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 84431bbd3..6a9983f83 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -22,6 +22,7 @@ NOT_CPYTHON36, NOT_CPYTHON36_OR_LINUX, PY27, + PY35, PY36, ensure_python_interpreter, get_dep_dist_names_from_pex, @@ -418,10 +419,8 @@ def test_interpreter_resolution_with_pex_python_path(): @pytest.mark.skipif(IS_PYPY) def test_interpreter_constraints_honored_without_ppp_or_pp(): - # Create a pex with interpreter constraints, but for not the default interpreter in the path. with temporary_dir() as td: - py36_path = ensure_python_interpreter(PY36) py35_path = ensure_python_interpreter(PY35) From 2d6c535a5b4da6088f898fa7be649df61566f0d7 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 10:29:51 +0000 Subject: [PATCH 05/11] Fix leaking pex_python_path --- tests/test_integration.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 6a9983f83..7301788b3 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -430,8 +430,7 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): PATH=":".join([ os.path.dirname(py36_path), os.path.dirname(py35_path) - ]), - PEX_VERBOSE="1" + ]) ) res = run_pex_command(['--disable-cache', '--interpreter-constraint===%s' % PY35, @@ -483,12 +482,19 @@ def test_interpreter_resolution_pex_python_path_precedence_over_pex_python(): def test_plain_pex_exec_no_ppp_no_pp_no_constraints(): with temporary_dir() as td: pex_out_path = os.path.join(td, 'pex.pex') - res = run_pex_command(['--disable-cache', - '-o', pex_out_path]) + env = make_env( + PEX_IGNORE_RCFILES="1", + PATH=os.path.dirname(os.path.realpath(sys.executable)) + ) + res = run_pex_command([ + '--disable-cache', + '-o', pex_out_path], + env=env + ) res.assert_success() stdin_payload = b'import os, sys; print(os.path.realpath(sys.executable)); sys.exit(0)' - stdout, rc = run_simple_pex(pex_out_path, stdin=stdin_payload) + stdout, rc = run_simple_pex(pex_out_path, stdin=stdin_payload, env=env) assert rc == 0 assert os.path.realpath(sys.executable).encode() in stdout From 2b9e1835ef4729a0c8bf343990713ad43dcedfc1 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 16:10:51 +0000 Subject: [PATCH 06/11] Only enforce constraints if they exist --- pex/pex_bootstrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pex/pex_bootstrapper.py b/pex/pex_bootstrapper.py index 8d7b1017b..142b9fe09 100644 --- a/pex/pex_bootstrapper.py +++ b/pex/pex_bootstrapper.py @@ -116,7 +116,7 @@ def maybe_reexec_pex(compatibility_constraints): elif ENV.PEX_PYTHON_PATH: target = _select_interpreter(ENV.PEX_PYTHON_PATH, compatibility_constraints) - else: + elif compatibility_constraints: # Apply constraints to target using regular PATH target = _select_interpreter( pex_python_path=None, From 355a646f14d0f7e84b46420caba62cd396e98691 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 17:11:13 +0000 Subject: [PATCH 07/11] Fix test_setup_interpreter_constraint? Replace subprocess with run_simple_pex --- tests/test_integration.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 7301788b3..b2c73b6ae 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1160,13 +1160,19 @@ def test_setup_interpreter_constraint(): interpreter = ensure_python_interpreter(PY27) with temporary_dir() as out: pex = os.path.join(out, 'pex.pex') + env = make_env( + PEX_IGNORE_RCFILES='1', + PATH=os.path.dirname(interpreter), + ) results = run_pex_command(['jsonschema==2.6.0', '--disable-cache', '--interpreter-constraint=CPython=={}'.format(PY27), '-o', pex], - env=make_env(PATH=os.path.dirname(interpreter))) + env=env) results.assert_success() - subprocess.check_call([pex, '-c', 'import jsonschema']) + + stdout, rc = run_simple_pex(pex, env=env, stdin='import jsonschema') + assert rc == 0 @pytest.mark.skipif(IS_PYPY, From 67c61fca8d21ecefb4a08f4651e4831307fa993e Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 17:13:24 +0000 Subject: [PATCH 08/11] Update docstring --- pex/pex_bootstrapper.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pex/pex_bootstrapper.py b/pex/pex_bootstrapper.py index 142b9fe09..5a0c7fd21 100644 --- a/pex/pex_bootstrapper.py +++ b/pex/pex_bootstrapper.py @@ -94,12 +94,14 @@ def maybe_reexec_pex(compatibility_constraints): interpreter specified by PEX_PYTHON. If PEX_PYTHON_PATH is set, it attempts to search the path for a matching interpreter in accordance with the interpreter constraints. If both variables are present in a pexrc, this function gives precedence to PEX_PYTHON_PATH and errors out if no - compatible interpreters can be found on said path. If neither variable is set, fall through to - plain pex execution using PATH searching or the currently executing interpreter. + compatible interpreters can be found on said path. + + If neither variable is set, we fall back to plain PEX execution using PATH searching or the + currently executing interpreter. If compatibility constraints are used, we match those constraints + against these interpreters. :param compatibility_constraints: list of requirements-style strings that constrain the Python interpreter to re-exec this pex with. - """ if os.environ.pop('SHOULD_EXIT_BOOTSTRAP_REEXEC', None): # We've already been here and selected an interpreter. Continue to execution. From 5297a21b6e9fc06d1c0da974b1c3663806aebd07 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 17:16:03 +0000 Subject: [PATCH 09/11] Test vs PY36 instead of PY35 --- tests/test_integration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index b2c73b6ae..cb346be06 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -433,7 +433,7 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): ]) ) res = run_pex_command(['--disable-cache', - '--interpreter-constraint===%s' % PY35, + '--interpreter-constraint===%s' % PY36, '-o', pex_out_path], env=env ) @@ -448,7 +448,7 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): # If the constraints are honored, it will have run python3.5 and not python3.6 if sys.version_info[0] == 3: - assert ("Python %s" % PY35) in str(stdout).split("\n")[0] + assert ("Python %s" % PY36) in str(stdout).split("\n")[0] @pytest.mark.skipif(NOT_CPYTHON36) From 3fc9904281bdcb91615e486498e346b97e113f93 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Tue, 19 Mar 2019 17:55:44 +0000 Subject: [PATCH 10/11] Missing b --- tests/test_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index cb346be06..36c426e29 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -1171,7 +1171,7 @@ def test_setup_interpreter_constraint(): env=env) results.assert_success() - stdout, rc = run_simple_pex(pex, env=env, stdin='import jsonschema') + stdout, rc = run_simple_pex(pex, env=env, stdin=b'import jsonschema') assert rc == 0 From 263aa7ab0a27d09b9f04c9ab507f38b118c53c57 Mon Sep 17 00:00:00 2001 From: Borja Lorente Date: Wed, 20 Mar 2019 10:06:15 +0000 Subject: [PATCH 11/11] Address Comments --- tests/test_integration.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 36c426e29..4c8bd5322 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -427,9 +427,9 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): pex_out_path = os.path.join(td, 'pex.pex') env = make_env( PEX_IGNORE_RCFILES="1", - PATH=":".join([ + PATH=os.pathsep.join([ + os.path.dirname(py35_path), os.path.dirname(py36_path), - os.path.dirname(py35_path) ]) ) res = run_pex_command(['--disable-cache', @@ -443,12 +443,12 @@ def test_interpreter_constraints_honored_without_ppp_or_pp(): stdin_payload = b'import sys; print(sys.executable); sys.exit(0)' stdout, rc = run_simple_pex(pex_out_path, stdin=stdin_payload, env=env) - print("BL: stdout = {}".format(stdout)) assert rc == 0 - # If the constraints are honored, it will have run python3.5 and not python3.6 - if sys.version_info[0] == 3: - assert ("Python %s" % PY36) in str(stdout).split("\n")[0] + # If the constraints are honored, it will have run python3.6 and not python3.5 + # Without constraints, we would expect it to use python3.5 as it is the minimum interpreter + # in the PATH. + assert str(py36_path).encode() in stdout @pytest.mark.skipif(NOT_CPYTHON36)