erikerlandson / st_tree

A fast and flexible c++ template class for tree data structures
https://github.com/erikerlandson/st_tree/wiki
Apache License 2.0
96 stars 20 forks source link

Ensure boost tests are linked statically or dynamically #23

Closed emmenlau closed 4 years ago

emmenlau commented 4 years ago

The flag BOOST_TEST_DYN_LINK should only be defined on shared library builds and not defined on static builds. This PR makes the flag dependent on the cmake setting for shared or static builds.

erikerlandson commented 4 years ago

This now only builds for me if I explicitly set cmake -DBUILD_SHARED_LIBS=TRUE .

emmenlau commented 4 years ago

Haha ok, that is quite unexpected. I will check whats going on!

emmenlau commented 4 years ago

Sorry to bug you with this issue, but could you post the error message, and whether you have any special boost options set for cmake? For me this builds not both static and shared...

erikerlandson commented 4 years ago

Sure, here's what I did. I did not use anything except cmake .:

[eje@localhost st_tree]$ git checkout pr23
Switched to branch 'pr23'
[eje@localhost st_tree]$ git clean -fdx

...

[eje@localhost st_tree]$ cmake .
-- The C compiler identification is GNU 8.3.1
-- The CXX compiler identification is GNU 8.3.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Boost version: 1.66.0
-- Found the following Boost libraries:
--   unit_test_framework
-- Configuring done
-- Generating done
-- Build files have been written to: /home/eje/git/st_tree

[eje@localhost st_tree]$ make 

...

[100%] Linking CXX executable unit_tests
/usr/bin/ld: /usr/lib/gcc/x86_64-redhat-linux/8/../../../../lib64/crt1.o: in function `_start':
(.text+0x24): undefined reference to `main'
collect2: error: ld returned 1 exit status
make[2]: *** [tests/CMakeFiles/unit_tests.dir/build.make:160: tests/unit_tests] Error 1
make[1]: *** [CMakeFiles/Makefile2:431: tests/CMakeFiles/unit_tests.dir/all] Error 2
make: *** [Makefile:130: all] Error 2
erikerlandson commented 4 years ago

This looks like it might be relevant: https://stackoverflow.com/questions/7781374/boost-test-linking

emmenlau commented 4 years ago

Ok I think I understand the problem a bit better now. It seems the flag BOOST_TEST_DYN_LINK should not depend on whether st_tree is build static vs dynamic as I previously assumed. But it should depend on whether st_tree links against a boost that is static vs dynamic.

However I'm slightly puzzled as to how users are expected to change this flag based on what kind of boost libraries are available. I could not find a cmake variable that indicates if boost was found static or dynamic.

So in summary, the build fails for you if BOOST_TEST_DYN_LINK is not set and fails for me if BOOST_TEST_DYN_LINK is set, and there is no easy way to set BOOST_TEST_DYN_LINK dynamically in cmake.

As a workaround, I've fixed this partially. The default behavior should continue to work for you. But if a user explicitly requests a static boost via cmake option -DBoost_USE_STATIC_LIBS=ON, then the boost flag BOOST_TEST_DYN_LINK will not be enabled. This helps me to build st_tree successfully.

Does that work for you, and seem reasonable?

erikerlandson commented 4 years ago

That seems to work, thanks!