-
-
Notifications
You must be signed in to change notification settings - Fork 649
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
Upgrade to pex 1.4.8 and eliminate workarounds. #6594
Conversation
Eeenteresting!: nedbat/coveragepy#715 I can hack around this. Fix forthcoming. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great that this closes a couple issues and kills off so much code! Thank you John.
I see you are working on fixing coverage library itself. If you want to merge this before that goes through, no worries on waiting for upstream. Seems like that could be a followup PR.
Yeah - I will not block. I think Ned takes a pretty conservative approach, especially when there is a workaround. For example: pants/src/python/pants/backend/python/tasks/coverage/plugin.py Lines 34 to 47 in 0390517
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
... and bit by an unrelated jupyter failure which I was working on independently anyhow:
Fix forthcoming for that in #6600; then I'll rebase here to pick up green CI. |
Pex now allows us to float setuptools high and run on python 3.7 and it supports sane platform expansion as well as graceful handling of non-standard `setuptools` platform reporting for Apple system interpreters allowing us to eliminate several workarounds. We also pickup a fix for concurrent pex extraction which is particularly useful when running pants from a pex using `PEX_FORCE_LOCAL`. Fixes pantsbuild#5922 Fixes pantsbuild#6363
We were always having our 1st sys.path entry nuked, but it just so happened the 0th position used to be held by the pex bootstrap which was no longer needed by the time coverage ran.
) ### Problem We originally had to manually pin setuptools to 5.4.1 to workaround pantsbuild/pants#3948. This is no longer the case thanks to multiple changes to Pex and Pants, most recently seen at pantsbuild/pants#6594. Continuing to pin to 5.4.1 prevents Python 3 from working, as seen with https://travis-ci.org/pantsbuild/setup/jobs/507117401#L553. ### Solution Remove all custom code resolving setuptools and let Pip install and resolve as it normally does. 1. 14 fewer lines of custom code, making the script easier for people to understand and for us to develop. * Note `pip` ships with `setuptools` by default for a reason. It's a sensible default. 1. We end up with the same version as Pants after all is done, which is the behavior we want. 1. If we ever needed to bump the pinned version, we would need people to curl the script again, which is very infrequent for them to do. ### Result Because we pin `setuptools` in Pants' `requirements.txt`, we end up getting the exact same setuptools that Pants is using for that version. This is the output after creating a new virtual env: ``` Successfully installed Markdown-2.1.1 Pygments-2.3.1 ansicolors-1.0.2 asn1crypto-0.24.0 asttokens-1.1.13 certifi-2019.3.9 cffi-1.11.1 chardet-3.0.4 configparser-3.5.0 contextlib2-0.5.5 cryptography-2.6.1 docutils-0.12 enum34-1.1.6 fasteners-0.14.1 faulthandler-2.6 future-0.17.1 idna-2.8 ipaddress-1.0.22 monotonic-1.5 packaging-16.8 pantsbuild.pants-1.15.0.dev4 pathspec-0.5.9 pex-1.5.3 ply-3.11 psutil-5.4.8 py-zipkin-0.17.0 pycparser-2.19 pyopenssl-17.3.0 pyparsing-2.3.1 pystache-0.5.3 pywatchman-1.4.1 requests-2.21.0 scandir-1.2 setproctitle-1.1.10 setuptools-40.4.3 six-1.12.0 subprocess32-3.2.7 thriftpy2-0.4.2 twitter.common.collections-0.3.10 twitter.common.confluence-0.3.10 twitter.common.dirutil-0.3.10 twitter.common.lang-0.3.10 twitter.common.log-0.3.10 twitter.common.options-0.3.10 urllib3-1.24.1 wheel-0.31.1 www-authenticate-0.9.2 New virtual environment successfully created at /Users/eric/.cache/pants/setup/bootstrap-Darwin-x86_64/1.15.0.dev4_py27. ```
Pex now allows us to float setuptools high and run on python 3.7 and
it supports sane platform expansion as well as graceful handling of
non-standard
setuptools
platform reporting for Apple systeminterpreters allowing us to eliminate several workarounds.
We also pickup a fix for concurrent pex extraction which is particularly
useful when running pants from a pex using
PEX_FORCE_LOCAL
.Fixes #5922
Fixes #6363
Fixes #6397