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.

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

abi3-py36 now passes tests #1320

Merged
merged 6 commits into from
Dec 20, 2020
Merged

abi3-py36 now passes tests #1320

merged 6 commits into from
Dec 20, 2020

Conversation

kngwyu
Copy link
Member

@kngwyu kngwyu commented Dec 15, 2020

Now cargo test --features=abi3-py36 fails since ffi::PyEval_InitThreads() is called after Py_InitializeEx.
So now py36 build is not actually forward-compatible.
We have a small fix for this problem (this PR), but this also suggests that py3* build can be broken in the future version.
Though I'm not sure the breakage is likely to happen in the user code, maybe we want to add a note about it.

@kngwyu kngwyu mentioned this pull request Dec 15, 2020
@kngwyu
Copy link
Member Author

kngwyu commented Dec 15, 2020

Oops, it looks like that datetime is not declared with Py_LIMITED_API....
I'm glad to find this major bug before releasing 0.13!

@kngwyu kngwyu force-pushed the abi3-py36-now-passes-tests branch from 29c3d48 to 17d0868 Compare December 15, 2020 15:05
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

👍 thanks, that's a nice catch!

I think a couple of these changes are done with the goal of minimising LOC & optimizations, but I don't agree with them. I'd prefer we kept what we had before for those cases. See my other comments.

src/err/mod.rs Outdated Show resolved Hide resolved
src/exceptions.rs Outdated Show resolved Hide resolved
src/gil.rs Outdated Show resolved Hide resolved
@davidhewitt
Copy link
Member

I merged #1329 which should fix the pypy issues - if you rebase on master CI should hopefully be green 🤞

@davidhewitt
Copy link
Member

Ok I merged #1331 and now hopefully this will work. I'm going to rebase this on master and then merge if it's ok.

@davidhewitt davidhewitt force-pushed the abi3-py36-now-passes-tests branch 3 times, most recently from ddb2c4e to c57324b Compare December 20, 2020 12:57
@davidhewitt
Copy link
Member

Ugh, still failing pypy tests!

I think the issue is that on pypy we can build the tests, but can't ever run them.

I'm going to work on yet another PR to improve the pypy CI...

@kngwyu
Copy link
Member Author

kngwyu commented Dec 20, 2020

Thanks.
Could I help to debug?
What is the precise commit where this starts failing?
#1325?

@davidhewitt davidhewitt force-pushed the abi3-py36-now-passes-tests branch from c57324b to db74cc8 Compare December 20, 2020 13:50
@davidhewitt
Copy link
Member

Ah, yes, that's it - for now I have disabled the pypy abi3-py36 tests being built; that should fix this PR and then I'll open a new PR later which re-enables building the tests.

@davidhewitt
Copy link
Member

🎉

@davidhewitt davidhewitt merged commit 11f0442 into master Dec 20, 2020
@davidhewitt davidhewitt deleted the abi3-py36-now-passes-tests branch December 20, 2020 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants