Skip to content

Commit

Permalink
Add ability to specify Python minor version when running ./pants (#…
Browse files Browse the repository at this point in the history
…7187)

## Problem
There are three issues when trying to run `./pants3` and switching between Python 3.6 and Python 3.7.

1) You must first `rm -rf build-support/pants_dev_deps.py3.venv` and then `rm -rf ~/.cache/pants/python_cache/interpreters` because the cache will have a bad symlink to the prior Python 3 interpreter. This is inconvenient, and the latter is surprising and difficult to remember.
2) If both Python 3.6 and Python 3.7 are discoverable on the `PATH`, there is no reliable way to specify that you want to use Python 3.7. The `virtualenv` code works by invoking `python3`, which defaults in most systems to the minimum version. The only workaround is to ensure `python3` points first to `python3.7` by messing with the PATH. While this is possible, it is not obvious to the user and it would be best to provide a solution that does not require messing with the PATH.
3) When running `./pants3`, we always override `PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS` to allow Python 3.6, so it is impossible at the moment to set a tighter constraint without modifying source code. Even if we resolve issues 1 and 2, we must address this issue to ensure that spawned subprocesses use the correct interpreter.

## Solution 
### Part 1 - renamed `venv` folders
Rename `py2.venv` -> `py27.venv` and split out `py3.venv` into `py36.venv` and `py37.venv`.

This allows us to have a distinct venv for each Python version, which is necessary to avoid redownloading/resymlinking dependencies anytime we change from 3.6 to 3.7. It is also necessary to fix problem #1 of having to run `rm -rf build-support/pants_dev_deps.py3.venv ~/.cache/pants/python_cache/interpreters` whenever making this change.

### Part 2 - `$PY` env var
We refactor `virtualenv` and `pants_venv` to require the caller to pre-set the env var `$PY`. This value stores the bin name, such as `python3.7`, to be used in setting up the venv.

If the user wants to constrain `./pants3` to a specific Python 3 interpreter, they may do so by prefixing the command with `PY=python3.7 ./pants3`. Otherwise, `./pants` will default to `python2.7` and `./pants3` will default to `python3` as before.

### Also changed - renames in `.travis.yml`
Update `ci.sh` and `.travis.yml` to specify the exact Python versions as Python 2.7 and 3.6, rather than Python 2 and Python 3. `ci.sh` does make a semantic change in that it now requires exactly Python 3.6 for Py3 shards, whereas earlier it only used Py3.6 de facto. Meanwhile, `.travis.yml` changes the OSX rust tests to use Python 2.7 to fix an issue with requiring exactly Python 3.6, updates the cache directories to use the new `venv` naming scheme, and renames `py2`->`py27` and `py3`->`py36` everywhere.

This is pre-work to make the Python 3.7 CI pull request easier to review.

## Alternative solutions considered
### `./pants3` interpreter flags
Originally this PR added the CI flags `./pants -6` and `./pants -7` to allow forcing Pants to use only Python 3.6 or 3.7, respectively.

This solution was rejected for being over-constrained and too complex.

### `./pants37`
Originally we considered adding a new script `./pants37` to specify Python 3.7, rather than `./pants3 -7`. 

This was rejected because it results in too many root-level scripts that are prefixed `./pants`, which is confusing. 

Further, it is not often that Pants devs will need to switch between Python 3.6 and 3.7—we certainly will not expect them to do this frequently (whereas we do encourage frequently changing between `./pants` and `./pants3`). We merely want the ability to run with Python 3.7, particularly for CI; it is not important for the option to be especially discoverable. 

## Result
Calling `./pants` or `./pants3` for the first time will delete the legacy venv folders and use the new naming scheme.

Scripts now behave as follows:
* `./pants` will use Python 2.7 as before
* `./pants3` will use the first `python3` bin on the PATH, as before
* `PY=python3.6 ./pants3` will use only Python 3.6
* `PY=python3.7 ./pants3` will use only Python 3.7

Switching between these options will trigger an engine rebuild the first time, and then after that will change the interpreter used with no cost.
  • Loading branch information
Eric-Arellano authored Jan 31, 2019
1 parent b510618 commit 5a6ac93
Show file tree
Hide file tree
Showing 8 changed files with 453 additions and 423 deletions.
506 changes: 255 additions & 251 deletions .travis.yml

Large diffs are not rendered by default.

20 changes: 12 additions & 8 deletions build-support/bin/ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Runs commons tests for local or hosted CI.
Usage: $0 (-h|-2fxbkmrjlpuneycitzsw)
-h print out this help message
-2 Run using Python 2 (defaults to using Python 3).
-2 Run using Python 2.7 (defaults to using Python 3.6).
-f run python code formatting checks
-x run bootstrap clean-all (assume bootstrapping from a
fresh clone)
Expand Down Expand Up @@ -58,7 +58,6 @@ EOF
python_unit_shard="0/1"
python_contrib_shard="0/1"
python_intg_shard="0/1"
python_two="false"

while getopts "h2fxbmrjlpeasu:ny:ci:tzw" opt; do
case ${opt} in
Expand Down Expand Up @@ -105,17 +104,22 @@ esac
# We're running against a Pants clone.
export PANTS_DEV=1

# Determine which Python interpreter to use for bootstrapping pex and for executing subprocesses.
# Order matters here. We must constrain subprocesses before running the bootstrap stage,
# or we will encounter the _Py_Dealloc error when bootstrapping a Python 3 PEX.
# Note that we set PY, and when running with Python 3, also set PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS.
# This would usually not be necessary when developing locally, because the `./pants` and `./pants3`
# scripts set these constraints for us already. However, we must set the values here because in non-bootstrap shards
# we run CI using `./pants.pex` instead of the scripts `./pants` and `./pants3`, so those scripts cannot set
# the relevant environment variables. Without setting these environment variables, the Python 3 shards will try to
# execute subprocesses using Python 2, which results in the _Py_Dealloc error (#6985), and shards that do not
# pull down `./pants.pex` but still use a virtualenv (such as Rust Tests) will fail to execute.
if [[ "${python_two:-false}" == "false" ]]; then
py_version_number="3"
py_version_number="3.6"
bootstrap_pants_script="./pants3"
export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS='["CPython>=3.6,<4"]'
export PANTS_PYTHON_SETUP_INTERPRETER_CONSTRAINTS="['CPython==${py_version_number}.*']"
else
py_version_number="2"
py_version_number="2.7"
bootstrap_pants_script="./pants"
fi
export PY="python${py_version_number}"
banner "Using Python ${py_version_number} to execute spawned subprocesses (e.g. tests)"

if [[ "${run_bootstrap:-false}" == "true" ]]; then
Expand Down
33 changes: 21 additions & 12 deletions build-support/pants_venv
Original file line number Diff line number Diff line change
Expand Up @@ -11,26 +11,35 @@ REQUIREMENTS=(
${REPO_ROOT}/pants-plugins/3rdparty/python/requirements.txt
)

venv_dir_prefix="${REPO_ROOT}/build-support/pants_dev_deps"

function venv_dir() {
py_version="py2"; [ "${PANTS_USE_PYTHON3}" = true ] && py_version="py3"
echo ${REPO_ROOT}/build-support/pants_dev_deps.${py_version}.venv
py_venv_version=$(${PY} -c 'import sys; print("".join(map(str, sys.version_info[0:2])))')
echo "${venv_dir_prefix}.py${py_venv_version}.venv"
}

function activate_venv() {
source "$(venv_dir)/bin/activate"
}

function create_venv() {
function remove_venv() {
rm -rf "$(venv_dir)"
# If `pants_dev_deps.venv` still exists, delete both its legacy folder and the cached
# Python interpreter folder, which contains problematic symlinks.
# Note we only perform these removals for the first time, to avoid continually deleting
# the cache when switching between using `pants_dev_deps.py2.venv` and `pants_dev_deps.py3.venv`.
legacy_venv_dir="${REPO_ROOT}/build-support/pants_dev_deps.venv"
if [ -d "${legacy_venv_dir}" ]; then
rm -rf "${legacy_venv_dir}"
rm -rf "${HOME}/.cache/pants/python_cache/interpreters"
fi
# If `pants_dev_deps.venv` or `pants_dev_deps.py{2,3}.venv` still exist, delete both the legacy folders
# and the cached Python interpreter folder, which contains problematic symlinks. Note we only perform
# these removals for the first time, to avoid continually deleting the cache when switching
# between valid venvs.
legacy_venv_dirs=("${venv_dir_prefix}.venv" "${venv_dir_prefix}.py2.venv" "${venv_dir_prefix}.py3.venv")
for legacy_venv_dir in "${legacy_venv_dirs[@]}"; do
if [ -d "${legacy_venv_dir}" ]; then
rm -rf "${legacy_venv_dirs[@]}"
rm -rf "${HOME}/.cache/pants/python_cache/interpreters"
break
fi
done
}

function create_venv() {
remove_venv
"${REPO_ROOT}/build-support/virtualenv" "$(venv_dir)"
}

Expand Down
Loading

0 comments on commit 5a6ac93

Please sign in to comment.