Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wheels built for the Python limited API on Windows should link against python3.dll, not minor version specific library like python39.dll #13167

Closed
lpsinger opened this issue May 2, 2024 · 4 comments · Fixed by #13171
Labels
modules:python Issues specific to the python module OS:windows Winodows OS specific issues

Comments

@lpsinger
Copy link

lpsinger commented May 2, 2024

Describe the bug
According to the Python C API reference manual:

On Windows, extensions that use the Stable ABI should be linked against python3.dll rather than a version-specific library such as python39.dll.

However, on Windows, Meson incorrectly links against the version-specific library when building limited API extensions in limited API Python packages. As a result, the packages are broken and cannot be used with Python versions that are newer than the version used at build time.

To Reproduce
See https://github.com/lpsinger/meson-windows-limited-api. Take a look at a recent GitHub Actions workflow log such as https://github.com/lpsinger/meson-windows-limited-api/actions/runs/8918072559/job/24492117501. Notice that when cibuildwheel gets to cp310-win_amd64, it does notice the limited API wheel that it had already built with Python 3.9, as expected:

Found previously built wheel example-0.0.1-cp39-abi3-win_amd64.whl, that's compatible with cp310-win_amd64. Skipping build step...

Then when it runs the unit tests, they fail with this error:

  ______________________ ERROR collecting test_example.py _______________________
  ImportError while importing test module 'D:\a\meson-windows-limited-api\meson-windows-limited-api\test_example.py'.
  Hint: make sure your test modules/packages have valid Python names.
  Traceback:
  ..\..\..\..\pypa\cibuildwheel\Cache\nuget-cpython\python.3.10.11\tools\lib\importlib\__init__.py:126: in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
  D:\a\meson-windows-limited-api\meson-windows-limited-api\test_example.py:1: in <module>
      from example import hello
  E   ImportError: DLL load failed while importing example: The specified module could not be found.

Take a look at one of the recent build artifacts containing the built packages, such as https://github.com/lpsinger/meson-windows-limited-api/actions/runs/8918072559/artifacts/1465707788.

If you unpack the wheel and look inside it, you can see that the C extension module example.pyd was linked against the wrong python DLL:

$ strings example.pyd | grep dll
KERNEL32.dll
api-ms-win-crt-environment-l1-1-0.dll
api-ms-win-crt-heap-l1-1-0.dll
api-ms-win-crt-runtime-l1-1-0.dll
api-ms-win-crt-stdio-l1-1-0.dll
api-ms-win-crt-string-l1-1-0.dll
api-ms-win-crt-time-l1-1-0.dll
python39.dll      <------------------------------- OOPS!
crtdll.c
dllentry.c
dllmain.c
__dll__
.rdata$.refptr.__native_dllmain_reason
.rdata$.refptr.__mingw_module_is_dll
__dll_characteristics__
__mingw_module_is_dll
.refptr.__mingw_module_is_dll
__native_dllmain_reason
_head_python39_dll
python39_dll_iname
.refptr.__native_dllmain_reason

Expected behavior
This Python C extension module should be linked against python3.dll, not python39.dll. The module should be importable and the tests should pass under Python 3.9, 3.10, 3.11, 3.12, etc.

system parameters

  • Is this a cross build or just a plain native build (for the same computer)? native build
  • what operating system (e.g. MacOS Catalina, Windows 10, CentOS 8.0, Ubuntu 18.04, etc.) _ Microsoft Windows Server 2022_
  • what Python version are you using 3.9.13
  • what meson --version 1.4.0
  • what ninja --version if it's a Ninja build 1.11.1.1
@eli-schwartz
Copy link
Member

if any(compiler.get_id() == 'msvc' for compiler in compilers.values()):
pydep_copy = copy.copy(pydep)
pydep_copy.find_libpy_windows(self.env, limited_api=True)
if not pydep_copy.found():
raise mesonlib.MesonException('Python dependency supporting limited API not found')

From your log:

    Version: 1.4.0
    Source dir: D:\a\meson-windows-limited-api\meson-windows-limited-api
    Build dir: D:\a\meson-windows-limited-api\meson-windows-limited-api\.mesonpy-r84voty2
    Build type: native build
    Project name: example
    Project version: undefined
    C compiler for the host machine: gcc (gcc 12.2.0 "gcc (x86_64-posix-seh-rev2, Built by MinGW-W64 project) 12.2.0")
    C linker for the host machine: gcc ld.bfd 2.39
    Host machine cpu family: x86_64
    Host machine cpu: x86_64
    Program python found: YES (C:\Users\runneradmin\AppData\Local\Temp\cibw-run-1_0nvaf4\cp39-win_amd64\build\venv\Scripts\python.exe)

Apparently that assumption was incorrect. :) /cc @amcn

@rgommers might be interested in this as well...

@eli-schwartz eli-schwartz added modules:python Issues specific to the python module OS:windows Winodows OS specific issues labels May 2, 2024
@amcn
Copy link
Contributor

amcn commented May 2, 2024

I'm investigating this presently.

@rgommers
Copy link
Contributor

rgommers commented May 2, 2024

The assumption being that the #pragma linker flag insertion mess is only happening for MSVC? I'm fuzzy on the details here, but I think this is what's happening: #10776 (comment).

At least Clang-cl and the Intel compilers also define _MSC_VER I believe, so compiler.get_id() == 'msvc' is probably too specific.

@amcn
Copy link
Contributor

amcn commented May 2, 2024

I was able to reproduce locally and I have a fix, which I am currently testing against the CI workflows in the above linked PR. It currently moves a lot of code around, but I hope to refine it to a smaller patch with feedback and test results.

amcn added a commit to amcn/meson that referenced this issue May 8, 2024
Based on the example in GH issue mesonbuild#13167, the limited API test has been
extended with a test to load the compiled module to ensure it can be
loaded correctly.
amcn added a commit to amcn/meson that referenced this issue May 8, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
amcn added a commit to amcn/meson that referenced this issue May 8, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
amcn added a commit to amcn/meson that referenced this issue May 27, 2024
Based on the example in GH issue mesonbuild#13167, the limited API test has been
extended with a test to load the compiled module to ensure it can be
loaded correctly.
amcn added a commit to amcn/meson that referenced this issue May 27, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
amcn added a commit to amcn/meson that referenced this issue May 27, 2024
Based on the example in GH issue mesonbuild#13167, the limited API test has been
extended with a test to load the compiled module to ensure it can be
loaded correctly.
amcn added a commit to amcn/meson that referenced this issue May 27, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
amcn added a commit to amcn/meson that referenced this issue Jun 11, 2024
Based on the example in GH issue mesonbuild#13167, the limited API test has been
extended with a test to load the compiled module to ensure it can be
loaded correctly.
amcn added a commit to amcn/meson that referenced this issue Jun 11, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
amcn added a commit to amcn/meson that referenced this issue Jun 11, 2024
Based on the example in GH issue mesonbuild#13167, the limited API test has been
extended with a test to load the compiled module to ensure it can be
loaded correctly.
amcn added a commit to amcn/meson that referenced this issue Jun 11, 2024
This commit fixes GH issue mesonbuild#13167 by linking to the correct
library under MINGW when the 'limited_api' kwarg is specified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modules:python Issues specific to the python module OS:windows Winodows OS specific issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants