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

Windows + MSVC: using debug + limited API does not inject a pragma to link to python3_d.lib #107585

Open
mattip opened this issue Aug 3, 2023 · 5 comments
Labels
build The build process and cross-build OS-windows

Comments

@mattip
Copy link
Contributor

mattip commented Aug 3, 2023

This came up in mesonbuild/meson#11745, where meson on MSVC is overriding the #pragma (comment lib... pragma. Shouldn't there be logic to link to python3_d.lib when using both debug and the limited API? Here is the code in question, it has 3 branches not 4:

cpython/PC/pyconfig.h

Lines 310 to 316 in a73daf5

# if defined(_DEBUG)
# pragma comment(lib,"python313_d.lib")
# elif defined(Py_LIMITED_API)
# pragma comment(lib,"python3.lib")
# else
# pragma comment(lib,"python313.lib")
# endif /* _DEBUG */

When I install python from https://www.python.org/downloads/windows/ and specify debug symbols in the installer, I do see 4 import libs in the libs folder.

Linked PRs

@mattip
Copy link
Contributor Author

mattip commented Aug 3, 2023

Even better: provide sysconfig options to get the import lib location and then remove this altogether (after a deprecation period)

FFY00 added a commit to FFY00/cpython that referenced this issue Aug 8, 2023
… MSVC

Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
Copy link
Member

FFY00 commented Aug 8, 2023

I definitely think sysconfig should provide this information, but I don't know enough about the MSVC build to evaluate the proposal to remove this entirely.

@eli-schwartz
Copy link
Contributor

Meson would definitely like to not have to deal with this header code.

Removing it entirely is a matter of some opinion. It's equivalent to what unix toolchains do, which is that if you need to link to a library, that is the job of the build system (probably your PEP 517 build backend, but potentially embedding libpython into another executable).

MSVC has a "clever trick" where you can stick your linker arguments into a header file and forcefully inject those linker arguments into any translation unit that includes said header.

  • Some people find this useful in reducing how much build system glue you need in order to use the header.
  • Other people find it frustrating because it results in unclear behavior, duplicative work (your build system already handles linking to a variety of libraries, including the python library), and... prevents you from working around bugs in the use of the forcefully injected linker flags.

Incidentally, if you follow the discussion on the linked mesonbuild ticket there's a fun reference to pybind11, which plays the trick of undefining _DEBUG just for Python.h in order to reach a world where you can debug your extension modules without requiring the use of a debug build of python itself. This is some dark sorcery. For the limited API use case, meson is going to simply pass /NODEFAULTLIB:python<XXX>.lib so that it can rely on the existing link arguments while monkey-patching Python.h for versions of python that don't have the fix for this issue.

Having to monkey-patch the linker like this is the kind of thing that's only necessary because it is forcing linker flags on you even when you're pretty sure you know better than your currently installed version of Python.h.

Maybe an acceptable compromise solution would be for pyconfig.h to include support for a macro such as -DPy_NO_MSVC_LIB_PRAGMA, so that the people who find the pragma troublesome could avoid generating it, while the people who really want this pragma1 still get to use it.

Footnotes

  1. I do not know who -- maybe it's for people that are compiling python extensions by spawning a MSVC dev command prompt and running cl.exe by hand, without a build system?

@rgommers
Copy link

Other people find it frustrating because it results in unclear behavior, duplicative work (your build system already handles linking to a variety of libraries, including the python library), and... prevents you from working around bugs in the use of the forcefully injected linker flags.

In case it matters at all: I agree with this. I ran into another such pragma in mesonbuild/meson#10776 (comment), and it was extremely difficult to debug. You're just chasing linker flags that appear to come out of nowhere, with no reasonable way of debugging the problem.

but I don't know enough about the MSVC build to evaluate the proposal to remove this entirely.

It's be better to get rid of all pragma usage at some point early in a new release cycle. @FFY00 do you mean the MSVC build of CPython itself? That should be fixable, right?

@serhiy-storchaka serhiy-storchaka added OS-windows build The build process and cross-build labels Jan 31, 2024
@zooba
Copy link
Member

zooba commented Jan 31, 2024

It wouldn't hurt too badly to remove it, though we'd have to make sure that the build backends are all aware of it and adapt their code. Definitely needs a sysconfig variable to provide the names though.

I was sure we'd added a preprocessor option to suppress it, though I don't see it in there now, so perhaps it never made it in for some reason.

FWIW, I think linking against a debug build of CPython should be quite an advanced scenario. So if build tools wanted to override the CRT debug option (pass /MD and /DNDEBUG instead of /MDd and /D:_DEBUG - they can still use /Od and /Zi flags, those are fine) and link against the release build unless given super special options, that'd be fine by me. You can probably even use /MDd with /DNDEBUG if you need the CRT's own assertions and extra runtime checks.


Oh, and I don't think you should redistribute a debug build at all. Debug symbols in some cases, but don't package up modules that expect to load into python_d.exe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build The build process and cross-build OS-windows
Projects
None yet
Development

No branches or pull requests

6 participants