GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-140239: Check for statx on Android by jbosboom · Pull Request #140395 · python/cpython

Conversation

@jbosboom

@jbosboom jbosboom commented Oct 21, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

#140249 inadvertently disabled checking for statx on Android because MACHDEP is "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, MACHDEP is linux and configure checks for statx; in the "Configure host Python" logs, MACHDEP is android and configure doesn't check for statx (would be just after checking for writev).

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 that prompted #140249).

Before merging:

  • Please look at the Android build log to make sure both the build and host Python configure check for statx. (The host Python check will result in no because the Android CI uses Android API level 24 and statx was introduced in Android API level 30.) Verified, the host Python configure checks for statx.
    • People are building for API levels higher than 30, given issue #123014, so this PR is not theoretical.
  • 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. (I can't do this myself.)

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" != linux check just below the code this PR changes, which does not exclude Android as may have been intended. Android introduced lchmod in API level 36. lchmod delegates to fchmodat with AT_SYMLINK_NOFOLLOW, and fchmodat has code that should work on non-symlinks and give the correct errno for 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.)

  • Issue: AIX build broken because of recent os.statx implementation. #140239

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@jbosboom
Check for statx on Android
0a8f9cc
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
jbosboom requested review from AA-Turner, corona10, emmatyping and erlend-aasland as code owners October 21, 2025 04:44
@bedevere-app bedevere-app Bot mentioned this pull request Oct 21, 2025
AIX build broken because of recent os.statx implementation. #140239
Closed
@bedevere-app bedevere-app Bot added the awaiting review label Oct 21, 2025
@ayappanec

ayappanec commented Oct 21, 2025

Copy link
Copy Markdown
Contributor

I didn't knew MACHDEP is not Linux in Android. Sorry for that. Thanks for all the details. Will be more thorough in the future.,

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jbosboom

jbosboom commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

@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.

ayappanec reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Oct 21, 2025

Copy link
Copy Markdown
Member

!buildbot Android

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Oct 21, 2025

Copy link
Copy Markdown

🤖 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: Android

The builders matched are:

  • AMD64 Android PR
  • aarch64 Android PR
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhsmith
mhsmith approved these changes Oct 21, 2025
View reviewed changes

@mhsmith mhsmith left a comment

Copy link
Copy Markdown
Member

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.

  • 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner and jbosboom reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@bedevere-app bedevere-app Bot added awaiting core review and removed awaiting review labels Oct 21, 2025
vstinner
vstinner approved these changes Oct 21, 2025
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

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.

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Oct 21, 2025
@vstinner
vstinner merged commit c788bfb into python:main Oct 21, 2025
6 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 21, 2025
@vstinner

vstinner commented Oct 21, 2025

Copy link
Copy Markdown
Member

I merged your PR, thanks.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Oct 21, 2025

Copy link
Copy Markdown
Member

buildbot/AMD64 Android PR Build done.

configure says checking for statx... yes. But I don't see test_os in run tests.

buildbot/aarch64 Android PR Build done.

configure says checking for statx... no.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhsmith

mhsmith commented Oct 21, 2025

Copy link
Copy Markdown
Member

As expected, the AMD64 build says statx... yes for the "build Python" for Linux, but no for the "host Python" for Android. And it looks like test_os appears normally at line 390 of the test log.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Oct 21, 2025

Copy link
Copy Markdown
Member

Oh ok, I looked at the wrong line. Thanks for correcting me :-)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

StanFromIreland pushed a commit to StanFromIreland/cpython that referenced this pull request Dec 6, 2025
@jbosboom @StanFromIreland
pythongh-140239: Check for statx on Android (python#140395)
ea6c7a9
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhsmith mhsmith mhsmith approved these changes
@vstinner vstinner vstinner approved these changes
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner
@corona10 corona10 Awaiting requested review from corona10 corona10 is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@emmatyping emmatyping Awaiting requested review from emmatyping emmatyping is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@jbosboom @ayappanec @vstinner @bedevere-bot @mhsmith
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.