Opened 5 years ago
Closed 5 years ago
#18681 closed enhancement (fixed)
Separate the rankwidth library into a standard package
Reported by:  mjo  Owned by:  

Priority:  major  Milestone:  sage6.8 
Component:  graph theory  Keywords:  
Cc:  jdemeyer, ncohen  Merged in:  
Authors:  Nathann Cohen  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  3681e06 (Commits)  Commit:  3681e06d8de78f2e65b73500ae1dfc6d8893a5e5 
Dependencies:  Stopgaps: 
Description (last modified by )
This is a followup to,
https://groups.google.com/forum/#!topic/sagedevel/HF52TdkH0fw
The author of the rankwidth (rw) package was very helpful in getting the library/executable packaged with autotools. The new release is available here:
http://pholia.tdi.informatik.unifrankfurt.de/~philipp/software/rw0.7.tar.gz
The first big change is that it uses autotools, obviously. So the standard ./configure && make
should work. But more importantly, we can now compile the shared library without the executable, and without the resulting dependency on igraph. If you use the default ./configure:
$ ./configure ... configure: error: Package requirements (igraph >= 0.6) were not met: No package 'igraph' found
But if you pass the enableexecutable=no
flag,
$ ./configure enableexecutable=no
it will succeed without igraph and build the library.
Change History (25)
comment:1 Changed 5 years ago by
 Cc jdemeyer added; jdemeyed removed
comment:2 followup: ↓ 3 Changed 5 years ago by
comment:3 in reply to: ↑ 2 Changed 5 years ago by
comment:4 Changed 5 years ago by
Okay, I'll do it tomorrow then.
comment:5 Changed 5 years ago by
 Branch set to public/18681
 Commit set to 938002493f84cee8e56e7e0a3ae504a1b8a60874
 Description modified (diff)
 Status changed from new to needs_review
New commits:
9380024  trac #18681: 'rw' as a package

comment:6 Changed 5 years ago by
 Commit changed from 938002493f84cee8e56e7e0a3ae504a1b8a60874 to 687888df22d84d548e1668062a9717e068b6d1b1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
687888d  trac #18681: 'rw' as a package

comment:7 Changed 5 years ago by
The license is GPL version 2 or later.
comment:8 Changed 5 years ago by
For upstream contact, I would mention the page http://pholia.tdi.informatik.unifrankfurt.de/~philipp/software/rw.shtml
comment:9 Changed 5 years ago by
These lines should be removed from the .pxd
file, since they are not in the API of rw
:
subset_t *slots uint_fast8_t num_vertices
Move this inside the cdef extern from "rankwidth_c/rw.h":
block (removing the stuff in quotes and uint_fast8
):
ctypedef int uint_fast8_t ctypedef int uint_fast32_t ctypedef int subset_t
comment:10 Changed 5 years ago by
 Status changed from needs_review to needs_work
comment:11 Changed 5 years ago by
And this is clearly a private function, this shouldn't be in the pxd
either:
cdef void set_am(int i, int j, int val)
comment:12 Changed 5 years ago by
 Commit changed from 687888df22d84d548e1668062a9717e068b6d1b1 to 8002dc43a5f94ce4a584dae56631ecb9d161ebc9
Branch pushed to git repo; I updated commit sha1. New commits:
8002dc4  trac #18681: Reviewer's comments

comment:13 followup: ↓ 14 Changed 5 years ago by
 Status changed from needs_work to needs_review
I made all modifications except the one about the licence: where did you see that? I said 'GPL2' in SPKG.txt
because that's what the COPYING
file contains.
Nathann
comment:14 in reply to: ↑ 13 ; followup: ↓ 16 Changed 5 years ago by
 Status changed from needs_review to needs_work
Replying to ncohen:
I made all modifications except the one about the licence: where did you see that? I said 'GPL2' in
SPKG.txt
because that's what theCOPYING
file contains.
Look in the code (rw.c
for example).
Note that Sage is released under GPL version 3 (the only license which is compatible with all standard packages), so if it really was GPL version 2 only, that would be a problem.
comment:15 Changed 5 years ago by
Do you really need this stuff in quotes:
"uint_fast8_t"
(same for the other two)
comment:16 in reply to: ↑ 14 ; followup: ↓ 19 Changed 5 years ago by
Yo !
Look in the code (
rw.c
for example).
Well, that's two contradicting sources. Should we ask upstream to use the same in both places?
Note that Sage is released under GPL version 3 (the only license which is compatible with all standard packages), so if it really was GPL version 2 only, that would be a problem.
Sigh... T_T
Nathann
comment:17 Changed 5 years ago by
 Commit changed from 8002dc43a5f94ce4a584dae56631ecb9d161ebc9 to f0556317ae56404b1171b4d990be769599a0af63
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
f055631  trac #18681: Reviewer's comments

comment:18 Changed 5 years ago by
 Status changed from needs_work to needs_review
comment:19 in reply to: ↑ 16 Changed 5 years ago by
Replying to ncohen:
Well, that's two contradicting sources.
It's not contradicting. The code is what matters. Essentially it says "this code is released until GPL v2 or later, see the file COPYING
for the GPL v2 text"
comment:20 followup: ↓ 21 Changed 5 years ago by
 Status changed from needs_review to needs_work
rw
should be a dependency of the Sage library in build/deps
.
comment:21 in reply to: ↑ 20 Changed 5 years ago by
 Status changed from needs_work to needs_review
rw
should be a dependency of the Sage library inbuild/deps
.
Arg... True :/
Can you think of a way to make this automatic?
Nathann
comment:22 Changed 5 years ago by
 Commit changed from f0556317ae56404b1171b4d990be769599a0af63 to 3681e06d8de78f2e65b73500ae1dfc6d8893a5e5
Branch pushed to git repo; I updated commit sha1. New commits:
3681e06  trac #18681: rw is a dependency of Sage

comment:23 Changed 5 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
comment:24 Changed 5 years ago by
Thanks!
comment:25 Changed 5 years ago by
 Branch changed from public/18681 to 3681e06d8de78f2e65b73500ae1dfc6d8893a5e5
 Resolution set to fixed
 Status changed from positive_review to closed
Have you started turning it into a Sage package?