-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add a check for python version in make
and gracefully exit rather than fail on python version 2.
#7109
Comments
maybe something like PYTHON_MINIMUM_VERSION = (3, 6, 0)
if sys.version_info < PYTHON_MINIMUM_VERSION:
logger.error("Kivy requires at least Python%d.%d.%d", *PYTHON_MININUM_VERSION) |
@tshirtman your solution is already implemented in setup.py, issue is it fails due to syntax error in py2 while it works with py3 |
Oh right, makes sense. |
but in which situation does the one in makefile helps? |
If you try to run
instead of
|
right, but few users actually do that, right? Maybe we should use a stub setup.py that is python2 compatible if we want that, and only conditionally import/run the actual (renamed) setup.py from it if we detect that we have a supported python version, then? |
@tshirtman I am not really sure of the consequences of that but #7111 does that, feel free to merge which ever method you feel works. |
Eh. I'm not sure we should have non standard setup.py (msetup.py) just for make users, which are pretty rare these days (except for us core-devs). The default cade should be for 3+, not 2. |
The makefile itself should check for 2.7 and fail, rather than putting the work in Python code itself. And for 3.x support, there's a standard python_requires or something similar metadata for minimum python version. This can be added to the config file. |
the other setup.py is not for make users, it's to allow anyone to run setup.py and get a legible error if they are using python2.7, it might be a bit heavy handed, but i don't see another way to avoid the syntax error otherwise (or avoiding using python3 syntax in setup.py). the effect would be something like $ python2.7 setup.py
Kivy requires Python >= (3, 6, 0) detected: (2, 7, 15)
$ python3.7 setup.py
(everything happens as usual) |
But you wouldn't use setup.py directly these days, except after you have a
working install, for normal users. So we just need to make sure that
installing with pip returns a clear error. We don't have to make it return
a nice error for users who don't use pip, because hopefully they would know
about Python2 not being supported.
Even editable install is installed with pip initially. So we just need to
add the appropriate metada and hopefully pip not even try to run setup.py
if the min version is violated.
…On Sun, Sep 27, 2020, 11:06 AM Gabriel Pettier ***@***.***> wrote:
the other setup.py is not for make users, it's to allow anyone to run
setup.py and get a legible error if they are using python2.7, it might be a
bit heavy handed, but i don't see another way to avoid the syntax error
otherwise (or avoiding using python3 syntax in setup.py).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#7109 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMRN7QNZGOZ2V4BFO5RDFDSH5IGRANCNFSM4R3OSS3Q>
.
|
Honestly my personal use case is using make, that’s why I wanted to add the Make only solution. This is good enough for users that compile from git. For the setup.py change I fear doing it this way is begging for consequences down the line…. |
Hm, i assumed because pip runs setup.py, it was a better solution, and i don't really see the point of having an error when you already have it installed, is that for when you run make without enabling your venv or something? |
Yes. Default python on osx is still python2, lol |
ok, i understand better, i guess the less intrusive fix is with the makefile then. |
Software Versions
make
Describe the bug
Make fails with
python
referring to python2 does not fail gracefullyExpected behavior
To Reproduce
Code and Logs and screenshots
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: