Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#42152 closed defect (fixed)

libbiosig: fails to build on snow leopard (Undefined symbols: _getline)

Reported by: mojca (Mojca Miklavec) Owned by: neurodroid (Christoph Schmidt-Hieber)
Priority: Normal Milestone:
Component: ports Version:
Keywords: snowleopard leopard tiger Cc: ryandesign (Ryan Carsten Schmidt)
Port: libbiosig

Description

The build on the Snow Leopard buildbot fails with:

[100%] Built target biosigstatic
Undefined symbols:
  "_getline", referenced from:
      _sread_atf in sopen_abf_read.c.o
      _sopen in biosig.c.o
      _sopen in biosig.c.o
      _sopen in biosig.c.o
      _sopen in biosig.c.o
      _sopen in biosig.c.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
make[2]: *** [libbiosig.dylib] Error 1
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
make[1]: *** [CMakeFiles/biosigshared.dir/all] Error 2
make[1]: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
make: *** [all] Error 2

See http://build.macports.org/builders/buildports-snowleopard-x86_64/builds/24062.

(I moved this into a new ticket from #42147.)

Attachments (4)

patch-CMakeLists.txt.diff (1.1 KB) - added by neurodroid (Christoph Schmidt-Hieber) 11 years ago.
libbiosig patch without getline from win32
Portfile-libbiosig.patch (917 bytes) - added by neurodroid (Christoph Schmidt-Hieber) 11 years ago.
libbiosig Portfile patch using gettext to provide getline on SL
patch-CMakeLists-v2.txt.diff (2.1 KB) - added by neurodroid (Christoph Schmidt-Hieber) 11 years ago.
patch for biosig 1.5.10 to build with MacPorts on all OS X versions
patch-v3.diff (2.7 KB) - added by neurodroid (Christoph Schmidt-Hieber) 11 years ago.
More general patch that uses the private getline implementation only on SL and independently of MacPorts.

Download all attachments as: .zip

Change History (27)

comment:1 Changed 11 years ago by mojca (Mojca Miklavec)

Just as a reminder: when this gets fixed, one also needs to force a rebuild of stimfit on SL (or commit any change on the Portfile).

comment:2 Changed 11 years ago by mojca (Mojca Miklavec)

See also #41694, #30785 and possibly others. It seems that getline is not available on 10.6 and earlier, so the upstream project needs to take that into account and be fixed accordingly.

If you want to make stimfit work on 10.6 faster than you can debug, it might be best bet to only make +biosig default (or maybe even only available) on 10.7 and later until you resolve the problem with getline.

comment:3 in reply to:  2 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Replying to mojca@…:

See also #41694, #30785 and possibly others. It seems that getline is not available on 10.6 and earlier, so the upstream project needs to take that into account and be fixed accordingly.

Yes, the problem (that I was not aware of) is all over the place. See also https://stackoverflow.com/questions/1117108/compiling-c-code-using-gnu-c-getline-on-mac-osx biosig has a getline implementation for win32 that we could make use of. I will try to add that to the cmake file.

If you want to make stimfit work on 10.6 faster than you can debug, it might be best bet to only make +biosig default (or maybe even only available) on 10.7 and later until you resolve the problem with getline.

If the getline fix isn't straightforward I'll do that.

comment:4 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Tentative fix using getline/getdelim/getlogin from win32: http://sourceforge.net/p/biosig/code/ci/32bd3b92ea6c8dc7027a180377fc9f671d88819b/

Would you like me to submit a separate Portfile patch here?

comment:5 Changed 11 years ago by mojca (Mojca Miklavec)

No need for a separate patch. I committed r116018.

I few changes I made:

  • I don't see any reason to fetch tiny patch files from external server, so I included the patch in repository.
  • I didn't increase the revision. There is no need to as it only fixes a compile failure and doesn't change any functionality. Those who already have a working version (including the buildbot) don't need to recompile anything.

comment:6 Changed 11 years ago by mojca (Mojca Miklavec)

But the patch doesn't work. See https://build.macports.org/builders/buildports-snowleopard-x86_64/builds/24064:

[ 66%] [ 69%] [ 71%] Building C object CMakeFiles/biosigshared.dir/win32/getline.c.o
Building C object CMakeFiles/biosigshared.dir/win32/getdelim.c.o
Building CXX object CMakeFiles/biosigstatic.dir/XMLParser/tinyxml.cpp.o
/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10/win32/getline.c:22: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘getline’
make[2]: *** [CMakeFiles/biosigshared.dir/win32/getline.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10/win32/getdelim.c:55: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘getdelim’
[ 73%] make[2]: *** [CMakeFiles/biosigshared.dir/win32/getdelim.c.o] Error 1
[ 76%] [ 78%] Building CXX object CMakeFiles/biosigstatic.dir/XMLParser/tinyxmlparser.cpp.o
Building CXX object CMakeFiles/biosigstatic.dir/XMLParser/tinyxmlerror.cpp.o
Building C object CMakeFiles/biosigstatic.dir/win32/getdelim.c.o
[ 80%] /opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10/win32/getdelim.c:55: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘getdelim’
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
Building C object CMakeFiles/biosigstatic.dir/win32/getline.c.o
make[1]: *** [CMakeFiles/biosigshared.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....
make[2]: *** [CMakeFiles/biosigstatic.dir/win32/getdelim.c.o] Error 1
make[2]: *** Waiting for unfinished jobs....
[ 83%] Building C object CMakeFiles/biosigstatic.dir/win32/getlogin.c.o
/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10/win32/getline.c:22: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘getline’
make[2]: *** [CMakeFiles/biosigstatic.dir/win32/getline.c.o] Error 1
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
make[1]: *** [CMakeFiles/biosigstatic.dir/all] Error 2
make[1]: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
make: *** [all] Error 2
make: Leaving directory `/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10'
Command failed:  cd "/opt/local/var/macports/build/_opt_mports_dports_science_libbiosig/libbiosig/work/biosig4c++-1.5.10" && /usr/bin/make -j8 -w all 
Exit code: 2

comment:7 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

*sigh

Will have to get my hands on a 10.6 machine then if blind patching doesn't cut it

Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Attachment: patch-CMakeLists.txt.diff added

libbiosig patch without getline from win32

comment:8 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

The attached patch and revised Portfile should provide getline on SL if the following:

$ nm /opt/local/lib/libgettextlib.dylib | grep getline

returns

U _getline

Currently setting up a 10.6 machine to say for sure but it may take a while until it's up and running.

Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Attachment: Portfile-libbiosig.patch added

libbiosig Portfile patch using gettext to provide getline on SL

comment:9 in reply to:  8 Changed 11 years ago by ryandesign (Ryan Carsten Schmidt)

Cc: ryandesign@… added
Keywords: snowleopard leopard tiger added

Replying to christsc@…:

The attached patch and revised Portfile should provide getline on SL if the following:

$ nm /opt/local/lib/libgettextlib.dylib | grep getline

returns

U _getline
$ nm /Volumes/Data/macports/snowleopard/lib/libgettextlib.dylib | grep getline
000000000001f0aa T _rpl_getline

comment:10 Changed 11 years ago by mojca (Mojca Miklavec)

Ryan, any help on this would be most welcome.

comment:11 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

So I have set up a 10.6 machine and I can confirm that linking against -lgettextlib fixes the problem.

comment:12 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

So sorry, just realized that I was using the wrong machine, this still isn't fixed.

Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

patch for biosig 1.5.10 to build with MacPorts on all OS X versions

comment:13 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

The attached patch (patch-CMakeLists-v2.txt.diff​) fixes a private implementation of getline that ships with biosig so that it can be compiled on Snow Leopard. The previous Portfile patch (Portfile-libbiosig.patch) is obsolete. The original Portfile from the svn repository (116018) should work (provided patch-CMakeLists.txt.diff​ is updated).

comment:14 Changed 11 years ago by mojca (Mojca Miklavec)

Before I commit anything: can you please explain me how the patch works?

What does the following do?

COMMAND ${GAWK} -f annotatedECG.awk extern/11073-10102-AnnexB.txt > "11073-10102-AnnexB.i"

and why is

#ifdef MACPORTS

needed? What does #include <unistd.h> do and where does that file come from? /usr/include/unistd.h or from /opt/local/sowhere/else?

I can commit it nonetheless, but I would like to roughly understand what I'm doing.

comment:15 Changed 11 years ago by mojca (Mojca Miklavec)

Also, if you can figure it out, I would be grateful for a livecheck code.

comment:16 in reply to:  14 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Replying to mojca@…:

Before I commit anything: can you please explain me how the patch works?

What does the following do?

COMMAND ${GAWK} -f annotatedECG.awk extern/11073-10102-AnnexB.txt > "11073-10102-AnnexB.i"

It generates some C definitions from a text description of IEEE Std 11073-10102-2012 (annotated ECG description) according to awk rules in annotatedECG.awk. This has already been added to the Linux Makefile (l.448) but hasn't made into cmake in time for release 1.5.10. I've committed it to the biosig repo so it will be included in release 1.5.11.

and why is

#ifdef MACPORTS

needed? What does #include <unistd.h> do and where does that file come from? /usr/include/unistd.h or from /opt/local/sowhere/else?

In Snow Leopard, ssize_t is defined in /usr/include/unistd.h. On Linux, including stdio.h seems to be sufficient to get ssize_t. I've committed this to the biosig repo as well so that this patch won't be necessary in 1.5.11

I can commit it nonetheless, but I would like to roughly understand what I'm doing.

Fair enough.

comment:17 Changed 11 years ago by mojca (Mojca Miklavec)

I will apply the patch, but I don't like the part of code that says

#ifdef MACPORTS

This needs to be solved in such a way that it will work outside of MacPorts as well.

comment:19 Changed 11 years ago by mojca (Mojca Miklavec)

Commit r116048 was successfully built on the buildbot.

comment:20 in reply to:  18 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Replying to mojca@…:

Approach of another project: https://dev.openwrt.org/ticket/9849, https://dev.openwrt.org/browser/trunk/tools/include/getline.h

I've committed a similar approach to the repo: https://sourceforge.net/p/biosig/code/ci/ac0a4d481703e48bbe2fe279ed682fae9e790acc/

The code is now only compiled on snow leopard and earlier, and unistd.h is also only included on snow leopard and earlier.

If you prefer this solution I could produce an updated patch.

Last edited 11 years ago by neurodroid (Christoph Schmidt-Hieber) (previous) (diff)

Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

Attachment: patch-v3.diff added

More general patch that uses the private getline implementation only on SL and independently of MacPorts.

comment:21 Changed 11 years ago by mojca (Mojca Miklavec)

The important part is that the upstream project works properly outside of MacPorts. Now that the project already builds on MacPorts and already has binary packages ready, we can just as well keep it that way (it won't make any difference at all) until the next package upgrade. (Unless you want me to apply this. Then I can do it.)

I also force-rebuilt stimfit on 10.6 successfully.

comment:22 Changed 11 years ago by mojca (Mojca Miklavec)

Resolution: fixed
Status: newclosed

comment:23 Changed 11 years ago by neurodroid (Christoph Schmidt-Hieber)

See ticket #42159 - in case the Portfile needs to be updated to resolve that ticket, the source patch could be updated at the same time.

Note: See TracTickets for help on using tickets.