Repository navigation
Can 'make lint-py PYTHON=python3' be a manditory Jenkins test? #1631
Description
Activity
/cc @nodejs/build
I guess the big change this would require would be Python 3 to be installed on all our CI hosts? Or is it possible to lint for Python 3 issues using Python 2? We do already run
make lint-pyon all CI test runs, but I guess that only covers Python 2 issues in the current state?I think it would be enough to test only on one host, with a custom job. And when the build supports Python 3, switch the job to use Python 3 fully.
Reacted by Ruben Bridgewater and Rich TrottI believe it would be best to move this issue to the build repo. Could someone do that (I do not have the necessary rights)? :)
Issue moved to
nodejs/buildThis has to be run on Python 3 because flake8 uses CPython to build an Abstract Syntax Tree and it then lints the AST. This is described in the Very Important Note on the top of http://flake8.pycqa.org
That being said, Python 3 was introduced 8+ years ago so today it is quite easy on all OSes to install both legacy Python and Python side-by-side on the same box and then do python2 -m flake8 [...] and python3 -m flake8 [...].
I think it would be enough to test only on one host, with a custom job. And when the build supports Python 3, switch the job to use Python 3 fully.
Oh, yeah, maybe we can just install Python 3 on the linter host(s) only and have it run there via a Jenkins config.
This effort can proceed when consensus is reached because nodejs/node#24954 has landed and make lint-py PYTHON=python2 and make lint-py PYTHON=python3 both pass.
Reacted by Rich TrottAdding to the Build WG meeting agenda.
If it's just for linting then it should be straight forward I think since it's limited to a few hosts.
But ...
Python 2 end-of-life is one year from now
Yikes .. I didn't know this but it has huge ramifications across the board for us. Some things off the top of my head:
- node-gyp and GYP in general .. I've mostly tuned out of the many attempts to make our GYP Python 3 compatible because it's such a mess (and mostly I'm just cranky at Google for being so terrible at tools—with a combination of pathological NIMBY and "bored of this, let's invent something new"). If Python 2 is genuinely something we shouldn't be relying on in a year, then we need to have a plan to migrate the entire ecosystem to a Python 3 version of GYP. That's going to take some care and skillful planning I think. Maybe it's time to consider a more radical move for addon building? Promote CMake? Revisit Fedor's gyp.js? Something else?
- Our custom Ansible stuff is only partly Python 3 compatible, mostly it works but there are special use cases where it fails still. There's a couple of PRs and issues open on this repo about this. On my macOS machine I get Python 3 with Ansible thanks to brew, but on my Linux machine I get Python 2 with Ansible, because
<reasons>.
Python 2 is still everywhere. If 2 is EOL so soon, why do we still have this kind of thing:
$ lsb_release -d Description: Ubuntu 18.04.1 LTS $ head -1 $(which ansible) #! /usr/bin/python2Have distros thrown up their hands too? I believe it's possible to install Python 3 on older hosts we maintain, like CentOS 6. Some of them are a bit awkward because a lot of internal tooling on Linux distros use Python so conflicts are really easy.
This feels to me like a big enough problem that it needs needs something at a higher level, a strategic initiative or short-lived working group to sort out all of the issues. Not that I really want to be thinking more than I have to about this mess but the pain is only going to increase. No, I'm not volunteering to lead such an initiative, our strategy of 🙈 🙉 🙊 isn't sustainable though.
https://fedoraproject.org/wiki/FinalizingFedoraSwitchtoPython3
- This is not just for linting. We will need to run parallel tests for many months. Putting both Python 2 and Python 3 on a single machine is quite easy these days (even on CentOS 6) so my recommendation would be that we put both on lots of build machines, not just on a few.
- Check out the current version of your favorite linux distro as it probably has Python 3 installed by default and not Python 2
- https://github.com/ansible/ansible is already fully supports Python 3
- https://github.com/chromium/gyp is already syntax compatible with Python 3
- https://github.com/nodejs/node-gyp needs work to become syntax compatible with Python 3
- https://github.com/nodejs/node is syntax compatible with Python 3 but its dependencies need work
Being syntax compatible != fully supporting.
- https://github.com/chromium/gyp is already syntax compatible with Python 3
Note that this is only as of ~1 week ago and it is still largely untested.
22 remaining items
Ready to go. I just wanted the patched code in
masterto get to as many people as possible.
(Still PR that were not rebased will fail linting, so I was hoping to minimize noise)Reacted by Christian ClaussOk so you are planning to turn it on in a few days or a week at most?
Ok so you are planning to turn it on in a few days or a week at most?
At most. I'll send a PSA now.
Activated for
node-test-linter(part ofnode-test-pull-request) -> https://ci.nodejs.org/job/node-test-linter/25180/consoleTesting job for
node-linter(part ofnode-test-pull-request-lite-pipeline) -> https://ci.nodejs.org/job/refack-node-linter/2/consoleReacted by Sakthipriyan Vairamani and Christian ClaussThanks massively... This will help us avoid any backsliding.
Closable now? Or is there still more to be done?
Closable now? Or is there still more to be done?
The pipeline variant hasn't gone live yet...
Reacted by Rich TrottCan we please get this resolves and closed? 303 daze left...
(Adding
build-agendaso it gets discussed at the next Build meeting if there isn't any significant progress between now and then.)IMO fixing the pipeline job has low ROI. It's mostly a convenience. The main job (https://ci.nodejs.org/job/node-test-linter/) runs python linting with Python3, so we have regression testing.
If someone wants to follow up with porting https://github.com/nodejs/build/blob/master/jenkins/pipelines/node-linter.jenkinsfile, that will be very helpful.
@rvagg @thefourtheye How do we progress this?
Is your feature request related to a problem? Please describe.
Python 2 end-of-life is one year from now. We have been working for a while and this repo's code now contains no Python 3 syntax errors or undefined names. make lint-py shows that we no longer have any syntax errors on either Python 2 or Python 3. It also shows that we have fixed all undefined names like basestring, cmp(), file, reduce(), raw_input(), unicode, xrange() that were removed in Python 3.
The port to Python 3 is not yet complete but the codebase is "syntax compatible" which is a state that we should now preserve and guard against any backsliding.
Describe the solution you'd like
After #24954 lands,it would be helpful if every Jenkins run required that make lint-py PYTHON=python2 and make lint-py PYTHON=python3 both pass. This will ensure that pull requests do not introduce syntax errors (like print without the parens) or undefined names (like raw_input()). This will help as we complete the port.Describe alternatives you've considered
Please describe alternative solutions or features you have considered.
Going snowboarding.
/cc @nodejs/python