Skip to content

bpo-42545: Check that all symbols in the limited ABI are exported #23616

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 4, 2020
Merged

bpo-42545: Check that all symbols in the limited ABI are exported #23616

merged 1 commit into from
Dec 4, 2020

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Dec 2, 2020 •
edited by bedevere-bot
Loading

@pablogsal pablogsal requested a review from vstinner December 2, 2020 20:55
@pablogsal pablogsal force-pushed the stable_abi branch 17 times, most recently from cc22074 to 80d409f Compare December 2, 2020 23:32
vstinner
vstinner reviewed Dec 4, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

Nice script!

Should we check if PC/python3dll.c is outdated?

@pablogsal
Copy link
Member Author

Should we check if PC/python3dll.c is outdated?

I considered that, but that file has some other functions being exposed that are Windows dependent so we cannot technically use this script that runs on Linux. I will try to figure out another PR to keep that file updated.

@vstinner
Copy link
Member

vstinner commented Dec 4, 2020

I considered that, but that file has some other functions being exposed that are Windows dependent so we cannot technically use this script that runs on Linux.

My request was more about checking from your script output if python3dll.c is complete or not. If there are differences between Windows and Linux in the ABI, maybe differences can be saved in a file used by your script?

I will try to figure out another PR to keep that file updated.

Sure. It's ok to first only focus on Linux.

@vstinner
Copy link
Member

vstinner commented Dec 4, 2020

The CI fails :-)

Some symbols from the stable ABI are missing: PyCFunction_New
Makefile:1922: recipe for target 'check-limited-abi' failed

@pablogsal pablogsal force-pushed the stable_abi branch 3 times, most recently from 16e35d9 to 5049076 Compare December 4, 2020 18:59
@pablogsal
Copy link
Member Author

The CI fails :-)

Whoops, wrong makefile invocation for shared libs. Fixed!

@pablogsal
Copy link
Member Author

pablogsal commented Dec 4, 2020 •
edited
Loading

If there are differences between Windows and Linux in the ABI, maybe differences can be saved in a file used by your script?

I checked and the file needs some love (functions missing or old functions remaining), so I am going to defer fixes to that file for a later PR, maybe checking with @zooba.

@pablogsal pablogsal merged commit 85f1ded into python:master Dec 4, 2020
@pablogsal pablogsal deleted the stable_abi branch December 4, 2020 22:06
@pablogsal
Copy link
Member Author

README.txt

?

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@pablogsal @adorilson
bpo-42545: Check that all symbols in the limited ABI are exported (py…
de97a64 
…thonGH-23616)
@pablogsal pablogsal mentioned this pull request Nov 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@pablogsal @vstinner @the-knights-who-say-ni @bedevere-bot