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

Require Python 3.7 #11408

Merged
merged 1 commit into from
Feb 13, 2021
Merged

Require Python 3.7 #11408

merged 1 commit into from
Feb 13, 2021

Conversation

skullydazed
Copy link
Member

Description

Bump our required Python version to 3.7. This also removes support for linux distributions which are currently broken.

Types of Changes

  • Core
  • Enhancement/optimization

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • I have read the CONTRIBUTING document.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@skullydazed skullydazed requested a review from a team January 2, 2021 17:19
@github-actions github-actions bot added cli qmk cli command core python labels Jan 2, 2021
@skullydazed
Copy link
Member Author

We could use some wider eyes on this one. Pinging people who have contributed some of the niche OS support:

@BlitzKraft @chax @csssuf @manuel-arguelles @petejohanson @shieldsd @skewwhiffy @umanwizard @zjp

@zjp
Copy link
Contributor

zjp commented Jan 2, 2021

Per https://www.gentoo.org/support/news-items/2020-04-22-python3-7.html Python 3.7 is now the default python version on Gentoo, so I think this is OK.

@BlitzKraft
Copy link
Contributor

BlitzKraft commented Jan 2, 2021

It looks good to me. Sabayon is rebranding to mocaccinoOS. I am not sure if it is relevant.

Is it possible to have the installer skip the uname check and have it use a generic tool with some warning? For example all debian based distros can use apt. So, check for an env variable, say, QMK_INSTALL_DISTRO. And if it exists, and set to a valid value, uname can be skipped, and the debian/ubuntu installer will be called. The installer logs/prints this preference.

The reason I mention this is because of the sabayon rebranding, as well as covering as many use cases as possible without having to account for the name of each distro uname string. User from elementary/popOS/zorin etc can set variable to debian, and the installer does its thing.

Edit: This is in addition to the current checks. Not a replacement. If the env var is not found/invalid, the install proceeds as it does in the current state.

@skullydazed
Copy link
Member Author

skullydazed commented Jan 2, 2021

Is it possible to have the installer skip the uname check and have it use a generic tool with some warning?

As long as ID_LIKE in /etc/os-release contains debian you should match the existing debian check. I don't know the history behind why sabayon/mocaccino has a separate script, but if that reason no longer applies we'd be very happy to have one less platform to explicitly support.

Edit: this works today on raspbian (source: I compile qmk on a pi occasionally)

@BlitzKraft
Copy link
Contributor

sabayon has a different package manager. Hence the need for the separate script. ID_LIKE is not guaranteed to be present, is it?

@skullydazed
Copy link
Member Author

We check both ID and ID_LIKE. Our grep is actually overly broad for the purpose, in that we're pulling in any variable from /etc/os-release that contains the string ID, but it's been "good enough" for a long time.

The use case you describe is already handled by /etc/os-release. If sabayon can use the same install script as another distribution that's the best way to invoke the proper script.

@chax
Copy link
Contributor

chax commented Jan 2, 2021

Solus currently uses pyrhon 3.7.9 as main python3 version, and since Solus is rolling release distro, version will only go up. You can see current version here: https://dev.getsol.us/source/python3/

Do you need me to test this PR?

@skullydazed
Copy link
Member Author

Do you need me to test this PR?

If you're confident it works fine there's no need to test now. There will be time to test all of develop before it's merged into master in a couple months time.

@skullydazed skullydazed merged commit cd336b2 into develop Feb 13, 2021
@skullydazed skullydazed deleted the bump_python branch February 13, 2021 18:26
@fauxpark fauxpark mentioned this pull request Aug 15, 2022
3 tasks
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli qmk cli command core python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants