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

Drop generated source from source distribution #539

Closed
methane opened this issue May 21, 2023 · 5 comments
Closed

Drop generated source from source distribution #539

methane opened this issue May 21, 2023 · 5 comments
Milestone

Comments

@methane
Copy link
Member

methane commented May 21, 2023

Thanks to cibuildwheel, we can provide wheels for most common platforms.
Dropping C++ source from source distribution will make build process simple.

@methane methane added this to the 1.1 milestone May 21, 2023
@jfolz
Copy link
Contributor

jfolz commented May 21, 2023

IMO it makes sense to have generated code in the source distribution. Other than removing Cython as a build dependency on the user's end, we can also be sure what code was compiled without worrying about bugs in different versions of Cython.

@methane
Copy link
Member Author

methane commented May 21, 2023

We can pin the version of cython to generate source by pyproject.toml.

https://pip.pypa.io/en/stable/reference/build-system/pyproject-toml/#build-time-dependencies

@ThomasWaldmann
Copy link
Contributor

ThomasWaldmann commented Sep 6, 2023

BTW, I recently restricted the cython version in borgbackup's pyproject.toml (and requirements.txt) and that just led to confusion / questions when some linux dist does not have that version.

E.g. for the stable borgbackup, i thought cython<3 might be conservative, as we used that for years already. But of course archlinux already has jumped on cython 3.0.x and does not have 0.29.x any more...

Thus, in the end, I removed the version restriction again. So everybody can use any working cython version they want and we also offer the generated C code (in our case, generated with cython 0.29.x).

So I also think bundling the generated C code in the sdist is a good idea.

@methane
Copy link
Member Author

methane commented Mar 10, 2024

OK. I decided not to do it.

@methane methane closed this as not planned Won't fix, can't repro, duplicate, stale Mar 10, 2024
@tacaswell
Copy link
Contributor

As an after the fact counter argument, the most frequent changes to cython are to the output (not to the input) with old generated code not compiling with newer versions of CPython (leaving aside how much of the not-really-public API of CPython that cython uses).

By including the generated c/c++ you are almost certainly giving up future compatibility of the sdist on newer versions of CPython (where the sdist is most valuable as there are not wheels for the future versions of CPython) that a user can not work around to avoid the chance of a bug in cython that users can easily work around (e.g. pip install cython==X.Y.Z; pip install --no-build-isolation msgpack) which will only impact users on versions/platforms that do not have a binary wheel (likely because they are on a future version of CPython!) or have explicitly opted out of wheels.

I have a project that tries to build the development branch of everything against the development branch of CPython (starting with the projects I am a maintainer on and gaining projects as needed). I have only seen a handful of actually regressions in Cython (and none in releases that I recall), but run into "sdist has pre-cythonized code that won't compile" frequently (it is probably the most common reason a project gets moved from pip install --pre foo to being installed from the default branch).

I also would not try to pin the version of cython in the pyproject.toml.

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

No branches or pull requests

4 participants