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

Android CPython upstreaming #980

Closed
mhsmith opened this issue Oct 2, 2023 · 7 comments
Closed

Android CPython upstreaming #980

mhsmith opened this issue Oct 2, 2023 · 7 comments

Comments

@mhsmith
Copy link
Member

mhsmith commented Oct 2, 2023

@freakboy3742: Let's use this issue as a place to keep notes before we approach the CPython core team.

Related:

@mhsmith mhsmith added this to the 15.1 milestone Oct 2, 2023
@mhsmith
Copy link
Member Author

mhsmith commented Oct 2, 2023

The existing patches are here.

Should upstream:

No need to upstream:

  • dynload_shlib.patch: Not required for CPython itself, but to support an awkward workaround in Chaquopy's importer for dynamic linker bugs on API levels older than 23. I'd be happy to just increase Chaquopy's minimum API level from 21 to 23 and get rid of this (Update minimum API level #718).

  • lfs.patch: Only required on 32-bit ABIs with API level less than 24.

  • sysroot_paths.patch: Only required on Python 3.11 and older.

This was referenced Oct 2, 2023
@mhsmith
Copy link
Member Author

mhsmith commented Oct 7, 2023

Most of the stdlib monkey patches aren't worth upstreaming, except for:

  • Redirecting stdout and stderr to logcat should probably be done unconditionally, as long as we can use sys.argv, sys.executable, or some other way to detect that we're embedded inside a regular app as opposed to any other executable.
    • stdin redirection might not be necessary at all if it always returns EOF on all known devices within our supported range (see #1083).

@mhsmith
Copy link
Member Author

mhsmith commented Oct 10, 2023

@freakboy3742: Following up on beeware/Python-Android-support#8, here are some notes on sys.platform etc:

sys.platform in Chaquopy currently returns linux. This is helpful in some situations (e.g. when determining the filename extension of shared libraries), and harmful in others (e.g. when assuming a standard filesystem layout under /usr, which Android doesn't have). It also makes it impossible for a requirements environment marker to distinguish between Linux and Android, which is the reason why we're not currently recommending the toga meta-package.

Overall, I agree it would be better if it wasn't linux anymore. So what should it be?

Based on the existing platforms:

  • It should not contain the OS version number (none of the common platforms do anymore).
  • It should not contain the architecture.

Someone at PyCon suggested linux-android, which also happens to be the format used in the GCC target triplet. This would work with the startswith idiom described in the sys.platform documentation, allowing Linux and Android to be treated either separately or together, depending on the situation.

Although I don't think the documentation has ever suggested using startswith for darwin, we could do the same thing there, i.e. darwin-ios.

Other platform indicators:

  • os.name should continue to be posix on both Android and iOS, obviously.
  • platform.system appears to be more user-oriented, although it is often used programmatically, as in pip. So it would make sense for this to be simply Android or iOS.

@mhsmith
Copy link
Member Author

mhsmith commented Oct 13, 2023

Although I don't think the documentation has ever suggested using startswith for darwin, we could do the same thing there, i.e. darwin-ios.

PEP 730 has now been created, proposing a sys.platform of simply ios. And the more I think about it, the more I feel like it's better to make a clean break from Linux on Android as well. For one thing, linux-android would mean that any existing code that's already following the recommendation to use startswith would need to add an and not clause to distinguish Linux from Android.

@mhsmith
Copy link
Member Author

mhsmith commented Jan 25, 2024

After updating the tests as shown on the android-test branch, we still have the following failures on Python 3.12.1 with Chaquopy:

Known solution:

  • test__xxinterpchannels, test__xxsubinterpreters, test_argparse, test_bdb, test_builtin, test_capi, test_docxmlrpc, test_ensurepip, test_frozen, test_import, test_importlib, test_interpreters, test_linecache, test_multibytecodec, test_pydoc, test_re, test_sys, test_sysconfig, test_syslog, test_trace, test_urllib2
    • Requires standard importer setup. This also includes anything that uses subinterpreters.
  • test_argparse
    • Expects sys.stdin to have a buffer attribute (#1083)
  • test_os
    • Conflicts with get_exec_path monkey patch
  • test_perfmaps
    • Assumes standard Linux filesystem layout. Will be skipped once sys.platform == "android", but make that change last because it'll probably cause some additional failures.
  • test_posix
    • Some privileged functions hard crash rather than return EPERM as the tests expect.
    • Some functions are privileged on Android but not on Linux.
  • test_ssl
    • OpenSSL ignores environment variables on Android. See Chaquopy monkey patch, but patching OpenSSL itself may be easier.

Unknown solution:

  • test_signal, test_threadsignals, test_wsgiref
    • Failures and hangs involving signals

@mhsmith
Copy link
Member Author

mhsmith commented Jan 31, 2024

I've pushed the test updates to the android-test branch in my CPython fork. And I'll create separate branches there for Chaquopy's existing CPython patches, in preparation for turning them into pull requests.

@mhsmith
Copy link
Member Author

mhsmith commented Mar 24, 2024

Continued in python/cpython#116622.

@mhsmith mhsmith closed this as completed Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant