Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-116622: Add Android testbed#117878

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

Merged
encukou merged 14 commits intopython:mainfrommhsmith:android-testbed
May 1, 2024
Merged

Conversation

mhsmith
Copy link
Member

@mhsmithmhsmith commentedApr 14, 2024
edited by bedevere-appbot
Loading

This PR adds a testbed app to run the Python test suite on Android. Instructions for using it are in Android/README.md.

Most of the files in this PR are boilerplate generated by the Android Studio new project wizard. To make it easier to review, I've split it into two commits:

  • An "empty project" commit with the output of the new project wizard.
  • A "testbed implementation" commit with the custom code.

Android app projects require a binary gradle-wrapper.jar, and two gradlew scripts to launch it. The integrity of these files can be verified by creating a new project in Android Studio Hedgehog – all three files in the new project should be identical to the versions in this PR.

Since myself and@freakboy3742 are now on the triage team, I've also added us to the CODEOWNERS file for Android and iOS respectively.

@mhsmithmhsmith requested a review fromencukouApril 14, 2024 18:40
@encukou
Copy link
Member

I can confirm that thegradle-wrapper.jar matches the one generated by Android Studio Hedgehog fromhttps://developer.android.com/studio/archive -- specifically, “Android Studio Hedgehog | 2023.1.1 Patch 2 January 23, 2024”.

But, one thing that's not clear to me: what is the license for these files?

In the Android Studio repository, I couldn't find any license at all.

The Android Studio download & installer made me agree to a “non-assignable, non-sublicensable” license, which limits what the SDK can be used for, and doesn't cover everything that Python's license allows.

@mhsmith
Copy link
MemberAuthor

The three filesgradlew,gradlew.bat andgradle-wrapper.jar (collectively the "Gradle wrapper"), originate fromGradle, which is used by Android Studio but is an independent project. Like the rest of Gradle, the wrapper is covered by the Apache license, and there's a notice indicating that at the top of the scripts.

@mhsmith
Copy link
MemberAuthor

Concerns were raised on Discord that the source of the Gradle files wasn't clear enough, so I've updated them from the current version of Gradle (8.7). Here's how to verify them:

  • Thegradlew andgradlew.bat scripts are from the root of the Gradle repository athttps://github.com/gradle/gradle/tree/v8.7.0.

  • Thegradle-wrapper.jar is from the Gradle release:

    • Go tohttps://gradle.org/releases/
    • Download the v8.7 "binary-only" package
    • Inside the ZIP, findlib/plugins/gradle-wrapper-8.7.jar
    • Inside the JAR (which is also a ZIP file), findgradle-wrapper.jar, which is the file I've just added to this PR.
    • If you look inside the inner JAR, you'll see that this version also contains a license notice.

@encukou
Copy link
Member

Thanks.
I am still worried about distributing compiled artifacts without corresponding source code, and I would prefer to put this in a separate repository, downloaded on demand for Android tests, than to include it in the CPython repo.

The testing instructions added inAndroid/README.md are rather manual; I don't think an extragit clone would make things much worse for users.

Including the jar means we're trusting a third party -- Gradle -- and their build process. I don't have a reason to trust them, and I don't think that CPython should implicitly vouch for them.
The text files are at least auditable -- though it's tedious. I don't see why we need to set RIGHT_MARGIN to 80, or include a fully commented-out config file example, to run the tests. If we treat these like binary blobs -- output of Android Studio -- they seem fine (and perhaps non-copyrightable); but they should go in e.g.cpython-bin-deps. If we treat them assource code, then I'll review them, and I'll ask you to prune things that are not needed -- but I think that'd just add work for everyone.

If there are any license issues found later, it's much easier to remove things if they're in an isolated repo/branch. (And it's also easier for over-cautious redistributors to not include the testbed.)


Note that I am a Linux distro packager by training, conditioned to include sources and avoid bundling. I might not be representing the CPython project well. I am definitely unfamiliar with mobile development practices. If you think I'm being unreasonable,please do seek other opinions, e.g. on Discourse.

@mhsmith
Copy link
MemberAuthor

mhsmith commentedApr 22, 2024
edited
Loading

Thanks, that all makes sense.

I am still worried about distributing compiled artifacts without corresponding source code, and I would prefer to put this in a separate repository, downloaded on demand for Android tests, than to include it in the CPython repo.

Even simpler than a separate repository would be to add an option to the android.py script to obtain the Gradle wrapper directly from Gradle – essentially automating the steps listed in my previous comment. This would be no different from downloading compilers from a third party, as we do on every platform.

The text files are at least auditable -- though it's tedious. I don't see why we need to set RIGHT_MARGIN to 80, or include a fully commented-out config file example, to run the tests. [...] If we treat them as source code, then I'll review them, and I'll ask you to prune things that are not needed -- but I think that'd just add work for everyone.

Actually I've got a pretty good idea of what can be pruned. My initial plan was to keep the app as close as possible to Android Studio's new project wizard output, but I guess that does include a lot of noise. Let me see what I can do.

encukou reacted with thumbs up emoji

@mhsmith
Copy link
MemberAuthor

OK, I've removed the Gradle wrapper, added an android.py subcommand to download it, and removed all the unused boilerplate from the app, which should make the PR a lot more manageable.

@encukou
Copy link
Member

Thank you! This looks good; I haven't tested it.
I'm not sure if the setup instructions will tleave the user withwget andunzip on all platforms. If not, could you add a note that they're required?
(Using Python instead of shelling out is probably not worth the work.)

@mhsmith
Copy link
MemberAuthor

I've added a list of the required tools to the README. And I've also switched fromwget tocurl, because it's included with macOS.

mhsmithand others added2 commitsApril 27, 2024 13:02
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Copy link
Member

@encukouencukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks good, thank you!
I was able to start the test suite! (There were some errors, and I didn't wait for it to finish on my non-accelerated VM, but that shouldn't block this PR.)

I have some more suggestions for the guide, to help newbies like me.
Additionally, consider introducing the cross-build therm "host" here:

Additionally

  Building for Android requires doing a cross-build where you have a "build"- Python to help produce an Android build of CPython. This procedure has been+ Python to help produce an Android build of CPython (the "host" build). This procedure has been  tested on Linux and macOS.

mhsmithand others added2 commitsApril 30, 2024 13:09
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@mhsmith
Copy link
MemberAuthor

mhsmith commentedApr 30, 2024
edited
Loading

I was able to start the test suite! (There were some errors, and I didn't wait for it to finish on my non-accelerated VM, but that shouldn't block this PR.)

Everything was passing on Android a couple of weeks ago, but unrelated development has introduced a few failures since then. The next priority will be to set up a buildbot so we can find out about such failures more quickly.

Additionally, consider introducing the cross-build term "host" here:

Done.

encukou reacted with thumbs up emojihugovk reacted with hooray emoji

Copy link
Member

@encukouencukou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Looks good! Thank you!

hugovk reacted with rocket emoji
@encukouencukou merged commit2520eed intopython:mainMay 1, 2024
SonicField pushed a commit to SonicField/cpython that referenced this pull requestMay 8, 2024
Add code and config for a minimal Android app, and instructions to build and run it.Improve Android build instructions in general.Add a tool subcommand to download the Gradle wrapper (with its binary blob). Androidstudio must be downloaded manually (due to the license).
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@freakboy3742freakboy3742freakboy3742 left review comments

@gpsheadgpsheadgpshead left review comments

@hugovkhugovkhugovk left review comments

@encukouencukouencukou approved these changes

@ezio-melottiezio-melottiAwaiting requested review from ezio-melottiezio-melotti is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@mhsmith@encukou@freakboy3742@gpshead@hugovk

[8]ページ先頭

©2009-2025 Movatter.jp