Opened 6 years ago
Closed 6 years ago
#18681 closed enhancement (fixed)
Separate the rankwidth library into a standard package
Reported by: | mjo | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.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 follow-up to,
https://groups.google.com/forum/#!topic/sage-devel/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.uni-frankfurt.de/~philipp/software/rw-0.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 --enable-executable=no
flag,
$ ./configure --enable-executable=no
it will succeed without igraph and build the library.
Change History (25)
comment:1 Changed 6 years ago by
- Cc jdemeyer added; jdemeyed removed
comment:2 follow-up: ↓ 3 Changed 6 years ago by
comment:3 in reply to: ↑ 2 Changed 6 years ago by
comment:4 Changed 6 years ago by
Okay, I'll do it tomorrow then.
comment:5 Changed 6 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 6 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 6 years ago by
The license is GPL version 2 or later.
comment:8 Changed 6 years ago by
For upstream contact, I would mention the page http://pholia.tdi.informatik.uni-frankfurt.de/~philipp/software/rw.shtml
comment:9 Changed 6 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 6 years ago by
- Status changed from needs_review to needs_work
comment:11 Changed 6 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 6 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 follow-up: ↓ 14 Changed 6 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 ; follow-up: ↓ 16 Changed 6 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 6 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 ; follow-up: ↓ 19 Changed 6 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 6 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 6 years ago by
- Status changed from needs_work to needs_review
comment:19 in reply to: ↑ 16 Changed 6 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 follow-up: ↓ 21 Changed 6 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 6 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 6 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 6 years ago by
- Reviewers set to Jeroen Demeyer
- Status changed from needs_review to positive_review
comment:24 Changed 6 years ago by
Thanks!
comment:25 Changed 6 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?