Skip to content

gh-88267: Avoid exporting functions when a static build is used #99888

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

Merged
merged 2 commits into from
Dec 9, 2022

Conversation

lakor64
Copy link
Contributor

@lakor64 lakor64 commented Nov 30, 2022

In a Python core static build scenario (for example: embedding in Windows), MSVC would export the Python symbols for such application as well.
This commit fixes this behavour allowing the application to not export python modules and remove the automatically generated lib file, wichi shouldn't be generated for an executable.

Modules such as _socket.pyd would be disabled in a static build as the linked environment would differ, therefore I don't think there is any use for having an application specifically export modules.

In case where such behavour would be required, I would suggest adding an extra macro to disable such exports (such as Py_NO_EXPORTS).

This behavour has been tested on Windows MSVC2022, on Linux I don't think GCC would automatically export such functions but I'm not an expert on that topic.

(Fixes #88267)

NOTE: I am aware that pythoncore currently doesn't build properly in static, I used vcpkg's https://github.com/microsoft/vcpkg/blob/master/ports/python3/0002-static-library.patch (updated for Py 3.12) along with some changes to PC/dl_nt.c to allow the python interpreter to properly run, I could send a PR for this changes in case.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@ghost
Copy link

ghost commented Nov 30, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

@kumaraditya303 kumaraditya303 requested a review from a team December 8, 2022 08:50
@zooba
Copy link
Member

zooba commented Dec 8, 2022

The change is fine, I think it does deserve a NEWS entry (under Build) though.

@netlify
Copy link

netlify bot commented Dec 8, 2022

Deploy Preview for python-cpython-preview ready!

Name Link
🔨 Latest commit b68b3fa
🔍 Latest deploy log https://app.netlify.com/sites/python-cpython-preview/deploys/6391ee0a03bce400086c5748
😎 Deploy Preview https://deploy-preview-99888--python-cpython-preview.netlify.app/whatsnew/changelog
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@lakor64
Copy link
Contributor Author

lakor64 commented Dec 8, 2022

The change is fine, I think it does deserve a NEWS entry (under Build) though.

Thank you for letting me know this (as it's my first time contributing in cpython I had no idea if this change required an entry) I've added an entry now via blurb. If I have to rebase the changes to master / modify the NEWS let me know.

Copy link
Member

@zooba zooba left a comment

Choose a reason for hiding this comment

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

Approved. I won't be around to merge today, but someone else can.

If there's a desire to backport, this should be safe enough. But it might make pre-3.12 builds a bit less predictable compared to knowing that you need to patch yourself.

@zooba zooba merged commit 3c53554 into python:main Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generation of an executable's library file when python is built is static
3 participants