search

madskjeldgaard / portedplugins

A collection of plugins for the SuperCollider sound environment, all of which are ported / remixed from elsewhere
GNU General Public License v3.0
180 stars 13 forks source link

CMake: use target_include_directories #9

Closed mossheim closed 3 years ago

mossheim commented 3 years ago

this seems to be intended based on usage and the following target_link_libraries command

madskjeldgaard commented 3 years ago

This is wonderful like your other PR's @brianlheim - thank you. I was meaning to ask - is this generally a good/recommended way of linking each plugin with a library?

madskjeldgaard commented 3 years ago

This is wonderful like your other PR's @brianlheim - thank you. I was meaning to ask - is this generally a good/recommended way of linking each plugin with a library?

I should clarify that I mean this workflow I have made with a seperate function and then calling it when the plugin is added with the files

mossheim commented 3 years ago

I was meaning to ask - is this generally a good/recommended way of linking each plugin with a library?

it's decent, i think. normally, you would just link a target against the library, and that would take care of the include directories as well. i checked, but couldn't confirm that this happens with your library dependencies. it's made more complicated by the fact that each plugin represents two targets.

another way to do it (but requires some more advance CMake knowledge) would be to create a single interface library target for each plugin, and then the scsynth/supernova targets linking against then. then link that interface library target against eigen and friends. that way, you'd only have to call target_link_libraries() once per plugin, and wouldn't have to have wrapper functions. but I think this is equally complicated compared to what you are doing now.

madskjeldgaard commented 3 years ago

Hi @brianlheim - I had to add a PUBLIC/PRIVATE flag to the include command you added, let me know if it doesn't look right https://github.com/madskjeldgaard/mkplugins/blob/c05ae18797d4c500c4dc4099af87702f488dbfd9/CMakeLists.txt#L90

thanks!

mossheim commented 3 years ago

yep that's right, sorry about that!

madskjeldgaard commented 3 years ago

No worries. Thanks for the help