fix 3.15+ append_to_inittab on platforms which aren't 64-bit little-endian

[davidhewitt]

Fixes #6143

On 3.15, m_slots: slots.0.get() as _ was hiding a cast from *mut [PySlot; N] to *mut PyModuleDef_Slot. On 64-bit little-endian this happens to be ok, which is why we never noticed.

[davidhewitt]

cc @decathorpe, do you happen to have a way to test with this patch to confirm?

I would also love if you have any ideas how we could test for 32-bit platform and/or s390x on CI; at the moment we run clippy for a bunch of platforms but I think you've hit a number of bugs on these platforms now. Hugely appreciate the patient reports but also feel like maybe it's time we considered if there's anything else we can do here to minimise the amount of issues which make it downstream to you.

[Person-93]

@davidhewitt I think QEMU can emulate both of those platforms. I have not tried using QEMU in a gtihub action runner, but it should work. So in CI, you can cross-compile and then run the test binary with QEMU.

[alex]

For 32-bit, it's possible to run a 32-bit Python binary on GHA Window. It's slow, though not as slow as emulation.

[decathorpe]

cc @decathorpe, do you happen to have a way to test with this patch to confirm?

Yes, I can. I've launched a test build with this PR included, I will report back once I get the results.

I would also love if you have any ideas how we could test for 32-bit platform and/or s390x on CI; at the moment we run clippy for a bunch of platforms but I think you've hit a number of bugs on these platforms now. Hugely appreciate the patient reports but also feel like maybe it's time we considered if there's anything else we can do here to minimise the amount of issues which make it downstream to you.

Yeah I think emulation is going to be the only workable solution here - Cirrus CI used to provide more architectures I think, but that's no longer an option.

@davidhewitt I think QEMU can emulate both of those platforms. I have not tried using QEMU in a gtihub action runner, but it should work. So in CI, you can cross-compile and then run the test binary with QEMU.

Relatedly, depending on what test coverage you'd want, I think you'd also need to compile on the target platform, not cross-compile. Last time I checked, doctests did not support getting cross-compiled, so without compiling on the target platform, those wouldn't be covered by tests.

[decathorpe]

Seems like the patch from this PR doesn't apply cleanly on top of 0.29.0, I'm getting compilation failures:

error[E0609]: no field `0` on type `PyModuleSlots`
   --> src/impl_/pymodule.rs:634:67
    |
634 |             assert_eq!((*module_def.ffi_def.get()).m_slots, SLOTS.0.get().cast());
    |                                                                   ^ unknown field
    |
    = note: available fields are: `pep_820_slots`, `pep_489_slots`
error[E0609]: no field `values` on type `PyModuleSlotsBuilder`
   --> src/impl_/pymodule.rs:652:35
    |
652 |         let second_last = builder.values[builder.len - 1];
    |                                   ^^^^^^ unknown field
    |
    = note: available fields are: `slots`, `len`
error[E0609]: no field `values` on type `PyModuleSlotsBuilder`
   --> src/impl_/pymodule.rs:653:28
    |
653 |         let last = builder.values[builder.len];
    |                            ^^^^^^ unknown field
    |
    = note: available fields are: `slots`, `len`

I will try to test this PR directly instead of applying it as a patch on top of 0.29.0.

EDIT: Nevermind, looks like the new code on the branch linked to this PR just doesn't compile.

[davidhewitt]

Sorry about that, the follow-up commit should hopefully fix the build.

Regarding platform testing, we actually already have a 32-bit windows build but only for latest stable Python. I guess we could add a test for in-development Python, though I'm about too many platforms for in-development Python(s) creating unneeded churn. Probably at beta stage it's fine?

I guess we need to make a similar decision for s390x, but it'd be even worse with QEMU emulation probably needing to build the in-dev Python version.

[codspeed-hq]

Merging this PR will degrade performance by 26.2%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 139 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
❌	into_u128_u64_max	1.5 µs	2 µs	-26.2%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing davidhewitt:pep-489-slots (b41e08b) with main (b3aa7da)}

[decathorpe]

Thanks - I can confirm that with the changes in this PR, pyo3 v0.29.0 now compiles + runs tests successfully on both i686 and s390x (and doesn't regress the others).

[davidhewitt]

Great, thanks!

[ngoldbaum]

Just one comment, otherwise this looks good.

For what it's worth, NumPy also recently switched our win32 CI job to run on 3.15t and it's been fine so far.

[ngoldbaum]

Can these variables have names that don't refer to PEPs? Using more descriptive names avoids the need to look up the PEPs unless someone really needs to.