-
-
Notifications
You must be signed in to change notification settings - Fork 654
Sirocco update to a pkg-config enabled version #40555
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
base: develop
Are you sure you want to change the base?
Conversation
@tobiasdiez - not sure, is the following the good fix for the meson build: --- a/src/sage/libs/meson.build
+++ b/src/sage/libs/meson.build
@@ -1,4 +1,3 @@
-sirocco = cc.find_library('sirocco', required: false, disabler: true)
# cannot be found via pkg-config
ecl = cc.find_library('ecl', required: false, disabler: true)
if not ecl.found() and not is_windows
@@ -65,6 +64,12 @@ else
conf_data.set('SAGE_MAXIMA', maxima_bin.full_path())
endif
+sirocco = dependency(
+ 'libsirocco',
+ version: '>=2.1.0',
+ required: false,
+ disabler: true,
+)
braiding = dependency(
'libbraiding',
version: '>=1.3.1', |
sirocco tarball is an unreleased version I made from my Sirocco PR miguelmarco/SIROCCO2#7 |
Documentation preview for this PR (built with commit a6827d0; changes) is ready! 🎉 |
Is the .pc file needed for the meson build? |
yes, and I checked that with the last commit meson build of sirocco interface works |
the meson build works just fine with the current release of sirocco (without pkgconfig). Please don't make it depend on an unofficial fork. |
it's not an unofficial fork, it's just a temporary fix, until @miguelmarco merges my PR |
Still, until it is merged and released this will break distros, including conda (you can see in the conda testers that sirocco is not detected). Please revert the meson bits, there is nothing broken that needs fixing there. |
It didn't work for me without pkg-config for libsirocco in SAGE_LOCAL. |
You can first try via pkg-config and if that's not working fallback to |
Might want to rebase over #40485 as well since it touches this line. |
OK, I've reverted the commit touching |
I uninstalled both
The same error without the patch. After
as it should be. I think the key is |
if I do the latter, I get a non-working Sage, for some reason. what you see is that enable-sirocco only builds the sirocco library, and the current build needs enable-sagemath_sirocco option to build the interface to sirocco. in meson build it is automatically detected whether libsirocco is available, and the interface is built (but it is getting an option to control whether it's actually built) |
Is there a way to build the interface to sirocco with --enable-sirocco? |
start |
I am trying, but I would like to know if it is possible when building in the classical way. |
@antonio-rojas - this is now based on the official new release. |
I can't test myself this month, but conda CI looks fine, so it should be ok on Arch too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but perhaps someone actually using that stuff is a better reviewer ;-)
I am trying to build sage with meson for the first time to see if these changes work for me. I had some issues, see this message. If I get it I have some doubts:
|
Please, this PR is a trivial update (apart from spkg-configure.m4, which you can only test on a system which has system-wide sirocco, or sirocco installed by hand in /usr/local or so) |
also fix a typo in(will be done elsewhere - in #40568)spkg-configure.m4
ofroman_numerals_py
, and fixspkg-configure.m4
offlint
to allow versions 3.3.*📝 Checklist
⌛ Dependencies