Uh oh!
There was an error while loading. Please reload this page.
gh-118201: Simplify conv_confname - #126089
Conversation
mhsmith
commented
Oct 28, 2024
!buildbot android |
bedevere-bot
commented
Oct 28, 2024
mhsmith
commented
Oct 28, 2024
!buildbot ios |
bedevere-bot
commented
Oct 28, 2024
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
commented
Oct 29, 2024
@sobolevn: You've done some work on the |
sobolevn
commented
Oct 29, 2024
Nope, sorry, I am not faimiliar with that code :( |
mhsmith
commented
Oct 31, 2024
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
commented
Oct 31, 2024
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 the |
mhsmith
commented
Oct 31, 2024
It looks like this code has remained almost unchanged since it was written in 1999 by @freddrake. |
JelleZijlstra
commented
Nov 1, 2024
Wow, that seems like a good reason to be extra careful with any changes. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
mhsmith
commented
Nov 10, 2024
!buildbot android |
bedevere-bot
commented
Nov 10, 2024
Uh oh!
There was an error while loading. Please reload this page.
mhsmith
commented
Nov 19, 2024
!buildbot android|ios |
bedevere-bot
commented
Nov 19, 2024
JelleZijlstra
commented
Nov 19, 2024
@mhsmith thanks, this looks good! I assume the buildbot failure are the result of the unrelated GC change that came in the other day? |
mhsmith
commented
Nov 19, 2024
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:
|
mhsmith
commented
Nov 19, 2024
!buildbot android|ios |
bedevere-bot
commented
Nov 19, 2024
mhsmith
commented
Nov 19, 2024
OK, looks like this is ready to merge. |
Thanks @mhsmith for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, @mhsmith and @JelleZijlstra, I could not cleanly backport this to |
(cherry picked from commit c5c9286)
GH-131375 is a backport of this pull request to the 3.13 branch. |
On Android and iOS we've been seeing intermittent buildbot failures where
os.confstr,os.pathconforos.sysconfraise 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 on
conv_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.
os.sysconfon iOS #118453