GH-107585: fix the stable API .lib file name in debug builds on MSVC #107761
Conversation
FFY00
commented
Aug 8, 2023
•
FFY00
commented
- Issue: Windows + MSVC: using debug + limited API does not inject a pragma to link to python3_d.lib #107585
… MSVC Signed-off-by: Filipe Laíns <[email protected]>
Signed-off-by: Filipe Laíns <[email protected]>
|
Would it be simpler just to enumerate all the cases and stick with complete strings? Rather than doing the concatenation tricks and potentially invading the macro namespace? At the very least, we should use |
There was a problem hiding this comment.
Definitely don't want to use preprocessor variables without a _Py_ prefix.
It might be possible to use a single multiline #pragma comment to avoid having to create variables at all, though that'll make it harder to search for this bit of code (which I know people do when they hit link errors).
Also, this code has changed recently for nogil, so it'll need reworking to account for the extra set of names.
| @@ -307,13 +307,19 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */ | |||
| /* So MSVC users need not specify the .lib | |||
| file in their Makefile (other compilers are | |||
| generally taken care of by distutils.) */ | |||
| # if defined(Py_LIMITED_API) | |||
| # define PINNED_VER "3" | |||
There was a problem hiding this comment.
_Py_ prefixes on the variables, please.
| # if defined(Py_LIMITED_API) | ||
| # define PINNED_VER "3" | ||
| # else | ||
| # define PINNED_VER "313" |
There was a problem hiding this comment.
I'm not sure exactly how the release manager finds these variables to update them, but we might need to update some of their docs/tools for this change.
|
When you're done making the requested changes, leave the comment: |
