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

Fixed find_package(DART) on optimizer components #637

Merged
merged 1 commit into from
Mar 24, 2016

Conversation

mkoval
Copy link
Collaborator

@mkoval mkoval commented Mar 17, 2016

Merging #630 uncovered some latent issues in DART's CMake configuration. The optional dart::optimizer components (bindings for nlopt, ipopt, and snopt) were not being added to the DARTTargets group and, thus, were not included in DARTTargets.cmake. This caused finding those components, e.g. via find_package(DART COMPONENTS nlopt), to fail even if they were built.

I thought the same issue would be present with FCLCollisionDetector and BulletCollisionDetector, but it appears that all of the available collision detectors are being linked into dart.so. There is dead code in those CMakeLists.txt file that indicates this was not always the case.

I found this surprising because merging all of these into one library seems undesirable. Was this an intentional change?

@jslee02 jslee02 added this to the DART 6.0.0 milestone Mar 18, 2016
@jslee02
Copy link
Member

jslee02 commented Mar 20, 2016

We intended to build DART as a single library just for simplicity at the moment. However, as we plan to build DART component wise, collision should follow the same convention of optimizer components.

What I'm not sure is if this should be done in DART 6.0 or KIDO 7.0.

Besides on collision, this PR looks good to merge.

@mxgrey
Copy link
Member

mxgrey commented Mar 21, 2016

I think we should break things out component-wise with DART 6.0.

I believe pretty strongly that the only difference between DART 6.0 and KIDO 7.0 should be naming, while everything about their build systems, APIs, and features should be identical (except obviously that any usage of "DART" would be replaced with "KIDO"). I think that would make for the smoothest possible transition for users and developers alike.

@mkoval
Copy link
Collaborator Author

mkoval commented Mar 24, 2016

I am fine with putting off the other component changes to a future version.

However, I would still like to merge this PR. It is currently impossible for me to use find_package(DART COMPONENTS dart-optimizer-nlopt) in my code that depends on DART. This worked in the previous version, so it is a clear regression.

@jslee02
Copy link
Member

jslee02 commented Mar 24, 2016

I believe pretty strongly that the only difference between DART 6.0 and KIDO 7.0 should be naming, while everything about their build systems, APIs, and features should be identical (except obviously that any usage of "DART" would be replaced with "KIDO"). I think that would make for the smoothest possible transition for users and developers alike.

I like this plan. We can do this in separate pull requests.

👍 to merge this.

@jslee02 jslee02 merged commit 4e67f0d into dartsim:master Mar 24, 2016
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.

3 participants