facebook / zstd

Zstandard - Fast real-time compression algorithm
http://www.zstd.net
Other
23.67k stars 2.1k forks source link

.pc file is broken when generated by CMake with explicit CMAKE_INSTALL_*DIR #2192

Closed tobim closed 3 years ago

tobim commented 4 years ago

Describe the bug When CMake is used to build zstd, it can be configured with explicit install paths for the includes, libraries, binaries and so on. When one of these paths is absolute, the generated libzstd.pc contains wrong fields.

To Reproduce Steps to reproduce the behavior:

$ mkdir build_ && cd $_
$ cmake ../build/cmake/ -DCMAKE_INSTALL_PREFIX=/tmp/usr -DCMAKE_INSTALL_INCLUDEDIR=/tmp/opt/include -DCMAKE_INSTALL_LIBDIR=/tmp/foobar/lib
$ cmake --build . --target all
$ cmake --build . --target install
$ cat /tmp/foobar/lib/pkgconfig/libzstd.pc
#   ZSTD - standard compression algorithm
#   Copyright (C) 2014-2016, Yann Collet, Facebook
#   BSD 2-Clause License (http://www.opensource.org/licenses/bsd-license.php)

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

Name: zstd
Description: fast lossless compression algorithm library
URL: http://www.zstd.net/
Version: 1.4.5
Libs: -L${libdir} -lzstd
Cflags: -I${includedir}

Expected behavior

...
prefix=/tmp/usr
exec_prefix=${prefix}
includedir=/tmp/opt/include
libdir=/tmp/foobar/lib
...
tobim commented 4 years ago

CMake has CMAKE_INSTALL_FULL_*DIR variables that can be used instead, but the ${prefix} parts would need to be removed from includedir and libdir.

There have been quite a few PRs regarding pkgconfig recently, and I don't know the motivation and requirements, but it seems to me that the variable preparation logic in both the Makefile and CMake based builds are too complicated.

felixhandte commented 4 years ago

@tobim, given the apparently mutually exclusive needs we've gotten from users of our pkg-config file, we've struggled to find a solution that supports everyone's use cases. In particular, we were requested to always leave ${prefix} in, in #1794. I'm not sure how we could accommodate that use case along with this at the same time. Any suggestions?

tobim commented 4 years ago

One approach I can think of is to take the absolute paths to the install locations includedir and libdir and extracting the common prefix directory. The result can then be used to assign prefix instead of the one that was passed on the command line.

felixhandte commented 3 years ago

Should be fixed in dev.