Skip to content
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

Move the planarity files to a spkg #18187

Closed
nathanncohen mannequin opened this issue Apr 14, 2015 · 84 comments
Closed

Move the planarity files to a spkg #18187

nathanncohen mannequin opened this issue Apr 14, 2015 · 84 comments

Comments

@nathanncohen
Copy link
Mannequin

nathanncohen mannequin commented Apr 14, 2015

The contents of sage/graphs/planarity_c is not Sage source but a non-vanilla copy of John Boyer's planarity code [1]. This branch turns it into a standard package.

Package: http://boxen.math.washington.edu/home/jdemeyer/spkg/planarity-2.2.0.tar.bz2 (upstream version 2.2.0 - Nauty + edited version of Nathann Cohen's autotools project)

[1] https://code.google.com/p/planarity/

Depends on #18431
Depends on #18428

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

CC: @mezzarobba @videlec @sagetrac-tmonteil @dcoudert @jdemeyer

Component: packages: standard

Author: Nathann Cohen, Jeroen Demeyer

Branch/Commit: 2ebb2d1

Reviewer: Jeroen Demeyer, Nathann Cohen

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

@nathanncohen nathanncohen mannequin added this to the sage-6.6 milestone Apr 14, 2015
@nathanncohen nathanncohen mannequin added the p: major / 3 label Apr 14, 2015
@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

Commit: 4f8f33d

@nathanncohen

This comment has been minimized.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

Branch: public/18187

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

Author: Nathann Cohen

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

New commits:

4f8f33dtrac #18187: Move the planarity files to a spkg

@jdemeyer
Copy link

comment:2

Is this meant to be a standard or optional package? In any case, the current branch obviously doesn't work if planarity is not installed:

$ make
...
building 'sage.graphs.planarity' extension
Executing 1 command (using 1 thread)
gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/src/sage-config/local/include -I/usr/local/src/sage-config/src -I/usr/local/src/sage-config/src/c_lib/include -I/usr/local/src/sage-config/src/sage/ext -I/usr/local/src/sage-config/local/include/python2.7 -c build/cythonized/sage/graphs/planarity.c -o build/temp.linux-x86_64-2.7/build/cythonized/sage/graphs/planarity.o -fno-strict-aliasing -w -fno-tree-dominator-opts
build/cythonized/sage/graphs/planarity.c:276:29: fatal error: planarity/graph.h: No such file or directory
 #include "planarity/graph.h"
                             ^
compilation terminated.
error: command 'gcc' failed with exit status 1
Makefile:19: recipe for target 'build' failed
make: *** [build] Error 1

Also: to be safe, you might want to rebase on top of #18145, especially if you make further changes to the build system.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

comment:3

Is this meant to be a standard or optional package?

A standard package. This code is already part of Sage.

Also: to be safe, you might want to rebase on top of #18145, especially if you make further changes to the build system.

Second time I have to rebase on top of that one :-P

Nathann

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

comment:4

I just checked, and there are no conflicts with #18145 (trivial merge).

Nathann

@jdemeyer
Copy link

Work Issues: spkg is not built

@jdemeyer
Copy link

comment:6

Replying to @nathanncohen:

I just checked, and there are no conflicts with #18145 (trivial merge).

I know, that ticket is just prone to conflicts. But it's also an important ticket. I hope you like the new syntax for optional packages in #18145.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 14, 2015

comment:7

I know, that ticket is just prone to conflicts. But it's also an important ticket. I hope you like the new syntax for optional packages in #18145.

Yepyep, it's indeed much much cleaner!!

Nathann

@jdemeyer
Copy link

comment:8

When building a library in a portable way, you must use some program like libtool.

@jdemeyer
Copy link

Changed work issues from spkg is not built to spkg is not built, make it portable

@jdemeyer
Copy link

comment:9

I wonder if you could not use install the .c and .h sources somewhere instead of building a library.

@jdemeyer
Copy link

comment:10

Or move planarity not to a spkg but to src/ext/planarity.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 22, 2015

comment:11

Or move planarity not to a spkg but to src/ext/planarity.

Hmmmm... I prefer the libtool way out. These files haven't been reviewed, so I do not think that they should be in our code directly.

@jdemeyer
Copy link

comment:12

Replying to @nathanncohen:

These files haven't been reviewed, so I do not think that they should be in our code directly.

Well, src/ext is not considered "our code", so that's not a problem.

@nathanncohen
Copy link
Mannequin Author

nathanncohen mannequin commented Apr 22, 2015

comment:13

What exactly is it meant to contain? There is also a src/sage/ext/ directory O_o

@jdemeyer
Copy link

comment:14

Replying to @nathanncohen:

What exactly is it meant to contain?

Well, that's not so clear :-)

At least, it contains certain non-Sage code for PARI and GAP.

@jdemeyer
Copy link

comment:58

This now works on OS X.

@vbraun
Copy link
Member

vbraun commented May 12, 2015

Dependencies: #15642

@vbraun
Copy link
Member

vbraun commented May 12, 2015

comment:59
>>> Trying to download http://www.sagemath.org/packages/upstream/planarity/planarity-2.2.0.tar.bz2
[...............................................]
Checksum: 3ef0985a2b8476123969cef5c1273d11286605f2 vs 90ff1db146aeffcc330cc9117a72e72aaf4a4fec
Invalid checksum for planarity-2.2.0.tar.bz2.tmp

You need to change the version number when changing the tarball. The buildbots have the old tarball cached now.

@jdemeyer
Copy link

comment:60

Replying to @vbraun:

You need to change the version number when changing the tarball. The buildbots have the old tarball cached now.

Sorry Volker, but I don't get it. Just because the buildbots have tested an earlier version, I have to change the tarball name? Can't you change the buildbot script such that it re-downloads the tarball every time (using wget -N if you use wget) or at least whenever the checksum check failed?

@vbraun
Copy link
Member

vbraun commented May 13, 2015

comment:61

The buildbot just relies on Sage to download tarballs.

@vbraun
Copy link
Member

vbraun commented May 13, 2015

comment:62

PS: #15642 will re-download tarballs whose checksum does not match

@jdemeyer
Copy link

comment:63

So, since you added the #15642 as dependency, there is no problem, right?

@vbraun
Copy link
Member

vbraun commented May 14, 2015

comment:64

Turns out sage-spkg doesn't even call sage-download-files if the checksum doesn't match, this is now #18417

@vbraun
Copy link
Member

vbraun commented May 14, 2015

Changed dependencies from #15642 to #15642, #18417

@vbraun
Copy link
Member

vbraun commented May 16, 2015

Changed dependencies from #15642, #18417 to #15642, #18428

@vbraun
Copy link
Member

vbraun commented May 16, 2015

comment:66

Fixing the downloading is now #18428

@jdemeyer
Copy link

comment:67

Needs work because of #18431

@jdemeyer
Copy link

Changed dependencies from #15642, #18428 to #18431, #18428

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2015

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

2ce8deaMerge tag '6.8.beta2' into t/18187/public/18187
2ebb2d1Add type file

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 3, 2015

Changed commit from 48ceb25 to 2ebb2d1

@vbraun
Copy link
Member

vbraun commented Jun 4, 2015

Changed branch from public/18187 to 2ebb2d1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants