-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-131531: android.py enhancements to support cibuildwheel #132870
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
Conversation
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@freakboy3742: Please review this PR, but don't actually merge it yet, as there may be some further updates depending on what happens on the cibuildwheel PR. |
!buildbot android |
This comment was marked as outdated.
This comment was marked as outdated.
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.
The broad strokes of this (especially the modifications to kick off an external test suite) make sense; a couple of questions inline about specific decisions/changes.
There's also the issue with the CI failures; not sure how many of those are transient issues caused by the state of main, and how many are caused by changes in this PR - at least one of the errors seems to be related to ctypes availability.
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 |
!buildbot android |
This comment was marked as outdated.
This comment was marked as outdated.
!buildbot android |
This comment was marked as outdated.
This comment was marked as outdated.
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.
The set of changes here all make sense, and it's passing CI; but in my local testing, I'm hitting a problem.
The problem appears to be caused by the intersection of a packaging issue with the binary dependencies, and a change in behavior in shutil.unpack_archive
.
The bzip2 download (possibly others, but the bzip2 build is where it fails) contains symlinks to absolute paths. android.py
uses shutil.unpack_archive()
to unpack the download; but if you use Python 3.14.0b1, this raises:
rkm@eunectes cpython % python3.14 Android/android.py configure-host aarch64-linux-android
> curl -Lf --retry 5 --retry-all-errors -o ./bzip2-1.0.8-2-aarch64-linux-android.tar.gz https://github.com/beeware/cpython-android-source-deps/releases/download/bzip2-1.0.8-2/bzip2-1.0.8-2-aarch64-linux-android.tar.gz
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 192k 100 192k 0 0 146k 0 0:00:01 0:00:01 --:--:-- 2381k
Traceback (most recent call last):
File "/Users/rkm/projects/python/cpython/Android/android.py", line 811, in <module>
main()
~~~~^^
File "/Users/rkm/projects/python/cpython/Android/android.py", line 787, in main
result = dispatch[context.subcommand](context)
File "/Users/rkm/projects/python/cpython/Android/android.py", line 198, in configure_host_python
unpack_deps(context.host, prefix_dir)
~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/rkm/projects/python/cpython/Android/android.py", line 180, in unpack_deps
shutil.unpack_archive(filename, prefix_dir)
~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/shutil.py", line 1432, in unpack_archive
func(filename, extract_dir, **kwargs)
~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/shutil.py", line 1349, in _unpack_tarfile
tarobj.extractall(extract_dir, filter=filter)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/tarfile.py", line 2389, in extractall
tarinfo = self._get_extract_tarinfo(member, filter_function, path)
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/tarfile.py", line 2443, in _get_extract_tarinfo
self._handle_fatal_error(e)
~~~~~~~~~~~~~~~~~~~~~~~~^^^
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/tarfile.py", line 2441, in _get_extract_tarinfo
tarinfo = filter_function(tarinfo, path)
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/tarfile.py", line 842, in data_filter
new_attrs = _get_filtered_attrs(member, dest_path, True)
File "/Library/Frameworks/Python.framework/Versions/3.14/lib/python3.14/tarfile.py", line 819, in _get_filtered_attrs
raise AbsoluteLinkError(member)
tarfile.AbsoluteLinkError: 'bin/bzless' is a link to an absolute path
This doesn't happen if you use Python 3.13 to run android.py
. This appears to be a side effect of the change in 3.14 to default to filter="data"
AFAICT, this isn't a problem in CI because it isn't using the build version of python to run android.py
- it's using whatever version is in the environment to run the buildbot server (I want to say 3.9...?)
It's also not an immediate problem, but it's safe to assume it will be as soon as 3.14 is final. I'm not sure if the fix is in this repo, fixing the filter argument; or to the binary package to fix the creation of symlinks. I suspect it's probably the latter, but I might be missing an obvious fix that can be applied to this repo.
Other than that, I think this is good to land and back port.
@@ -3,7 +3,7 @@ | |||
: "${HOST:?}" # GNU target triplet | |||
|
|||
# You may also override the following: | |||
: "${api_level:=24}" # Minimum Android API level the build will run on | |||
: "${ANDROID_API_LEVEL:=24}" # Minimum Android API level the build will run on |
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.
I'm not sure whether there will be a merge conflict on this line when backporting to 3.13, but either way, the API level on that branch should remain at 21. Apart from that, this file should be identical on all branches.
Yes, this was a bug in the bzip2 Makefile, so I've added a patch and a new release in cpython-android-source-deps, and updated this PR to use it. It doesn't look like any of the other tarballs had the same problem.
3.12 actually, which was the current stable version when I set up the buildbot. It probably wouldn't work as far back as 3.9 because it uses some fairly new asyncio APIs. |
!buildbot android |
🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 6f18528 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F132870%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
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.
That updated bzip2 archive looks like it has done the trick, so this is good to go!
Thanks @mhsmith for the PR, and @freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…honGH-132870) Modifies the environment handling and execution arguments of the Android management script to support the compilation of third-party binaries, and the use of the testbed to invoke third-party test code. (cherry picked from commit 2e1544f) Co-authored-by: Malcolm Smith <smith@chaquo.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
Sorry, @mhsmith and @freakboy3742, I could not cleanly backport this to
|
GH-135158 is a backport of this pull request to the 3.14 branch. |
…-132870) (#135158) Modifies the environment handling and execution arguments of the Android management script to support the compilation of third-party binaries, and the use of the testbed to invoke third-party test code. (cherry picked from commit 2e1544f) Co-authored-by: Malcolm Smith <smith@chaquo.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
…el (pythonGH-132870) Modifies the environment handling and execution arguments of the Android management script to support the compilation of third-party binaries, and the use of the testbed to invoke third-party test code. (cherry picked from commit 2e1544f) Co-authored-by: Malcolm Smith <smith@chaquo.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
GH-135164 is a backport of this pull request to the 3.13 branch. |
…-132870) (#135164) Modifies the environment handling and execution arguments of the Android management script to support the compilation of third-party binaries, and the use of the testbed to invoke third-party test code. (cherry picked from commit 2e1544f) Co-authored-by: Malcolm Smith <smith@chaquo.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
#131532 added an Android release package format for the use of external tools such as cibuildwheel. This PR adds a number of features to that package to allow third-party wheels to be built and tested against it:
android.py env
command which prints necessary environment variables (CC, CFLAGS, etc.).android.py test
command so it can be used to run any test suite, not only Python's own tests. This involves adding the following options:--site-packages
to copy code into the app--cwd
to copy files into the app's working directory-c
to run a Python statement-m
to run a Python moduleThe corresponding cibuildwheel PR is pypa/cibuildwheel#2349.