refactor: cleanup code

[messense]

See https://github.com/PyO3/pyo3/blob/f5357f9395f8ac3632ad0e32ea4c0b2ea6a14b07/pyo3-build-config/src/impl_.rs#L1522-L1524

[messense]

Fixes https://github.com/PyO3/pyo3/runs/6371408437?check_suite_focus=true

[ravenexp]

While this change is technically correct, this also makes it impossible to build wheels for Windows by just adding --target x86_64-pc-windows-gnu. This is very unfortunate, because x86_64-pc-windows-msvc is a pain to use on Linux.

Sadly, MinGW Python is not a thing since version 3.5. Since it is not supported by the upstream at all, it is not possible to publish and distribute wheels for it.
Then there is the MinGW-w64 software distribution, which has it's own Python build, but it's a Unix Python that happens to run on Windows. And this one uses python3.y.dll library names in place of python3y.dll used by the official Windows build.

Ideally, the standard Windows Python DLL name should be used when cross-compiling, and the MinGW one should be selected only when building things for the MinGW distribution on Windows, i.e. when python3-dll-a is also being built with x86_64-pc-windows-gnu (as a part of the build script). pyo3-build-config could then also be updated to use the same heuristic by default.

What do you think?

[ravenexp]

We need something less surprising for the users here...

[adamreichold]

I also wonder whether this link is strictly necessary: I have been building extensions using MinGW but targeting the libraries from the MSVC-based upstream CPython releases.

[adamreichold]

Oh, I just noticed that you explained all of this in #17 (comment). Sorry for noise.

[davidhewitt]

I have been building extensions using MinGW but targeting the libraries from the MSVC-based upstream CPython releases.

Same here actually 😅

[adamreichold]

Ideally, the standard Windows Python DLL name should be used when cross-compiling, and the MinGW one should be selected only when building things for the MinGW distribution on Windows, i.e. when python3-dll-a is also being built with x86_64-pc-windows-gnu (as a part of the build script). pyo3-build-config could then also be updated to use the same heuristic by default.

At a first glance, this sounds reasonable to me. It should definitely be documented though as it does feel a bit brittle.

[ravenexp]

At a first glance, this sounds reasonable to me. It should definitely be documented though as it does feel a bit brittle.

Yes, this looks super convoluted, but it's my best guess wrt what the users would expect to get when they type cargo build --target. Most Rust users do not associate the x86_64-pc-windows-gnu rustc target with MSYS2 specifically.

[messense]

Admittedly I don't quite understand the issue here since personally I don't build wheels using mingw64.

I've checked locally on Ubuntu that this works with PyO3/pyo3#2364, all of the following commands succeeded

# abi3
cargo build --manifest-path examples/maturin-starter/Cargo.toml --features abi3 --target x86_64-pc-windows-gnu
cargo xwin build --manifest-path examples/maturin-starter/Cargo.toml --features abi3 --target x86_64-pc-windows-msvc
# non-abi3
export PYO3_CROSS_PYTHON_VERSION=3.9
cargo build --manifest-path examples/maturin-starter/Cargo.toml --features generate-import-lib --target x86_64-pc-windows-gnu
cargo xwin build --manifest-path examples/maturin-starter/Cargo.toml --features generate-import-lib --target x86_64-pc-windows-msvc

Another idea, can we change it to generate both python3y.dll.a and python3.y.dll.a? (Or just copy python3y.dll.a to python3.y.dll.a).

[adamreichold]

I've checked locally on Ubuntu that this works with PyO3/pyo3#2364, all of the following commands succeeded

I think the problem would manifest when loading such an extension using the official CPython builds under Windows (providing python3x.dll) instead of the MinGW builds (which provide python3.x.dll).

[ravenexp]

Another idea, can we change it to generate both python3y.dll.a and python3.y.dll.a? (Or just copy python3y.dll.a to python3.y.dll.a).

This will not work because on Windows, the import library name and the DLL library name do not actually have to match (crazy!).
The DLL file name the linker output will link to is determined by the LIBRARY statement in the original .DEF file, not the import library file name, as I thought before:

python3-dll-a/src/python39.def

Line 6 in ac9a080

LIBRARY "python39.dll"

[ravenexp]

I think the problem would manifest when loading such an extension using the official CPython builds under Windows (providing python3x.dll) instead of the MinGW builds (which provide python3.x.dll)

Yes, this is what I'm worrying about, given that 3 out of 4 commenters in this thread have been using the MinGW cross target to build Python wheels for Windows, not for MSYS2.

I double checked and found out that the MSYS2 Python build actually provides libpython3.y.dll, not python3.y.dll, as I wrote above. What is more interesting, on Arch Linux, which I use, the Python DLL is named either python3y.dll or libpython3.y.dll depending on whether you install the binary package or the source package:

https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=mingw-w64-python-bin#n54
https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=mingw-w64-python

On the up side, it looks like the distributions that use libpython3.y.dll also install _sysconfigdata*.py, which can be used by PyO3 to cross-compile via PYO3_CROSS_LIB_DIR. Native compilation should also work on MSYS2 using the provided Python interpreter to query the build config.

So my proposal here is to change the PyO3 defaults instead to always assume python3y.dll for Windows targets, and to leave the python3-dll-a configuration as is. MSYS2 Python should be treated as a custom build, which requires either a working interpreter or a library directory with _sysconfigdata*.py. Actually, this is exactly how PyO3 worked before the python3-dll-a patches were merged, so I don't think we are breaking much backwards compatibility here.

[messense]

OK, I've repurposed this PR as a code cleanup.

[ravenexp]

LGTM, thanks!

[messense]

So my proposal here is to change the PyO3 defaults instead to always assume python3y.dll for Windows targets

Done in PyO3/pyo3@ea37acb