Skip to content
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

Drop supporting Trusty and use imported targets for dependencies #1212

Merged
merged 52 commits into from
Jan 9, 2019

Conversation

jslee02
Copy link
Member

@jslee02 jslee02 commented Jan 8, 2019

This PR is a sequel of #1209, and more will be followed to modernize CMake scripts of DART. Here are two main changes in this PR.

  • Drop supporting Ubuntu 14.04 (Trusty) in DART 6.7. The main reason is to use the recent CMake features (e.g., target-based dependency management), which requires CMake >= 3. Also, this is to reduce the effort of supporting the too old version of dependencies.

  • Use IMPORTED targets for dependency libraries where available. For the dependencies that don't provide targets, define the targets in the finding modules, Find<package>.cmake. Use the same target name when the corresponding dependency provides the target in their latest version, which is not released yet.


Before merging a pull request

  • Set version target by selecting a milestone on the right side

when a downstream library imports DART. This is to import targets of the dependency libraries (if available) so that the dependency library is properly linked when the library is a target but not an absolute path to the library. Without this, simply linking to a dependency target will fail as #1200.
@scpeters
Copy link
Collaborator

scpeters commented Jan 9, 2019

I'm having trouble building this on mojave.

First it has trouble linking to assimp:

[ 65%] Linking CXX shared library ../lib/libdart.dylib
ld: library not found for -lassimp
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/libdart.6.7.0.dylib] Error 1

Then if I manually add the path to the assimp library to LIBRARY_PATH, it gets a little farther, then fails to find a framework:

make[2]: *** No rule to make target `/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/System/Library/Frameworks/GLUT.framework/GLUT', needed by `lib/libdart-gui.6.7.0.dylib'.  Stop.
make[1]: *** [dart/gui/CMakeFiles/dart-gui.dir/all] Error 2

@scpeters
Copy link
Collaborator

scpeters commented Jan 9, 2019

The release-6.7 branch builds fine on my mojave machine

@jslee02
Copy link
Member Author

jslee02 commented Jan 9, 2019

@scpeters I think there were two issues in building on Mojave, which should be fixed by the last few commits.

First, I switched to use the upstream config file of assimp, but they use a hard-coded library path that doesn't work for macOS. So I fall back to using own module file, Findassimp.cmake.

Second, GLUT::GLUT seems not working for Mojave or maybe for all the macOSs. This is because FindGLUT.cmake, provided by CMake, uses a different library path for the link library property of GLUT::GLUT from GLUT_LIBRARIES. The link library property of GLUT::GLUT additionally has /GLUT suffix, and it causes the build error you saw. So I just had to fall back to using variables for GLUT.

@scpeters
Copy link
Collaborator

scpeters commented Jan 9, 2019

I just tested 7c13f5d on our mojave test machine, and it builds successfully. Nice work!

@jslee02
Copy link
Member Author

jslee02 commented Jan 9, 2019

Thanks for the testing! Let me merge this PR once Travis CI becomes happy.

@jslee02 jslee02 merged commit d486a92 into release-6.7 Jan 9, 2019
@jslee02 jslee02 deleted the cmake_modernize branch January 9, 2019 23:59
jslee02 added a commit that referenced this pull request Jan 10, 2019
@scpeters scpeters mentioned this pull request Apr 16, 2019
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants