| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Hum, I think this is a backwards incompatible change, which could result in downstream code breaking. While I think we should probably have a better way to identify Android platforms, I don't know exactly how I feel about this approach. I think at least we should get the input from people shipping and packaging Python on Android.
Sorry, something went wrong.
|
@freakboy3742 was actually the one who convinced me to go this way – his reasoning is on the PEP's Discourse thread here. |
Sorry, something went wrong.
As I said in the discussion thread on PEP 738 - I'm a hard +1 on sys.platform == "android". Yes, this is technically backwards incompatible with the current state of Android support in the CPython source tree. However, I'd counter that:
Repeating what I said in the discuss thread: Yes, technically, Android is a Linux. However, that is a distinction that isn't helpful in practice, because “Linux” isn’t just a kernel - it’s lots of other device expectations, filesystem expectations, userspace expectations, and more. The most practical manifestation of this: If Android is indeed "a Linux", why are any patches needed to CPython at all? Shouldn't Android "just work" as a Linux, or fall out of trivial configuration checks? Unless I'm mistaken, that there are less conditional branches in the test suite for FreeBSD support than are required for Android. If that isn't an indication that Android isn't "a Linux" in practice, I don't know what is. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks for chiming in @freakboy3742.
Looks good to me. I had to look twice at the test_sysconfig change, though 😄
Sorry, something went wrong.
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
|
@erlend-aasland: I've applied your suggestions, so I think this should be ready to merge now. |
Sorry, something went wrong.
There was a problem hiding this comment.
The PR looks good to me, the only thing missing is updating the documentation.
We should add Android to the "For other systems (...)" list in the sys.platform documentation (Doc/library/sys.rst), and add a versionchanged field noting this change.
After that, I think we are good to merge, thanks @mhsmith!
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
|
I have made the requested changes; please review again. I've also cleaned up the sys.platform documentation as follows:
|
Sorry, something went wrong.
|
Thanks for making the requested changes! @erlend-aasland, @FFY00: please review the changes made to this pull request. |
Sorry, something went wrong.
Python from 3.13 onwards supports Android as an official platform. As a part of this official support, there were a bunch of changes including `platform.system()` returning "Android" and `sys.platform` being "android" instead of "Linux" and "linux" on earlier versions. This is a breaking change. Also try to be backwards compatible while running on Android for older python versions in environments like Termux This maturin bug was found as a part of testing of builds of python packages for Termux as part of Python 3.13 migration. Downstream Python distribution update PR: termux/termux-packages#27739 Ideally we would like to ensure that get_python_os() returns "linux" for Python versions <= 3.12, but that'd require additional work in supplying python version to get_python_os(), but that'd complicate some things. This works well enough, and shouldn't be necessary in future when Python >= 3.13 becomes the standard target by most (which should ideally happen soon since upstream Python is supporting Android from this version). My analysis of maturin reveal that get_python_os() returning "android" shouldn't cause any effects on Android targets that do need to be patched. Issue also reported to maturin in PyO3#2945 Partially based of off work by @robertkirkman in the linked issue. Further fixup was done to ensure suitability for upstream acceptance. Ref: python/cpython#116215
Python from 3.13 onwards supports Android as an official platform. As a part of this official support, there were a bunch of changes including `platform.system()` returning "Android" and `sys.platform` being "android" instead of "Linux" and "linux" on earlier versions. This is a breaking change. Also try to be backwards compatible while running on Android for older python versions in environments like Termux This maturin bug was found as a part of testing of builds of python packages for Termux as part of Python 3.13 migration. Downstream Python distribution update PR: termux/termux-packages#27739 Ideally we would like to ensure that get_python_os() returns "linux" for Python versions <= 3.12, but that'd require additional work in supplying python version to get_python_os(), but that'd complicate some things. This works well enough, and shouldn't be necessary in future when Python >= 3.13 becomes the standard target by most (which should ideally happen soon since upstream Python is supporting Android from this version). My analysis of maturin reveal that get_python_os() returning "android" shouldn't cause any side effects for Python <= 3.12 on Android. Issue also reported to maturin in #2945 Partially based of off work by @robertkirkman in the linked issue. Further fixup was done to ensure suitability for upstream acceptance. Ref: python/cpython#116215 CC @mhsmith Android support for Python
| Back | FazBrowse Home | New Git URL |
As discussed in PEP 738, although Android is based on Linux, it differs in enough significant ways that a separate name is justified.
Related changes:
I've replaced all instances of hasattr(sys, "getandroidapilevel") with a sys.platform check. This is generally easier to read, and more consistent with how the other platforms are handled.
I've checked all the places I could find that compare sys.platform to "linux", and added "android" where appropriate. These are mostly in the tests, but there are also a few in the stdlib.