JohnCremona / eclib

The eclib package includes mwrank (for 2-descent on elliptic curves over Q) and modular symbol code used to create the elliptic curve database.
GNU General Public License v2.0
21 stars 16 forks source link

Building with --with-boost fails due to missing -lpthread #41

Closed antonio-rojas closed 5 years ago

antonio-rojas commented 6 years ago

When building with boost enabled, linking fails

libtool: link: g++ -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z -Wl,relro -Wl,-z -Wl,now -o .libs/nfhpcurve nfhpcurve.o  -L/usr/lib -L/usr/lib64 ../libsrc/.libs/libec.so -lflint -lpari -lntl -lboost_system -lboost_thread
/usr/bin/ld: ecnf.o: undefined reference to symbol 'pthread_join@@GLIBC_2.2.5'
/usr/lib/libpthread.so.0: error adding symbols: DSO missing from command line

Adding -lpthread to LDFLAGS fixes it

JohnCremona commented 6 years ago

That is strange since I did add -pthread to CFLAGS (see line 120 of configure.ac) in order to compile with NTL but I did not need to add -lpthread to LDFLAGS. If you are able to see what change I should have made to configure.ac, or elsewhere, I would appreciate it.

SnarkBoojum commented 6 years ago

Hmmmm... if you add -pthread to CFLAGS, that means the flag doesn't get used by the linking lines, and hence it's normal to have issues at link-stage.

Trying to add it by hand doesn't look honest: isn't there a configure.ac macro which will take care of setting all flags correctly and in a portable fashion?

JohnCremona commented 6 years ago

I have now added -lpthread to LDFLAGS "manually" too and it works for me but then that was also true before. This looks relevant: https://stackoverflow.com/questions/17055279/autotools-for-pthreads-not-setting-correct-linker-flags
but I cannot test that now as I'm about to go on holiday.

JohnCremona commented 6 years ago

The version tagged v20180815 fixes this (it works for me and even make distcheck works now).

antonio-rojas commented 6 years ago

No, it doesn't fix it for me.

antonio-rojas commented 6 years ago

It turns out that this is caused by not having the ax_pthread.m4 macro installed. This should probably be shipped with eclib's source.

JohnCremona commented 5 years ago

Surely not: on my systems this file is in /usr/share/aclocal so is a system file, which (in ubuntu) is in the package autoconf-archive.

If I download the tarball https://github.com/JohnCremona/eclib/archive/v20180815.tar.gz, extract it and cd into the directory, then run libtoolize; autoreconf -i then configure and make work. This is on ubuntu 16.04.

JohnCremona commented 5 years ago

@antonio-rojas Is this now fixed for you in the current master branch?

antonio-rojas commented 5 years ago

Yes, it works with autoconf-archive

antonio-rojas commented 5 years ago

This is broken again after 207f8fa4250591d6d0b95de8a1400747657a12dc

The variable PTHREAD_LDFLAGS is not defined anywhere

JohnCremona commented 5 years ago

A patch would be appreciated.

antonio-rojas commented 5 years ago

Just revert 207f8fa4250591d6d0b95de8a1400747657a12dc I guess? Not sure which issues it was meant to fix, there are no details in the commit message.

JohnCremona commented 5 years ago

The code as is, and configured --with-boost, works fine for me on one machine (ubuntu 18.04.1) but not on another (ubuntu 16.04.5). Same with plain "./configure". And Travis is happy with the linux builds, and was happy with osx until they stopped supporting the osx version which was in the .travis file. What a pain. I am making no more changes in the near future, anyone else is of course welcome to.

JohnCremona commented 5 years ago

To contradict myself once more, the latest commit d702f44 which even got a new tag v20190205.1 now works for me on both machines. It seems wrong to me that one has to add $(PTHREAD_CFLAGS) into AM_LDFLAGS, surely PTHREAD_LDFLAGS shuold be correct. And it is on some platforms. I am hoping to hear some good news now; if so I'll update today's release.

antonio-rojas commented 5 years ago

Thanks, it's all good now - even without autoconf-archive