Conversation
3c3bcf5 to
f84c84f
Compare
54c1583 to
21c079d
Compare
|
This should fix RPATH handling for good. The only thing not included is automatic translation of the There is one case in which this may break projects that work now: when libraries or modules require setting an RPATH to dynamically link to a library installed in the Python install path (with something like @rgommers I think I added test cases for all scenarios we discussed. It would be great if you could test with packages that may be affected and that do not pin the meson-python version to any released version. |
87b4cee to
1fa120b
Compare
macOS requires using `@loader_path` in place of `$ORIGIN`.
This shows that build RPATHs are not correctly stripped.
This does not introduce any functional changes, except removing duplicates entries from RPATH. Fixes mesonbuild#813.
There is no need to perform the check for every native file installed.
for packages using internal shared libraries relocated by meson-python. Fixes mesonbuild#711.
Revise tests to exercise support when executed with Meson > 1.6
Requires Meson 1.9.0.
|
I think it is about time that we straighten out our RPATH story, thus tentatively marking this for the next release. However, it still needs a decision regarding the handling of the anchors in |
|
@rgommers I want to add extended tests in CI that test building SciPy (and maybe some other projects later). I had a quick look at how to do it and I didn't find what the preferred way of building SciPy is these days. Should I use pixi, spin, or something else? Or is downloading the source distribution and installing with pip or uv the way to go? The latter seems the easiest but pip does not have a way to install the build dependencies for a package and disabling build isolation is by far the simplest to test a local version of meson-python. |
|
Hmm, are you sure - that's a really heavy build (10-15 min), and it's nontrivial to set up because of the BLAS dependency. Or are you thinking as a one-off or some sort of extended test suite? We do test And that doesn't test all RPATH rewriting issues. I think what we probably need for more coverage is a macOS SciPy job against main? Re building:
I did plan on getting back to this PR after the release. It'd be great to get this merged soon. |
|
I was thinking to add an extended test suite gated on all the existing CI jobs to complete successfully. However, I didn't expect SciPy to take 15 min to compile, it is a long time since I had to do it. Do you think it is worth having something like this in our CI? |
|
Does ccache not help here? I know that scipy takes a while because it has 1200 compile edges, with quite a bit of C++. On my laptop (znver5, -j16) it takes But on a second run, it completes significantly faster: |
|
(Needing to compile all of highs despite a system package existing for it, surely is not helping compile times. Compiling that package from source with |
I'm inclined to say no here. I think there's a lot of value in our CI being simple and very fast. We'd make the completion time way longer, and the setup quite a bit more maintenance-intensive. I'd much rather add more CI jobs in SciPy's CI that use meson-python's main branch on different platforms. I do like the idea of better integration testing though. I think we can (a) push more of our large users to test our
Yes it does, and we use it in SciPy - but that comes with its own extra work; two job steps per build, cache management, having to run CI jobs on |
I did a bit of searching, and it's recommended in our docs, and SciPy isn't the only project that used it.
If I can find three significant projects using the feature quickly, and we've recommended it in our docs for a while, there will be more users. We should add a backwards compat shim. That doesn't cost much; the Meson behavior of retaining a literal |
|
My review plan here is more a test plan - since I can't just read through the code and reason about changed test packages for completeness, I'll plan to add some new ones with more layouts, and run that in CI on my fork on Linux and macOS. And then add NumPy (same as SciPy, just builds faster), VapourSynth and dwave-optimization builds, and check that they all work without patching and don't contain duplicate RPATHs. |
|
This PR should also update the docs on shared libraries, so we recommend the new preferred form. Which I think is |
|
Translating This PR implements what IMO is correct handling of RPATH entries, including removal of build RPATH entries. This will break packages that have been relying (without realizing it, I assume) on build RPATH entries. The anchor translation will help the project that have (no-op till now) |
Sure, I understand that. However, it had the effect of working in practice - one bug cancelling out another made that so. Let's make it easy: I'll see if I can find a simple-enough way of adding in backwards compat handling here. If I don't succeed, there's nothing to discuss. If it does work, it's probably easy to decide to add it in, so we can hard affected people rather than hard-break them in the next release. |
If you mean adding the RPATH anchor translation, that should be easy. Knowing the code, I can do it quickly, I think. |
|
Yes, that's what I meant. I was actually planning to leave it alone right now, focus on testing, see something fail (the two projects in the list above), and then get the respective CI jobs green again with a bit of backwards compat handling. |
Builds on top of #783 and replaces #724
Fixes #725
Fixes #813