Skip to content

cython: force through env variable#35995

Merged
alalazo merged 2 commits intospack:developfrom
haampie:fix/cython-recython!
Mar 17, 2023
Merged

cython: force through env variable#35995
alalazo merged 2 commits intospack:developfrom
haampie:fix/cython-recython!

Conversation

@haampie
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

@alalazo alalazo force-pushed the fix/cython-recython! branch from 2a4a888 to 83ac763 Compare March 17, 2023 10:26
@alalazo alalazo enabled auto-merge (squash) March 17, 2023 10:26
@alalazo alalazo merged commit fd70a2c into spack:develop Mar 17, 2023
@haampie haampie deleted the fix/cython-recython! branch March 18, 2023 11:13
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.

3 participants