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-118201: Simplify conv_confname#126089

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
JelleZijlstra merged 6 commits intopython:mainfrommhsmith:simplify-conv-confname
Nov 19, 2024

Conversation

@mhsmith
Copy link
Member

@mhsmithmhsmith commentedOct 28, 2024•
edited
Loading

On Android and iOS we've been seeing intermittent buildbot failures whereos.confstr,os.pathconf oros.sysconf raise a ValueError. This is not a failure in the underlying system call, but a failure to look up a mapping from strings to integers before making the system call.

I haven't been able to explain this (see#118201 for details), but I have found a way to simplify the relevant code, which will hopefully fix the issue, or at least give us some more information about it.

All three functions depend onconv_confname, which currently maps from strings to integers using a binary search in a C array. This PR changes it to useos.{confstr,pathconf,sysconf}_names, which are dicts containing the same information.

I also took the opportunity to improve the test coverage of this area.

@mhsmith
Copy link
MemberAuthor

!buildbot android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@mhsmith for commit2a3e50a 🤖

The command will test the builders whose names match following regular expression:android

The builders matched are:

  • aarch64 Android PR
  • AMD64 Android PR

@mhsmith
Copy link
MemberAuthor

!buildbot ios

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@mhsmith for commit2a3e50a 🤖

The command will test the builders whose names match following regular expression:ios

The builders matched are:

  • iOS ARM64 Simulator PR

@mhsmith
Copy link
MemberAuthor

mhsmith commentedOct 29, 2024•
edited
Loading

Well I'm not sure why it ran the buildbots on all platforms when I only asked for two, but I've checked the failures, and all of them are already present on the main branch.

@mhsmith
Copy link
MemberAuthor

@sobolevn: You've done some work on theposix module recently; are you able to review this PR?

@sobolevn
Copy link
Member

Nope, sorry, I am not faimiliar with that code :(

@mhsmith
Copy link
MemberAuthor

I'm not sure if any active developer is familiar with this code, but I hope the change should be fairly self-explanatory.@JelleZijlstra: are you able to take a look, or can you recommend anyone else?

@JelleZijlstra
Copy link
Member

I can take a look but I'm also not familiar so I'll need a bit of time to familiarize myself with it. Have you tried looking at thegit blame for the affected functions to see if any were written by people who are still active?

@JelleZijlstraJelleZijlstra self-requested a reviewOctober 31, 2024 22:19
@mhsmith
Copy link
MemberAuthor

It looks like this code has remained almost unchanged since it was written in 1999 by@freddrake.

@JelleZijlstra
Copy link
Member

Wow, that seems like a good reason to be extra careful with any changes.

@mhsmith
Copy link
MemberAuthor

!buildbot android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@mhsmith for commita214081 🤖

The command will test the builders whose names match following regular expression:android

The builders matched are:

  • AMD64 Android PR
  • aarch64 Android PR

@mhsmith
Copy link
MemberAuthor

!buildbot android|ios

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@mhsmith for commit8a026f1 🤖

The command will test the builders whose names match following regular expression:android|ios

The builders matched are:

  • iOS ARM64 Simulator PR
  • aarch64 Android PR
  • AMD64 Android PR

@JelleZijlstra
Copy link
Member

@mhsmith thanks, this looks good! I assume the buildbot failure are the result of the unrelated GC change that came in the other day?

@mhsmithmhsmith added needs backport to 3.13bugs and security fixes and removed needs backport to 3.13bugs and security fixes labelsNov 19, 2024
@mhsmith
Copy link
MemberAuthor

Yes, and it looks like that's been fixed on the main branch now, so I'll try another run.

Let's not backport this PR to 3.13, as it's a relatively disruptive change, and a backport isn't really necessary:

  • On iOS, the relevant tests are already skipped.
  • On Android, the tests have never failed on the 3.13 branch.

@mhsmith
Copy link
MemberAuthor

!buildbot android|ios

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@mhsmith for commit8a026f1 🤖

The command will test the builders whose names match following regular expression:android|ios

The builders matched are:

  • aarch64 Android PR
  • iOS ARM64 Simulator PR
  • AMD64 Android PR

@mhsmith
Copy link
MemberAuthor

OK, looks like this is ready to merge.

@JelleZijlstraJelleZijlstra merged commitc5c9286 intopython:mainNov 19, 2024
50 checks passed
ebonnal pushed a commit to ebonnal/cpython that referenced this pull requestJan 12, 2025
@mhsmithmhsmith added the needs backport to 3.13bugs and security fixes labelMar 17, 2025
@miss-islington-app
Copy link

Thanks@mhsmith for the PR, and@JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington-app
Copy link

Sorry,@mhsmith and@JelleZijlstra, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker c5c9286804e38c95fe717f22ce1bf2f18eee5b17 3.13

mhsmith added a commit to mhsmith/cpython that referenced this pull requestMar 17, 2025
@bedevere-app
Copy link

GH-131375 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelMar 17, 2025
freakboy3742 pushed a commit that referenced this pull requestMar 18, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Intermittent "unrecognized configuration name" failure on iOS and Android

4 participants

@mhsmith@bedevere-bot@sobolevn@JelleZijlstra
[8]ページ先頭
©2009-2025 Movatter.jp