Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[3.12] gh-105020: Share tp_bases and tp_mro Between Interpreters For All Static Builtin Types (gh-105115)#105124

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

miss-islington
Copy link
Contributor

@miss-islingtonmiss-islington commentedMay 31, 2023
edited by bedevere-bot
Loading

Ingh-103912 we added tp_bases and tp_mro to each PyInterpreterState.types.builtins entry. However, doing so ignored the fact that both PyTypeObject fields are public API, and not documented as internal (as opposed to tp_subclasses). We address that here by reverting back to shared objects, making them immortal in the process.
(cherry picked from commit7be667d)

Co-authored-by: Eric Snowericsnowcurrently@gmail.com

…ll Static Builtin Types (pythongh-105115)Inpythongh-103912 we added tp_bases and tp_mro to each PyInterpreterState.types.builtins entry.  However, doing so ignored the fact that both PyTypeObject fields are public API, and not documented as internal (as opposed to tp_subclasses).  We address that here by reverting back to shared objects, making them immortal in the process.(cherry picked from commit7be667d)Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
@ericsnowcurrently
Copy link
Member

@Yhg1s, what do I need to do to fix the ABI check? It's a false positive.

@Yhg1s
Copy link
Member

@Yhg1s, what do I need to do to fix the ABI check? It's a false positive.

As the failed check says:

The up to date ABI file should be attached to this build as an artifact.To learn more about this check: https://devguide.python.org/setup/#regenerate-the-abi-dump

@zooba recently merged support for the CI check to produce the necessary artifact (#105088) but I don't think the devguide has been updated yet. (You may need to update the branch to get the updated check?)

For the record, the failure is not a false positive, it's just a change in the ABI that is okay (because all direct use of the changed struct is internal-only)

ericsnowcurrently reacted with thumbs up emoji

@zooba
Copy link
Member

zooba commentedJun 1, 2023
edited
Loading

The devguide has a mention now, but not step-by-step instructions (which would probably require screenshots of the GitHub UI that would likely go out of date and are probably too big for the doc page).

When the PR check fails, the associated run will have the updated ABI file attached as an artifact. After release manager approval, you can download and add this file into your PR to pass the check.

(Worth pointing out that "the PR check" has already been mentioned in the section, so it's not being newly introduced in the new text.)

Only trick seems to be that the artifacts are listed on the Summary page, not the specific check that published it. Perhaps we can figure out how to infer the URL to the summary page and add a link in that message? In this case, it'd behttps://github.com/python/cpython/actions/runs/5127462177?pr=105124#artifacts.

Changes to _PyRuntime caused the ABI check to fail.  We're okay to move forward since the *public* ABI wasn't touched.
@ericsnowcurrently
Copy link
Member

@zooba I was able to use that artifact, which saved me a bunch of work. Thanks!

@ericsnowcurrentlyericsnowcurrentlyenabled auto-merge (squash)June 1, 2023 22:22
@ericsnowcurrentlyericsnowcurrently merged commitc38ceb0 intopython:3.12Jun 1, 2023
@miss-islingtonmiss-islington deleted the backport-7be667d-3.12 branchJune 1, 2023 22:25
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@Yhg1sYhg1sAwaiting requested review from Yhg1s

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@miss-islington@ericsnowcurrently@Yhg1s@zooba@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp