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 — bpo-43795: Remove Py_FrozenMain from the Limited API & Stable ABI by encukou · Pull Request #26241 · python/cpython

Conversation

@encukou

@encukou encukou commented May 19, 2021 •
edited by miss-islington
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Py_FrozenMain was added to the Limited C API in bpo-42591 (3.10.0a4);
but to fix that issue it would be enough to add it to the regular C API.

The function is undocumented, tests were added very recently (bpo-44131),
and most importantly, it is not present in all builds of Python, as
the linker sometimes omits it as unused.
It should be added back when these issues are fixed.

Note that this does not affect Python's regular C API.

https://bugs.python.org/issue43795

Automerge-Triggered-By: GH:encukou

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
encukou added 2 commits May 19, 2021 15:02
@encukou
bpo-43795: Remove Py_FrozenMain from the Limited API & Stable ABI
f35f65b
@encukou
Don't define Py_FrozenMain with Py_LIMITED_API
eb57a3b
@encukou encukou added the needs backport to 3.10 label May 19, 2021
@encukou
encukou requested review from pablogsal and vstinner May 19, 2021 13:31
@encukou
encukou requested a review from a team as a code owner May 19, 2021 13:31
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label May 19, 2021
@bedevere-bot bedevere-bot added the awaiting core review label May 19, 2021
@encukou
Add a blurb
dac5c8c
@encukou
encukou force-pushed the pep652-no-frozenmain branch from 215efd4 to dac5c8c Compare May 19, 2021 13:31
@encukou
Fix ReST syntax in blurb
7aa20ce
@encukou

encukou commented May 19, 2021

Copy link
Copy Markdown
Member Author

See also the discussion at capi-sig: https://mail.python.org/archives/list/[email protected]/thread/5QLI3NUP3OSGLCCIBAQOTX4GEJQBWJ6F/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner reviewed May 20, 2021
View reviewed changes
Comment thread Include/pylifecycle.h Outdated
/* Py_FrozenMain is kept out of the Limited API until documented and present
in all builds of Python */
#ifndef Py_LIMITED_API
PyAPI_FUNC(int) Py_FrozenMain(int argc, char **argv);

@vstinner vstinner May 20, 2021

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.

Please move it into Include/cpython/pylifecycle.h instead.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Misc/NEWS.d/next/C API/2021-05-19-15-09-47.bpo-43795.WAHRxt.rst Outdated
@@ -0,0 +1,2 @@
:c:func:`Py_FrozenMain`, is no longer considered part of the Limited API and Stable ABI,
to which it was added in 3.10.0a4 (bpo-23730).

@vstinner vstinner May 20, 2021

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.

Py_FrozenMain was part of the limited C API in Python 3.9 and before, I don't get "to which it was added in 3.10.0a4", I understand that you're talking about the stable ABI. Since Python 3.10 is not released yet, maybe remove the Stable ABI part and only mention that you remove Py_FrozenMain from the limited C API? Or just write two sentences to clarify the change ;-)

Also, bpo-23730 is unrelated to Py_FrozenMain.

I like to document changes in the limited C API in https://docs.python.org/dev/whatsnew/3.10.html#c-api-changes (in the Removed section).

Sorry, something went wrong.

Uh oh!

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

@shihai1991 shihai1991 May 20, 2021

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.

Hm, I find this sentence from PEP-0384: This PEP proposes to define a stable set of API functions which are guaranteed to be available for the lifetime of Python 3. Do we effect any user when the 3.10 version isn't released?

Sorry, something went wrong.

Uh oh!

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

@encukou encukou May 21, 2021

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.

@vstinner, I updated to just a single sentence.
I see "Limited API" as not well defined before 3.10, unlike the Windows stable ABI list. But just noting that it's removed works.

@shihai1991 The trouble is that the set of APIs introduced in PEP 384 wasn't maintained the way the PEP intended.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
encukou added 2 commits May 21, 2021 11:23
@encukou
Move Py_FrozenMain definition ot Include/cpython
35bcbfb
@encukou
Update news & whatsnew
860fdd0
vstinner
vstinner approved these changes May 25, 2021
View reviewed changes

@vstinner vstinner left a comment

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, thanks for documenting the removal.

I just have a comment about the What's New entry. Feel free to change it or not.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.10.rst Outdated

* The undocumented function ``Py_FrozenMain`` has been removed from the
limited API. The function is mainly useful for custom builds of Python,
and it is not always present when Python is built as a shared library.

@vstinner vstinner May 25, 2021

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.

"when Python is built as a shared library" the symbol is exported, do you mean the opposite? Maybe omit this sentence since I would like this issue to be fixed :-D

Sorry, something went wrong.

Uh oh!

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

@encukou encukou May 25, 2021

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.

OK, I'll remove it. But the fix could reword this, too :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels May 25, 2021
encukou
encukou commented May 25, 2021
View reviewed changes
Comment thread Doc/whatsnew/3.10.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@encukou
Update Doc/whatsnew/3.10.rst
b333913
@encukou encukou added the 🤖 automerge label May 25, 2021
@miss-islington
miss-islington merged commit d168569 into python:main May 25, 2021
@bedevere-bot bedevere-bot removed the awaiting merge label May 25, 2021
@miss-islington

miss-islington commented May 25, 2021

Copy link
Copy Markdown
Contributor

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@encukou
encukou deleted the pep652-no-frozenmain branch May 25, 2021 11:42
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 25, 2021
@encukou @miss-islington
bpo-43795: Remove Py_FrozenMain from the Limited API & Stable ABI (py…
76995e3 
…thonGH-26241)

Py_FrozenMain was added to the Limited C API in [bpo-42591]() (3.10.0a4);
but to fix that issue it would be enough to add it to the regular C API.

The function is undocumented, tests were added very recently ([bpo-44131]()),
and most importantly, it is not present in all builds of Python, as
the linker sometimes omits it as unused.
It should be added back when these issues are fixed.

Note that this does not affect Python's regular C API.
(cherry picked from commit d168569)

Co-authored-by: Petr Viktorin <[email protected]>
@encukou

encukou commented May 25, 2021

Copy link
Copy Markdown
Member Author

Thanks for the review!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented May 25, 2021

Copy link
Copy Markdown

GH-26353 is a backport of this pull request to the 3.10 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.10 label May 25, 2021
pablogsal pushed a commit that referenced this pull request May 25, 2021
@miss-islington @encukou
bpo-43795: Remove Py_FrozenMain from the Limited API & Stable ABI (GH…
68b9c20 
…-26241) (GH-26353)

Py_FrozenMain was added to the Limited C API in [bpo-42591]() (3.10.0a4);
but to fix that issue it would be enough to add it to the regular C API.

The function is undocumented, tests were added very recently ([bpo-44131]()),
and most importantly, it is not present in all builds of Python, as
the linker sometimes omits it as unused.
It should be added back when these issues are fixed.

Note that this does not affect Python's regular C API.
(cherry picked from commit d168569)

Co-authored-by: Petr Viktorin <[email protected]>

Co-authored-by: Petr Viktorin <[email protected]>
@pablogsal pablogsal mentioned this pull request Jul 18, 2021
[3.10] Correct the order of regen-abidump #27228
Closed
@encukou encukou mentioned this pull request Jun 7, 2022
Implement PEP 652 -- Maintaining the Stable ABI #87961
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shihai1991 shihai1991 shihai1991 left review comments
@vstinner vstinner vstinner approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal

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.

6 participants

@encukou @miss-islington @bedevere-bot @vstinner @shihai1991 @the-knights-who-say-ni
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.