Skip to content

Disable speedups on GraalPy same as on PyPy#339

Merged
etrepum merged 2 commits intosimplejson:masterfrom
timfel:patch-1
Sep 24, 2025
Merged

Disable speedups on GraalPy same as on PyPy#339
etrepum merged 2 commits intosimplejson:masterfrom
timfel:patch-1

Conversation

@timfel
Copy link
Contributor

@timfel timfel commented Sep 23, 2025

With this change when building from source GraalPy does not receive the native extension (which does not help much, same as on PyPy)

Copy link
Member

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Would it also make sense to add graalpy to the CI matrix?

@timfel
Copy link
Contributor Author

timfel commented Sep 23, 2025

Would it also make sense to add graalpy to the CI matrix?

I didn't see PyPy in there, so I assumed it's not desired. Afaict the sdist is only tested against 3.13 and the cibuildwheel testing is done while skipping PyPy. I'm happy to enable tests against GraalPy and PyPy :)

@etrepum
Copy link
Member

etrepum commented Sep 23, 2025

Would it also make sense to add graalpy to the CI matrix?

I didn't see PyPy in there, so I assumed it's not desired. Afaict the sdist is only tested against 3.13 and the cibuildwheel testing is done while skipping PyPy. I'm happy to enable tests against GraalPy and PyPy :)

It builds and tests on all supported platforms, but the source tarball artifact uploaded to pypi as the sdist is only built once. Doesn't make sense to upload the same tarball N times.

IIRC the pypy testing was removed because cibuildwheel isn't supporting it anymore but if there's a supported way to build and test with graal then I'd be happy to accept that.

@timfel timfel force-pushed the patch-1 branch 2 times, most recently from e0b6000 to 3eea9c5 Compare September 23, 2025 17:59
@timfel
Copy link
Contributor Author

timfel commented Sep 23, 2025

Well, cibuildwheel does support building with PyPy and with GraalPy, but in general it does not support building pure Python wheels at all. And since the C code is not used on PyPy and GraalPy, cibuildwheel will refuse to build on those. (https://github.com/timfel/simplejson/actions/runs/17954795969/job/51063190663#step:4:2668) - so the only way is to add a test step for PyPy and GraalPy.

@etrepum
Copy link
Member

etrepum commented Sep 23, 2025

testing those platforms in a separate job or jobs would be fine too

Copy link
Member

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Looks great, thank you! Will merge after the CI suite has finished

@etrepum etrepum enabled auto-merge September 23, 2025 18:37
With this change when building from source GraalPy does not receive the native extension (which does not help much, same as on PyPy)
auto-merge was automatically disabled September 24, 2025 07:13

Head branch was pushed to by a user without write access

@timfel
Copy link
Contributor Author

timfel commented Sep 24, 2025

Looks great, thank you! Will merge after the CI suite has finished

Thanks. Python 2 has been too long, I forgot getattr fails there without a default 🤦

@etrepum etrepum enabled auto-merge September 24, 2025 12:49
@etrepum etrepum disabled auto-merge September 24, 2025 12:50
@etrepum etrepum enabled auto-merge September 24, 2025 13:43
@etrepum etrepum merged commit 5f9f96f into simplejson:master Sep 24, 2025
9 checks passed
@timfel timfel deleted the patch-1 branch September 24, 2025 17:44
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.

2 participants

Comments