Opened 9 years ago

Last modified 9 years ago

#12655 closed enhancement

Update PolyBoRi to release 0.8.1 — at Version 36

Reported by: AlexanderDreyer Owned by: AlexanderDreyer
Priority: major Milestone: sage-5.0
Component: packages: standard Keywords:
Cc: PolyBoRi, malb, burcin Merged in:
Authors: Alexander Dreyer Reviewers: Martin Albrecht, Jeroen Demeyer
Report Upstream: None of the above - read trac for reasoning. Work issues:
Branch: Commit:
Dependencies: #12656 Stopgaps:

Status badges

Description (last modified by jdemeyer)

PolyBoRi's next minor release will be out soon. 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

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

Change History (36)

comment:1 Changed 9 years ago by AlexanderDreyer

  • Description modified (diff)

comment:2 Changed 9 years ago by AlexanderDreyer

  • Status changed from new to needs_review

comment:3 Changed 9 years ago by AlexanderDreyer

  • Dependencies set to #12656

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.

comment:4 Changed 9 years ago by AlexanderDreyer

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

comment:5 Changed 9 years ago by AlexanderDreyer

Also works without #12656 (not recommended).

comment:6 Changed 9 years ago by malb

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

comment:7 Changed 9 years ago by AlexanderDreyer

  • Description modified (diff)

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.

comment:8 follow-up: Changed 9 years ago by malb

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

comment:9 in reply to: ↑ 8 Changed 9 years ago by AlexanderDreyer

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.)

comment:10 Changed 9 years ago by malb

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

comment:11 Changed 9 years ago by AlexanderDreyer

  • Description modified (diff)

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.)

comment:12 Changed 9 years ago by malb

  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to positive_review

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.

comment:13 Changed 9 years ago by AlexanderDreyer

Finally, the rc became release, indeed!

comment:14 follow-up: Changed 9 years ago by 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.

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

comment:15 in reply to: ↑ 14 ; follow-up: Changed 9 years ago by AlexanderDreyer

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?

comment:16 in reply to: ↑ 15 Changed 9 years ago by AlexanderDreyer

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.

comment:17 Changed 9 years ago by AlexanderDreyer

  • Description modified (diff)
  • Status changed from positive_review to needs_work

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.

comment:18 Changed 9 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review

comment:19 follow-up: Changed 9 years ago by fbissey

I agree that "custom.py" is not a patch to upstream in any way. It is a custom configuration and there is nothing in it that couldn't be actually be shifted in spkg-install. Putting it in custom.py probably keep things cleaner. If you don't want to pollute the patch folder could we consider putting it in the spkg root directory?

comment:20 in reply to: ↑ 19 ; follow-up: Changed 9 years ago by jdemeyer

Replying to fbissey:

If you don't want to pollute the patch folder could we consider putting it in the spkg root directory?

I agree with this. I saw the file in patches/ and assumed it was a patch to some existing PolyBoRi file. If it's not a patch, it doesn't belong in patches/.

comment:21 Changed 9 years ago by AlexanderDreyer

Ok, I moved custom.py to the spkg root. The spkg-install is now more generic: it takes any *.patches in patches and apply them to the sources. The updated spkg is at the ususal place.

comment:22 in reply to: ↑ 20 Changed 9 years ago by AlexanderDreyer

Replying to jdemeyer:

Replying to fbissey:

If you don't want to pollute the patch folder could we consider putting it in the spkg root directory?

I agree with this. I saw the file in patches/ and assumed it was a patch to some existing PolyBoRi file. If it's not a patch, it doesn't belong in patches/.

I fixed that, so the package needs a new review.

comment:23 follow-up: Changed 9 years ago by fbissey

Sorry to be a little bit annoying. Now that custom.py is in the root folder of the spkg could you add a "special files" section to SPKG.txt, list custom.py and explain what it is.

comment:24 in reply to: ↑ 23 Changed 9 years ago by AlexanderDreyer

Replying to fbissey:

Sorry to be a little bit annoying. Now that custom.py is in the root folder of the spkg could you add a "special files" section to SPKG.txt, list custom.py and explain what it is.

No problem! You'll find an updated spkg at the usual place.

comment:25 Changed 9 years ago by jdemeyer

  • Reviewers changed from Martin Albrecht to Martin Albrecht, Jeroen Demeyer
  • Status changed from needs_review to positive_review

Looks good to me.

comment:26 Changed 9 years ago by malb

  • Status changed from positive_review to needs_work

comment:27 Changed 9 years ago by AlexanderDreyer

  • Status changed from needs_work to needs_review

Hi, I updated the spkg. Please have a look at it.

comment:28 Changed 9 years ago by malb

  • Status changed from needs_review to positive_review

Back to positive review. Thanks!

comment:29 Changed 9 years ago by jhpalmieri

  • Status changed from positive_review to needs_work

At least in combination with #12369, this fails to build on OpenSolaris on x86 (David Kirkby's machine hawk): it gives me

scons: done reading SConscript files.
scons: Building targets ...
writing config.h...
gcc -o Cudd/cudd/cuddAPI.pic.o -c -std=c99 -O3 -Wno-long-long -Wreturn-type -g -fPIC -mmmx -msse -msse2 -msse3 -g -fPIC -Wall -pedantic -O2 -KPIC -DPBORI_NDEBUG -DSIZEOF_VOID_P=4 -DSIZEOF_INT=4 -DSIZEOF_LONG=4 -DPBORI_HAVE_GD -DPBORI_HAVE_TR1_UNORDERED_MAP -DPBORI_HAVE_M4RI -Ilibpolybori/include -Igroebner/include -I/export/home/palmieri/testing/GCC-no-check/sage-5.0.beta10-gcc/local/include -I/export/home/palmieri/testing/GCC-no-check/sage-5.0.beta10-gcc/local/include/python2.7 -ICudd Cudd/cudd/cuddAPI.c
gcc: error: unrecognized option ‘-KPIC’
scons: *** [Cudd/cudd/cuddAPI.pic.o] Error 1

In contrast, the 0.8.0.p1 spkg builds fine.

On Solaris on sparc (mark2), in combination with #12369, I get this:

Starting build...
Removing old PolyBoRi install...
Done removing old PolyBoRi install.
Running build_polybori...
scons: Reading SConscript files ...
Platform:  sunos5
Variable LIBSUFFIX not in default environment!
Platform:  sunos5
Detecting type sizes... no
Could not detect type sizes (maybe compile/link flags trouble)! Exiting.

comment:30 Changed 9 years ago by AlexanderDreyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

Thanks, I think I found the issue. Please try out the following: http://boxen.math.washington.edu/home/dreyer/spkg/polybori-0.8.1.p1.spkg

Note: I updated the spkg from #12754 which already fixed other things (for instance gcc-4.7 compatibility.)

comment:31 follow-up: Changed 9 years ago by jhpalmieri

With this package, on both Solaris and OpenSolaris, I get this:

Starting build...
Removing old PolyBoRi install...
Done removing old PolyBoRi install.
Running build_polybori...
scons: Reading SConscript files ...
NameError: name 'CCFLAGS' is not defined:
  File "/export/home/palmieri/testing/sage-5.0.beta7-gcc/spkg/build/polybori-0.8.1.p1/src/polybori-0.8.1/SConstruct", line 241:
    if is_gcc():
  File "/export/home/palmieri/testing/sage-5.0.beta7-gcc/spkg/build/polybori-0.8.1.p1/src/polybori-0.8.1/SConstruct", line 239:
    compilerenv = Environment(ENV = os.environ, options = opts)
  File "/export/home/palmieri/testing/sage-5.0.beta7-gcc/local/lib/scons-1.2.0/SCons/Environment.py", line 975:
    variables.Update(self)
  File "/export/home/palmieri/testing/sage-5.0.beta7-gcc/local/lib/scons-1.2.0/SCons/Variables/__init__.py", line 169:
    execfile(filename, {}, values)
  File "custom.py", line 6:
    CCFLAGS += ["-Wno-long-long", "-Wreturn-type"]
Error building PolyBoRi.

Should CCFLAGS be CFLAGS? If I fix this, I get a similar error for GD_LIBS. I ended up applying the following patch:

  • custom.py

    diff --git a/custom.py b/custom.py
    a b  
    11import os
    22import sys
    33
    4 # FIXME: Do we really want to *overwrite* the flags (e.g. if set by the user)?
    5 # Answer: Should be fixed now.
    6 CCFLAGS += ["-Wno-long-long", "-Wreturn-type"]
    7 
    84# Note: PolyBoRi still appends DEFAULT_*FLAGS (overwrite those, if necessary)
    95if 'CFLAGS' in os.environ:
    106  CFLAGS = os.environ['CFLAGS'].split(' ')
     7else:
     8  CFLAGS = []
     9
     10# FIXME: Do we really want to *overwrite* the flags (e.g. if set by the user)?
     11# Answer: Should be fixed now.
     12CFLAGS += ["-Wno-long-long", "-Wreturn-type"]
    1113
    1214if 'CXXFLAGS' in os.environ:
    1315  CXXFLAGS = os.environ['CXXFLAGS'].split(' ')
     
    1517if 'CPPFLAGS' in os.environ:
    1618  CCFLAGS = os.environ['CPPFLAGS'].split(' ')
    1719
     20if 'GD_LIBS' in os.environ:
     21  GD_LIBS = os.environ['GD_LIBS'].split(' ')
     22else:
     23  GD_LIBS = []
     24
    1825GD_LIBS+=["png12","z"]
    1926
    2027# FIXME: Should we include LDFLAGS here? (see above)

The spkg-install file is odd, though: some places it refers to CFLAGS, others to CCFLAGS, so I'm not sure my changes are correct. In any case, it worked on OpenSolaris, but on Solaris I got the same error as before ("Could not detect type sizes"). This may be related to #12369.

comment:32 in reply to: ↑ 31 ; follow-ups: Changed 9 years ago by AlexanderDreyer

Replying to jhpalmieri:

With this package, on both Solaris and OpenSolaris, I get this: Should CCFLAGS be CFLAGS? If I fix this, I get a similar error for GD_LIBS. I ended up applying the following patch:

Sorry, I made a mistake. I did not have a recent Sage at hand on Solaris, so I tried that out without custom.py. (I need to fix that upstream again. Your patch is a good work around.)

The spkg-install file is odd, though: some places it refers to CFLAGS, others to CCFLAGS, so I'm not sure my changes are correct.

CFLAGS are flags for pure C, CCFLAGS for C or C++.

In any case, it worked on OpenSolaris, but on Solaris I got the same error as before ("Could not detect type sizes"). This may be related to #12369.

There a file called config.log. Can you post it somewhere?

comment:33 in reply to: ↑ 32 Changed 9 years ago by AlexanderDreyer

Sorry, I made a mistake. I did not have a recent Sage at hand on Solaris, so I tried that out without custom.py. (I need to fix that upstream again. Your patch is a good work around.)

But the CCFLAGS should still be there, for instance:

CCFLAGS = ["-O3", "-Wno-long-long", "-Wreturn-type"]

comment:34 Changed 9 years ago by jhpalmieri

If I add a print statement at the top of custom.py ("print CCFLAGS"), then on OS X or sage.math, I see ['-O3']. On OpenSolaris, I get a NameError saying that CCFLAGS is not defined.

With Solaris (skynet machine mark2), the entire contents of config.log:

file /home/palmieri/mark2/GCC/sage-5.0.beta10-gcc/spkg/build/polybori-0.8.1.p1/src/polybori-0.8.1/SConstruct,line 653:
        Configure(confdir = .sconf_temp)
scons: Configure: Detecting type sizes... 
.sconf_temp/conftest_0.c <-
  |
  |        #include <stdio.h>
  |        int main(int argc, char **argv) {
  |          printf("SIZEOF_VOID_P=%u SIZEOF_INT=%u SIZEOF_LONG=%u",
  |            (unsigned)sizeof(void*), (unsigned)sizeof(int), (unsigned)sizeof(long));
  |          return 0;
  |        }
  |        
gcc -o .sconf_temp/conftest_0.o -c -Wno-long-long -Wreturn-type -std=c99 -O3 -DPBORI_NDEBUG -I/home/palmieri/mark2/GCC/sage-5.0.beta10-gcc/local/include .sconf_temp/conftest_0.c
gcc -o .sconf_temp/conftest_0 -Wl,-rpath=\$ORIGIN/../build/usr/local/lib -z origin .sconf_temp/conftest_0.o -L/home/palmieri/mark2/GCC/sage-5.0.beta10-gcc/local/lib
ld: fatal: option -dn and -P are incompatible
ld: fatal: Flags processing errors
collect2: ld returned 1 exit status
scons: Configure: no

comment:35 in reply to: ↑ 32 Changed 9 years ago by jdemeyer

Replying to AlexanderDreyer:

CFLAGS are flags for pure C, CCFLAGS for C or C++.

I don't think this is standard usage. Normally, one uses CFLAGS for C and CXXFLAGS for C++. Of course, you can additionally support CCFLAGS if you want.

comment:36 Changed 9 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.