GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||
GeezerVison is showing github.com — gh-97588: Fix ctypes structs by matthiasgoergens · Pull Request #97702 · python/cpython Conversation
|
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit ba61051 🤖 If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This is about as good as it'll be for 3.13. So there is a decision to make:
I'm torn here. Does anyone want to play a more impartial judge? Or just chip in with an opinion? @zooba, @gpshead For an idea about what could go into 3.14: Things that are under-tested or missing, where fixes might break working code:
Missing features that would IMO be needed before we can claim that ctypes can match a C struct in the common ABIs:
Things needed for better testability or implementability of the above:
For this PR: all buildbots fail on only:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This stuff has been broken since forever, and even this PR has been slumbering for a long time. So I would vote in favour of taking your time and fixing this properly, and merging it into whatever version of Python is the one currently being worked on when you are ready. But I have no strong feelings either way. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
!buildbot s390x SLES PR |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit bc1225b 🤖 The command will test the builders whose names match following regular expression: The builders matched are:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
This is a tough one. I'm leaning in the same direction -- this is substantial work, and if you merge now you may spend the entire beta period having to tweak it, and possibly the 3.14 release cycle regretting it. If you had come with this two weeks ago I would have said "put it in", but now it's very close to beta 1. You could talk to the RM for a 3rd opinion. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@gpshead, you asked me to look into this. Was that to try to get it in 3.13, or just to move it forward? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I like the conclusions above that Matthias and Guido make: Save this for 3.14. IIRC I asked you to look into it because it seemed hairy enough to understand the details of that I didn't imagine it would ever go forward without a committer dedicating time to understand it as you have. You've done a lot. Even if this went in for 3.13 I'd call it an improvement, but given more potential work to be done has been identified in the process, waiting for 3.14 isn't going to hurt anyone and will avoid repeated behavior-fix-transition pain. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Btw, if you are taking your time to fix this properly, I would like to suggest migrating the intricate logic that calculates the offsets etc from the specification from C to Python. That way it's much easier to understand and maintain, and this calculation only happens once per struct definition, so it shouldn't be in any inner loop and doesn't need to be C-level fast. When I wrote my prototype that you kindly took over, I actually started sketching out my logic in Python before translating it into C. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thank you for confirming! 3.14 it is. Yes, rewriting this in Python is the way to go.
Yup. There are a few modules like that; I was diving into argparse when you invited me here :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I'll merge this when I'll be available to fix any issues quickly: after PyCon US, or at the sprints if someone wants to pair up there. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Argh, I hit Enter while I was editing the commit message. Sorry for that :( |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Structure layout, and especially bitfields, sometimes resulted in clearly wrong behaviour like overlapping fields. This fixes Co-authored-by: Gregory P. Smith <[email protected]> Co-authored-by: Petr Viktorin <[email protected]>
|
FYI: this appears to have broken tests on x86 (32-bit) Linux, see gh-121938. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.