-
Notifications
You must be signed in to change notification settings - Fork 26
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
=sci-mathematics/sagemath-standard-10.3 fails as: AttributeError: Can't pickle local object '_prepare_extension_detection.<locals>.<lambda>'
#783
Comments
Tl;DR: If I uninstall it with More words: Looks like def _prepare_extension_detection(dist: Distribution) -> None:
# Setuptools needs to know that it has extensions modules
orig_has_ext_modules = dist.has_ext_modules
dist.has_ext_modules = lambda: orig_has_ext_modules() or _has_cmake(dist) # type: ignore[method-assign]
# ... which fails to pickle/unpickle across process boundaries somewhere in the build system. It does not look like |
Hum... This is something I may take upstream if I can reproduce it with vanilla sage. This is quite the intrusive behavior. |
|
Just got bitten. It is pulled via pandas[full-support] which pulls blosc which in turns wants scikit-build :P I am expecting other packages will get impacted at some point. sage itself is supposed to move to meson as a build system in the not so distant future, that will help. |
As well here
when building 10.4.beta8. |
This happened after switching system python to |
That's surprising. What exactly pulls scikit-core? For me, ultimately it was coming from pandas. |
Not sure. I do not have
Will investigate tomorrow. |
|
|
Masking and removing |
@minrk - it appears that using scikit in pyzmq build process creates problems for building SageMath Can this be overcome with some kind of monkey-patching, or perhaps a fix at pyzmq? |
Presumably the patch or fix belongs in either scikit-build-core or whatever in your build system is causing the build stages to be pickled, which doesn't seem like a typical situation or a safe assumption. That wouldn't be pyzmq, which I don't believe is in control when whatever is pickling things, especially since this will affect any packages using scikit-build-core, not just one. Unless I'm misreading the situation. |
There are triggers that may be unusual. scikit-build-core interferes with sage building because sage happens to use multiprocessing as part of the process. I am pretty sure things would not fail if it did not. |
What happens if you use a saner way to build Sage, e.g. sagemath/sage#36524 ? |
I am seriously considering it, but this is a considerable PR and it is not that easy to just drop it in. |
I must say that PR also has the issue of putting essential build system stuff in what is traditionally |
Why isn't this bit https://github.com/sagemath/sage/blob/e5f42fac70317c33655a3d3f882d732fdb635354/src/sage/misc/cython.py#L397 and after not work in this context? Does something like this need to be reproduced in sage_setup? Also, the thing that really hurt is the lambda, if you replace it with some |
I have very carefully looked at what |
pyzmq seems to build fine with the patched scikit-build-core, but more things should be tried. |
Amazing, this does seem to work. The original issue was with Sage (maybe python) being unable to pickle a lambda in a package (scikit-build-core) which was not used by Sage. So why do the changes in a not needed package prevent Sage from trying to pickle the same lambda? |
I'd submit it anyway - let them try it. Surely they have CI to test it better than us. |
I think it needs a little bit of tidying up. The style of the code is not matching the re-plumbing I have made. I think some renaming following the kind of convention setuptools-rust has used is in order, but that's trivial. The point of the matter is setuptools picks up on all extensions published in the entrypoint file in the egg-info directory. What scikit-build-core did was package their "cmake-extension" definition in the wrong entrypoint. It mostly works because of the internal implementation, but what I did is just move in a more appropriate entrypoint, which is not interfering with the building. And when you follow the setuptools-rust model, you end up simplify the code because you do not need to encapsulate everything in an object. |
Submitted to scikit-build-core. |
This is a sort of follow up to #413 and #414. In sage-on-gentoo we discovered that the presence of scikit-build-core breaks the building sagemath (cschwan/sage-on-gentoo#783). scikit-build-core also happens to be an indirect dependency for building sagemath's documentation. On the basis that the presence of setuptolls_rust does not break sagemath's build, I studied the difference with scikit-build-core. I noticed that the elements `bundled in the single "finalize_distribution_options" entrypoint in scikit is split between two different setuptools entrypoints. This PR reproduce that setup and does a bit of clean up. The bundling of the old _prepare_extension_detection and _prepare_build_cmake_command into finalize_distribution_options being redundant after attributing them directly to separate entrypoints. I also renamed the functions to better characterise them. Using this PR, sagemath builds without problems and pyzmq, which uses scikit-build-core also builds properly. --------- Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Henry Schreiner <HenrySchreinerIII@gmail.com>
seems they liked your patch, and merged it with changes. Good. |
And they cut a new release including the stuff which means we will be able to move beyond the patch once that version is in the tree. |
scikit-build-core-0.9.6 is in the tree and I have added to the list of keywords for sage-10.3 and sage-9999 so the issue should go away naturally now. |
Considering this issue fixed now. |
Tried to build
sagemath-standard
to reproduce a possible tool chain failure. Got the build failure as:# emerge -av1 sagemath-standard
# emerge --info
The text was updated successfully, but these errors were encountered: