build/pkgs/gap: Update to 4.13.1, require >= 4.13
#38169
Conversation
|
Documentation preview for this PR (built with commit ebee948; changes) is ready! 🎉 |
|
The automatically run incremental builds do not work well because of the soname change. Building from scratch: https://github.com/mkoeppe/sage/actions/runs/9425219803, https://github.com/mkoeppe/sage/actions/runs/9425219809 |
|
The Linux tests are OK, but on the macOS platforms, we see |
|
I also see this in a local build on macOS. |
|
@mkoeppe Yes, it looks like a missing or incorrect rpath. Since you have a local build, you could do this: |
|
Thanks! I get |
|
Looks to me like the libgap.9.dylib has been installed without properly setting the install_name |
|
The error seems to be that So the problem is not that the rpath is missing, the load command itself is wrong. (This is in the python extension module, libgap.so, not in libgap.dylib). Adding an -rpath option won't help. You could repair the load path with macher or install_name_tool in a post-build script. But it would be better if you could get the linker to use the correct load command for libgap when building the extension module. |
|
Thanks! I agree |
|
I am not sure what you mean by the install name. I think you might mean the LC_ID_DYLIB load command. Running macher info on libgap.dylib in SageMath 10.3, I see: |
|
Yes, same thing, as per |
|
I think the rpath may be added when the linker sees a -L command and when it actually links with a library in that directory. Can you find the link command used for libgap.cpython-312-darwin.so. Also I don't understand the path src/sage/libs/gap/libgap.cpython-312-darwin.so -- why isn't that library in the venv? |
We are using an editable build |
|
Hmmm. The ld man page does suggest that if there were an install_name in libgap.so then it would have added the @rpath to the load command. You can use macher set_id to add an LC_ID_DYLIB load command. You could try that on libgap.so and rebuild the extension module to see if it fixes the problem. |
|
Yes, it's a fix that I've had to make several times in past, basically for every package that tries to build a shared library without using libtool... |
|
It works now. Thanks a lot for the discussion @culler |
|
Works for me with system GAP, modulo the already known segfaults in |
|
Thank you! |
<!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> - Based on sagemath#37884 by @tornaria - Fixes sagemath#37616 ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> - Depends on sagemath#38144 (merged here for testing) URL: sagemath#38169 Reported by: Matthias Köppe Reviewer(s):
In gap 4.13 there are some improvements, e.g. converting fp groups to permutation groups, computing abelianization of fp groups, which lead to different generators. This commit fixes doctests so they pass using gap 4.13.
|
Needs rebasing against 10.5b1. |
|
I still have a couple of doctests failure with gap 4.13.1 after applying this PR (once corrected for #38155). Or at least I would think they are related to gap 4.13.1 and I guess it could be down to the presence or absence or wrong version of particular gap packages, it would be nice to get to the bottom of it. |
|
I notice that #37884 was marking those two failures that I observe as random, but these changes have not been included in this PR. |
|
Merged the latest version of #37884 |
|
Thank you! |
|
I'm getting the same failures in src/sage/categories/simplicial_sets.py as the CI |
|
I took a look at the failures in |
|
OK, so I looked at the CI.
It looks like something is not installing the right gap or rebuilding correctly. |
The root of this issue is that sage: H.<e> = FreeGroup()
sage: G = H/[e^2]
sage: [g for g in G]used to return |
This works in a build not using this branch but develop with system gap in Fedora 40. On the other side I have tests with time outs. For |
|
The |
|
But it PASSES if I additionally install
and their dependencies. Time to binary search through the list to find out which one does the magic. |
|
Ok, it's the polenta package that makes the simplicial sets tests pass. Here are what I see as the remaining work items:
|
Follow-up to: * sagemath#37884 * sagemath#38169 With the four additional work items I mentioned in a comment on the latter: 1. Everything has been rebased 2. There's a new feature to detect the polenta GAP package 3. The three failing simplicial sets tests have been marked `# needs gap_package_polenta` 4. I backported gap-system/gap#5796 to the GAP spkg so that the optional GRAPE tests will pass (untested). Let's see what the CI has to say... URL: sagemath#38804 Reported by: Michael Orlitzky Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
Follow-up to: * sagemath#37884 * sagemath#38169 With the four additional work items I mentioned in a comment on the latter: 1. Everything has been rebased 2. There's a new feature to detect the polenta GAP package 3. The three failing simplicial sets tests have been marked `# needs gap_package_polenta` 4. I backported gap-system/gap#5796 to the GAP spkg so that the optional GRAPE tests will pass (untested). Let's see what the CI has to say... URL: sagemath#38804 Reported by: Michael Orlitzky Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
Follow-up to: * sagemath#37884 * sagemath#38169 With the four additional work items I mentioned in a comment on the latter: 1. Everything has been rebased 2. There's a new feature to detect the polenta GAP package 3. The three failing simplicial sets tests have been marked `# needs gap_package_polenta` 4. I backported gap-system/gap#5796 to the GAP spkg so that the optional GRAPE tests will pass (untested). Let's see what the CI has to say... URL: sagemath#38804 Reported by: Michael Orlitzky Reviewer(s): Dima Pasechnik, Enrique Manuel Artal Bartolo
|
Done as part of #38804 |
📝 Checklist
⌛ Dependencies