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

cython: force through env variable #35995

Merged
merged 2 commits into from
Mar 17, 2023
Merged

Conversation

haampie
Copy link
Member

@haampie haampie commented Mar 10, 2023

So, the cython folks used to recommend shipping generated C files, kinda like
autotools packages include generated configure scripts and docs etc so you need
fewer dependencies.

But it turns out: cython relies a lot on cpython internals, meaning that it's
bound to be forward incompatible with Python, and that may even include
patch releases of Python, e.g. 3.10.1 -> 3.10.2.

Since cython itself is only a small build dep, and it takes less time to
generate C files than to compile them, it'd be better to force regeneration
using the new CYTHON_FORCE_REGEN environment variable.

I've opened a backport for it to 0.29, let's see if that gets accepted, then
the lowerbound for the env variable can be changed:

cython/cython#5307

@adamjstewart
Copy link
Member

I like this idea in theory. I think we'll find that a lot of packages will now need a cython build dep that didn't previously have one. We'll see how reliably running cython actually works. There are also likely many packages that manually remove cythonized files that we should clean up. I don't think anyone is currently using cython 3+, so I would keep this as a draft until we add a patch to older versions.

@adamjstewart adamjstewart self-assigned this Mar 10, 2023
@haampie
Copy link
Member Author

haampie commented Mar 14, 2023

We can also apply cython/cython#5307 as a patch in Spack 🤔

@spackbot-app spackbot-app bot added the patch label Mar 14, 2023
@haampie
Copy link
Member Author

haampie commented Mar 14, 2023

This still rebuilds the world, needs #36103, which of course takes an eternity because gitlab has to catch up with develop

adamjstewart
adamjstewart previously approved these changes Mar 14, 2023
Copy link
Member

@adamjstewart adamjstewart left a comment

Choose a reason for hiding this comment

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

This looks great! Confirmed that the patch applies all the way back to 0.29. The older versions will become deprecated and be removed if we can figure out the first version that supported Python 3.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants