Switch from PolyBoRi to BRiAl

[ohanar]

Right now PolyBoRi relies on scons (which is still Python 2 only), and has some Python 2 specific syntax in its bindings. These issues will need to be resolved before Sage can support Python 3.

On this ticket we update to the first version of BRiAl which includes a new (incomplete, but sufficient for our purposes) autotools based system.

To achieve this, we fork upstream polybori, see https://github.com/BRiAl/BRiAl

Tarball: https://github.com/BRiAl/BRiAl/releases/download/0.8.4.3/brial-0.8.4.3.tar.bz2

Depends on #19085

CC: @malb

Component: packages: standard

Author: R. Andrew Ohana

Branch/Commit: 7ff3b09

Reviewer: François Bissey

Issue created by migration from https://trac.sagemath.org/ticket/18437

[ohanar]

comment:1

Reposted from #15530:

fyi, I started working on an autotools based build system for polybori at https://github.com/ohanar/PolyBoRi/tree/autotools. I'll probably work on it a bit more next weekend during SD 64.25, but the important bits (the c++ libraries) are currently working with only a couple of hacks. What remains (for sage at least) is just a little cleanup and throwing in the bit of the python bindings we use.

---

I also don't know how we should handle this with regards to upstream, since upstream is dead. I certainly don't want to take over maintenance for polybori, but we'll probably need to make some sort of fork.

[kiwifb]

comment:2

I think there are two separate issues:

1. polybori is using a internal copy of a third party package that may not be dead upstream (I think it has updates once in a while): cudd.
2. the python bindings

We could package the libraries properly and then have a separate python binding package, unless polybori is altering its upstream significantly beyond the build system. The python binding could be moved in sage making it "a collective" responsibility.

[kiwifb]

comment:3

Current version of cudd in polybori is 2.5.0 but upstream at http://vlsi.colorado.edu/~fabio/ has a version 2.5.1 - it is somewhat alive.

[ohanar]

comment:4

Replying to @kiwifb:

I think there are two separate issues:

1. polybori is using a internal copy of a third party package that may not be dead upstream (I think it has updates once in a while): cudd.
2. the python bindings

We could package the libraries properly and then have a separate python binding package, unless polybori is altering its upstream significantly beyond the build system. The python binding could be moved in sage making it "a collective" responsibility.

Polybori has made some real modifications to cudd (I don't think that they are too significant though). From looking at the code it seems like they at least tried to support using an unmodified version, however I don't know if it actually works. Also, cudd's build system looks to be a joke (e.g. assumes sizeof(void *) == 4 unless you explicitly tell it otherwise), so I don't know how much I would trust it.

[kiwifb]

comment:5

Doesn't sound good. Autotooling something like that is definitely a good idea. But that puts the burden of maintenance on us. Mind you in Gentoo we have patches to autotool all the individual components of suitesparse independently of upstream, so it has been/is being done.

[ohanar]

comment:6

So I looked a bit more into how exactly polybori modifies cudd, and other than the build system changes, it appears to be just one of two things:

1. Prefixing the symbols to not collide with another installation of cudd
2. stripping out code that polybori doesn't use (there is only one case that polybori makes any meaningful changes to do this).

Given this, I don't think it would be hard to make polybori work with a vanilla version of cudd. Also, since upstream cudd is not completely dead, the author might be receptive to a better build system if we provide it.

[ohanar]

Author: R. Andrew Ohana

[ohanar]

comment:7

I contacted the author of Cudd on Tuesday and have yet to hear back from him. Given that, I think I would prefer not packaging Cudd separately, since maintaining one autotools based system (for polybori) is easier than two (for polybori and for cudd).

I've posted a branch, which uses the tarball at https://github.com/ohanar/PolyBoRi/releases/download/0.8.4/polybori-0.8.4.tar.gz. It doesn't include any of the python bindings, nor any of the tests, so beyond the build system, it is a bit of a fork of polybori. (Maybe we should rename it to something like cropped-polybori.)

---

New commits:

083fabf	cleanup settings in polybori in module_list.py
f5b2b78	move polybori python interface into sage
8ee9176	use new autotools based polybori

[ohanar]

Commit: 8ee9176

[ohanar]

Branch: u/ohanar/polybori

[kiwifb]

comment:8

I can easily test the new stuff in Gentoo. I can even test your stuff from git master if that helps.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b6aad41

use non-broken version of polybori

[sagetrac-git]

Changed commit from 8ee9176 to b6aad41

[ohanar]

comment:10

Yes, I that would be good (at the very least to make sure that I haven't somehow made things work with old artifacts lying around). Point in fact, I just realized that I pushed a little prematurely, as I forgot to include a few headers in the dist. The new tarball is https://github.com/ohanar/PolyBoRi/releases/download/0.8.5/polybori-0.8.5.tar.bz2.

I'm actually using the autotools branch, but at the moment the tip is the new release I just cut. Since this is new, I can squash the history at some point if desired.

[jdemeyer]

comment:12

Exactly what does this patch do? It looks like you're moving part of polybori into the Sage library, why that?

[jdemeyer]

comment:14

Extraordinary measures (forking polybori???) need extraordinary justification...

[kiwifb]

comment:15

Upstream being dead is ample justification. We had that conversation elsewhere and decided that it was the way to go.

[jdemeyer]

comment:16

Replying to @kiwifb:

We had that conversation elsewhere

Pointers please...

[kiwifb]

comment:17

We had some talk over at #15530 comment:32 but I think the confirmation that Alexander and Michael had completely abandoned ship were posted on a thread on sage-devel. I'll dig that up.

[kiwifb]

comment:18

From sage-devel "Python 3 focused Sage Days" thread:
Hi, here's a reply from a PolyBoRi developer:

---

Subject: Re: [Polybori-discuss] Fwd: Re: [sage-devel] Re: Python 3 focused
Sage Days
Date: Saturday 10 Jan 2015, 22:19:16
From: Alexander Dreyer adreyer@t-online.de
To: Martin Albrecht martinralbrecht@googlemail.com, Polybori Discuss
polybori-discuss@lists.sourceforge.net

Hi Martin,
Unfortunately, Andrew's right, PolyBoRi died when its developers left the
scientific community. I'm not sure, what's the best way for SAGE to deal with
it. Porting PolyBoRi's py files shouldn't be a big deal, and Sage already uses
it's own Cython-bindings for the data structures. What remains is scons.
Perhaps, it's easier to set up a setup.py or another build system than
removing PolyBoRi from all the crypto modules. (You probably know the
dependencies better than me.) For working around lots of scons issues, most of
the SConstruct file is plain python anyway.

Best regards,
Alexander

[jdemeyer]

comment:19

Even if upstream polybori is dead and even if you fork it, that still doesn't mean it should be moved to the Sage library.

[kiwifb]

comment:20

Fair point. The question then is "does anyone care to have to install sage to get polybori instead of having it as stand alone package?". If it has users as a stand alone package then it should be kept split. If not, that's at the discretion of whoever actually maintains the code.

[jdemeyer]

comment:21

Replying to @kiwifb:

Fair point. The question then is "does anyone care to have to install sage to get polybori instead of having it as stand alone package?".

I don't think that's "the question".

All the packaging effort in Sage is really converging towards separating packages from the Sage library proper.

At the very least, moving polybori in the Sage library is a sufficiently big change that it should be discussed on sage-devel.

[kiwifb]

comment:71

If that's the case, yes I do have an idea. But I'll look at the log more in depth first. It looks like missing sse2 instruction but the build is i486 so it shouldn't have them either.

[kiwifb]

comment:72

I think I know what to do, expect a pull request shortly.

[malb]

comment:73

Here is what I do in M4RIE

https://bitbucket.org/malb/m4rie/src/HEAD/m4/ax_m4ri_flags.m4?at=master

but I think this should be replaced by pkg-config like this

https://autotools.io/pkgconfig/pkg_check_modules.html

[kiwifb]

comment:74

Funny, I wrote what I called a vile hack but at some point it was looking very very close to that. I just didn't have the step to use cat afterwards. For some reason I didn't want to go that way.

[sagetrac-git]

Changed commit from 2a799b9 to 61c0186

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

61c0186

brial: version bump which includes M4RI's SIMD CFLAGS

[ohanar]

comment:76

ok, hopefully now fixed

[jdemeyer]

comment:78

Isn't this a typo?

+ $(INST)/$(BRAIL) \

[kiwifb]

comment:79

Replying to @jdemeyer:

Isn't this a typo?

+ $(INST)/$(BRAIL) \

Looks like one. I wonder how I got it to build "without that problem" on two different machines.

[jdemeyer]

comment:80

Replying to @kiwifb:

Looks like one. I wonder how I got it to build "without that problem" on two different machines.

$(INST)/$(BRAIL) becomes $(INST)/ which is an existing directory...

The problem would start if you upgrade $(BRIAL), then the Sage library would not be automatically rebuilt.

[vbraun]

comment:81

Here is a build failure because the dependencies are not correct, looks like the Sage library is built before brial:

http://build.sagemath.org/release/builders/%20%20slow%20Oxford%20ARM%20%28Ubuntu%2012.10%29%20incremental/builds/182/steps/compile/logs/stdio

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

7ff3b09

brial: fix typo in deps

[sagetrac-git]

Changed commit from 61c0186 to 7ff3b09

[kiwifb]

comment:84

OK, positive review - take 3.

[vbraun]

Changed branch from u/ohanar/brial to 7ff3b09