GeezerVison

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

GeezerVison is showing github.com — Add guidelines for changing/removing from the Limited API by encukou · Pull Request #778 · python/devguide

Conversation

encukou commented Dec 7, 2021

Copy link

Copy Markdown

Member

See the CPython PR: python/cpython#29956
and mailing list discussion: https://mail.python.org/archives/list/[email protected]/thread/64VOMHNR37N4HVPWAHKHJM4UFYJPVKSV/

Sorry, something went wrong.

Uh oh!

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

All reactions


          Add guidelines for changing/removing from the Limited API

58bf107

the-knights-who-say-ni added the CLA signed label

Dec 7, 2021

encukou mentioned this pull request

Dec 7, 2021

bpo-43795: Document stable_abi.txt format and contents python/cpython#29956

Merged

arhadthedev reviewed

Dec 7, 2021

View reviewed changes

Comment thread

c-api.rst Outdated

Comment on lines +133 to +134

		.. _macro: https://github.com/python/cpython/blob/3.10/Objects/object.c#L2298-L2308
		.. _redefinition: https://github.com/python/cpython/blob/main/Include/object.h#L603-L606

arhadthedev Dec 7, 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.

Is it safe to refer lines in branches instead of tags and commits? For example, Py_NewRef shifted up by 56 lines after it was redefined.

Suggested change

	.. _macro: https://github.com/python/cpython/blob/3.10/Objects/object.c#L2298-L2308
	.. _redefinition: https://github.com/python/cpython/blob/main/Include/object.h#L603-L606
	.. _macro: https://github.com/python/cpython/blob/v3.10.1/Objects/object.c#L2299-L2307
	.. _redefinition: https://github.com/python/cpython/blob/53a03aa/Include/object.h#L547-L550

Sorry, something went wrong.

Uh oh!

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

encukou reacted with heart emoji

All reactions

❤️ 1 reaction

encukou Dec 7, 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.

Thanks for catching that! I failed to copy/paste the right URL.

Sorry, something went wrong.

Uh oh!

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

All reactions


          Link to a tag rather than a branch

bae3bb1

vstinner reviewed

Dec 9, 2021

View reviewed changes

Comment thread

c-api.rst

		- the Backwards Compatibility Policy (:pep:`387`) is followed, and
		- the Stable ABI is not broken -- that is, extensions compiled with
		Limited API of older versions of Python continue to work on
		newer versions of Python.

vstinner Dec 9, 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.

Is it possible to export a function which always fail, if the function has been removed? Removing PyUnicode_InternImmortal() sounds like an interesting challenge: https://bugs.python.org/issue41692

Maybe the function can be replaced with a function... which does nothing but log an error?

Sorry, something went wrong.

Uh oh!

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

All reactions

arhadthedev Dec 9, 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.

Is it possible to export a function which always fail, if the function has been removed?

I believe, only if the function is not vital for already compiled modules (so its failure won't hamper their main activity). That's actually the whole point of backwards compatibility, to not break third party things that already work.

Sorry, something went wrong.

Uh oh!

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

All reactions

encukou Dec 9, 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.

IMO, after it's removed from the API and when SSTATE_INTERNED_IMMORTAL needs to be removed, PyUnicode_InternImmortal can do an unpaired INCREF: make the string immortal, but leak it.
See https://www.python.org/dev/peps/pep-0652/#stable-abi:

Future Python versions may deprecate some members of the Stable ABI. Deprecated members will still work, but may suffer from issues like reduced performance or, in the most extreme cases, memory/resource leaks.

Sorry, something went wrong.

Uh oh!

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

All reactions

gvanrossum reviewed

Dec 9, 2021

View reviewed changes

Comment thread

c-api.rst Outdated

		its size must not change.
		- Exported symbols (functions and data) must continue to be available
		as exported symbols. Specifically, a function can only be converted
		to a `static inline` function (or macro) if Python also continues to

gvanrossum Dec 9, 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.

More backticks?

Sorry, something went wrong.

Uh oh!

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

All reactions


          Add more backticks

b7f731f

encukou force-pushed the c-api branch from 4d58b8d to b7f731f Compare

December 14, 2021 13:07


          Undo unrelated change

e751783

encukou merged commit 5c8a021 into python:main

Dec 14, 2021

encukou deleted the c-api branch

December 14, 2021 13:53

encukou commented Dec 14, 2021

Copy link

Copy Markdown

Member Author

Thanks for the reviews!

If you have any more questions, let me know! I thought I left breadcrumbs in all the right places, but I see people coming from directions that are only obvious in retrospect :)

All reactions

Sorry, something went wrong.

Uh oh!

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

AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request

Jun 17, 2022


          Add guidelines for changing/removing from the Limited API (python#778)

74cacfe

Reviewers

vstinner

vstinner left review comments

gvanrossum

gvanrossum left review comments

arhadthedev

arhadthedev left review comments

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.

5 participants

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.