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-104799: Move location of type_params AST fields (GH-104828)#104974

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 26, 2023
edited by github-actionsbot
Loading

…4828)(cherry picked from commitba73473)Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@JelleZijlstra
Copy link
Member

Seems like this changes the ABI! cc@Yhg1s

I don't know whether the ABI change is relevant in practice, because external users shouldn't be constructing these internal AST nodes anyway. So maybe we should decide this is a false positive. If we think this is in fact a real issue that could affect users, I would vote to revert the change on main too and leave#104799 unfixed, as it's a relatively minor issue and the AST doesn't guarantee full backwards compatibility anyway.

AlexWaygood reacted with thumbs up emoji

@AlexWaygood
Copy link
Member

AlexWaygood commentedMay 30, 2023
edited
Loading

@Yhg1s, thoughts on this, as release manager? My preference would be to get this merged into 3.12 ASAP, as it minimizes the backwards-compatibility breakage between 3.11 and 3.12 that comes from thePEP-695 implementation. But if the failing ABI check makes that impossible, then we should revert the change that's been made onmain (which this PR is backporting) ASAP.

@AlexWaygoodAlexWaygood requested a review fromYhg1sMay 30, 2023 10:19
@JelleZijlstra
Copy link
Member

JelleZijlstra commentedMay 30, 2023
edited by AlexWaygood
Loading

@Yhg1s said on Discord that the ABI change is fine (thanks!). I'm running the script to regenerate the ABI locally now.

@JelleZijlstra
Copy link
Member

Unfortunately I couldn't get the script to work. On Mac something is wrong with libmpdec

gcc -I./Modules/_decimal/libmpdec -DCONFIG_64=1 -DANSI=1 -DHAVE_UINT128_T=1 -fno-strict-overflow -DNDEBUG -g -O3 -Wall -g3 -O0 -g3 -O0  -std=c11 -Werror=implicit-function-declaration -fvisibility=hidden  -I./Include/internal  -I. -I./Include   -fPIC -fPIC -c ./Modules/_decimal/_decimal.c -o Modules/_decimal/_decimal.ogcc -shared      Modules/_decimal/_decimal.o -lm Modules/_decimal/libmpdec/libmpdec.a  -o Modules/_decimal.cpython-312-aarch64-linux-gnu.so/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getprec':/src/./Modules/_decimal/_decimal.c:740: undefined reference to `mpd_getprec'/usr/bin/ld: Modules/_decimal/_decimal.o: in function `context_getemax':/src/./Modules/_decimal/_decimal.c:741: undefined reference to `mpd_getemax'

On an AWS Ubuntu instance I geterror="docker-credential-ecr-login can only be used with Amazon Elastic Container Registry.".

@AlexWaygood
Copy link
Member

AlexWaygood commentedMay 30, 2023
edited
Loading

I used the workflow Steve is demo-ing over at#105088 to regen the ABI via GitHub Actions on my CPython fork. Ithink I did it right. At least the GitHub Actions check on this PR is now passing!

zooba reacted with laugh emoji

@JelleZijlstra
Copy link
Member

Thanks! That diff looks pretty reasonable.

AlexWaygood reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@Yhg1sYhg1sAwaiting requested review from Yhg1s

Assignees

@JelleZijlstraJelleZijlstra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@miss-islington@JelleZijlstra@AlexWaygood@bedevere-bot

[8]ページ先頭

©2009-2025 Movatter.jp