ENH: support shared libraries inside packages #257
Conversation
There was a problem hiding this comment.
Yes. This is exactly what we need. I think it is a non-breaking change. I did this change in #238 and it worked with NumPy as well.
May be we can accept ––destdir as an optional argument and pass it only when available. That way if someone needs it they can pass it via spin build ––destdir. In simple words, its just like what we did for ––prefix in spin.
I don't think anyone needs it - I'd avoid adding it unless there's a real-world use case. |
|
Good catch, thanks @rgommers! Happy to move to |
This drops the use of staged installs with `--destdir` in favor of using `--prefix` for the install location. The problem is that RPATHs in extension modules end up pointing to what Meson thinks is the final install location (i.e., inside the prefix) rather than to the staging directory. `--destdir` seems meant for packaging, as an actual staging area, while for `spin` the final install directory is `build-install` (by default), there is no intent to later put this package into `/usr` or `C:\`. Hence using `--prefix` seems like the correct thing to do. The one test change here is to a test that was incorrect. `meson setup --prefix` expects an absolute path, and `/foobar` isn't a path that exists or can be created. Addresses the issue discussed in PR 238 - the `spin build` behavior before this change cannot be made to work for SciPy, because the internal shared library in `scipy.special` keeps breaking. It should also address the problem discussed in issue spin#176.
rgommers
force-pushed
the
fix-sharedlibs
branch
3 times, most recently
from
dd3c945 to
8620c27
Compare
This tests using a shared library from a Python extension module, which is relevant for SciPy (scipy.special contains such a shared library).
rgommers
force-pushed
the
fix-sharedlibs
branch
from
8620c27 to
e1cee14
Compare
|
I can't quite tell what's up with the editable install failures on Windows. Will need to switch to a Windows machine to see what's up - could be something trivial, but if it's not then it's probably relevant for SciPy as well when we switch to |
|
Windows is very finicky; I have a test machine here for that purpose, and can help debug if that'd be helpful. |
Thanks. I'll try to have a look at it in the coming days, but if you have time to look at it that may be helpful. I haven't quite followed along with how editable install support in |
|
|
This drops the use of staged installs with
--destdirin favor of using--prefixfor the install location. The problem is that RPATHs in extension modules end up pointing to what Meson thinks is the final install location (i.e., inside the prefix) rather than to the staging directory.--destdirseems meant for packaging, as an actual staging area, while forspinthe final install directory isbuild-install(by default), there is no intent to later put this package into/usrorC:\. Hence using--prefixseems like the correct thing to do.The one test change here is to a test that was incorrect.
meson setup --prefixexpects an absolute path, and/foobarisn't a path that exists or can be created.Addresses the issue discussed in gh-238 - the
spin buildbehavior before this change cannot be made to work for SciPy, because the internal shared library inscipy.specialkeeps breaking. It should also address the problem discussed in issue gh-176.It may be my fault for suggesting the use of
--destdirearly on inspin's life - I think I understand the tradeoffs a bit better now.@stefanv question about testing - do you want me to extend
example_pkgwith a shared library? It will look likesharedlib-in-package/mypkg/sub/*in mesonbuild/meson-python#700. It's a little bit of extra complexity, but it should be pretty robust since it mirrors what SciPy has internally.