gh-140239: Check for statx on Android #140395
Conversation
Android has Linux's statx, but MACHDEP is "android" on Android, so configure doesn't check for statx on Android. Base the check for statx on ac_sys_system instead, which is "Linux-android" on Android, "Linux" on other Linux distributions, and "AIX" on AIX (which has an incompatible function named statx).
jbosboom
requested review from
AA-Turner,
corona10,
emmatyping and
erlend-aasland
as code owners
|
I didn't knew |
|
@ayappanec I appreciate that you're keeping up with the main branch and reporting issues promptly. Don't worry about this. Given the maybe-also-wrong check right below it, it took me about 30 minutes to figure this out. |
|
!buildbot Android |
|
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 0a8f9cc 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F140395%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
There was a problem hiding this comment.
- If the Android buildbots still exist and use API level 30 or higher (#123014 indicates they "will" use 34), please run the Android buildbots to make sure the tests pass.
The buildbots may run on API level 34, but they build against API level 21 or 24, so they still won't detect the function. However, I've tested this PR manually by setting the ANDROID_API_LEVEL environment variable to 30, and it builds and passes the statx tests.
|
I merged your PR, thanks. |
configure says
configure says |
|
As expected, the AMD64 build says |
|
Oh ok, I looked at the wrong line. Thanks for correcting me :-) |
5 participants
#140249 inadvertently disabled checking for
statxon Android becauseMACHDEPis "android" on Android. You can verify this by looking at the logs from the Android runner from #140249. This is a cross-compile. In the "Configure build Python" logs,MACHDEPislinuxand configure checks forstatx; in the "Configure host Python" logs,MACHDEPisandroidand configure doesn't check forstatx(would be just after checking forwritev).Base the check for
statxonac_sys_systeminstead, which isLinux-androidon Android,Linuxon other Linux distributions, andAIXon AIX (which has an incompatible function named statx that prompted #140249).Before merging:
Please look at the Android build log to make sure both the build and host Python configure check forVerified, the host Python configure checks forstatx. (The host Python check will result innobecause the Android CI uses Android API level 24 andstatxwas introduced in Android API level 30.)statx.cc @ayappanec as author of #140249
cc @vstinner as reviewer of statx-related PRs
cc @mhsmith as Android buildbot operator and Android maintainer
(There's a
"$MACHDEP" != linuxcheck just below the code this PR changes, which does not exclude Android as may have been intended. Android introducedlchmodin API level 36.lchmoddelegates tofchmodatwithAT_SYMLINK_NOFOLLOW, andfchmodathas code that should work on non-symlinks and give the correcterrnofor symlinks, so I think the behavior is acceptable, but we aren't testing it. That can be a separate issue/PR if it's a problem.)