Python 3.8 support on Android

[inclement]

Needs cleaning up so not for merging yet (in particular want to retain the py3.7 patches for py3.7 builds), but this seems to work.

[opacam]

Nice work 👍

Thanks @inclement, I will review it when you finish the last touches, aaa...and it would be great to have the python 3.8 patch uploaded so we can to test this 😄

[jgavris]

+1 ...If you can upload the patch for 3.8 we're more than willing to test it.

[inclement]

Doing some testing now, but this is intended to be basically final (barring bugfixes).

[AndreMiras]

Nice! Looking forward to testing it soon

[inclement]

I've tested that this is working. There's a minor inconvenience that you have to remember to change both the python3 and hostpython3 versions if targeting something different to the default, but that's something we can fix later.

[inclement]

Thanks for the reviews. Looks like the only test failure is now in travis on macOS, with the error: [Errno 2] No such file or directory: '/usr/local/include/python3.8/pyconfig.h' that I've seen other people mention before, presumably because the Travis builds are using 3.7. I've changed the travis.yml to use 3.8 to see if that works, but does anyone know why osx (and only osx) is using the system's Python.h in the build?

[inclement]

Looks like 3.8 isn't available on macos with travis anyway, so updating the travis.yml doesn't do anything useful. Let's leave it at 3.7 for now, it's fine to run p4a under this version and we can sort out multi-version testing later if we want.

This leaves the question of what to do with the failing macos build. Does anyone expect to commit a fix for the pyconfig.h problem any time soon (or maybe I already missed one?)?

[inclement]

Just ran the tests on the develop branch again, to establish that the macos pyconfig.h issues is present there as well. I think this PR can therefore be merged, the tests are only failing in one place, the same as on trunk.

[AndreMiras]

Awesome work! I only have one concern. Why bumping the default version to some "cutting edge" Python? Specially since it breaks the build for osx.
My concern with having broken build on develop branch is we may start overlook on other broken things in the future as the build would be red anyway.
So probably keeping the default to 3.7 in the recipe is an easy enough workaround until somebody sorts things up on osx?

[AndreMiras]

Why bumping the default version to something "cutting edge"? I think it's great that we support it, but maybe it's a bit early to bump it as a default at the same time?

[inclement]

On inspection, I don't think it's actually breaking the build for osx, since that build is also failing for the same reason on develop. Not sure what's up there - some osx environment change? I've seen users reporting similar issues there. About bumping the version, I was taking the view that if this actively breaks something then that's probably something we want to know about anyway. Really it shouldn't cause any issues. I don't have a strong preference if you want to leave it at 3.7.1.

…

On 16/02/2020 18:23, Andre Miras wrote: ***@***.**** approved this pull request. Awesome work! I only have one concern. Why bumping the default version to some "cutting edge" Python? Specially since it breaks the build for osx. My concern with having broken build on develop branch is we may start overlook on other broken things in the future as the build would be red anyway. So probably keeping the default to 3.7 in the recipe is an easy enough workaround until somebody sorts things up on osx? ------------------------------------------------------------------------ In pythonforandroid/recipes/python3/__init__.py <#2044 (comment)>: > @@ -18,15 +19,24 @@ class Python3Recipe(GuestPythonRecipe): :class:`~pythonforandroid.python.GuestPythonRecipe` ''' - version = '3.7.1' + version = '3.8.1' Why bumping the default version to something "cutting edge"? I think it's great that we support it, but maybe it's a bit early to bump it as a default at the same time? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2044?email_source=notifications&email_token=AAJVBG7YADSTAKLO2QBIMYDRDGADVA5CNFSM4J6ZFO5KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCVWFC4Q#pullrequestreview-359420274>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJVBG23C3BEFVTFFGXNW7DRDGADVANCNFSM4J6ZFO5A>.

[AndreMiras]

Re the broken osx build, I just saw from your other PR, the osx build is broken no matter what #1975
So I understand now this is not related to this one either and I misunderstood that 👍

[inclement]

Thanks. I'm going to go ahead and merge this, we can always change the default version later if preferred.