fix #143 by setting LIB_INSTALL_DIR CMake var

[schmittlauch]

This is a simple fix for #143 by reverting parts of 327bf14.
I don't know though whether this fits the intended modernized CMake workflow.

[neheb]

This is more or less correct.

Currently in OpenWrt, libdir= comes up as blank and includedir=/evhtp , which is totally wrong. This is currently used to fix: https://github.com/openwrt/packages/blob/master/libs/libevhtp/Makefile#L49

In most other pkgconfig files that I've seen, this is the format:

prefix=/usr
exec_prefix=/usr
libdir=${exec_prefix}/lib
includedir=${prefix}/include

which works well in OpenWrt.

[schmittlauch]

Please do not hardcode prefixes like /usr but instead set them during the build – or more precisely install – phase!
Distributions like NixOS or Guix do not adhere to the FHS but set different prefixes depending on build inputs and versions. Using ${CMAKE_INSTALL_PREFIX} works correctly for us.

[neheb]

ping @cotequeiroz

[cotequeiroz]

Please do not hardcode prefixes like /usr but instead set them during the build – or more precisely install – phase!
Distributions like NixOS or Guix do not adhere to the FHS but set different prefixes depending on build inputs and versions. Using ${CMAKE_INSTALL_PREFIX} works correctly for us.

I get what @neheb wants. He does not want to hardcode anything, but to use the prefix and exec_prefix variables, which can be easily overriden, as the first part of libdir and includedir. They are usually set that way by autotools. cmake has the GNUInstallDirs module to help.

So the following patch should do it that way:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -14,6 +14,7 @@ include(CheckIncludeFiles)
 include(CheckTypeSize)
 include(CheckCCompilerFlag)
 include(TestBigEndian)
+include(GNUInstallDirs)

 check_function_exists(strndup HAVE_STRNDUP)
 check_function_exists(strnlen HAVE_STRNLEN)
diff --git a/evhtp.pc.in b/evhtp.pc.in
index a7b351f..fbddc51 100644
--- a/evhtp.pc.in
+++ b/evhtp.pc.in
@@ -1,6 +1,7 @@
 prefix=@CMAKE_INSTALL_PREFIX@
-libdir=@LIB_INSTALL_DIR@
-includedir=@INCLUDE_INSTALL_DIR@/evhtp
+exec_prefix=${prefix}
+libdir=${exec_prefix}/@CMAKE_INSTALL_LIBDIR@
+includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@/evhtp

 Name: libevhtp
 Description: A more flexible replacement for libevent's httpd API

Note that this changes behavior. Currently, on gentoo-x86_64, libevht will install libs to /usr/local/lib by default. If you include the GNUInstallDirs module, they will be installed in /usr/local/lib64--which should be right for my system, btw.

Edit:
I forgot to show the resulting evhtp.pc file:

$ head -n4 evhtp.pc
prefix=/usr/local
exec_prefix=${prefix}
libdir=${exec_prefix}/lib64
includedir=${prefix}/include/evhtp

[neheb]

Not quite:

mangix@mangix-pc:~/devstuff/openwrt/staging_dir/target-mips_24kc_musl/usr/lib/pkgconfig$ cat * | grep exec_prefix=\$\{ | wc -l
22
mangix@mangix-pc:~/devstuff/openwrt/staging_dir/target-mips_24kc_musl/usr/lib/pkgconfig$ cat * | grep exec_prefix=/usr | wc -l
166

[cotequeiroz]

Keep in mind that unlike autotools, cmake does not do prefix/exec_prefix distinction. It will still work the same way. You could also just use @CMAKE_INSTALL_PREFIX@ again.

[neheb]

Does CMake even use pkgconfig? I see files under /usr/lib/cmake which I'm guessing do what pkgconfig does for autotools. I see

/usr/lib/cmake/libevhtp/libevhtpConfig.cmake
/usr/lib/cmake/libevhtp/libevhtpConfigVersion.cmake
/usr/lib/cmake/libevhtp/libevhtpTargets.cmake
/usr/lib/cmake/libevhtp/libevhtpTargets-release.cmake

As far as pkgconfig files go, autotools should probably be the focus.

[cotequeiroz]

Yes, they have their own package management tool. Keep in mind that you may want to build non-cmake-aware apps with the library.

[schmittlauch]

@cotequeiroz Your suggestion works fine for me as well, I just tested it under NixOS.

So how to proceed? Will you open a new PR with your proposed patch or shall I adapt it into this one?

[cotequeiroz]

You choose. It’s such a small set of changes, that I would prefer not to do it on its own. Besides, we lose the discussion. If you want to give me credit, just write it in the commit message.

[schmittlauch]

incorporated the proposal of @cotequeiroz

[schmittlauch]

So is there any interest in merging this tiny fix?

[CLAassistant]

All committers have signed the CLA.