Update PolyBoRi to release 0.8.1

[alexanderdreyer]

PolyBoRi's next minor release is out now.
There were no changes of the interface between PolyBoRi 0.8.0 and 0.8.1, so we just have to update the sources from here:

https://sourceforge.net/projects/polybori/files/polybori/0.8.1

Current spkg

• Install http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1.p1.spkg

This also fixes building with GCC-4.7.0, see #12751 for the GCC-4.7.0 metaticket.

Depends on #12656
Depends on #12750
Depends on #12799

Upstream: None of the above - read trac for reasoning.

CC: @sagetrac-PolyBoRi @malb @burcin

Component: packages: standard

Author: Alexander Dreyer

Reviewer: Martin Albrecht, Jeroen Demeyer, Leif Leonhardy

Merged: sage-5.0.beta13

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

[alexanderdreyer]

comment:1

The first experiment is here http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1rc3.p0.spkg

[alexanderdreyer]

comment:3

Even though the spkg works without a patch to the sage library, I recommend to use the patch from #12656, also. It fixed inconsistencies between the original interface and Sage's Cython-bases reimplementation.

[alexanderdreyer]

Dependencies: #12656

[alexanderdreyer]

comment:4

For sage-5.0.beta7 (with #12656) the spkg installs and tests well (make ptestlong) on a SuSE Enterprise 11 AMD64.

[alexanderdreyer]

comment:5

Also works without #12656 (not recommended).

[malb]

comment:6

I can confirm that doctests pass on geom.math with 5.0.beta8.

[alexanderdreyer]

comment:7

Here's an updates spkg.
http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1rc5.p0.spkg
Note: upstream PolyBoRi can detect and use Sage (if available), so we do not have to patch PyPolyBoRi.py every time again and again.

[malb]

comment:8

Since it's still an RC, do we aim for 'positive review' already?

[alexanderdreyer]

comment:9

Replying to @malb:

Since it's still an RC, do we aim for 'positive review' already?

It should be final now, but I'd like to wait for response from non-Linux people. Maybe you can give a conditional review for the case, the spkg doesn't change any more. (There's no rc5 marker inside the spkg, so we just have to rename it, if we are lucky.)

[malb]

comment:10

Sounds like a plan. conditional positive review it is :)

[alexanderdreyer]

comment:11

Unfortunately, I had to rebundle the spkg, because the directory name contained rc5 (D'Oh!).
Here is the updated spkg: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1.p0.spkg
Also, there is one last-minute change in the source (not doctesting optional polybori.plot, if prerequsite dot is not available):
https://bitbucket.org/brickenstein/polybori/changeset/b3a12752980c

The recent spkg installed on SuSE11 AMD64 and OS X 10.5 ppc. make ptestlong succeeded on SuSE (OS X still running.)

[malb]

comment:12

Hey, I checked the SPKG for obvious problems and it looks clean. I didn't install it again and ran tests, but those the buildbot can figure out. So: positive review.

[malb]

Reviewer: Martin Albrecht

[alexanderdreyer]

comment:13

Finally, the rc became release, indeed!

[jdemeyer]

comment:14

It is recommended to use patch for patching SPKGs, see http://sagemath.org/doc/developer/patching_spkgs.html. If not, at least add a corresponding .patch file for custom.py.

I'm not setting this ticket back to needs_work for this but it SHOULD be fixed.

[alexanderdreyer]

comment:15

Replying to @jdemeyer:

It is recommended to use patch for patching SPKGs, see http://sagemath.org/doc/developer/patching_spkgs.html. If not, at least add a corresponding .patch file for custom.py.

A patch might become part of upstream sources eventually.
But custom.py is a user configuration file, which will never be upstream itself (of course the mechanism already supported there).
Of course, we can generate a patch against /dev/null. but does it really makes sense?

[alexanderdreyer]

comment:16

Replying to @alexanderdreyer:

Of course, we can generate a patch against /dev/null. but does it really makes sense?

Ok, answering myself: it's part of the directory patch, so of course one should consider it as a patch to upstream. There's no config directory mention here http://sagemath.org/doc/developer/producing_spkgs.html , so patching is the only option here.

[alexanderdreyer]

comment:17

The updated spkg is now here:
http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1.p0.spkg
Also, I added two minor fixes from upstream. One fixes a preprocessor misconception (#if instead of #ifdef, and the other fixes a doctest prerequisites check, which prevented ipbori's self test to check polybori.plot.

So, unfortunately we need a review again.

[alexanderdreyer]

comment:60

Replying to @nexttime:

Replying to @nexttime:

module_list.py certainly needs work w.r.t. dependencies.
[...]
That's because a couple of Sage library extension modules didn't get rebuilt, still referring to the old, deleted PolyBoRi shared library.

Actually only the pbori module needs to get rebuilt; while touching $SAGE_LOCAL/include/polybori.h doesn't help, touching $SAGE_LOCAL/include/polybori/* does (afterwards running ./sage -b).

I'll add this to the spkg.

Also builds with GCC 4.7.0 (Sage 5.0.beta10, Ubuntu 10.04.4 x86_64).

I noticed that different flags get passed to gcc/g++ depending on GCC's version, or probably the settings of CC and CXX. Seems rather a side-effect than intended behaviour...

W.r.t. M4RI's flags: It should be sufficient to only pass / add the contents of __M4RI_SIMD_CFLAGS, and probably only when compiling some of the modules.

This is not related to the upgrade, so I'd prefer a new ticket on this.

[alexanderdreyer]

comment:61

Replying to @nexttime:

Furthermore, it's quite funny to use g++ -std=c++98 while using long long; I get hundreds of warnings for that, since -Wall is also there, but not -Wno-long-long. (-Wall comes from M4RI's CFLAGS as well.)

I double-checked that. Per default the spkg sets -Wno-long-long, It's probably overwritten by some C...FLAGS. Should I block -Wno-long-long such that it is never overwritten by the environment (like -std=c++98)?

[alexanderdreyer]

comment:62

Replying to @nexttime:
This issue is now #12799.

[jdemeyer]

comment:63

Replying to @alexanderdreyer:

Actually only the pbori module needs to get rebuilt; while touching $SAGE_LOCAL/include/polybori.h doesn't help, touching $SAGE_LOCAL/include/polybori/* does (afterwards running ./sage -b).

I'll add this to the spkg.

I think it's better to change module_list.py instead of the spkg.

[alexanderdreyer]

Changed dependencies from #12656, #12750 to #12656, #12750, #12799

[alexanderdreyer]

Changed work issues from Fix dependencies in module_list.py and/or touch some headers from spkg-install. to none

[alexanderdreyer]

comment:64

Replying to @jdemeyer:

I think it's better to change module_list.py instead of the spkg.

This part is now #12799, so I add this as dependency and remove the work issue.

I also updated the spkg to use -Wno-long-long for C++ here: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1.p1.spkg

[alexanderdreyer]

comment:65

Replying to @nexttime:

W.r.t. M4RI's flags: It should be sufficient to only pass / add the contents of __M4RI_SIMD_CFLAGS, and probably only when compiling some of the modules.

@malb What do you think: Would this be enough?

[nexttime]

comment:66

Replying to @jdemeyer:

Replying to @alexanderdreyer:

Actually only the pbori module needs to get rebuilt; while touching $SAGE_LOCAL/include/polybori.h doesn't help, touching $SAGE_LOCAL/include/polybori/* does (afterwards running ./sage -b).

I'll add this to the spkg.

I think it's better to change module_list.py instead of the spkg.

IIRC the problem isn't only which files the pbori module depends on, but also that the headers have their original file date from upstream, i.e., aren't generated during build nor (currently) touched from spkg-install, such that they'll usually be older than a present pbori module.

If one doesn't want to change the timestamps, one could make the pbori module just (or also) depend on the .so (without a version suffix), since the (target of the) latter will always have a fresh modification time.

[alexanderdreyer]

comment:67

Replying to @nexttime:

If one doesn't want to change the timestamps, one could make the pbori module just (or also) depend on the .so (without a version suffix), since the (target of the) latter will always have a fresh modification time.

The libraries are already listed as libraries=['polybori', 'polybori_groebner', 'gd', 'png12', 'm4ri']. Aren't they considered as dependencies anyway?

But the timestamp of include/polybori/config.h varies, so #12799 fixes the problem.

[nexttime]

comment:68

Replying to @alexanderdreyer:

Replying to @nexttime:

If one doesn't want to change the timestamps, one could make the pbori module just (or also) depend on the .so (without a version suffix), since the (target of the) latter will always have a fresh modification time.

The libraries are already listed as libraries=['polybori', 'polybori_groebner', 'gd', 'png12', 'm4ri']. Aren't they considered as dependencies anyway?

Of course the libraries are listed, since they need to get linked to the module, but Cython doesn't automatically consider their timestamps for dependency checking, i.e., which modules need to get rebuilt.

But the timestamp of include/polybori/config.h varies, so #12799 fixes the problem.

Then I'd add just that there.

[alexanderdreyer]

comment:69

Replying to @nexttime:

Then I'd add just that there.

Done. Is this ticket positively reviewed again?

[nexttime]

comment:70

Replying to @alexanderdreyer:

Replying to @nexttime:

Then I'd add just that there.

Done. Is this ticket positively reviewed again?

Can't tell. Jeroen last set it from needs_review to needs_work, and the issues he mentioned seem to be fixed, as well as mine regarding the dependencies (now at #12799, which this ticket depends on).

If the rest of the spkg already had positive review, you could probably revert it to that, but the other reviewers can probably tell better.

[alexanderdreyer]

comment:71

Replying to @jdemeyer:

I think SPKG.txt needs some work:

1. The Changelog messages don't tell much. If you add a patch, you should mention that fact in the Changelog. You should add a ticket number in the Changelog messages.
2. The patches should be documented.

Also done (see above). @jdemeyer Back to positive?

[nexttime]

comment:72

Replying to @nexttime:

Replying to @alexanderdreyer:

Is this ticket positively reviewed again?

[...] the issues he mentioned seem to be fixed, as well as mine regarding the dependencies (now at #12799, which this ticket depends on).

... which now has positive review.

[jdemeyer]

comment:73

Positive review then... provided it survives testing.

[jdemeyer]

Merged: sage-5.0.beta13