Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-131531: Addandroid.py package command#131532
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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 mechanics of what is here generally looks good; it's the higher-level design that gives me pause.
In particular, I'm unclear on the motivation behind moving the compilation process - and especially thebuild target - out of thecross-build folder.
From a purely practical perspective, thebuild folder can be re-used between emscripten, iOS and Android; so it make some sense to keep it in a common location. This set of changes would mean needing to keep an independent set of builds for Android.
More generally, it's breaking the expectation that a build can be performed "out-of-tree" in a (mostly arbitrary) location.
If the issue is ensuring that theprefix directory is in a known location for testing purposes, would it not be better to keep theAndroid folder as a source directory, and "package" the distribution artefact using a temporary folder that is a clone of the parts of the Android source folder that are known to be required, plus a copy of theprefix directory in the expected location?
That would also allow the distribution artefact to include files that aren't in the Android folder - for example, license files and the like that may be required for distribution purposes, but won't be duplicated in theAndroid folder itself because they're common across distribution artefacts on all platforms.
That approach would also be consistent with how the iOS testbed works. The iOS testbed has 2 modes: "clone" and "run". The iOS folder contains the source project; during a test pass, "clone" is used to copy the testbed to a new location, and is provided a path to a compiled iOS Framework as part of that clone process. The test is then run on the cloned version, not the original source location.
The iOS testbeddoesn't manage the initial build, or building the final release artefact, but it could - and probably should as part of any move to make an official iOS artefact). However, the general approach of using theiOS/Android folder purely as a source, rather than a working directory, should work for both platforms, and for both testing and distribution packaging. It also allows the distribution process to be "opt in" on what should be distributed, rather than excluding specific artefacts from a complete build (which could easily lead to new files being inadvertently included in a distribution artefact because we forget to exclude them).
Uh oh!
There was an error while loading.Please reload this page.
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 |
… rather than extracting it from a Gradle release
…oth the source layout and the packaged layout
mhsmith commentedMar 29, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
OK, those are all good points. I've moved the build directories back to |
!buildbot android |
bedevere-bot commentedMar 29, 2025
🤖 New build scheduled with the buildbot fleet by@mhsmith for commit150137f 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131532%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
I have made the requested changes; please review again. |
Thanks for making the requested changes! @freakboy3742: please review the changes made to this pull request. |
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.
One question inline about a possible simplification of the directory structure.
The only other question I have is about how this will interact with the next piece of work that you've flagged (running a third-party test suite).
The current state of the code involves handling the "running in the source tree" case as a special case, and that identification seems to be based on the name of the parent directory, and the existence of apyconfig.h.in file. The existence ofpyconfig.h.in file will be a reasonably robust test, but it won't be infallible; I'm wondering it that potential flaw could be avoided entirely by running the normal test suite against the directory structure "as packaged", rather than making theinSourcetree special case?
Thatmight be part of what you've got in mind for the next piece of work, especially given that it may require some sort of copy/clone/unpack process for the packaged artefact. I'm happy defer that work for later if that's what you've got in mind; I just wanted to make sure I understood the scope for the next piece.
Android/android.py Outdated
| os.chdir(subdir("build", clean=context.clean)) | ||
| if context.clean: | ||
| clean("build") | ||
| os.chdir(subdir("build", "build", create=True)) |
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.
Is there a purpose to this extra level ofbuild here? It makes at leastsome sense in the android folder, because of theprefix,build anddist subfolders; but even there, I'm not sure I see why thedist andprefix folders couldn't be mounted in the `cross-build/*-linux-android - my read of the OOT build handling is that it's designed for this sort of "output artefact" storage, without the need for another level of indirection.
I'm mostly being aware that this results in across-build/build/build/build folder which... seems like too many builds :-)
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 I see why the
distandprefixfolders couldn't be mounted in the `cross-build/*-linux-android
That's a good idea. Done.
For comparison, the other scripts that use thecross-build directory are:
Tools/wasm/wasi.py– Builds in the top-level triplet directory; doesn't use anyprefixdirectory.Tools/wasm/emscripten/__main__.py– Usesbuildandprefixsubdirectories, though it doesn't useprefixso heavily as Android does. Maybe it could be migrated to the Android layout next time the script is updated.
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.
Agreed there's an opportunity to make the emscripten build consistent.
I've also got some informal build scripts for iOS that I really should formalize into some utility methods... but FWIW, they all use the triplet directory.
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 |
mhsmith commentedMar 31, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That was my reason for originally keeping everything in the But even then, there was still a need to detect whether we were in a source tree, because of the code in build.gradle.kts which copies the standard library from its source directory ( However, after this PR is merged I am planning to update the buildmaster script so it will:
I haven't worked out exactly how to deal with third-party test suites, but I'll leave that to a separate PR. |
!buildbot android |
bedevere-bot commentedMar 31, 2025
🤖 New build scheduled with the buildbot fleet by@mhsmith for commitc537d97 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131532%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
I have made the requested changes; please review again. |
Thanks for making the requested changes! @freakboy3742: please review the changes made to this pull request. |
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.
Filling in details of a conversation@mhsmith had in a private channel - there may be some additional cleanups possible, but if they are, those will likely fall out of the next piece of work to make the testbed suitable for use in testing third-party projects.
fe5c4c5 intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@mhsmith for the PR, and@freakboy3742 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Adds a `package` entry point to the `android.py` build script to supportcreating an Android distribution artefact.(cherry picked from commitfe5c4c5)Co-authored-by: Malcolm Smith <smith@chaquo.com>
GH-131960 is a backport of this pull request to the3.13 branch. |
bedevere-bot commentedApr 1, 2025
|
The Fedora test failure has to be a false positive - this doesn't change any code that Fedora would be running. |
bedevere-bot commentedApr 1, 2025
|
Adds a `package` entry point to the `android.py` build script to supportcreating an Android distribution artefact.
Uh oh!
There was an error while loading.Please reload this page.
This PR makes two changes:
buildandprefixdirectories fromcpython/cross-buildintocpython/Android.android.py packagecommand which creates a tarball from a subset of theAndroiddirectory, including all the files useful either for embedding Python in an app, or building third-party packages against it.The resulting package is about 30 MB. Here's anexample artifact (temporary URL).
The package includes a copy of the testbed and the android.py script, which can be used to run the CPython test suite outside of the context of the source tree. A future PR will add the ability to run test suites of third-party packages.
For Python 3.13 and older, we can host these packages in the Chaquopy Maven Central repository for the use of cibuildwheel and Chaquopy itself. From 3.14 onwards, they will hopefully be hosted on python.org.
📚 Documentation preview 📚:https://cpython-previews--131532.org.readthedocs.build/