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.

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

Add support for running with Python 3.6 and 3.7 #32

Merged
merged 55 commits into from Mar 16, 2019

Conversation

Projects
None yet
4 participants
@Eric-Arellano
Copy link
Contributor

commented Mar 12, 2019

Problem

Now that we're releasing Pants with Python 3 wheels (pantsbuild/pants#7197, overall migration tracked by pantsbuild/pants#6062), we need to add a way for every day Pants users to consume the upgrade.

See #30 for more context, including the design goals.

Solution

Parse the pants_runtime_python_version from pants.ini to determine which Python interpreter to use. If it's missing, default to Python 2.7.

This requires modifying the venv folders to support multiple Python versions. Rather than using the naming scheme ${pants_version}, we now consistently use ${pants_version}_py{py_major_minor}, regardless of if the user is using pants_runtime_python_version.

It also requires upgrading virtualenv to avoid a deprecation warning when running with Python 3.

Finally, we add tests to ensure this all works.

Result

Running ./pants like normal for the first time will bootstrap Pants again and create the new folder 1.15.dev3_py{27,36,37}.

Changing the $PYTHON env var or the pants_runtime_python_version in pants.ini will change which Python version is used to run Pants.

When changing interpreters for the first time, it will bootstrap Pants again. After that, you can change without issue.

Eric-Arellano added some commits Mar 12, 2019

Parse targeted Python version via pants.ini value
Rather than using a sentinal value, read in the pants.ini value for `pants_engine_python_version`. Default to Py27 because that's what we've always used and most people will have this.
Remove pants_engine_python_version from pants.ini
We first need to land pantsbuild/pants#7363 and then release Pants with it or else the option will error out.
Fix interpreter_version failing script when interpreter_version not f…
…ound in pants.ini

For some reason it was exiting the script?! I have no idea why, because the same code works for parsing the pants_version.

This does the same thing and has the benefit of using way less lines.
Squashed commit of the following:
commit e8cf735
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:36:16 2019 -0700

    Trusty Py34 is from system, not pyenv

commit 1f7d236
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:30:08 2019 -0700

    OSX requires language: general

commit 4941956
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:23:28 2019 -0700

    Move constant to top of file

    Think this is the style used in most Python projects and ours.

commit c478ecb
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:19:29 2019 -0700

    Set Pyenv for Trusty

commit 30aab56
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:16:58 2019 -0700

    Try pyenv approach

    Path wrangling didn't work.

commit 9b9e6be
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:12:11 2019 -0700

    Global python entry -> build matrix

    See if moving back to osx_setup and linux_setup works.

commit 0dc87dc
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:09:18 2019 -0700

    Maybe fix Trusty using Py3.4

commit aaab4bf
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:05:50 2019 -0700

    Test if OSX comes pre-installed with Python?

commit 96cb6ac
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:02:44 2019 -0700

    Remove unneeded anchors from OSX shards

    Turns out Precise comes installed with 3.6 when requested! So we only need Pyenv for OSX, not Linux.

commit 4a312a4
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 09:01:31 2019 -0700

    Try to debug Trusty image using Py3.4

commit d511d99
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:48:28 2019 -0700

    Remove stray whitespace

commit 126ae2f
Merge: cd28677 4aee51a
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:47:56 2019 -0700

    Merge branch 'gh-pages' of github.com:pantsbuild/setup into ci-py

commit 4aee51a
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:46:38 2019 -0700

    Test Ubuntu Xenial (#31)

    Travis added Xenial back in November. See https://blog.travis-ci.com/2018-11-08-xenial-release.

    Adding this test results in better coverage.

commit cd28677
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:36:55 2019 -0700

    Add  prefix to script. Oops

commit 4afcdcf
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:24:43 2019 -0700

    Add comment explaining why we rewrite pants.ini like this

commit ef5bf71
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:22:55 2019 -0700

    Tweak ci.py formatting

commit 599ae46
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 08:19:38 2019 -0700

    Try to install Py36 on each shard

    Also modernize .travis.yml to use all the cool goodies we've learned in Pants's CI like anchors and the brew addon package!

commit 717271f
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 07:50:10 2019 -0700

    Remove PythonVersion and fix --pants-version unspecified

    --pants-version unspecified would fail because of a bad config, that the contrib module would refer to pants_version even though that line was deleted.

commit d1f9f0f
Author: Eric Arellano <ericarellano@me.com>
Date:   Tue Mar 12 07:40:38 2019 -0700

    Add ci.py to CI to allow parameterizing pants_version and python_version

Eric-Arellano added some commits Mar 12, 2019

Implement naming scheme
Will look like this: 1.14.dev1_py36.
Update interpreter selection test to modify pants.ini
We won't be able to resolve Py3 from PyPi until the release anyway, so we should setup the test correctly from the start. Once the Pypi release happens, we can rebase and get green CI
Upgrade version to 1.15.0.dev4
We'll need this version to get CI passing so that it can consume Py3 wheels and parse pants_engine_python_version.

Eric-Arellano added a commit that referenced this pull request Mar 13, 2019

Add ci.py helper script to allow parametrizing the pants_version (#33)
### Problem
The `./pants` script claims to support running with both a pinned `pants_version` in the `pants.ini` and with the `pants_version` unspecified.

We currently do not test this unspecified branch, which we should be doing to guarantee that functionality always works.

Further, this is pre-work for #32, which will add `--python-version` as another parameter for `ci.py` to allow an unspecified Python version vs. `pants.ini` specifying 2.7 or 3.6 (FYI resulting [`.travis.yml`](https://github.com/pantsbuild/setup/pull/32/files#diff-354f30a63fb0907d4ad57269548329e3)).

### Solution
Introduce a new `ci.py` helper script that uses argparse to have the parameter `--pants-version {unspecified,config}`.

#### Requires adding Python 3 to CI shards
Because this script uses modern Python 3—i.e. enums, f-strings, type hints, keyword-only args, and `subprocess.run()`—it requires Python 3.5+ to run.

Even though we can't yet run `./pants` with Python 3 until #32 lands, this PR does some of the pre-work in adding Python 3 to each of the shards so that we can run this script. This is valid to do because every single shard is going to run the tests with both Py27 and Py36—unlike Pants where we have dedicated shards for each interpreter—so there is only benefit in ensuring each shard has Py27 and Py36.

#### Also updates test's pinned `pants_version`

@Eric-Arellano Eric-Arellano marked this pull request as ready for review Mar 13, 2019

@Eric-Arellano

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

Reviewers: CI is going to fail on this one until we release 1.15.0.dev4, because we need the Py3 wheels and pantsbuild/pants#7363 to parse pants_engine_python_version in pants.ini.

Stu and I came up with a strategy to (hopefully) still land this change before sending the release email to pants-devel. Right after we finish the release to PyPi, I'll update the pants.ini to point to the new version and run CI. If all goes green, we'll merge this PR and send the email to pants-devel about how to consume this upgrade. If CI fails, we simply push off the announced release one week and go with a silent release.

@stuhood
Copy link
Member

left a comment

Thanks. One blocker related to the option name, but the rest looks reasonable.

Show resolved Hide resolved ci.py Outdated
Show resolved Hide resolved ci.py
Show resolved Hide resolved pants Outdated

Eric-Arellano added some commits Mar 14, 2019

Check interpreter is valid
Copy idiom we use in Pants' `virtualenv` file.
Nest conditionals in write_config_entry
Usually I don't like nested conditionals, but it reads better here.
@stuhood
Copy link
Member

left a comment

Looks good: thanks!

Show resolved Hide resolved pants Outdated

Eric-Arellano added some commits Mar 14, 2019

Restore pants.ini if command fails and --python-version changed
Same fix we made in #36 for setting up pants versions. Forgot to also apply it to setup python version.
@illicitonion
Copy link

left a comment

Works for me, though I don't know really anything about the setup repo :)

Show resolved Hide resolved ci.py Outdated

Eric-Arellano added some commits Mar 15, 2019

Remove unnecessary except clause
Daniel pointed out we don't need this. We not only don't need it, but we shouldn't have it. No matter what, we need to do the cleanup, regardless of the exception caught.
Extract temporarily_rewrite_config() context manager
Do not repeat yourself. Makes it a bit more clear also why we need to read original config and have the try finally.
Show resolved Hide resolved pants Outdated
@@ -22,9 +22,19 @@ def __str__(self):
return self.value


class PythonVersion(Enum):

This comment has been minimized.

Copy link
@cosmicexplorer

cosmicexplorer Mar 15, 2019

Love enums!!

Eric-Arellano added some commits Mar 15, 2019

Revert bad setting of '-x'
Sorry for accidently pushing!
Rename local variables to reflect new function name
No longer hardcoded to Python.
@cosmicexplorer
Copy link

left a comment

Thanks!!

Eric-Arellano added some commits Mar 15, 2019

Fix unnamed value in ci.py
We gotta get Pylint in this repo. Python only complains about unknown values once it's evaluated.
Upgrade virtualenv to 16.4.3
Running ./pants with Py3.6 or 3.7 would result in this error:
```
./pants
WARN] /Users/eric/.cache/pants/setup/bootstrap-Darwin-x86_64/1.15.0.dev4_py36/lib/python3.6/site.py:165: DeprecationWarning: 'U' mode is deprecated
  f = open(fullname, "rU")

WARN] /Users/eric/.cache/pants/setup/bootstrap-Darwin-x86_64/1.15.0.dev4_py36/lib/python3.6/site.py:165: DeprecationWarning: 'U' mode is deprecated
  f = open(fullname, "rU")

No goals specified.
Use `pants goals` to list goals.
Use `pants help` to get help.
```

To fix this, we must upgrade to 16.4.3, as the deprecation warning was fixed between 16.0.0 and this release.
See if python -m ensurepip works
I think the issue is that the wrong pip is being used? This looks like a Pip issue rather than twitter commons.
Revert "See if python -m ensurepip works"
This reverts commit fa1f224.

Still fails :/ but passes locally.
Squashed commit of the following:
commit 4799cfa
Author: Eric Arellano <ericarellano@me.com>
Date:   Fri Mar 15 23:59:37 2019 -0700

    Remove pinned setuptools
Squashed commit of the following:
commit c738904
Author: Eric Arellano <ericarellano@me.com>
Date:   Sat Mar 16 01:11:30 2019 -0700

    Remove bad leftover line

commit 4799cfa
Author: Eric Arellano <ericarellano@me.com>
Date:   Fri Mar 15 23:59:37 2019 -0700

    Remove pinned setuptools

Eric-Arellano added a commit that referenced this pull request Mar 16, 2019

Skip flaky Pantsd tests for OSX 10.12 shard (#40)
### Problem
Due to pantsbuild/pants#6714, and maybe pantsbuild/pants#7323, the OSX 12 shard frequently flakes when Pantsd is enabled, as seen [here](https://travis-ci.org/pantsbuild/setup/jobs/507257503#L432), [here](https://travis-ci.org/pantsbuild/setup/jobs/507238744#L545), and [here](https://travis-ci.org/pantsbuild/setup/jobs/507238744#L681).

This blocks #32, as that PR will result in each shard running with Pantsd a total of 6 more times, which results in an almost certain guarantee at least one shard will flake.

### Solution
Temporarily skip pantsd tests for OSX 10.12.

We do not completely remove the shard, as the first two tests still work and are useful to include.

@Eric-Arellano Eric-Arellano merged commit 416ab35 into pantsbuild:gh-pages Mar 16, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:py3 branch Mar 16, 2019

@Eric-Arellano Eric-Arellano referenced this pull request Mar 19, 2019

Closed

Add support for Python 3 #30

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.