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

Replace compile-time dependency on pari by run-time dispatch #125

Closed
mkoeppe opened this issue Feb 21, 2021 · 10 comments · Fixed by #166
Closed

Replace compile-time dependency on pari by run-time dispatch #125

mkoeppe opened this issue Feb 21, 2021 · 10 comments · Fixed by #166
Milestone

Comments

@mkoeppe
Copy link

mkoeppe commented Feb 21, 2021

The current design is not compatible with modern Python packaging, in which PEP517/518 dictate that there is no such thing as an "optional build dependency".
Also, if installing a wheel one can never be sure whether it has been built with the PARI connection or not.

This should be replaced by a hook into which PARI (via cypari/cypari2) can install itself.

@embray
Copy link
Collaborator

embray commented Mar 9, 2021

I was thinking about this just yesterday. I admit I don't 100% understand how the PARI integration is supposed to work or what it's for. But AFAICT it could be replaced with a more generic hook. Such a hook might be useful for other interfaces as well, such as for GAP.

@embray
Copy link
Collaborator

embray commented Mar 9, 2021

If nothing else, for easier pip-install, --without-pari should be the default, rather than --with-pari.

@mkoeppe
Copy link
Author

mkoeppe commented Mar 10, 2021

Let's look into creating the hook sometime during in the Sage 9.4 development cycle.

@dimpase
Copy link
Member

dimpase commented Dec 1, 2021

I'm able to build wheels (for manylinux and macOS) for a cython module dependent on an external C++ library in https://github.com/dimpase/primecountpy
using https://github.com/pypa/cibuildwheel

OK, here we'd need to build libpari - should not make a difference.

Should I try?

@mkoeppe
Copy link
Author

mkoeppe commented Dec 1, 2021

No, the best way forward is to merge #149

@dimpase
Copy link
Member

dimpase commented Dec 1, 2021

Doesn't #149 need newer Cython?

@dimpase
Copy link
Member

dimpase commented Dec 1, 2021

If I recall correctly, one reason Pari gets in here is that its threads implementation is a bit funny, and if it's not called early things go bad, one gets some weird TLS-related segfaults. (But maybe it's my PTSD speaks up :-))

@dimpase
Copy link
Member

dimpase commented Dec 1, 2021

Anyhow, we just got a wheel builder in here, Pari or no Pari (the wheels we build do have Pari linked in).

@kliem
Copy link

kliem commented Dec 1, 2021

If I recall correctly, one reason Pari gets in here is that its threads implementation is a bit funny, and if it's not called early things go bad, one gets some weird TLS-related segfaults. (But maybe it's my PTSD speaks up :-))

#149 removes exactly this dependency. Pari headers provide PARI_SIGINT_block and PARI_SIGINT_pending. This is all we need. What we aim for is somehow getting this stuff on runtime, if available.

I don't know if this is even possible and less so, if it is possible to integrate such a mechanism in cysignals. I do have a workaround by just passing pointers on runtime. #149 can get those pointers, provided that cypari2 has sagemath/cypari2#109 merged.

@dimpase
Copy link
Member

dimpase commented Dec 1, 2021 via email

@mkoeppe mkoeppe added this to the 1.12.0 milestone Oct 4, 2023
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 a pull request may close this issue.

4 participants