Bias selecting the current interpreter. #783
Conversation
Preiously we made some effort towards this, but were not thorough. Fix all interpreter selection logic to be both lazy and bias towards re-using the current interpreter, only falling back to picking the minimum version compatible interpreter from all possible compatibile interpreters as the very last step. Addresses item 1 in pex-tool#782.
| @@ -574,7 +574,7 @@ def to_python_interpreter(full_path_or_basename): | |||
| pex_python_path = rc_variables.get('PEX_PYTHON_PATH', '') | |||
| else: | |||
| pex_python_path = "" | |||
| interpreters = find_compatible_interpreters(pex_python_path, constraints) | |||
| interpreters = list(iter_compatible_interpreters(pex_python_path, constraints)) | |||
There was a problem hiding this comment.
Why create a list here? Wouldn't it cause the same performance problem again (albeit only a build time)? Could we do something like this?
try:
interpreter = next(iter_compatible_interpreters(pex_python_path, constraints))
except StopIteration:
die('Could not find compatible interpreter', CANNOT_SETUP_INTERPRETER)
Or is it specifically important to choose the min interpreter here?
There was a problem hiding this comment.
I was just gunning for changing ~no logic and, yeah, it was only at build time so even less of a concern.
There was a problem hiding this comment.
Thing is, pex build time can be build tool user runtime. For example, when iterating on some tests and rebuilding the test sources chroot, this would run every time I think?
There was a problem hiding this comment.
It would. Keep in mind two things though:
- The interpreters list is used below to build a multi-interpreter / multi-platform pex.
- There are only actually more than one interpreter if either more than one --python are passed (ie:
--python=python2.7 --python=python3) or one or more--interpreter-constraintis passed.
Due to item 1, we must materialize the full list. The 1st half of item 2 is a feature, the second half of item 2 may be a bug in the ballpark of #658 and #694.
Biasing interpreter selection towards the current interpreter was implemented in pex-tool#783 but not test was added. Add a test that fails due to a bug in path trimming logic and fix the bug.
Biasing interpreter selection towards the current interpreter was implemented in #783 but not test was added. Add a test that fails due to a bug in path trimming logic and fix the bug.
Preiously we made some effort towards this, but were not thorough.
Fix all interpreter selection logic to be both lazy and bias towards
re-using the current interpreter, only falling back to picking the
minimum version compatible interpreter from all possible compatibile
interpreters as the very last step.
Addresses item 1 in #782.