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

build: relax upper limit on python version check #48136

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bnoordhuis
Copy link
Member

We can't entirely remove the limit because we want to print a helpful message with python installs we found but we can at least relax it to a much wider range.

Python 3.x is very likely compatible with 3.x-1 so don't be too uptight about what versions we accept, as long as it's python >= 3.6.

Refs: #48130

We can't entirely remove the limit because we want to print a helpful
message with python installs we found but we can at least relax it to
a much wider range.

Python 3.x is very likely compatible with 3.x-1 so don't be too uptight
about what versions we accept, as long as it's python >= 3.6.

Refs: nodejs#48130
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. labels May 23, 2023
@sxa
Copy link
Member

sxa commented May 23, 2023

Noting that when I suggested relaxing the version check (Giving the user information on how to override it if they wanted to do so at their own risk) it was rejected: #41051

So while I'd be in favour of this as an even more strong solution that what I proposed, there may be disagreement elsewhere ;-)

@Trott Trott added the python PRs and issues that require attention from people who are familiar with Python. label May 23, 2023
@Trott
Copy link
Member

Trott commented May 23, 2023

@nodejs/python

@Trott Trott requested a review from cclauss May 23, 2023 17:31
@cclauss
Copy link
Contributor

cclauss commented May 24, 2023

From my perspective, I would like us to use https://devguide.python.org/versions as our metric. If the Python Core Team supports a version of Python then we should also support it.

Python 3.6 is EOL (since 2021) and Python 3.12 is currently a late alpha an early beta pre-release. Python 3.7 goes EOL in one month.

If folks want to use EOL or pre-release versions of Python then we should provide minimal (not recommended) support. If folks are paying for long-term support for an OS that has an EOL version of Python then their OS provider can support them and we are no longer obliged to do so. If folks are using an OS that is beyond the standard support and are not paying for LTS, then their problems are not for us to solve.

Building on secure platforms and being able to use new language optimizations and features helps both our safety and velocity. Python 3.11 is 25% faster than Python 3.10 and this optimization trend will continue in Py3.12 and then 3.13 annual releases. Supporting four versions of Python is sufficient. Let's encourage security and performance upgrades.

if sys.version_info[:2] in acceptable_pythons:
major, minor, patch = sys.version_info[:3]
print('Node.js configure: Found Python {}.{}.{}...'.format(major, minor, patch))
if major > 3 or major == 3 and minor >= 6:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if major > 3 or major == 3 and minor >= 6:
if sys.version_info >= (3, 8):

https://devguide.python.org/versions/#versions

@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 13, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 13, 2023
@nodejs-github-bot
Copy link
Collaborator

@AdamMajer
Copy link
Contributor

This just managed to hit a snag with Python 3.12 in Fedora Rawhide. distutils was removed, so from distutils.version import StrictVersion is now broken in configure.

https://peps.python.org/pep-0632/
python/cpython#92584

Simply relaxing the check, while it works most of the time, it can cause issues when not tested.

@aduh95
Copy link
Contributor

aduh95 commented May 11, 2024

This needs a rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. python PRs and issues that require attention from people who are familiar with Python.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants