dbry / WavPack

WavPack encode/decode library, command-line programs, and several plugins
BSD 3-Clause "New" or "Revised" License
371 stars 67 forks source link

cmake: Install header in same location #125

Closed diizzyy closed 2 years ago

diizzyy commented 2 years ago

CMake currently installs header file in root of includedir while a GNU Autotools installs in wavpack/. Consumers expect to find the header in the latter so make sure to use the same path

dbry commented 2 years ago

This is not an area I know much about, but I am used to the include file being in /usr/local/include/wavpack/

@evpobr should this just go in?

Thanks!

evpobr commented 2 years ago

It looks like it. But I don't see the code that does it, only this:

https://github.com/dbry/WavPack/blob/b9efddc4a5fb63294cd8be63716d3d8da3efc137/Makefile.am#L101

evpobr commented 2 years ago

If so, then it will be necessary to correct this line as well:

https://github.com/dbry/WavPack/blob/b9efddc4a5fb63294cd8be63716d3d8da3efc137/CMakeLists.txt#L204

About CMAKE_INSTALL_INCLUDEDIR: https://cmake.org/cmake/help/latest/module/GNUInstallDirs.html

evpobr commented 2 years ago

And this:

https://github.com/dbry/WavPack/blob/b9efddc4a5fb63294cd8be63716d3d8da3efc137/CMakeLists.txt#L652

diizzyy commented 2 years ago

204 is also correct? 652 would break things as users expect to find the header in wavpack/wavpack.h not wavpack.h so changing that part would be wrong?

evpobr commented 2 years ago

My mistake. If user must include wavack/wavpack.h, not wavpack.h, you don't need my changes.

dbry commented 2 years ago

Okay @evpobr and @diizzyy, so this is good to go in as is? Thanks!

diizzyy commented 2 years ago

Yes

evpobr commented 2 years ago

Yes 😊