Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove --use-first-matching-interpreter misfeature #1076

Merged
merged 1 commit into from Oct 14, 2020
Merged

Remove --use-first-matching-interpreter misfeature #1076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 1 addition & 38 deletions pex/bin/pex.py
View file
Open in desktop
Expand Up @@ -526,22 +526,6 @@ def configure_clp_pex_environment(parser):
"distributions can be found.",
)

group.add_option(
"--use-first-matching-interpreter",
dest="use_first_matching_interpreter",
default=False,
action="callback",
callback=parse_bool,
help=(
"If multiple interpreters are valid, use the first one, which is the minimum "
"compatible Python version. Normally, when multiple interpreters match, Pex will "
"resolve requirements for each interpreter; this allows the resulting Pex to be "
"compatible with more interpreters, such as different Python versions. However, "
"resolving for multiple interpreters will take longer to build, and the resulting PEX "
"may be larger."
),
)

parser.add_option_group(group)


Expand Down Expand Up @@ -803,18 +787,6 @@ def to_python_interpreter(full_path_or_basename):
)

interpreter = min(interpreters) if interpreters else None
if options.use_first_matching_interpreter and interpreters:
if len(interpreters) > 1:
unused_interpreters = set(interpreters) - {interpreter}
TRACER.log(
"Multiple interpreters resolved, but only using {} because "
"`--use-first-matching-interpreter` was used. These interpreters were matched but "
"will not be used: {}".format(
interpreter.binary,
", ".join(interpreter.binary for interpreter in sorted(unused_interpreters)),
)
)
interpreters = [interpreter]

try:
with open(options.preamble_file) as preamble_fd:
Expand Down Expand Up @@ -850,16 +822,7 @@ def walk_and_do(fn, src_dir):
pex_info.pex_root = options.runtime_pex_root
pex_info.strip_pex_env = options.strip_pex_env

# If we're only building the PEX for the first of many interpreters due to
# `--use-first-matching-interpreter` selection, we do not want to enable those same interpreter
# constraints at runtime, where they could lead to a different interpreter being selected
# leading to a failure to execute the PEX. Instead we rely on the shebang set by that single
# interpreter to pick out a similar interpreter at runtime (for a CPython interpreter, the
# shebang will be `#!/usr/bin/env pythonX.Y` which should generally be enough to select a
# matching interpreter. To be clear though, there are many corners this will not work for
# including mismatching abi (python2.7m vs python2.7mu) when the PEX contains platform specific
# wheels, etc.
if options.interpreter_constraint and not options.use_first_matching_interpreter:
if options.interpreter_constraint:
for ic in options.interpreter_constraint:
pex_builder.add_interpreter_constraint(ic)

Expand Down
47 changes: 0 additions & 47 deletions tests/test_integration.py
View file
Open in desktop
Expand Up @@ -619,53 +619,6 @@ def test_interpreter_resolution_pex_python_path_precedence_over_pex_python():
assert correct_interpreter_path in stdout


def test_use_first_matching_interpreter():
# type: () -> None
py35_path = ensure_python_interpreter(PY35)
py36_path = ensure_python_interpreter(PY36)
env = make_env(PEX_PYTHON_PATH=os.pathsep.join((py35_path, py36_path)))

def run_pex_with_py36(use_first_matching_flag):
with temporary_dir() as output_dir:
pex_out_path = os.path.join(output_dir, "first_matching.pex")
args = [
"--disable-cache",
"--interpreter-constraint=>=3.5",
"-o",
pex_out_path,
"psutil==5.7.0",
]
if use_first_matching_flag:
args.append("--use-first-matching-interpreter")
res = run_pex_command(args, env=env)
res.assert_success()

pex_info = PexInfo.from_pex(pex_out_path)
assert (
[] if use_first_matching_flag else [">=3.5"]
) == pex_info.interpreter_constraints

# Running with Python 3.5 should always work because that is the first matching
# interpreter.
stdin_payload = b"import sys, psutil; print(sys.executable); sys.exit(0)"
stdout, rc = run_simple_pex(pex_out_path, stdin=stdin_payload, env=env)
assert rc == 0
assert py35_path in stdout.decode()

stdout, rc = run_simple_pex(
pex_out_path, stdin=stdin_payload, env=make_env(PEX_PYTHON_PATH=py36_path)
)
return stdout, rc

without_flag_stdout, without_flag_rc = run_pex_with_py36(use_first_matching_flag=False)
assert without_flag_rc == 0
assert py36_path in without_flag_stdout.decode()

with_flag_stdout, with_flag_rc = run_pex_with_py36(use_first_matching_flag=True)
assert with_flag_rc == 1
assert bool(re.search(r"Needed.*cp-36-cp36m", with_flag_stdout.decode()))


def test_plain_pex_exec_no_ppp_no_pp_no_constraints():
# type: () -> None
with temporary_dir() as td:
Expand Down