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

[omplapp] Use external dependency ompl via CMake target instead of embedding #14

Open
WangWeiLin-MV opened this issue Aug 5, 2024 · 4 comments

Comments

@WangWeiLin-MV
Copy link

Hi,

I am a member of the VCPKG project. I found that omplapp repeatedly builds "ompl" from the submodule instead of depends on the built libraries microsoft/vcpkg#40151.

I see that ompl/ompl#1066 ompl/ompl#1067 adds the CMake targets ompl::ompl, which allow for convenient target_link_libraries(main PRIVATE ompl::ompl), but omplapp not uses it, and it bypasses ompl/CMakeLists.txt to compile its components like this omplapp/CMakeLists.txt#L252-L257:

add_subdirectory(ompl/doc)
add_subdirectory(ompl/src)
add_subdirectory(ompl/py-bindings)
add_subdirectory(ompl/tests)
add_subdirectory(ompl/demos)
add_subdirectory(ompl/scripts)

And there are many different parts between the two files omplapp/CMakeLists.txt and ompl/CMakeLists.txt. Splitting the two projects need a lot of changes.

I would like to ask if it is possible to modify omplapp to be a project that depends on ompl, rather than an ompl variant that shares a lot of code? This is very helpful to us, and I think this will also make it easier to develop omplapp.

If you have any questions please let me know. Thanks in advance.

@Ryanf55
Copy link
Contributor

Ryanf55 commented Sep 8, 2024

Yes, agree. It will be easier after this PR.
#13

If omplapp called find_package(ompl CONFIG) before it brought it in as a submodule, would that fix it for you?

@WangWeiLin-MV
Copy link
Author

If omplapp called find_package(ompl CONFIG) before it brought it in as a submodule, would that fix it for you?

Yes, the duplicate build could be avoided in that way.

@Ryanf55
Copy link
Contributor

Ryanf55 commented Sep 9, 2024

@mamoll Can you move this to omplapp? This issue is specific to omplapp

@zkingston zkingston transferred this issue from ompl/ompl Sep 9, 2024
@zkingston
Copy link
Member

@Ryanf55 done.

@zkingston zkingston reopened this Sep 9, 2024
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

No branches or pull requests

3 participants