Python style checker dependencies not consumable for cross-py2/py3 linting iff building a single-python pex #7158
Comments
|
And for the record: this was exposed by the deprecation removal in #7063... apparently we had not actually whitelisted any Py3-only targets, so we weren't actually linting them before. |
I'm not sure how you concluded that. Does the pants pex contain the python checkstyle plugin or not? Have you run with PEX_VERBOSE=9? |
|
If you can add more detail that would be great. I was able to nominally do what you describe as failing with: And: $ git diff --cached
diff --git a/contrib/python/examples/src/python/three/BUILD b/contrib/python/examples/src/python/three/BUILD
new file mode 100644
index 000000000..b801e5d3a
--- /dev/null
+++ b/contrib/python/examples/src/python/three/BUILD
@@ -0,0 +1,3 @@
+python_library(
+ compatibility='>=3.4,<4',
+)
diff --git a/contrib/python/examples/src/python/three/lint.py b/contrib/python/examples/src/python/three/lint.py
new file mode 100644
index 000000000..d2f799eac
--- /dev/null
+++ b/contrib/python/examples/src/python/three/lint.py
@@ -0,0 +1,5 @@
+def func1():
+ pass
+
+def func2():
+ passAnd then: The And this is what I'd expect since the checker wheel is universal. |
|
@illicitonion reports what now seems to be a related issue running pants from sources (https://www.pantsbuild.org/howto_develop.html). I likewise don't repro his issue. Do you all include the checker wheel in your wheelhouse?: https://pypi.org/project/pantsbuild.pants.contrib.python.checks.checker/1.14.0.dev2/#files |
|
@jsirois : I believe that a critical bit of the repro (have updated the description) is that this is a pants pex built from published wheels using: pants/build-support/bin/release.sh Lines 520 to 523 in 60fc885 This results in a pex containing the following So the wheel inside the pex is a 2.7 tagged wheel. |
|
Thanks - that helps. I'll dig a bit more. NB: the wheel is universal though: |
|
I can almost guarantee you this is due to |
Hah. You're right... good eye.
Is #7159 likely to help? or would we need to have the wheel building shards do something to request a multi-python wheel? |
jsirois
changed the title
|
Actually - the description is still not right. |
|
#7159 would be a bandaid. Fundamentally you are trying to buold a pex that never does tool / plugin resolves yet still works for 2 and 3. If you left the checker out of the pex, it would resolve as 2 or 3 as needed. Since you don't, the pex really needs to be built for both 2 + 3 simultaneously. Anything else works by luck. |
|
I'll play around here with building a proper pex to make sure this is even possible today. |
|
Yeah - so this is not possible to fix correctly until pantsbuild.pants* are released for python2 and python3. The #7159 bandaid should work, but expect a blowup later. |
We use pants/build-support/bin/release.sh Lines 31 to 33 in 60fc885 |
|
Sigh - understood. I was hoping for an exact command line - it's nice to have exact repro data. |
The environment looks something like: |
|
Alright - there are a host of issues here. The checker tool is mainly a red herring. The primary issue to contend with is building a comprehensive pex that must run some or all portions under python2 and python3. After some quick experiments, fwict, a pex supporting this cannot be built in-general. In specific cases it can happen to work - namely when all relevant dists and their transitivie deps have py2.py3 wheels published / resolvable. In general though this precondition can't be relied upon, and in the specific case of |
|
@jsirois : Thank you for thinking about this. In terms of short term workarounds, might it be a reasonable hack to temporarily rename the Empirically, everything other than style checking is working in our environment, so while there is undoubtedly more work to do to unblock a universal pex, the primary case that we need to unblock the next stable release is the ability to check Py3 code from a Py2/single-python pex. |
|
I think it would be more reasonable to remove the future dependency from the checker. Looks like we only do: Seems to me not worth the baggage of building a fake wheel when the dep can be dropped with a few lines of code in, say, common.py: https://github.com/pantsbuild/pants/blob/f30c612f7b9c70e0b1f4cf234d8c9155a8b27508/contrib/python/src/python/pants/contrib/python/checks/checker/common.py |
|
Actually - it already deps on six - which is well behaved. I'll spin up a quick PR. |
### Problem `future==0.16.0` cannot be resolved in the context of `linux_x86_64-cp-36-cp36m` (see the subtext of #7158), and it seems likely that `future==0.17.1` can be, since its pypi entry claims compatibility with python 3.6. Either way, updating to the most recent version seems win-win. ### Solution Update to `future==0.17.1`.
PEX does not support multi-python pexes in-general and in-particular it cannot properly build a pex for a py2/3 compatible sdist today. Work around this limitation by removing future (which is distributed as an sdist) from the checker deps. Hack to workaround #7158
|
#7178 is confirmed working as a workaround using the example py3 linting target I posted a diff of above: |
|
Thank you John: have confirmed that this fix works in the case in the description in our repo. Would closing this in favor of #6450 be reasonable, or should I open something to cover the specific case of "the checker's dep's need to be maximally compatible"? |
|
Closing in favor of #6450 is reasonable since that work will be blocked on the series of pex issues that need to be resolved to support the pex side of that issue - which I've added a note about. |
…7340) This reverts commit 3a81e86. ### Problem The use of the `asttokens` library, now within the pythonstyle checker pex, is causing a variant of #7158 when consumed outside of this repo. Additionally, some files fail to parse with: ``` for tok in tokenize.generate_tokens(StringIO(str_node_text).readline): File "/opt/ee/python/2.7/lib/python2.7/tokenize.py", line 374, in generate_tokens ("<tokenize>", lnum, pos, line)) File "<tokenize>", line 3 "else echo 'no_op'; fi)" ``` Suppressing the lint only works if the lint runs without error, so this makes it impossible to consume outside of the pants repo right now (or at least, impossible in a lot of repos outside of the pants repo). ### Solution - Revert #7286. ### Result This will be unreverted asap when the above issues are investigated and fixed.
### Problem The workaround for #7158 was to remove usage of `future` in the python style `checker` pex, but documentation was not left behind to defend against re-addition, and so it regressed. ### Solution Switch from `future` to `six`, and add a comment. ### Result Checking py3 code from within a py2 pex should work.
When attempting to use a py2 pants pex (edit: built using
build-support/bin/release.sh -p) to lint a py3 target in an external repo at master (technically 0406f0f), the following error occurs:At least with regard to the
checkerwheel, this is likely related to #6450. Thefuturedependency is something else... possibly we just need an update to the latest version?The text was updated successfully, but these errors were encountered: