Opened 5 years ago
Closed 5 years ago
#24015 closed defect (fixed)
fix libhomfly on OSX
Reported by: | dimpase | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | sage-8.2 |
Component: | packages: optional | Keywords: | |
Cc: | embray, mmarco | Merged in: | |
Authors: | Dima Pasechnik | Reviewers: | Jeroen Demeyer |
Report Upstream: | Completely fixed; Fix reported upstream | Work issues: | |
Branch: | f3349e0 (Commits, GitHub, GitLab) | Commit: | f3349e0fa9584b4a37918fed0cd370bb1a3f4cbf |
Dependencies: | Stopgaps: |
Description (last modified by )
as reported on sage-devel linking on OSX fails with "duplicate symbol" errors.
[libhomfly-1.0r2] libtool: link: cc -dynamiclib -Wl,-undefined -Wl,dynamic_lookup -o .libs/libhomfly.0.dylib .libs/bound.o .libs/control.o .libs/dllink.o .libs/homfly.o .libs/knot.o .libs/model.o .libs/order.o .libs/poly.o -lgc -L/Volumes/Sage/sage/local/lib -g -O2 -Wl,-rpath -Wl,/Volumes/Sage/sage/local/lib -install_name /Volumes/Sage/sage/local/lib/libhomfly.0.dylib -compatibility_version 1 -current_version 1.0 -Wl,-single_module -Wl,-exported_symbols_list,.libs/libhomfly-symbols.expsym [libhomfly-1.0r2] duplicate symbol _first in: [libhomfly-1.0r2] .libs/bound.o [libhomfly-1.0r2] .libs/control.o [libhomfly-1.0r2] duplicate symbol _list in: [libhomfly-1.0r2] .libs/bound.o [libhomfly-1.0r2] .libs/control.o ...
there is also an error in poly.c, where a non-void function returns nothing. This is trivially fixed by
diff --git a/lib/poly.c b/lib/poly.c index 48e9a68..307d1de 100644 --- a/lib/poly.c +++ b/lib/poly.c @@ -76,7 +76,7 @@ char *p_show(Poly *p) { fprintf(stream, "0"); fclose(stream); - return; + return NULL; } for (first = 1, pt = p->term, pend = pt+p->len; pt != pend; ++pt)
As it was being fixed, more problems were uncovered, and then a pesky GC bug, that went away after adding an explicit GC_INIT()
call.
It appears that all is fixed by https://github.com/miguelmarco/libhomfly/pull/11
the (official!) tarball is here: https://github.com/miguelmarco/libhomfly/releases/download/1.02r4/libhomfly-1.02r4.tar.gz
Change History (19)
comment:1 Changed 5 years ago by
Description: | modified (diff) |
---|
comment:2 Changed 5 years ago by
Authors: | → Dima Pasechnik |
---|
comment:4 Changed 5 years ago by
Report Upstream: | N/A → Reported upstream. No feedback yet. |
---|
C changes proposed to upstream: https://github.com/miguelmarco/libhomfly/pull/8
comment:5 Changed 5 years ago by
Report Upstream: | Reported upstream. No feedback yet. → Fixed upstream, in a later stable release. |
---|
still, for OSX we need to update GC to a very recent version, see https://github.com/miguelmarco/libhomfly/pull/8 for details.
comment:6 Changed 5 years ago by
Branch: | → u/dimpase/libhomflyfix |
---|---|
Commit: | → a3c3ac74800c71d07316bdc273f6cb8ada30504c |
Description: | modified (diff) |
Milestone: | sage-8.1 → sage-8.2 |
Status: | new → needs_review |
the last remaining problem was the lack of GC_INIT()
call in the library (something that is apparently not needed on Linux, but the docs
say that that some platforms might need it...)
With https://github.com/miguelmarco/libhomfly/pull/11 it is all fixed, also on the current Sage's GC version on OSX.
New commits:
a3c3ac7 | new version and checksum
|
comment:7 Changed 5 years ago by
NB: the tarball is just a proof of concept, corresponding to commit b68b5b2aac2e89b17312208080562f5754d3da2d on https://github.com/dimpase/libhomfly
Once the upstream is done with merging, we should get the official tarball...
comment:8 Changed 5 years ago by
I made a new release upstream. It is https://github.com/miguelmarco/libhomfly/releases/tag/1.02r4
The tarball is at https://github.com/miguelmarco/libhomfly/archive/1.02r4.tar.gz
comment:9 Changed 5 years ago by
Commit: | a3c3ac74800c71d07316bdc273f6cb8ada30504c → c623d6b4355df57d8e53f4fb9ea6299704d19321 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c623d6b | new version and checksum
|
comment:10 Changed 5 years ago by
Commit: | c623d6b4355df57d8e53f4fb9ea6299704d19321 → 5b5e57b59b5e9f7cbf071e704720def4b3bbab94 |
---|
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
5b5e57b | new version and checksum; minor SPKG.txt update
|
comment:11 Changed 5 years ago by
Commit: | 5b5e57b59b5e9f7cbf071e704720def4b3bbab94 → 7f277d991bc884663782ff23b5663b103b7d4ed2 |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
7f277d9 | tarball needs autoreconf run, thus updated
|
comment:12 Changed 5 years ago by
Description: | modified (diff) |
---|
I had to update the tarball from github by running autoreconf -if
and re-tarring then.
Should all be ready now.
comment:13 Changed 5 years ago by
Description: | modified (diff) |
---|---|
Summary: | fix libhomfly building on OSX → fix libhomfly on OSX |
One little detail about GC_INIT()
: it is not needed on the current Boehm GC master at https://github.com/ivmai/bdwgc, but is needed on few releases (7.4 and 7.6), as well as on the current Sage's version of GC.
No it builds with SAGE_CHECK=yes
, and all the relevant Sage tests pass, too, on OSX 10.12 as well as on Linux.
comment:14 Changed 5 years ago by
Commit: | 7f277d991bc884663782ff23b5663b103b7d4ed2 → f3349e0fa9584b4a37918fed0cd370bb1a3f4cbf |
---|
Branch pushed to git repo; I updated commit sha1. New commits:
f3349e0 | use the official tarball
|
comment:15 Changed 5 years ago by
Description: | modified (diff) |
---|
OK, so now even the tarball is official!
comment:16 Changed 5 years ago by
Report Upstream: | Fixed upstream, in a later stable release. → None of the above - read trac for reasoning. |
---|
I think we need a menu choice saying "Fixes fully merged in the current upstream release". :-)
comment:17 Changed 5 years ago by
Report Upstream: | None of the above - read trac for reasoning. → Completely fixed; Fix reported upstream |
---|
comment:18 Changed 5 years ago by
Reviewers: | → Jeroen Demeyer |
---|---|
Status: | needs_review → positive_review |
comment:19 Changed 5 years ago by
Branch: | u/dimpase/libhomflyfix → f3349e0fa9584b4a37918fed0cd370bb1a3f4cbf |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
I have a fix for the C code madness, now onto some cython trouble, hopefully minor: