Skip to content
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

The GAP system #34472

Closed
wants to merge 104 commits into from
Closed

The GAP system #34472

wants to merge 104 commits into from

Conversation

orlitzky
Copy link
Contributor

Close bug 49282 ahead of its 20th anniversary by adding GAP, a new gap-pkg eclass, the dev-gap category, and a collection of useful GAP packages under dev-gap.

Based largely upon the work of @kiwifb in the sage-on-gentoo overlay.

@orlitzky orlitzky added the do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI. label Dec 25, 2023
@orlitzky
Copy link
Contributor Author

orlitzky commented Dec 25, 2023

There are two known issues with this PR:

  1. The dev-gap/browse package hangs the SageMath test suite
  2. dev-gap/polenta throws warnings when GAP starts, causing the SageMath test suite to fail: Startup warnings gap-packages/polenta#9

@orlitzky
Copy link
Contributor Author

@kiwifb please let me know what you think. I just sent the new category RFC to the mailing list, but will wait for feedback before posting the eclass. All GAP tests should pass now (once you work out the circular deps), and the sage test suite passes if you uninstall browse and polenta.

@kiwifb
Copy link
Contributor

kiwifb commented Dec 25, 2023

Well, so far I am very impressed. I never took the time to work out testing for example. The browse/polenta stuff is weird, I may have a look later but I am in serious holiday mode.

@orlitzky
Copy link
Contributor Author

I'm not expecting you to drop what you're doing and review this on Christmas. Enjoy yourself. Happy holidays :)

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2023-12-25 18:08 UTC
Newest commit scanned: f6f66a0
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/a1fdc06b5d/output.html

@orlitzky
Copy link
Contributor Author

The polenta issue has been fixed (thanks @fingolfin), leaving only the weird browse issue that I've been dreading.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-11 17:47 UTC
Newest commit scanned: 40194ed
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/cfb3d1675e/output.html

Copy link

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! A few minor remarks as inline comments.

I'd be happy to help with the Browse issue, but I don't know what it is exactly, and I don't use Sage, so I am not sure how to reproduce it

eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
@orlitzky
Copy link
Contributor Author

I'd be happy to help with the Browse issue, but I don't know what it is exactly, and I don't use Sage, so I am not sure how to reproduce it

I recently taught Sage to use the system copy of GAP and its packages, making this a lot easier to test.

If sage is using GAP without Browse installed, you can test for example the following file:

$ sage -t --long --timeout=0 src/sage/graphs/generators/distance_regular.pyx
Running doctests with ID 2024-01-12-09-31-09-832296b0.
...
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 249.7 seconds
    cpu time: 246.1 seconds
    cumulative wall time: 246.0 seconds

After installing Browse, however, it hangs indefinitely. (This may also require atlasrep). You can kill it with Ctrl-C after it gets stuck, and the traceback implicates Browse:

$ sage -t --long --timeout=0 src/sage/graphs/generators/distance_regular.pyx
...
<Ctrl-C>
usr/lib/python3.11/site-packages/cysignals/signals.cpython-311-x86_64-linux-gnu.so(+0x8930)[0x7fe047dd9930]
/usr/lib/python3.11/site-packages/cysignals/signals.cpython-311-x86_64-linux-gnu.so(+0x89ff)[0x7fe047dd99ff]
/usr/lib/python3.11/site-packages/cysignals/signals.cpython-311-x86_64-linux-gnu.so(+0xb9cb)[0x7fe047ddc9cb]
/lib64/libc.so.6(+0x37f40)[0x7fe048e8af40]
/usr/lib/python3.11/site-packages/cysignals/signals.cpython-311-x86_64-linux-gnu.so(+0xf2c0)[0x7fe047de02c0]
/lib64/libc.so.6(+0x37f40)[0x7fe048e8af40]
/lib64/libc.so.6(tcsetattr+0x75)[0x7fe048f49a35]
/lib64/libtinfow.so.6(_nc_set_tty_mode_sp+0x39)[0x7fdf78063659]
/lib64/libtinfow.so.6(cbreak_sp+0x93)[0x7fdf7805b633]
/lib64/libncurses.so.6(newterm_sp+0x26b)[0x7fe03da6fe7b]
/lib64/libncurses.so.6(newterm+0x3c)[0x7fe03da7012c]
/lib64/libncurses.so.6(initscr+0x70)[0x7fe03da6c470]
/usr/lib64/gap/pkg/browse/bin/amd64/ncurses.so(+0x88b7)[0x7fe03daa58b7]
...

I don't know much beyond that except that I also had to pipe the tests in our eclass through tee to keep Browse from clobbering my terminal while testing.

The same issue manifests on arch linux as well: sagemath/sage#31761 (comment)

@orlitzky
Copy link
Contributor Author

I don't know much beyond that except that I also had to pipe the tests in our eclass through tee to keep Browse from clobbering my terminal while testing.

This also hides the problem when running the sage test suite. The following works,

$ sage -t --long --timeout=0 --verbose src/sage/graphs/generators/distance_regular.pyx &> foo.txt

but redirecting only stdout or stderr individually was insufficient.

@kiwifb
Copy link
Contributor

kiwifb commented Jan 12, 2024

That issue with browse is very reminiscent of the problem with xgap years ago (2013 cschwan/sage-on-gentoo#189 and there should be more on sage-devel and possibly on trac), which could cause this kind of problems and more with sage. More problems because at the time a gap session would be started in the background when starting sage I think. At the time I was trying to install every single packages of the gap tarball and for some reason they would all load.

@orlitzky
Copy link
Contributor Author

That issue with browse is very reminiscent of the problem with xgap years ago (2013 cschwan/sage-on-gentoo#189 and there should be more on sage-devel and possibly on trac), which could cause this kind of problems and more with sage. More problems because at the time a gap session would be started in the background when starting sage I think. At the time I was trying to install every single packages of the gap tarball and for some reason they would all load.

Before the recent changes to support system GAP, a larger collection of packages was being loaded at startup, and any additional optional packages used by the default set are loaded as well. So you could wind up with quite a lot. Moreover, at some point, reset_gap_workspace() was made to load every installed package. I actually started working on a fix for that earlier today when I was messing around with Browse: sagemath/sage#37049

eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
eclass/gap-pkg.eclass Show resolved Hide resolved
eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
fi
}

# @FUNCTION: gap-pkg_enable_tests
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused. It looks as if you're supposed to call this if package has tests but the test phase is run independently of whether it was called. I suppose that would lead to people missing it and effectively ending up with missing deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a helper function for the boilerplate associated with a package that has a test suite and has runtime deps that aren't normally build-time deps. In that case, we want to add IUSE=test, add RESTRICT="!test? ( test )", and add all runtime deps as build-time deps when USE=test is set.

You could do that all yourself, of course, but it's handy to have a function that does it.

Is your issue with the phrase "enable tests?" If so, I just stole it from distutils_enable_tests and elisp-enable-tests, which serve similar purposes. (Better to be consistent than perfect.)

dev-gap/atlasrep/atlasrep-2.1.7.ebuild Show resolved Hide resolved
dev-gap/atlasrep/atlasrep-2.1.7.ebuild Outdated Show resolved Hide resolved
dev-gap/autodoc/autodoc-2022.10.20.ebuild Outdated Show resolved Hide resolved
dev-gap/browse/browse-1.8.21.ebuild Outdated Show resolved Hide resolved
dev-gap/ctbllib/ctbllib-1.3.6.ebuild Show resolved Hide resolved
dev-gap/digraphs/digraphs-1.6.3.ebuild Outdated Show resolved Hide resolved
@orlitzky
Copy link
Contributor Author

This just showed up today while building sci-mathematics/gap. Not sure if it was something I did, or something in ::gentoo. I'll have to figure it out before I can address the other comments:

libtool: Version mismatch error.  This is libtool 2.4.7, but the
libtool: definition of this LT_INIT comes from libtool 2.4.6.
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.
make: *** [Makefile.rules:423: build/obj/src/main.c.lo] Error 63
make: *** Waiting for unfinished jobs....
libtool: Version mismatch error.  This is libtool 2.4.7, but the
libtool: definition of this LT_INIT comes from libtool 2.4.6.
libtool: You should recreate aclocal.m4 with macros from libtool 2.4.7
libtool: and run autoconf again.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-14 13:27 UTC
Newest commit scanned: bf1f693
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/5805415db5/output.html

@fingolfin
Copy link

@orlitzky mayve helpful: gap-system/gap#5383

@orlitzky
Copy link
Contributor Author

@orlitzky mayve helpful: gap-system/gap#5383

That's certainly the issue, and we're used to getting ourselves into trouble when re-running autotools, but what I find baffling is that the issue just recently appeared. I've been using libtool-2.4.7 for over a year. My best guess is that I added the eautoreconf late in the process, around the same time I started testing corner cases like LIBTOOL=slibtool.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-14 14:58 UTC
Newest commit scanned: bde2d2a
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/070ec0cd50/output.html

GAP_PKG_HTML_DOCDIR="htm"

# The "exam" directory contains examples... but they're loaded by
# read.g, and actually used by dev-gap/polenta!

Choose a reason for hiding this comment

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

Suggested change
# read.g, and actually used by dev-gap/polenta!
# read.g, and actually used by dev-gap/alnuth!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alnuth's test suite actually passes without its own examples, but the polenta test suite doesn't (without alnuth's examples). So the comment is as intended... but if some part of alnuth is using its own examples then it would be less confusing to have it say alnuth and not polenta.

eclass/gap-pkg.eclass Outdated Show resolved Hide resolved
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-14 17:27 UTC
Newest commit scanned: c9b8ea1
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/a68fcf6352/output.html

@kiwifb
Copy link
Contributor

kiwifb commented Jan 14, 2024

@orlitzky mayve helpful: gap-system/gap#5383

That's certainly the issue, and we're used to getting ourselves into trouble when re-running autotools, but what I find baffling is that the issue just recently appeared. I've been using libtool-2.4.7 for over a year. My best guess is that I added the eautoreconf late in the process, around the same time I started testing corner cases like LIBTOOL=slibtool.

Trip down memory lane cschwan/sage-on-gentoo#690 took me a while to figure it out and I historically fixed it with cschwan/sage-on-gentoo@f764f7e

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-14 19:42 UTC
Newest commit scanned: 75805eb
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/6322ccdcc8/output.html

Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
…GORY}

Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Some day, this might pull in app-emacs/gap-mode, which does not yet
exist. I tried to package it but it throws several warnings in the
latest version of emacs:

  https://gitlab.com/gvol/gap-mode/-/issues/13

For now let's drop the flag since it doesn't do anything.

Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Without AC_CONFIG_MACRO_DIRS in configure.ac, running eautoconf clobbers
the important m4_include directives in aclocal.m4, leading to libtool
version errors in the build. A quick "sed" does the trick.

Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Since removing the --bindir and --libdir flags from the default econf,
this function is unused.

Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Max Horn <horn@mathematik.uni-kl.de>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Max Horn <horn@mathematik.uni-kl.de>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
The comment listing the file extensions to be installed as documentation
was outdated, as such things are doomed to be. It has been changed to
less precise and more accurate.

Suggested-by: Max Horn <horn@mathematik.uni-kl.de>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
The default value for this variable was essentially being set inside
gap-pkg_src_install, rather than at the top-level. We move it to the
top level so that the eclass man page correctly documents it.

Suggested-by: Michał Górny <mgorny@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
In case François and I get hit by the same bus, but mainly for symmetry:
sci-mathematics is also listed as a co-maintainer on the individual
dev-gap packages.

Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
This is where skel.ebuild puts it.

Suggested-by: Michael Mair-Keimberger <mmk@levelnine.at>
Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
@orlitzky
Copy link
Contributor Author

I left some comments on this PR - hope thats fine. Further more i also suggest to move S=... below SRC_URI to be inline with the skel.ebuild

Perfect, thanks. I did move S too. The force-push was needed to keep up with all the recent category moves.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-17 01:27 UTC
Newest commit scanned: 83390ce
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/7c1cf30623/output.html

@orlitzky
Copy link
Contributor Author

I'm going to re-test everything with clang-17 and when GAP has memcheck/valgrind enabled, but if all goes well, I plan to merge this tonight or tomorrow and sneak it in as a late entry to @akhuettel's 2023 round-up.

@orlitzky
Copy link
Contributor Author

Merged, thanks everyone.

@orlitzky orlitzky closed this Jan 22, 2024
@thesamesam
Copy link
Member

Massive thanks to everybody here - so wonderful to see this in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge Please DO NOT MERGE this PR. It will not be assigned but it will be scanned by CI.
Projects
None yet
8 participants