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

FindMagnumPlugins.cmake interaction with Assimp #87

Closed
celentes opened this issue Jun 10, 2020 · 4 comments
Closed

FindMagnumPlugins.cmake interaction with Assimp #87

celentes opened this issue Jun 10, 2020 · 4 comments

Comments

@celentes
Copy link

Documentation on using AssimpImporter recommends building assimp as a subproject:

add_subdirectory(assimp EXCLUDE_FROM_ALL)
add_library(Assimp::Assimp ALIAS assimp)

But I prefer to keep them globally installed, so I was trying to get that to work, and what I have noticed is that FindMagnumPlugins refers to it this way:

find_package(Assimp)
set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY INTERFACE_LINK_LIBRARIES Assimp::Assimp)

But pulling assimp from the master right now, I have noticed that it perfectly installs and provides assimp package with assimp::assimp as a target, and if I go ahead and change that to

find_package(assimp)
set_property(TARGET MagnumPlugins::${_component} APPEND PROPERTY INTERFACE_LINK_LIBRARIES assimp::assimp)

then it works and finds it correctly.

@celentes
Copy link
Author

Any reason I am missing to keep it inconsistent?

@mosra
Copy link
Owner

mosra commented Jun 10, 2020

The (uppercase) Assimp package is searched for and exposed by magnum's own FindAssimp.cmake, so you'll need to copy that file into your modules/ dir along with FindMagnumPlugins. The docs mention both ways, either a subproject setup or a system-wide installation, so you probably just missed the part that suggested to take the FindAssimp module also.

That said, if you have an idea how to make the docs clearer, let me know, I'll fix them.

@mosra mosra added this to the 2020.0a milestone Jun 10, 2020
@mosra
Copy link
Owner

mosra commented Jun 10, 2020

The reason why magnum has its own FindAssimp instead of find_package(assimp CONFIG), btw., is because historically Assimp didn't provide a CMake imported target, it was missing various parts, or distributions didn't ship the cmake files from it. Plus find_package(assimp CONFIG) wouldn't work with Assimp as a CMake subproject, so the Find module kinda abstracts that away.

@mosra
Copy link
Owner

mosra commented Jun 26, 2020

Ok, I think there's nothing left to do here. A related issue is mosra/magnum#436, which could avoid the need to explicitly copy the FindAssimp.cmake. Closing this in favor of that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants