FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-141671: PyMODINIT_FUNC: apply `__declspec(dllexport)` on Windows by encukou · Pull Request #141672 · python/cpython · GitHub

/ cpython Public

gh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Windows - #141672

Merged
encukou merged 11 commits into
python:mainfrom
encukou:pymodexport-always-export
Jan 7, 2026
ConversationCommits (11)ChecksFiles changed
Merged

gh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Windows#141672
encukou merged 11 commits into
python:mainfrom
encukou:pymodexport-always-export

Conversation

encukou commented Nov 17, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

encukou requested a review from zooba November 17, 2025 16:48
bedevere-app Bot mentioned this pull request Nov 17, 2025
Closed
encukou mentioned this pull request Nov 17, 2025
Open
25 tasks

encukou commented Nov 17, 2025 •
edited
Loading

Copy link
Copy Markdown
Member Author

cc @jaraco / @pfmoore: what are your thoughts on removing /EXPORT from setuptools/distutils on Python 3.15+?

zooba reviewed Nov 17, 2025
View reviewed changes
Comment thread Include/exports.h Outdated

#if defined(_WIN32) || defined(__CYGWIN__)
#if defined(Py_ENABLE_SHARED)
#if !defined(Py_BUILD_CORE) || defined(Py_ENABLE_SHARED)

zooba Nov 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm not 100% sure about this... don't we need it for our own exports?

encukou Nov 18, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Only for Py_ENABLE_SHARED -- see #99888 that added the #else.

zooba Nov 18, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah, I see we do these checks again everywhere else we use the macros. No problem them.

pfmoore commented Nov 17, 2025

Copy link
Copy Markdown
Member

TBH, I have no view. My knowledge of C is very out of date, so I wouldn't trust anything I might say anyway...

da-woods commented Nov 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

The /EXPORT from setuptools has definitely caused Cython an amount of pain before (although some of that was self-inflicted....). It'd be nice not to have to define PyInit_ just to to make the name exist. Although I suspect we can't really avoid it in a realistic timeline.

(Edit: admittedly we also aren't the people that it's designed to help either)

encukou commented Dec 16, 2025

Copy link
Copy Markdown
Member Author

Here's a version that removes some redundant defines (setting things to defaults).

vstinner approved these changes Dec 17, 2025
View reviewed changes

vstinner left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM. This change mostly moves code around, and just add __declspec(dllexport) on Windows.

Comment thread Include/exports.h

#define PyMODINIT_FUNC _PyINIT_FUNC_DECLSPEC PyObject*
#define PyMODEXPORT_FUNC _PyINIT_FUNC_DECLSPEC PyModuleDef_Slot*
#ifndef PyMODINIT_FUNC

vstinner Dec 17, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Supporting an already defined PyMODINIT_FUNC/PyMODEXPORT_FUNC is a new feature. Is it really worth it? I'm not against it, just curious.

encukou Dec 17, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's an escape hatch: if there's an unexpected problem with this PR, this can allow people to hotfix it without patching or waiting for a new release.

encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 19, 2025

bedevere-bot commented Dec 19, 2025

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit 5091b79 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F141672%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 19, 2025
AraHaan reviewed Dec 29, 2025
View reviewed changes
Comment thread Include/exports.h
#if defined(Py_BUILD_CORE)
#define _PyINIT_EXPORTED_SYMBOL Py_EXPORTED_SYMBOL
#else
#define _PyINIT_EXPORTED_SYMBOL __declspec(dllexport)

AraHaan Dec 29, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think a better option instead is to define Py_EXPORTED_SYMBOL here and then use that.

Also I feel like an escape hatch like Py_EMBED_MODULE where it basically uses Py_LOCAL_SYMBOL instead could be an option as well for when one does not want the module init function exported due to intending to use PyImport_AppendInitTab within the exe that they directly build the module into.

encukou Jan 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't want to change the meaning of Py_EXPORTED_SYMBOL.

For the inittab, you can avoid PyMODINIT_FUNC/PyMODEXPORT_FUNC, and just use a normal declaration.

encukou merged commit 8565ddd into python:main Jan 7, 2026
46 checks passed
encukou deleted the pymodexport-always-export branch January 7, 2026 12:09
rgommers mentioned this pull request Jan 13, 2026
Open
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
pythongh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Win…
417f14e 
…dows (pythonGH-141672)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

zooba zooba left review comments
vstinner vstinner approved these changes
+1 more reviewer
AraHaan AraHaan left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

Back | FazBrowse Home | New Git URL