-
-
Notifications
You must be signed in to change notification settings - Fork 40.2k
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
Make python a required build dependency #5784
Conversation
If you have one of the affected systems please give this a try to make sure it works. If you don't yet have python 3 installed:
If you already have python3 installed you can skip steps 2 and 3. |
Tagging people who have contributed some of the less used platforms. If you are on this list please take a look at this PR and ensure that your OS still works and installs python3. @manuel-arguelles @Chris-Johnston @vatho @florensie @zjp @vomindoraan @BlitzKraft @shieldsd |
@skullydazed For sabayon, python3 is the default python. Changing the line 109 to Also, the colon in the line 109 is not correct. It needs to be a hyphen, for specifying version numbers. For python3.5, the package name is I tested the script, and works with the corrections. Without specifying version, python3.6.6 is installed. With specifying 3.5.5-r1, the corresponding package is installed. |
+1 for Slackware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested working on Linux Mint 18.3 and 19.1
$ cat /etc/upstream-release/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04 LTS"
$ cat /etc/upstream-release/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=18.04
DISTRIB_CODENAME=bionic
DISTRIB_DESCRIPTION="Ubuntu 18.04 LTS"
using the steps provided.
@BlitzKraft Thanks for the report. Does my latest commit fix those issues? |
Yes, it does fix the issues @skullydazed |
It might take me a minute to check -- it's right around finals for me and every class I'm in has a project due next week. If no one has checked on Gentoo by Friday, I'll look at it. |
@zjp That's fine. If we end up merging this sooner and there is a problem with gentoo a new PR can be created to fix it up. |
Tested it on arch linux updated today. Kernel 5.0.10.
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for arch linux. Python 3.7.3 is the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things. LGTM otherwise.
@@ -534,6 +534,8 @@ endef | |||
%: | |||
# Check if we have the CMP tool installed | |||
cmp $(ROOT_DIR)/Makefile $(ROOT_DIR)/Makefile >/dev/null 2>&1; if [ $$? -gt 0 ]; then printf "$(MSG_NO_CMP)"; exit 1; fi; | |||
# Ensure that python3 is installed. This check can be removed after python is used in more places. | |||
if ! python3 --version 1> /dev/null 2>&1; then printf "$(MSG_PYTHON_MISSING)"; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if ! python3 --version 1> /dev/null 2>&1; then printf "$(MSG_PYTHON_MISSING)"; fi | |
if ! python3 --version >/dev/null 2>&1; then printf "%s" "$(MSG_PYTHON_MISSING)"; fi |
SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
Also change the stdout redirection to be consistent with the rest of the file.
@@ -81,3 +81,7 @@ MSG_FILE_TOO_BIG = $(ERROR_COLOR)The firmware is too large!$(NO_COLOR) $(CURRENT | |||
MSG_FILE_TOO_SMALL = The firmware is too small! $(CURRENT_SIZE)/$(MAX_SIZE)\n | |||
MSG_FILE_JUST_RIGHT = The firmware size is fine - $(CURRENT_SIZE)/$(MAX_SIZE) ($(FREE_SIZE) bytes free)\n | |||
MSG_FILE_NEAR_LIMIT = The firmware size is approaching the maximum - $(CURRENT_SIZE)/$(MAX_SIZE) ($(FREE_SIZE) bytes free)\n | |||
MSG_PYTHON_MISSING = $(WARN_COLOR)WARNING:$(NO_COLOR)\n \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSG_PYTHON_MISSING = $(WARN_COLOR)WARNING:$(NO_COLOR)\n \ | |
MSG_PYTHON_MISSING = $(WARN_COLOR)WARNING:$(NO_COLOR)\n\ |
I think there's no need for the extra space.
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
Make python a required build dependency (qmk#5784)
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
Sorry to necro this so late but I was able to get my environment set up. Per the Python page on the wiki, it's very integral to Gentoo, so it's unlikely to not be installed on the system. The default right now is Python 3.6 I'll say looks good to me, even without dev-lang/python in the install script. If, periodically, pulling the repo breaks something for me, I'll take a look at why and update. |
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
* Make python a required build dependency * Add missing color * fixup sabayon linux per @BlitzKraft
As described in #5776 this adds python to all the OS install scripts.
Description
We need to test on the following platforms:
Types of Changes
Issues Fixed or Closed by This PR
Checklist