dyne / frei0r

A large collection of free and portable video plugins
https://frei0r.dyne.org/
GNU General Public License v2.0
420 stars 90 forks source link

CMakeLists: Don't hardcode pkgconfig install dir #81

Closed Othko97 closed 5 years ago

Othko97 commented 5 years ago

Currently the pkgconfig files are hardcoded to be installed to $CMAKE_INSTALL_PREFIX/lib/pkgconfig, which affects installations that are using a differente library directory. This patch allows the use of the GNUInstallDirs package to modify the installation libdir at build time and to install the pkgconfig files to $CMAKE_INSTALL_LIBDIR/pkgconfig.

Addresses #79

A previous attempt was made in #80, which failed without the GNUInstallDirs package being required.

rrrapha commented 5 years ago

Like this, the pkgconfig file goes to $CMAKE_INSTALL_LIBDIR/pkgconfig. The libraries are still installed in the hardcoded lib directory. Is this intended?

ddennedy commented 5 years ago

A variable is not hard-coding even if it defaults to $ptefix/lib.. Not sure what your asking.

rrrapha commented 5 years ago

I was confused by the following behaviour:

$ cmake -DCMAKE_INSTALL_LIBDIR=mylibdir .
$ make install
...
-- Installing: /usr/local/mylibdir/pkgconfig/frei0r.pc
-- Installing: /usr/local/lib/frei0r-1/facebl0r.so
-- Installing: /usr/local/lib/frei0r-1/facedetect.so
...
ddennedy commented 5 years ago

Looks like I do not get e-mail notification when a pull request is updated without a comment. Something for us all to keep in mind on our pull requests in the future. :-)

The libraries are still installed in the hardcoded lib directory. Is this intended?

Indeed, I see the problem. Technically that is separate from this, but it is most definitely incorrect behavior. I have changes tested to fix that. I know frei0r.h specifies "lib", but I think one needs to realize that depends on configuration without having to state it explicitly. After all, this info also assumes install prefix is /usr.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 8466329..94a63e8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -30,9 +30,9 @@ if (NOT CMAKE_BUILD_TYPE)
       FORCE)
 endif (NOT CMAKE_BUILD_TYPE)

-set (LIBDIR lib/frei0r-1)
-set (FREI0R_DEF ${CMAKE_SOURCE_DIR}/msvc/frei0r_1_0.def)
-set (FREI0R_1_1_DEF ${CMAKE_SOURCE_DIR}/msvc/frei0r_1_1.def)
+set (LIBDIR "${CMAKE_INSTALL_LIBDIR}/frei0r-1")
+set (FREI0R_DEF "${CMAKE_SOURCE_DIR}/msvc/frei0r_1_0.def")
+set (FREI0R_1_1_DEF "${CMAKE_SOURCE_DIR}/msvc/frei0r_1_1.def")

 # --- custom targets: ---
 INCLUDE( cmake/modules/TargetDistclean.cmake OPTIONAL)
@@ -46,7 +46,8 @@ add_subdirectory (src)
 # Generate frei0r.pc and install it.
 set (prefix "${CMAKE_INSTALL_PREFIX}")
 set (exec_prefix "${CMAKE_INSTALL_PREFIX}")
-set (libdir "${CMAKE_INSTALL_PREFIX}/lib")
+set (libdir "${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}")
 set (includedir "${CMAKE_INSTALL_PREFIX}/include")
 configure_file ("frei0r.pc.in" "frei0r.pc" @ONLY)
ddennedy commented 5 years ago

See also 910121ab6f8bea0910171a7cc95a8032efd0c7ec