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:

Status badges

Description (last modified by dimpase)

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 vdelecroix

  • Cc vdelecroix added

comment:2 Changed 5 years ago by dimpase

  • Branch set to u/dimpase/frobbyupdate
  • Commit set to 1680e35d25c64b949da2103f926112afb2afbbb2

New commits:

1680e35initial commit and a minimal patch, so that it builds

comment:3 Changed 5 years ago by git

  • Commit changed from 1680e35d25c64b949da2103f926112afb2afbbb2 to 339252f5c9bb9a6d08fb726d9eb980b6a26a9ddc

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

339252fAll M2 patches integrated, rpath to link correctly

comment:4 Changed 5 years ago by dimpase

  • 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 dimpase

  • Authors set to Dima Pasechnik
  • Cc mkoeppe added

comment:6 follow-up: Changed 5 years ago by embray

Does frobby have any dependencies that should be listed?

comment:7 in reply to: ↑ 6 Changed 5 years ago by dimpase

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: Changed 5 years ago by embray

Probably still good to add a dependencies file for that. You can use $(MP_LIBRARY) for this.

comment:9 Changed 5 years ago by git

  • Commit changed from 339252f5c9bb9a6d08fb726d9eb980b6a26a9ddc to f1f80e4e1a8c8f2e665794ab21a2c313b931c37f

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

f1f80e4frobby needs mpir/gmp

comment:10 in reply to: ↑ 8 Changed 5 years ago by dimpase

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 mkoeppe

Upstream tarball = ?

comment:12 Changed 5 years ago by dimpase

  • Description modified (diff)

comment:13 follow-up: Changed 5 years ago by 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 1

but then it happily continues ("All normal tests passed.")

comment:14 follow-up: Changed 5 years ago by mkoeppe

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 dimpase

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 1

but 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: Changed 5 years ago by dimpase

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 mkoeppe

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 mkoeppe

(However, Debian for example does not install "frobgrob".)

comment:19 Changed 5 years ago by tscrim

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 mkoeppe

  • Status changed from needs_review to positive_review

Follow-up (frobgrob): #21004.

comment:21 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name

comment:22 Changed 5 years ago by mkoeppe

  • Reviewers set to Matthias Koeppe

comment:23 Changed 5 years ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:24 Changed 5 years ago by vbraun

  • Branch changed from u/dimpase/frobbyupdate to f1f80e4e1a8c8f2e665794ab21a2c313b931c37f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.