Opened 5 years ago
Closed 5 years ago
#20905 closed defect (fixed)
converting frobby into a new-style package
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.3 |
Component: | packages: optional | Keywords: | |
Cc: | vdelecroix, mkoeppe | Merged in: | |
Authors: | Dima Pasechnik | Reviewers: | Matthias Koeppe |
Report Upstream: | N/A | Work issues: | |
Branch: | f1f80e4 (Commits, GitHub, GitLab) | Commit: | f1f80e4e1a8c8f2e665794ab21a2c313b931c37f |
Dependencies: | Stopgaps: |
Description (last modified by )
currenly frobby is broken due to a C++ issue which Macaulay2 people know how to fix.
This ticket converts frobby into a new-style package and applies this fix, and other patches they came up with.
The upstream tarball is here: http://www.broune.com/frobby/frobby_v0.9.0.tar.gz
Change History (24)
comment:1 Changed 5 years ago by
- Cc vdelecroix added
comment:2 Changed 5 years ago by
- Branch set to u/dimpase/frobbyupdate
- Commit set to 1680e35d25c64b949da2103f926112afb2afbbb2
comment:3 Changed 5 years ago by
- Commit changed from 1680e35d25c64b949da2103f926112afb2afbbb2 to 339252f5c9bb9a6d08fb726d9eb980b6a26a9ddc
Branch pushed to git repo; I updated commit sha1. New commits:
339252f | All M2 patches integrated, rpath to link correctly
|
comment:4 Changed 5 years ago by
- Description modified (diff)
- Status changed from new to needs_review
SPKG_CHECK=yes sage -i frobby
returns OK, and all the tests in the frobby interface pass. There are some C++ issues in the debugging version of the program, but they can be ignored for our purposes.
comment:5 Changed 5 years ago by
- Cc mkoeppe added
comment:6 follow-up: ↓ 7 Changed 5 years ago by
Does frobby have any dependencies that should be listed?
comment:7 in reply to: ↑ 6 Changed 5 years ago by
Replying to embray:
Does frobby have any dependencies that should be listed?
it only depends on a standard Sage package (MPIR - which is impersonating GMP here).
comment:8 follow-up: ↓ 10 Changed 5 years ago by
Probably still good to add a dependencies
file for that. You can use $(MP_LIBRARY)
for this.
comment:9 Changed 5 years ago by
- Commit changed from 339252f5c9bb9a6d08fb726d9eb980b6a26a9ddc to f1f80e4e1a8c8f2e665794ab21a2c313b931c37f
Branch pushed to git repo; I updated commit sha1. New commits:
f1f80e4 | frobby needs mpir/gmp
|
comment:10 in reply to: ↑ 8 Changed 5 years ago by
Replying to embray:
Probably still good to add a
dependencies
file for that. You can use$(MP_LIBRARY)
for this.
done. Indeed, this seems to be the correct thing to do.
comment:11 Changed 5 years ago by
Upstream tarball = ?
comment:12 Changed 5 years ago by
- Description modified (diff)
comment:13 follow-up: ↓ 15 Changed 5 years ago by
sage -f -c frobby
gives:
[frobby-0.9.0.p2] Running the test suite for frobby-0.9.0.p2... [frobby-0.9.0.p2] g++ -Wall -I /Users/mkoeppe/cvs/sage/local/include -Wno-uninitialized -Wno-unused-parameter -std=c++11 -g -D DEBUG -fno-inline -Wextra -Wno-uninitialized -Wno-unused-parameter -c src/main.cpp -o bin/debug/main.o [frobby-0.9.0.p2] In file included from /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/map:60:0, [frobby-0.9.0.p2] from src/CliParams.h:24, [frobby-0.9.0.p2] from src/Action.h:21, [frobby-0.9.0.p2] from src/main.cpp:20: [frobby-0.9.0.p2] /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/bits/stl_tree.h: In member function 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_create_node(_Args&& ...)': [frobby-0.9.0.p2] /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/bits/stl_tree.h:420:14: error: '__tmp' does not name a type [frobby-0.9.0.p2] ::new(__tmp) _Rb_tree_node<_Val>; [frobby-0.9.0.p2] ^ [frobby-0.9.0.p2] make[2]: *** [bin/debug/main.o] Error 1
but then it happily continues ("All normal tests passed.")
comment:14 follow-up: ↓ 16 Changed 5 years ago by
Also, there is the optional frobgrob script, which depends on 4ti2. See the manual
comment:15 in reply to: ↑ 13 Changed 5 years ago by
Replying to mkoeppe:
sage -f -c frobby
gives:[frobby-0.9.0.p2] Running the test suite for frobby-0.9.0.p2... [frobby-0.9.0.p2] g++ -Wall -I /Users/mkoeppe/cvs/sage/local/include -Wno-uninitialized -Wno-unused-parameter -std=c++11 -g -D DEBUG -fno-inline -Wextra -Wno-uninitialized -Wno-unused-parameter -c src/main.cpp -o bin/debug/main.o [frobby-0.9.0.p2] In file included from /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/map:60:0, [frobby-0.9.0.p2] from src/CliParams.h:24, [frobby-0.9.0.p2] from src/Action.h:21, [frobby-0.9.0.p2] from src/main.cpp:20: [frobby-0.9.0.p2] /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/bits/stl_tree.h: In member function 'std::_Rb_tree_node<_Val>* std::_Rb_tree<_Key, _Val, _KeyOfValue, _Compare, _Alloc>::_M_create_node(_Args&& ...)': [frobby-0.9.0.p2] /Users/mkoeppe/cvs/sage/local/include/c++/4.9.2/bits/stl_tree.h:420:14: error: '__tmp' does not name a type [frobby-0.9.0.p2] ::new(__tmp) _Rb_tree_node<_Val>; [frobby-0.9.0.p2] ^ [frobby-0.9.0.p2] make[2]: *** [bin/debug/main.o] Error 1but then it happily continues ("All normal tests passed.")
yes, I have seen this. This concerns debugging version of the program only, IMHO. My C++ is too rusty to get any idea what goes wrong there, besides vague recollections of "iterator traits" etc I got nightmares about... Hopefully it's OK as it is.
comment:16 in reply to: ↑ 14 ; follow-up: ↓ 17 Changed 5 years ago by
Replying to mkoeppe:
Also, there is the optional frobgrob script, which depends on 4ti2. See the manual
should 4ti2 be in frobby dependencies?
comment:17 in reply to: ↑ 16 Changed 5 years ago by
Replying to dimpase:
Replying to mkoeppe:
Also, there is the optional frobgrob script, which depends on 4ti2. See the manual
should 4ti2 be in frobby dependencies?
If I understand the manual correctly, 4ti2 is only used by the frobgrob script, which is currently not installed. I think it should be installed; and 4ti2 should be a "recommended package" rather than a real dependency (like those many packages that one "should" install before polymake).
comment:18 Changed 5 years ago by
(However, Debian for example does not install "frobgrob".)
comment:19 Changed 5 years ago by
Judging only from the warning, I think the warning is coming from the fact that the forward declaration of new
in the header has an input parameter __tmp
that not been given an explicit type. Just add the type given in the implementation to the header and it should go away.
comment:20 Changed 5 years ago by
- Status changed from needs_review to positive_review
Follow-up (frobgrob): #21004.
comment:22 Changed 5 years ago by
- Reviewers set to Matthias Koeppe
comment:23 Changed 5 years ago by
- Status changed from needs_work to positive_review
comment:24 Changed 5 years ago by
- Branch changed from u/dimpase/frobbyupdate to f1f80e4e1a8c8f2e665794ab21a2c313b931c37f
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
initial commit and a minimal patch, so that it builds