search

Libvisual / libvisual

Libvisual Audio Visualization
http://libvisual.org/
81 stars 31 forks source link

Core (Build): Add CMake export configuration #294

Closed kaixiong closed 1 year ago

kaixiong commented 1 year ago

@hartwork

Hello @kaixiong,

I got a chance at a first look and a basic before-after comparison with regard to installed files now. Three things caught my attention:

Thanks, it's much appreciated. CMake export targets are still a bit unfamiliar for me.

* Template file `Config.cmake.in` is rendered to filename `libvisual-config.cmake`. My vote for renaming the template file to `libvisual-config.cmake.in` for clarity.

Well spotted, I missed this when editing the copy-paste from CMake docs.

* `PATH_VARS` includes `LV_INCLUDE_DIR` but I cannot find use of a variable `PACKAGE_LV_INCLUDE_DIR` anywhere. Is it a leftover ir is there intention in keeping it around?

Yeah it's unnecessary, the variable has been removed. The include path is passed into the package configuration via a different route i.e. INSTALL_INTERFACE expression in target_include_directories.

* There seems to de-facto change in actors data target location from `<prefix>/share/actor` to `<prefix>/share/libvisual-0.5/actor` which seems like a clear improvement or even a build system bugfix. I am trusting that this transition is as complete as it seems from quick look.

The plugin registry picks up the folders correctly, so no problem there.

Not sure how this went unnoticed for so long :shrug:

For completeness, I've also just added libvisual-config.cmake and libvisual-config-version.cmake to .gitignore.