Opened 4 years ago

Closed 4 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:

Status badges

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 4 years ago by dimpase

  • Description modified (diff)

comment:2 Changed 4 years ago by dimpase

  • Authors set to 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 4 years ago by dimpase

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

comment:4 Changed 4 years ago by dimpase

  • Report Upstream changed from N/A to Reported upstream. No feedback yet.

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

comment:5 Changed 4 years ago by dimpase

  • Report Upstream changed from Reported upstream. No feedback yet. to 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 4 years ago by dimpase

  • Branch set to u/dimpase/libhomflyfix
  • Commit set to a3c3ac74800c71d07316bdc273f6cb8ada30504c
  • Description modified (diff)
  • Milestone changed from sage-8.1 to sage-8.2
  • Status changed from new to 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:

a3c3ac7new version and checksum

comment:7 Changed 4 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 4 years ago by git

  • Commit changed from a3c3ac74800c71d07316bdc273f6cb8ada30504c to c623d6b4355df57d8e53f4fb9ea6299704d19321

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

c623d6bnew version and checksum

comment:10 Changed 4 years ago by git

  • Commit changed from c623d6b4355df57d8e53f4fb9ea6299704d19321 to 5b5e57b59b5e9f7cbf071e704720def4b3bbab94

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 4 years ago by git

  • Commit changed from 5b5e57b59b5e9f7cbf071e704720def4b3bbab94 to 7f277d991bc884663782ff23b5663b103b7d4ed2

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

7f277d9tarball needs autoreconf run, thus updated

comment:12 Changed 4 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 4 years ago by dimpase

  • Description modified (diff)
  • Summary changed from fix libhomfly building on OSX to 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 4 years ago by git

  • Commit changed from 7f277d991bc884663782ff23b5663b103b7d4ed2 to f3349e0fa9584b4a37918fed0cd370bb1a3f4cbf

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

f3349e0use the official tarball

comment:15 Changed 4 years ago by dimpase

  • Description modified (diff)

OK, so now even the tarball is official!

comment:16 Changed 4 years ago by dimpase

  • Report Upstream changed from Fixed upstream, in a later stable release. to 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 4 years ago by jdemeyer

  • Report Upstream changed from None of the above - read trac for reasoning. to Completely fixed; Fix reported upstream

comment:18 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:19 Changed 4 years ago by vbraun

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