GeezerVison

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

That's looks like a reasonable suggestion, bit possibly out of scope for this PR.

Less intrusively, I could add whitespace in the test expectations, and remove it before comparing.

Let's see how the core devs feel.

Thanks for the review!

@abalkin: Any chance you could take a look at this?

@brettcannon: I'd argue this is type-bugfix, not type-enhancement - the implementation in master produces invalid buffers in almost all cases. This isn't just adding support for _pack_ (an enhancement), but also fixing the behavior for when _pack_ is absent.

@mdickinson, I really appreciate that you were able to review #5576 a few weeks ago. Do you think you'd be able to take a look at this one too?

@eric-wieser Yes, I'll take a look. It won't be this weekend, though, I'm afraid.

@eric-wieser: Changes LGTM; thank you.

@abalkin Python 3.12a5 is due tomorrow, which means that if we leave this PR open for another couple of days we'll end up having to nudge the news entry again. (That was poor timing on my part - sorry about that.) But I think this is ready, so I'll take the EAFP approach: I'll merge, and we can tweak later if necessary.

@mdickinson - my only concern was with the floor log10 computation that I felt could be done better. If it looks good enough to you, I have no objections to the merge.

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Installed with X 3.x has failed when building commit 90d85a9.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/464/builds/3771) and take a look at the build logs.
4. Check if the failure is related to this commit (90d85a9) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/464/builds/3771

Failed tests:

• test_ctypes

Failed subtests:

• test_native_types - test.test_ctypes.test_pep3118.Test.test_native_types

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

413 tests OK.

1 test failed:
test_ctypes

17 tests skipped:
test_asdl_parser test_check_c_globals test_clinic test_devpoll
test_gdb test_ioctl test_kqueue test_launcher test_msilib
test_peg_generator test_perf_profiler test_startfile
test_winconsoleio test_winreg test_winsound test_wmi
test_zipfile64

1 re-run test:
test_ctypes

Total duration: 22 min

Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.installed/build/target/lib/python3.12/test/test_ctypes/test_pep3118.py", line 27, in test_native_types
    self.assertEqual(normalize(v.format), normalize(fmt))
AssertionError: 'T{<b:x:3x<Q:y:}' != 'T{<b:x:7x<Q:y:}'
- T{<b:x:3x<Q:y:}
?        ^
+ T{<b:x:7x<Q:y:}
?        ^

Urgh. That looks like a legitimate failure on 32-bit platforms, introduced by this PR. @eric-wieser I think this is a problem with the test rather than the core logic. Do you agree?

Yes, you're right; the cases that don't set _pack_ at all are platform-dependent.

I think a test written for 32 bit platforms would pass on both; so replacing the uint64 with uint32s should make the problem go away

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot x86 Gentoo Non-Debug with X 3.x has failed when building commit 90d85a9.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/58/builds/3813) and take a look at the build logs.
4. Check if the failure is related to this commit (90d85a9) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/58/builds/3813

Failed tests:

• test_ctypes

Failed subtests:

• test_native_types - test.test_ctypes.test_pep3118.Test.test_native_types

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

419 tests OK.

10 slowest tests:

• test_math: 4 min 3 sec
• test_multiprocessing_spawn: 3 min 45 sec
• test_asyncio: 2 min 46 sec
• test_concurrent_futures: 2 min 45 sec
• test_tokenize: 1 min 46 sec
• test_multiprocessing_forkserver: 1 min 32 sec
• test_unparse: 1 min 24 sec
• test_multiprocessing_fork: 1 min 15 sec
• test_signal: 1 min 12 sec
• test_compileall: 1 min 2 sec

1 test failed:
test_ctypes

14 tests skipped:
test_check_c_globals test_devpoll test_ioctl test_kqueue
test_launcher test_msilib test_peg_generator test_perf_profiler
test_startfile test_winconsoleio test_winreg test_winsound
test_wmi test_zipfile64

1 re-run test:
test_ctypes

Total duration: 26 min 53 sec

Traceback (most recent call last):
  File "/buildbot/buildarea/cpython/3.x.ware-gentoo-x86.nondebug/build/Lib/test/test_ctypes/test_pep3118.py", line 27, in test_native_types
    self.assertEqual(normalize(v.format), normalize(fmt))
AssertionError: 'T{<b:x:3x<Q:y:}' != 'T{<b:x:7x<Q:y:}'
- T{<b:x:3x<Q:y:}
?        ^
+ T{<b:x:7x<Q:y:}
?        ^

I think a test written for 32 bit platforms would pass on both [...]

I'll give that a go; PR shortly. I'm wondering whether I'm going to end up hitting problems with ILP32 platforms not being consistent about whether uint32 is aliased to unsigned int or to unsigned long, though.

PR shortly

#101587