Modify configure to explain how to bypass python version check if required. #41051
Conversation
Signed-off-by: Stewart X Addison <sxa@redhat.com>
nodejs-github-bot
added
build
There was a problem hiding this comment.
This codebase makes liberal use of f-strings which are syntax errors in Python < 3.6 so let's not encourage our users to shoot themselves in the foot. Supporting their misadventures will distract the valuable focus of maintainers.
Also, note that Python 3.6 will reach its end of life in 22 days.
https://github.com/psf/black set max-line-length to 88 characters.
configure
Outdated
| @@ -32,4 +32,10 @@ else: | |||
| python_cmd_path = which(python_cmd) | |||
| if python_cmd_path and 'pyenv/shims' not in python_cmd_path: | |||
| sys.stderr.write('\t{} {}\n'.format(python_cmd_path, ' '.join(sys.argv[:1]))) | |||
| if sys.version_info > (3, 10): | |||
| sys.stderr.write('Running "configure" with newer versions of python has not been tested and may not work.\n') | |||
There was a problem hiding this comment.
| sys.stderr.write('Running "configure" with newer versions of python has not been tested and may not work.\n') | |
| sys.stderr.write('Running "configure" with newer versions of Python has not been tested and may not work.\n') |
configure
Outdated
| else: | ||
| sys.stderr.write('Running "configure" with a pre-3.6 version of python is not supported and will likely not work.\n') | ||
| sys.stderr.write('To bypass this check (not recommended) run "python ./configure.py\n"') |
There was a problem hiding this comment.
| else: | |
| sys.stderr.write('Running "configure" with a pre-3.6 version of python is not supported and will likely not work.\n') | |
| sys.stderr.write('To bypass this check (not recommended) run "python ./configure.py\n"') |
sxa
force-pushed
the
python_warning_enhancement
branch
from
d6e24f2 to
394873d
Compare
Signed-off-by: Stewart X Addison <sxa@redhat.com>
sxa
force-pushed
the
python_warning_enhancement
branch
from
394873d to
41ab0b1
Compare
I've adjusted the message in that situation to make it explicit that it will not work with an earlier version, and not display the message on how to bypass. Also capitalised |
There was a problem hiding this comment.
I don't think this is a good idea to warn about new versions. The latest python versions are going to be used frequently by most people working on core. The check however would soon be outdated and would have to be maintained instead.
Having a warning about older versions SGTM.
Not sure if I'd agree on that. My development environment generally doesn't use anything other than the version supplied with my linux distribution, so on the three machines I have next to me one has 3.6, one has 3.7 and the other has 3.9. In the past I have fallen foul of the check and this PR is an attempt to highlight when that occurs and offer users a path forwards rather than just leaving them confused.
I would not have considered it to onerous to update the check along with the supported list in the file - it's a relatively small script so unlikely that it wouldn't get spotted. I could modify the check to explicitly look for <3.6 (which we know won't work, as per Christian's comment) and leave the |
|
Sorry @sxa but after all this discussion, my vote would be to close this PR with unmerged changes. The current Python version logic seems to work well at preventing the use of python_version < 3.6 or python_version > 3.10 and it is easiest to change in just one location. As @BridgeAR rightly points out, we do not want to encourage a lot of experimentation with unsupported versions until after our CI tests have been updated to stress test those versions. |
|
I do understand that we don't want people to use older versions of python, but I'd expect we need to test at some point with newer versions. If this is too much would adding the explanation to the documentation be an alternative? |
|
I don't really agree with not givin the user any hints but since it seems this isn't going to be approved I'll close it. As mentioned by MD I can look at possibly adding something to the documentation. |
As new versions of python come out it's possible that a user will be using a later one than our supported list. This PR adds the message in that situation telling people how they can bypass the check and to encourage raising an issue/PR to support the new version if it works. It also adds information about the lowest supported version with an indication that earlier ones are unlikely to work (I could just remove this check, but it's good to be nice to users!)
I don't think this will be too controversial unless there is an objection to the new lines not being wrapped at <80 characters :-)