Skip to content

Upgrade GAP, libsemigroups#41700

Open
trevorkarn wants to merge 7 commits intosagemath:developfrom
trevorkarn:bumpgap-4.15.1
Open

Upgrade GAP, libsemigroups#41700
trevorkarn wants to merge 7 commits intosagemath:developfrom
trevorkarn:bumpgap-4.15.1

Conversation

@trevorkarn
Copy link
Contributor

@trevorkarn trevorkarn commented Feb 26, 2026

Upgrade GAP to 4.15.1 and libsemigroups to 3.5.1.

Fixes #41653 and implicitly #40186

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • 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

@github-actions
Copy link

github-actions bot commented Feb 26, 2026

Documentation preview for this PR (built with commit c92ca31; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@antonio-rojas
Copy link
Contributor

libsemigroups can't be updated to v3 because the GAP semigroups package only supports v2. There is also a new version 5.5.4 of semigroups, it makes sense to update it too.

@dimpase
Copy link
Member

dimpase commented Feb 26, 2026

[gap_packages-4.15.1] [......................................................................]
[gap_packages-4.15.1] Setting up build directory /Users/dima/software/sage/local/var/tmp/sage/build/gap_packages-4.15.1
[gap_packages-4.15.1] Applying patches from ../patches...
[gap_packages-4.15.1] Applying ../patches/digraphs-planarity-4.x.patch
[gap_packages-4.15.1] patching file 'pkg/digraphs/src/planar.c'
[gap_packages-4.15.1] 1 out of 1 hunks failed--saving rejects to 'pkg/digraphs/src/planar.c.rej'
[gap_packages-4.15.1] Error applying '../patches/digraphs-planarity-4.x.patch'
[gap_packages-4.15.1] ************************************************************************
[gap_packages-4.15.1] Error applying patches
[gap_packages-4.15.1] ************************************************************************
[gap_packages-4.15.1] Please email sage-devel (http://groups.google.com/group/sage-devel)
[gap_packages-4.15.1] explaining the problem and including the log files
[gap_packages-4.15.1]   /Users/dima/software/sage/logs/pkgs/gap_packages-4.15.1.log

more patches to take care of

@trevorkarn
Copy link
Contributor Author

[gap_packages-4.15.1] [......................................................................]
[gap_packages-4.15.1] Setting up build directory /Users/dima/software/sage/local/var/tmp/sage/build/gap_packages-4.15.1
[gap_packages-4.15.1] Applying patches from ../patches...
[gap_packages-4.15.1] Applying ../patches/digraphs-planarity-4.x.patch
[gap_packages-4.15.1] patching file 'pkg/digraphs/src/planar.c'
[gap_packages-4.15.1] 1 out of 1 hunks failed--saving rejects to 'pkg/digraphs/src/planar.c.rej'
[gap_packages-4.15.1] Error applying '../patches/digraphs-planarity-4.x.patch'
[gap_packages-4.15.1] ************************************************************************
[gap_packages-4.15.1] Error applying patches
[gap_packages-4.15.1] ************************************************************************
[gap_packages-4.15.1] Please email sage-devel (http://groups.google.com/group/sage-devel)
[gap_packages-4.15.1] explaining the problem and including the log files
[gap_packages-4.15.1]   /Users/dima/software/sage/logs/pkgs/gap_packages-4.15.1.log

more patches to take care of

Thanks - is this your output of make? I didn't see this error when building gap_packages on my system, should I expect something different with a different OS?

@dimpase
Copy link
Member

dimpase commented Feb 26, 2026

Thanks - is this your output of make?
yes
I didn't see this error when building gap_packages on my system, should I expect something different with a different OS?

This patch comes from #40153

I have hard time imagining it would fail on macOS, but somehow magically work on Linux.

You either have to enable building gap_packages via ./configure, or
explicitly run make gap_packages

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure make gap_packages works, and then run tests

@trevorkarn
Copy link
Contributor Author

Thanks - is this your output of make?
yes
I didn't see this error when building gap_packages on my system, should I expect something different with a different OS?

This patch comes from #40153

I have hard time imagining it would fail on macOS, but somehow magically work on Linux.

You either have to enable building gap_packages via ./configure, or explicitly run make gap_packages

Indeed. I can reproduce your patch issue when I explicitly run make gap_packages.

@trevorkarn
Copy link
Contributor Author

All tests pass locally when I run sage -tp 10 -a --long. Does this include the gap_packages as well, or do I have to manually test them somehow?

@antonio-rojas
Copy link
Contributor

As mentioned above, you need to revert the libsemigroups upgrade, the semigroups package shipped by sage does not support v3.

@trevorkarn
Copy link
Contributor Author

As mentioned above, you need to revert the libsemigroups upgrade, the semigroups package shipped by sage does not support v3.

Thanks for the reminder. Do you have a case in mind of where this causes a failure? I ask because it feels slightly misleading to have all of the tests pass when the semigroups package doesn't support v3. So I'd like to add a test that fails on v3 and passes with any compatible version.

@antonio-rojas
Copy link
Contributor

As mentioned above, you need to revert the libsemigroups upgrade, the semigroups package shipped by sage does not support v3.

Thanks for the reminder. Do you have a case in mind of where this causes a failure? I ask because it feels slightly misleading to have all of the tests pass when the semigroups package doesn't support v3. So I'd like to add a test that fails on v3 and passes with any compatible version.

Just try to install the semigroups package, it will not even compile.

@dimpase
Copy link
Member

dimpase commented Feb 28, 2026

look for gap_packages in the output of ./configure -h and act accordingly

@trevorkarn
Copy link
Contributor Author

As mentioned above, you need to revert the libsemigroups upgrade, the semigroups package shipped by sage does not support v3.

Thanks for the reminder. Do you have a case in mind of where this causes a failure? I ask because it feels slightly misleading to have all of the tests pass when the semigroups package doesn't support v3. So I'd like to add a test that fails on v3 and passes with any compatible version.

Just try to install the semigroups package, it will not even compile.

Indeed it does not even compile. Is this something that we should update on a separate PR, or is it fine to leave it as v2 indefinitely?

@trevorkarn
Copy link
Contributor Author

look for gap_packages in the output of ./configure -h and act accordingly

I see that this output tells me how to install gap_packages (by running ./configure --enable-gap_packages). I'm not seeing anything about how to run tests in the output of ./configure -h. Am I missing something?

@dimpase
Copy link
Member

dimpase commented Feb 28, 2026

if gap_packages is installed then it will be tested

@dimpase
Copy link
Member

dimpase commented Feb 28, 2026

As mentioned above, you need to revert the libsemigroups upgrade, the semigroups package shipped by sage does not support v3.

Thanks for the reminder. Do you have a case in mind of where this causes a failure? I ask because it feels slightly misleading to have all of the tests pass when the semigroups package doesn't support v3. So I'd like to add a test that fails on v3 and passes with any compatible version.

Just try to install the semigroups package, it will not even compile.

Indeed it does not even compile. Is this something that we should update on a separate PR, or is it fine to leave it as v2 indefinitely?

leave it as the highest version supported by Semigroups package as packaged by GAP

@trevorkarn
Copy link
Contributor Author

Ok I think most recent push addresses all comments

@cxzhong cxzhong requested a review from dimpase March 2, 2026 07:27
@antonio-rojas
Copy link
Contributor

semigroups 5.6.0 with support for libsemigroups v3 was just released, so those can now be upgraded too.

@dimpase
Copy link
Member

dimpase commented Mar 6, 2026

semigroups 5.6.0 with support for libsemigroups v3 was just released, so those can now be upgraded too.

This would need splitting Semigroups into its own spkg?

@antonio-rojas
Copy link
Contributor

semigroups 5.6.0 with support for libsemigroups v3 was just released, so those can now be upgraded too.

This would need splitting Semigroups into its own spkg?

It is its own spkg already

@antonio-rojas
Copy link
Contributor

Note that there is a bug in the semigroups tarball that breaks building with external libsemigroups. A workaround is echo "3.5.1" > .LIBSEMIGROUPS_VERSION

See semigroups/Semigroups#1143 (comment)

@trevorkarn
Copy link
Contributor Author

Note that there is a bug in the semigroups tarball that breaks building with external libsemigroups. A workaround is echo "3.5.1" > .LIBSEMIGROUPS_VERSION

See semigroups/Semigroups#1143 (comment)

In which directory should I do this? I tried in the build directory SAGE_ROOT/local/var/tmp/sage/build/semigroups-5.6.0 and in SAGE_ROOT/local/var/tmp/sage/build/semigroups-5.6.0/src, and reran with KEEP_BUILT_SPKGS=yes and neither of those work. Also how should we get this into Sage? Should this go in the patches directory? Or should we wait until it is fixed upstream?

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Mar 9, 2026

In which directory should I do this? I tried in the build directory SAGE_ROOT/local/var/tmp/sage/build/semigroups-5.6.0 and in SAGE_ROOT/local/var/tmp/sage/build/semigroups-5.6.0/src, and reran with KEEP_BUILT_SPKGS=yes and neither of those work. Also how should we get this into Sage? Should this go in the patches directory? Or should we wait until it is fixed upstream?

In spkg-install.in, right before the sdh_configure call. You'll need to run autogen.sh too.

@dimpase
Copy link
Member

dimpase commented Mar 9, 2026

hmm, no, I think this should be a patch?
in SAGEROOT/build/pkgs/semigroup/patches/

un-tar the tarball somewhere,
run "git init", "git commit -m1", create the file, "git add" it, hen put the output of "git diff" in the sudirectory patches/, with suffix ".patch"

@dimpase
Copy link
Member

dimpase commented Mar 9, 2026

or is it an unconfigured tarball?

@trevorkarn
Copy link
Contributor Author

or is it an unconfigured tarball?

I'm not sure, but it seems to work with the patch approach. Does that answer the question?

@dimpase
Copy link
Member

dimpase commented Mar 10, 2026

or is it an unconfigured tarball?

I'm not sure, but it seems to work with the patch approach. Does that answer the question?

Yep, all good then.

@trevorkarn
Copy link
Contributor Author

or is it an unconfigured tarball?

I'm not sure, but it seems to work with the patch approach. Does that answer the question?

Yep, all good then.

Ok great - is there anything else I should do for this pr?

@trevorkarn trevorkarn self-assigned this Mar 10, 2026
@dimpase
Copy link
Member

dimpase commented Mar 10, 2026

do the tests pass with gap_packages spkg correctly installed?

@trevorkarn
Copy link
Contributor Author

do the tests pass with gap_packages spkg correctly installed?

I believe so. Here is the test log to make sure I am interpreting it correctly.
sagetest.log

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

update GAP to version 4.15.1

4 participants