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:

GitHub link to the corresponding issue

Description (last modified by dimpase)

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 dimpase

Description: modified (diff)

comment:2 Changed 5 years ago by dimpase

Authors: Dima Pasechnik

I have a fix for the C code madness, now onto some cython trouble, hopefully minor:

sage -t --warn-long 231.9 src/sage/libs/homfly.pyx
**********************************************************************
File "src/sage/libs/homfly.pyx", line 65, in sage.libs.homfly.homfly_polynomial_string
Failed example:
    from sage.libs.homfly import homfly_polynomial_string # optional - libhomfly
Exception raised:
    Traceback (most recent call last):
      File "/Volumes/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/Volumes/Sage/sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.libs.homfly.homfly_polynomial_string[0]>", line 1, in <module>
        from sage.libs.homfly import homfly_polynomial_string # optional - libhomfly
    ImportError: dlopen(/Volumes/Sage/sage/local/lib/python2.7/site-packages/sage/libs/homfly.so, 2): Symbol not found: _fmemopen
      Referenced from: /Volumes/Sage/sage/local/lib/libhomfly.0.dylib
      Expected in: flat namespace
     in /Volumes/Sage/sage/local/lib/libhomfly.0.dylib
**********************************************************************

comment:3 Changed 5 years ago by dimpase

duh, there is no fmemopen on OSX, as simple as this...

comment:4 Changed 5 years ago by dimpase

Report Upstream: N/AReported upstream. No feedback yet.

C changes proposed to upstream: https://github.com/miguelmarco/libhomfly/pull/8

comment:5 Changed 5 years ago by dimpase

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 dimpase

Branch: u/dimpase/libhomflyfix
Commit: a3c3ac74800c71d07316bdc273f6cb8ada30504c
Description: modified (diff)
Milestone: sage-8.1sage-8.2
Status: newneeds_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:

a3c3ac7new version and checksum

comment:7 Changed 5 years ago by dimpase

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

Commit: a3c3ac74800c71d07316bdc273f6cb8ada30504cc623d6b4355df57d8e53f4fb9ea6299704d19321

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

c623d6bnew version and checksum

comment:10 Changed 5 years ago by git

Commit: c623d6b4355df57d8e53f4fb9ea6299704d193215b5e57b59b5e9f7cbf071e704720def4b3bbab94

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

5b5e57bnew version and checksum; minor SPKG.txt update

comment:11 Changed 5 years ago by git

Commit: 5b5e57b59b5e9f7cbf071e704720def4b3bbab947f277d991bc884663782ff23b5663b103b7d4ed2

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

7f277d9tarball needs autoreconf run, thus updated

comment:12 Changed 5 years ago by dimpase

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 dimpase

Description: modified (diff)
Summary: fix libhomfly building on OSXfix 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 git

Commit: 7f277d991bc884663782ff23b5663b103b7d4ed2f3349e0fa9584b4a37918fed0cd370bb1a3f4cbf

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

f3349e0use the official tarball

comment:15 Changed 5 years ago by dimpase

Description: modified (diff)

OK, so now even the tarball is official!

comment:16 Changed 5 years ago by dimpase

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 jdemeyer

Report Upstream: None of the above - read trac for reasoning.Completely fixed; Fix reported upstream

comment:18 Changed 5 years ago by jdemeyer

Reviewers: Jeroen Demeyer
Status: needs_reviewpositive_review

comment:19 Changed 5 years ago by vbraun

Branch: u/dimpase/libhomflyfixf3349e0fa9584b4a37918fed0cd370bb1a3f4cbf
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.