-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 a problem with the way we were calling virtualenv #2145
Conversation
Just testdrove the PRs changes on my local build machine, seems to work. |
Thanks you @zurgeg for the initiative. |
It worked for me, dunno why it doesn't work on Travis CI.
|
I think it is caused by there being 2 different versions, |
Finally just got those builds to work (I think). It had to do with a single whitespace on a line. |
For some reason the |
@AndreMiras turns out my first build was lucky... I just experienced your same issue, changed python3 to virtualenv (Should work anywhere), and it started working. |
@zurgeg ,
Try changing:
|
Thanks for investing time into this as I haven't had time lately. |
mmm...I think that |
Oh yeah, I'll do that, also doing what I think works for all Python versions. |
Finally got those builds to work! So apparently no matter if you use |
But wait now you removed the |
Oh... Hold on, I though it was --python=..., so I thought that was problematic, hold on... |
Also you know you can try things locally e.g. using Docker so you don't have to wait for the CI which is probably slower than your local build. |
Just fixed your issue @AndreMiras! |
pythonforandroid/build.py
Outdated
partition(".")[0] | ||
), | ||
'venv' | ||
'-p python{}'.format( |
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 I understand correctly this didn't change, but we only changed venv = sh.Command('virtualenv')
then maybe don't introduce a change here if we don't end up fixing that line. I mean rollback the full arg --python=
and the indentation so it's clear what the actual fix is
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.
Okay, doing that now...
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 some reason it gives path =python3 not found from --python=python3
.
Also @opacam I don't think using |
Okay, going way back to my |
This is properly painful right? Honestly we don't mind much about Python 3.3, it's not like it wasn't already a nightmare to support 3.6+ and multiple platforms. |
Yeah, I think the other PR is better, closing this one when it’s merged.
|
Thanks for bring this up to our attention and helping out debugging this complex one |
This PR fixes a problem where any buildozer build errors with
Can not import virtualenv.__main__; virtualenv is not a package
as I tested and when I made this modification it worked.