New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[gr-fec] include GSL_LDFLAGS in CMakeLists to avoid linking errors in dependent in-trees #1490

Merged
merged 1 commit into from Feb 3, 2018

Conversation

Projects
None yet
3 participants
@sbmueller
Member

sbmueller commented Oct 21, 2017

These changes are required in order to build GR on macOS 10.13 with MacPorts. Also tested on Fedora 25, but further testing e.g. on Ubuntu would be sensible. If these changes are not provided, I get a linker error: ld: library not found for -lgsl. @bastibl hope that doesn't interfere with #1247, where you actually deleted this.

@bastibl

This comment has been minimized.

Member

bastibl commented Oct 21, 2017

Can you make sure that your GSL libraries are properly installed, in particular, that the install name is set. For me it looks like this.

otool -D libgsl.dylib 
libgsl.dylib:
/usr/local/opt/gsl@1/lib/libgsl.0.dylib

If it doesn't use an absolute path, you can try changing the install name with something like

install_name_tool -id "<absolute path>" libgsl.dylib
@sbmueller

This comment has been minimized.

Member

sbmueller commented Oct 21, 2017

Installation looks fine to me:

otool -D libgsl.dylib
libgsl.dylib:
/opt/local/lib/libgsl.23.dylib

Can you elaborate why you deleted ${GSL_LDFLAGS}? Is there a case where it'll break anything? I've noticed it is still there for other in-trees like gr-fec/swig/CMakeLists.txt (without GSL_FOUND in gr-fec/CMakeLists.txt).

@bastibl

This comment has been minimized.

Member

bastibl commented Oct 21, 2017

Can you please do VERBOSE=1 make and post the compile command. Atm, I don't find where -lgsl should come from.

@sbmueller

This comment has been minimized.

Member

sbmueller commented Oct 21, 2017

One entry of the (huge) output is the linking of the gr-atsc swig stuff:

[ 97%] Linking CXX shared module _atsc_swig.so
cd /Users/senpo/gnuradio/src/gnuradio/build/gr-atsc/swig && /opt/local/bin/cmake -E cmake_link_script CMakeFiles/_atsc_swig.dir/link.txt --verbose=1
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -O3 -DNDEBUG -bundle -Wl,-headerpad_max_install_names  -o _atsc_swig.so CMakeFiles/_atsc_swig.dir/atsc_swigPYTHON_wrap.cxx.o -Wl,-rpath,/Users/senpo/gnuradio/lib -undefined dynamic_lookup ../lib/libgnuradio-atsc.3.7.12git.dylib ../../gr-analog/lib/libgnuradio-analog.3.7.12git.dylib ../../gr-filter/lib/libgnuradio-filter.3.7.12git.dylib ../../gr-fft/lib/libgnuradio-fft.3.7.12git.dylib /opt/local/lib/libfftw3f.dylib /opt/local/lib/libfftw3f_threads.dylib ../../gr-fec/lib/libgnuradio-fec.3.7.12git.dylib ../../gr-blocks/lib/libgnuradio-blocks.3.7.12git.dylib ../../gnuradio-runtime/lib/libgnuradio-runtime.3.7.12git.dylib ../../gnuradio-runtime/lib/pmt/libgnuradio-pmt.3.7.12git.dylib ../../volk/lib/libvolk.1.3.dylib /opt/local/lib/liborc-0.4.dylib -lgsl -lgslcblas -lm /opt/local/lib/libboost_date_time-mt.dylib /opt/local/lib/libboost_program_options-mt.dylib /opt/local/lib/libboost_filesystem-mt.dylib /opt/local/lib/libboost_system-mt.dylib /opt/local/lib/libboost_regex-mt.dylib /opt/local/lib/libboost_thread-mt.dylib /opt/local/lib/libboost_chrono-mt.dylib /opt/local/lib/libboost_atomic-mt.dylib /opt/local/lib/liblog4cpp.dylib 

Clang is invoked with the -lgsl flag. This is the case for gr-wavelet, gr-fec, gr-dtv and gr-atsc, where the flag is used in every clang command.
Error yields:

ld: library not found for -lgsl
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [gr-atsc/swig/_atsc_swig.so] Error 1
make[1]: *** [gr-atsc/swig/CMakeFiles/_atsc_swig.dir/all] Error 2

${GSL_LDFLAGS} is missing in gr-dtv's and gr-atsc's swig/CMakeLists.txt while it is there for gr-wavelet and gr-fec, so the problem just occurs in the first two of those in-trees.

@bastibl

This comment has been minimized.

Member

bastibl commented Oct 22, 2017

Sorry for annoying you with this, but, in stead of your patch, can you please try changing GSL_LIBRARIES to GSL_LDFLAGS here:

https://github.com/gnuradio/gnuradio/blob/master/gr-fec/lib/CMakeLists.txt#L117

@sbmueller

This comment has been minimized.

Member

sbmueller commented Oct 22, 2017

@bastibl you're not annoying, I'm glad you're looking into this! :) Your proposed change worked, I'll follow up with a commit including this.

@sbmueller sbmueller force-pushed the sbmueller:libgsl branch from 99ba69e to db6b8db Oct 22, 2017

@sbmueller sbmueller changed the title from [gr-dtv/gr-atsc] add gsl libraries to swig cmake files to [gr-fec] include GSL_LDFLAGS in CMakeLists to avoid linking errors in dependent in-trees Oct 22, 2017

@bastibl

This comment has been minimized.

Member

bastibl commented Oct 22, 2017

Great, I think that's the better approach. It changes gr-fec to allow using it as (what cmake calls) transitive library dependency. This way, we don't have to clutter cmake files with library flags for indirect dependencies. This might be easier to maintain.

@mbr0wn mbr0wn requested a review from bastibl Feb 3, 2018

@mbr0wn

This comment has been minimized.

Member

mbr0wn commented Feb 3, 2018

@bastibl @sbmueller Can you two please sort this out? @bastibl can you approve this PR when you're happy? Then, @marcusmueller or I will merge.

@bastibl

This comment has been minimized.

Member

bastibl commented Feb 3, 2018

I'm happy with this. The last commit is as suggested.

@bastibl

bastibl approved these changes Feb 3, 2018

@mbr0wn mbr0wn merged commit acb0464 into gnuradio:master Feb 3, 2018

4 of 5 checks passed

bb/Ubuntu_14_04_64_py2_qt4_next Build done.
Details
bb/Ubuntu_14_04_64_py2_qt4_master Build done.
Details
bb/Ubuntu_16_04_64_py2_qt4_master Build done.
Details
bb/Ubuntu_16_04_64_py2_qt5_next Build done.
Details
bb/pull_request_runner Build done.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment