Skip to content
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

bpo-43725: Add CI step to check changes in the exported ABI #25188

Closed
wants to merge 1 commit into from
Closed

bpo-43725: Add CI step to check changes in the exported ABI #25188

wants to merge 1 commit into from

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Apr 4, 2021 •
edited by bedevere-bot
Loading

@pablogsal pablogsal changed the title Add CI step to check changes in the exported ABI bpo-43725: Add CI step to check changes in the exported ABI Apr 4, 2021
@pablogsal pablogsal force-pushed the abicheck branch 6 times, most recently from 311b675 to 953b0f1 Compare April 4, 2021 20:55
@pablogsal
Copy link
Member Author

Unfortunately seems that doing this check with two different compiler versions produce some different DWARF and the tool identifies the differences as errors :(

@pablogsal pablogsal closed this Apr 4, 2021
gpshead
gpshead approved these changes Apr 4, 2021
View reviewed changes
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

Neat, lets try it out! I suppose this shows up as a non-blocking check by default? Release branch owners can presumably decide if this is reliable enough to make it blocking.

@pablogsal pablogsal reopened this Apr 4, 2021
@pablogsal
Copy link
Member Author

I will try to investigate how to overcome this:

Unfortunately seems that doing this check with two different compiler versions produce some different DWARF and the tool identifies the differences as errors :(

@pablogsal
Copy link
Member Author

@gpshead Check my message in https://bugs.python.org/msg390213 with the analysis of this solution.

erlend-aasland
erlend-aasland reviewed Apr 4, 2021
View reviewed changes
.github/workflows/build.yml
@@ -48,6 +48,28 @@ jobs:
git diff --name-only origin/$GITHUB_BASE_REF.. | grep -qvE '(\.rst$|^Doc|^Misc)' && echo '::set-output name=run_tests::true' || true
fi

check_abi:
name: 'Check if the ABI has changed'
runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd pin the runner, to ensure that the job runs with at least the same major GCC version every time. Ref. bpo-43382.

@pablogsal pablogsal closed this Apr 6, 2021
@pablogsal pablogsal deleted the abicheck branch May 19, 2021 18:57
@gpshead gpshead mentioned this pull request May 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erlend-aasland erlend-aasland erlend-aasland left review comments

@gpshead gpshead gpshead approved these changes

Assignees
No one assigned
Labels
awaiting core review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pablogsal @gpshead @erlend-aasland @the-knights-who-say-ni @bedevere-bot