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

Fix locating system python when it's not in $PATH (weird but happens, apparently) #1856

Merged
merged 1 commit into from Jun 10, 2019
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jun 10, 2019

Testing a fix for locating system python on travis. This will most likely not work yet but I need to see the travis output to figure out what is wrong (it's a travis-specific problem) hence I'm already making the pull request This fixes the tests breaking when the system python is not in the $PATH env var for some reason (usually that doesn't happen, but with travis & python3.7 for some reason it does)

@ghost ghost added the WIP label Jun 10, 2019
@ghost
Copy link
Author

ghost commented Jun 10, 2019

@opacam this appears to make it work, the main remaining issues being that virtualenv needs to be installed for every python version (including the new python 3.7, which doesn't seem to have it right now) and that #1852 also needs to be merged to get rid of all the test errors

@ghost
Copy link
Author

ghost commented Jun 10, 2019

For what it's worth, the problem was that travis uses a python that is not in $PATH. I have never seen that before so the code trying to locate the system python didn't handle this, but it should work now with this pull request 👍 😄

@ghost ghost changed the title [WIP] Test a fix for locating system python on travis [WIP] Fix locating system python when it's not in $PATH (weird but happens, apparently) Jun 10, 2019
opacam
opacam previously approved these changes Jun 10, 2019
Copy link
Member

@opacam opacam left a comment

Choose a reason for hiding this comment

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

Thanks @Jonast for the quick fix ❤️

👍 👏

It would be great that @AndreMiras or @inclement took a look on this and click the merge button if they are fine with this...

About the suggestions: feel free to apply that if you want but since that seems that you are the maintainer of this part of code, it's fine to me to keep that in your code style 😉

@ghost
Copy link
Author

ghost commented Jun 10, 2019

fne to me to keep that in your code style

my style is ugly 😱 lol. I just tend to forget, always good to poke me about it

@opacam
Copy link
Member

opacam commented Jun 10, 2019

Aaa...don't worry about the code style...I have a solution for that

I will tell you my secret...I tend to use a little trick before commit/push anything...:laughing:

There is a python module named black wich formats the code in a nice way...since I discovered that, it has been arround my head that we should make use of this module in p4a...it would allow us to forget about those code style discussions and focus with the important stuff. It can be integrated with travis (to perform a code style test and uses a pyproject.toml file to do the configuration). It also can be executed targeting a file only or a folder.

I really recommend you that take a look on this project, I think that it's worth it 😉

Copy link
Member

@opacam opacam left a comment

Choose a reason for hiding this comment

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

¡¡¡Good work @Jonast!!!
👍

@opacam opacam added the Priority: High After critical issues are fixed, these should be dealt with before any further issues. label Jun 10, 2019
@inclement
Copy link
Member

Looks like travis failed because the last run predates the wheel fix, I've set it running again.

@ghost ghost changed the title [WIP] Fix locating system python when it's not in $PATH (weird but happens, apparently) Fix locating system python when it's not in $PATH (weird but happens, apparently) Jun 10, 2019
@AndreMiras AndreMiras merged commit 4e67d23 into kivy:develop Jun 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High After critical issues are fixed, these should be dealt with before any further issues. WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants