GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-141671: PyMODINIT_FUNC: apply `__declspec(dllexport)` on Windows by encukou · Pull Request #141672 · python/cpython

Conversation

@encukou

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: PyMODINIT_FUNC does not apply __declspec(dllexport) on Windows #141671

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
encukou added 2 commits November 17, 2025 17:42
@encukou
pythongh-141671: Use __declspec(dllexport) when not building core
39d18d1
@encukou
blurb
ad9110c
@encukou
encukou requested a review from zooba November 17, 2025 16:48
@bedevere-app bedevere-app Bot added the awaiting core review label Nov 17, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Nov 17, 2025
PyMODINIT_FUNC does not apply __declspec(dllexport) on Windows #141671
Closed
@encukou encukou mentioned this pull request Nov 17, 2025
Implement PEP 793 – PyModExport #140550
Open
19 of 25 tasks
@encukou

encukou commented Nov 17, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zooba
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 zooba Nov 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@encukou encukou Nov 18, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@zooba zooba Nov 18, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@pfmoore

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...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@da-woods

da-woods commented Nov 18, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou added 8 commits November 18, 2025 11:07
@encukou
Use _PyINIT_EXPORTED_SYMBOL
fd34aff
@encukou
Use __declspec(dllexport) outside core
c0e0a83
@encukou
Allow embedders to customize
5abc323
@encukou
Remove duplication with defaults
82ad329
@encukou
Remove duplication with default
35a088a
@encukou
Reduce diff
b39a5a8
@encukou
Merge in the main branch
13da2c0
@encukou
Link macro that's now documented
5091b79
@encukou

encukou commented Dec 16, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner approved these changes Dec 17, 2025
View reviewed changes

@vstinner vstinner left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Include/exports.h

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

@vstinner vstinner Dec 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@encukou encukou Dec 17, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Dec 17, 2025
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 19, 2025
@bedevere-bot

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 19, 2025
AraHaan
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 AraHaan Dec 29, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@encukou encukou Jan 7, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@encukou
Merge branch 'main' into pymodexport-always-export
8818fae
@encukou
encukou merged commit 8565ddd into python:main Jan 7, 2026
46 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou deleted the pymodexport-always-export branch January 7, 2026 12:09
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 7, 2026
@rgommers rgommers mentioned this pull request Jan 13, 2026
Don't automatically add PyInit_<name> to export symbols in Python 3.15+ pypa/distutils#395
Merged
thunder-coding pushed a commit to thunder-coding/cpython that referenced this pull request Feb 15, 2026
@encukou @thunder-coding
pythongh-141671: PyMODINIT_FUNC: apply __declspec(dllexport) on Win…
417f14e 
…dows (pythonGH-141672)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@encukou @pfmoore @da-woods @bedevere-bot @vstinner @zooba @AraHaan
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.