#29369 closed enhancement (fixed)
spkgconfigure.m4 for brial
Reported by:  mjo  Owned by:  mjo 

Priority:  major  Milestone:  sage9.1 
Component:  build: configure  Keywords:  
Cc:  embray, fbissey, mkoeppe, dimpase  Merged in:  
Authors:  Michael Orlitzky  Reviewers:  Matthias Koeppe, Dima Pasechnik 
Report Upstream:  N/A  Work issues:  
Branch:  40a59a4 (Commits, GitHub, GitLab)  Commit:  
Dependencies:  Stopgaps: 
Description
Nothing unusual here except that this is a C++ library with no pkgconfig file, and that makes it a bit harder to search for. I've assumed that any version we find is acceptable, which probably is not quite true.
Change History (42)
comment:1 Changed 2 years ago by
 Branch set to u/mjo/ticket/29369
 Cc embray fbissey mkoeppe dimpase added
 Commit set to b262c17edb2ee783141662503a24bf641d8a8abf
 Status changed from new to needs_review
comment:2 followups: ↓ 5 ↓ 42 Changed 2 years ago by
This is a matter for another ticket but I think the python part should be merged into sage proper. A bit of history. PolyBoRi? use to have its own python bindings. sage also had its own python bindings to PolyBoRi?. The python part of PolyBoRI (now BRiAl) could work with either the PolyBoRI or sage bindings. In BRiAl we never bothered with porting the python bindings of PolyBoRi?  they are not there anymore. I removed all bits that could have used the PolyBoRi? bindings from what is now sagebrial in Gentoo. So at the moment
 Brial is needed to build sage because sage builds binding to brial
 sagebrial needs sage's binding at runtime to be usable
 sage needs sagebrial at run time for some stuff
I think sagebrial should just be merged in sage.
comment:3 Changed 2 years ago by
build/pkgs/brial/distros/
needs filling
comment:4 Changed 2 years ago by
At the moment we cannot use python packages in the system python  such as presumably the one of brial. See #29023
comment:5 in reply to: ↑ 2 Changed 2 years ago by
 Status changed from needs_review to needs_work
Replying to fbissey:
This is a matter for another ticket but I think the python part should be merged into sage proper.
Ah, I didn't realize that sage's brial is installing TWO things, both the C++ library and a python package. I've only detected the C++ library, so this will have to wait until something happens with python  either adding support for system python modules, or just moving sagebrial into sage in this case.
(I'm still running a ptestlong on this branch but presumably it will fail.)
comment:6 followup: ↓ 8 Changed 2 years ago by
a less invasive option that moving sagebrial into sage would be to split the package into two parts, python and nonpython (they may use the same tarball). Then the work done here is perfectly useful for brial proper, and sagebrial becomes another story.
comment:7 Changed 2 years ago by
 Commit changed from b262c17edb2ee783141662503a24bf641d8a8abf to 4c3c18c666929aacd4417babb45a7c8a5df04967
comment:8 in reply to: ↑ 6 Changed 2 years ago by
 Status changed from needs_work to needs_review
Replying to dimpase:
a less invasive option that moving sagebrial into sage would be to split the package into two parts, python and nonpython (they may use the same tarball). Then the work done here is perfectly useful for brial proper, and sagebrial becomes another story.
Yeah, for now this is the path of least resistance (although the whole thing can probably be copy/pasted into src/sage/libs/brial). I factored the python module out into a new standard sage_brial package and now everything looks OK.
comment:10 Changed 2 years ago by
You can throw an upgrade on top of my commits if you want, I just didn't feel like adding more variables to this ticket (especially since my goal is to never again use the SPKG in the first place).
comment:11 Changed 2 years ago by
 Branch changed from u/mjo/ticket/29369 to u/mkoeppe/ticket/29369
comment:12 Changed 2 years ago by
 Commit changed from 4c3c18c666929aacd4417babb45a7c8a5df04967 to c4f45b4b3cfc04e722d3c12c71fb639ece81c4c2
Branch pushed to git repo; I updated commit sha1. New commits:
c4f45b4  fix for debian

comment:13 Changed 2 years ago by
comment:14 Changed 2 years ago by
 Reviewers set to Matthias Koeppe
comment:15 Changed 2 years ago by
 Commit changed from c4f45b4b3cfc04e722d3c12c71fb639ece81c4c2 to 94b22ed480852ca34a2a02edade8d00d160a9f00
Branch pushed to git repo; I updated commit sha1. New commits:
94b22ed  build/pkgs/{brial,sage_brial}: Add upstream_url

comment:16 Changed 2 years ago by
comment:17 Changed 2 years ago by
OK, looks good. I'll wait for MacOS tests to finish.
comment:18 Changed 2 years ago by
better macOS tests scheduled here: https://github.com/mkoeppe/sage/actions/runs/64318155 (hopefully I fixed the broken parallelization of the build)
comment:19 Changed 2 years ago by
brial's build or sage's one?
comment:20 Changed 2 years ago by
I gather it's Sage's builds using brial from Homebrew
comment:21 followup: ↓ 24 Changed 2 years ago by
system brial needs m4ri and boost, yet
20200327T06:01:37.8676320Z Checking whether SageMath should install SPKG brial... 20200327T06:01:37.8715050Z checking whether any of boost m4ri is installed as or will be installed as SPKG... yes; install brial as well 20200327T06:01:37.8715730Z configure: no suitable system package found for SPKG brial
and then the build dies on pynac (not surprisingly)
The problem is in m4ri
20200327T06:01:37.8106410Z Checking whether SageMath should install SPKG m4ri... 20200327T06:01:37.8144280Z checking whether any of libpng is installed as or will be installed as SPKG... no 20200327T06:01:37.8422490Z checking for m4ri >= 20140914... no 20200327T06:01:37.8673980Z configure: no suitable system package found for SPKG m4ri
there is a nonstandard location formula/installation availabe:
https://github.com/brewsci/homebrewscience/blob/master/Formula/m4ri.rb
comment:22 Changed 2 years ago by
 Reviewers changed from Matthias Koeppe to Matthias Koeppe, Dima Pasechnik
 Status changed from needs_review to positive_review
let's merge this, and fix for MacOS/Cygwin, if/as needed.
comment:23 followup: ↓ 25 Changed 2 years ago by
 Status changed from positive_review to needs_work
Because it depends on m4ri and we only accept very new versions of that, this only uses brial on very few system configurations.
ubuntueoanpython2
(https://github.com/mkoeppe/sage/runs/537774470), ubuntufocal
(https://github.com/mkoeppe/sage/runs/537774488) and archlinuxlateststandard
(https://github.com/mkoeppe/sage/runs/537774841) work well.
The test failures on debianbullseyestandard
(https://github.com/mkoeppe/sage/runs/537774574) are probably unrelated.
There's no brial package for homebrew, so there's nothing to test.
But it does not find brial on fedora31standard
(https://github.com/mkoeppe/sage/runs/537774738). Please take a look
comment:24 in reply to: ↑ 21 Changed 2 years ago by
Replying to dimpase:
there is a nonstandard location formula/installation availabe:
https://github.com/brewsci/homebrewscience/blob/master/Formula/m4ri.rb
https://github.com/brewsci/homebrewscience is unmaintained. Let's not use it.
comment:25 in reply to: ↑ 23 Changed 2 years ago by
Replying to mkoeppe:
But it does not find brial on
fedora31standard
(https://github.com/mkoeppe/sage/runs/537774738). Please take a look
Is brialdevel installed there?
comment:26 Changed 2 years ago by
 Commit changed from 94b22ed480852ca34a2a02edade8d00d160a9f00 to 4ad36bdea8a5a81df6324d16556984a4119d43ee
Branch pushed to git repo; I updated commit sha1. New commits:
4ad36bd  build/pkgs/brial/distros/fedora.txt: Add brialdevel

comment:27 Changed 2 years ago by
Another round of tests at https://github.com/mkoeppe/sage/actions/runs/64668026
comment:28 followup: ↓ 29 Changed 2 years ago by
fedora28standard (https://github.com/mkoeppe/sage/runs/539636819):
[sagelib9.1.beta8] build/cythonized/sage/rings/polynomial/pbori.cpp:60026:55: error: use of deleted function ‘polybori::BooleConstant& polybori::BooleConstant::operator=(polybori::BooleConstant&&)’ [sagelib9.1.beta8] __pyx_v_self>_pbconst = BooleConstant(__pyx_v_value); [sagelib9.1.beta8] ^ [sagelib9.1.beta8] In file included from /usr/include/polybori/BoolePolynomial.h:42, [sagelib9.1.beta8] from /sage/src/sage/libs/polybori/pb_wrap.h:1, [sagelib9.1.beta8] from build/cythonized/sage/rings/polynomial/pbori.cpp:684: [sagelib9.1.beta8] /usr/include/polybori/BooleConstant.h:40:7: note: ‘polybori::BooleConstant& polybori::BooleConstant::operator=(polybori::BooleConstant&&)’ is implicitly deleted because the default definition would be illformed: [sagelib9.1.beta8] class BooleConstant: [sagelib9.1.beta8] ^~~~~~~~~~~~~ [sagelib9.1.beta8] /usr/include/polybori/BooleConstant.h:40:7: error: nonstatic const member ‘const bool polybori::BooleConstant::m_value’, can’t use default assignment operator [sagelib9.1.beta8] error: command 'gcc' failed with exit status 1
comment:29 in reply to: ↑ 28 Changed 2 years ago by
Replying to mkoeppe:
fedora28standard (https://github.com/mkoeppe/sage/runs/539636819):
[sagelib9.1.beta8] build/cythonized/sage/rings/polynomial/pbori.cpp:60026:55: error: use of deleted function ‘polybori::BooleConstant& polybori::BooleConstant::operator=(polybori::BooleConstant&&)’ ... [sagelib9.1.beta8] error: command 'gcc' failed with exit status 1
See, this is why I became a mathematician...
comment:30 Changed 2 years ago by
Right, for the endless fun with other mathematicians' software...
comment:31 Changed 2 years ago by
 Branch changed from u/mkoeppe/ticket/29369 to u/mjo/ticket/29369
 Commit changed from 4ad36bdea8a5a81df6324d16556984a4119d43ee to 40a59a410b7be086264f09cbefe9c03cb94e5c77
 Status changed from needs_work to needs_review
This one rejects brial0.8.5.
New commits:
40a59a4  Trac #29369: detect only newish versions of brial.

comment:32 followup: ↓ 33 Changed 2 years ago by
And I am physicist, which would make me the practical one  except I do more theoretical physics, so some people would say I am a mathematician with the wrong clothes. So, would something upstream make your job easier? Like a .pc file :)
I am upstream's maintainer after all. Pull requests welcome.
comment:33 in reply to: ↑ 32 Changed 2 years ago by
Replying to fbissey:
And I am physicist, which would make me the practical one  except I do more theoretical physics, so some people would say I am a mathematician with the wrong clothes. So, would something upstream make your job easier? Like a .pc file :)
I am upstream's maintainer after all. Pull requests welcome.
A .pc
file will be invaluable the next time around, but here we need to detect the versions that have already shipped. I probably should have asked for help writing the test program, but it didn't look that hard until it was too late =)
comment:34 Changed 2 years ago by
comment:35 Changed 2 years ago by
comment:36 Changed 2 years ago by
 Status changed from needs_review to positive_review
comment:37 Changed 2 years ago by
 Branch changed from u/mjo/ticket/29369 to 40a59a410b7be086264f09cbefe9c03cb94e5c77
 Resolution set to fixed
 Status changed from positive_review to closed
comment:38 Changed 2 years ago by
 Commit 40a59a410b7be086264f09cbefe9c03cb94e5c77 deleted
Followup: I see crashes in running doctests on fedora30standard
(https://github.com/mkoeppe/sage/runs/572856797), which uses system brial 1.2.51
sage t src/sage/rings/polynomial/multi_polynomial_sequence.py # Killed due to abort sage t src/sage/rings/polynomial/pbori.pyx # Killed due to abort sage t src/sage/rings/polynomial/polynomial_ring_constructor.py # Killed due to abort sage t src/sage/sat/boolean_polynomials.py # Killed due to abort sage t src/sage/sat/solvers/dimacs.py # Killed due to abort
comment:39 Changed 2 years ago by
This is now #29490
comment:40 Changed 2 years ago by
 Owner changed from (none) to mjo
comment:41 Changed 2 years ago by
In src/module_list.py there is
Extension('sage.rings.polynomial.pbori', ... depends = [SAGE_INC + "/polybori/" + hd + ".h" for hd in ["polybori", "config"]], extra_compile_args = m4ri_extra_compile_args),
which causes constant rebuilds with system package, as these headers are no longer in SAGE_INC
. It's a minor annoyance, but still.
comment:42 in reply to: ↑ 2 Changed 22 months ago by
Replying to fbissey:
This is a matter for another ticket but I think the python part should be merged into sage proper. [....] So at the moment
 Brial is needed to build sage because sage builds binding to brial
 sagebrial needs sage's binding at runtime to be usable
 sage needs sagebrial at run time for some stuff
I think sagebrial should just be merged in sage.
This is now #30332
I left off the libpng dependency, because I think brial needs it if and only if mr4i does. I also left off the python dependency, but maybe I shouldn't have?
New commits:
Trac #29369: clarify message from SAGE_SPKG_DEPCHECK.
Trac #29369: new spkgconfigure.m4 for brial.