-
-
Notifications
You must be signed in to change notification settings - Fork 478
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
pkgconf to dependencies of libsemigroups #38870
base: develop
Are you sure you want to change the base?
Conversation
without it, libsemigroups will be useless for GAP's semigroups package
Documentation preview for this PR (built with commit 9785adf; changes) is ready! 🎉 |
I think pkgconf should be a (build only, after the pipe) dependency of gap_packages instead, right? The GAP semigroups package uses it to detect libsemigroups: https://github.com/semigroups/Semigroups/blob/main/m4/ax_check_libsemigroup.m4 libsemigroups, on the other hand, only uses pkgconf to detect things that sage disables anyway: https://github.com/search?q=repo%3Alibsemigroups%2Flibsemigroups%20PKG_CHECK_MODULES&type=code (I guess we could make this more explicit with |
I don't think Eigen was disabled in the previous version of the libsemigroups package. When I fixed the issue with TextFlow.hpp I got another error in the include statement in the following block of code from digraph.hpp:
I think that means that LIBSEMIGROUPS_EIGEN_ENABLED was defined in the previous version. |
The spkg-install for libsemigroups does,
so unless that's buggy somehow, it should be disabled (we don't have eigen in the sage distro). |
I was able to successfully build libsemigroups and gap_packages after making these two changes.
It should be possible to make the include statement in digraph.hpp work correctly by adding a -I option somewhere, but I don't know where to do that and this worked for me. I also don't know what it means when @orlitzky says "we don't have eigen in the sage distro". |
I meant that we don't have |
And could you please review this PR? |
For me, everything builds fine after |
I was. I think this commit changes the wrong package:
|
libsemigroups could not be used by its Sage consumer(s) without pkg-config. And (probably) it might fail tests. |
Only the consumers that rely on pkg-config for detection need pkg-config. Others would be fine without it. The packages that need pkg-config should depend on pkg-config...
You can quickpkg up your dev-util/pkgconf, emerge -C it, and try :)
This is handled by automake which doesn't know anything about pkg-config: https://github.com/libsemigroups/libsemigroups/blob/main/Makefile.am#L39 |
without it, libsemigroups will be useless for GAP's semigroups package
see sage-release for a discussion
📝 Checklist
⌛ Dependencies