davisking / dlib

A toolkit for making real world machine learning and data analysis applications in C++
http://dlib.net
Boost Software License 1.0
13.46k stars 3.37k forks source link

[Bug]: DLIB_IN_PROJECT_BUILD incorrectly puts -llapack into `Libs` of `dlib-1.pc` #2969

Closed nh2 closed 3 months ago

nh2 commented 3 months ago

What Operating System(s) are you seeing this problem on?

Linux (x86-64)

dlib version

19.24.4

Python version

No response

Compiler

GCC 13

Expected Behavior

No response

Current Behavior

With -DDLIB_IN_PROJECT_BUILD=on, dlib generates this .pc file:

Libs: -L${libdir} -ldlib -L/path/to/lapack-3/lib -llapack

This is wrong, because as far as I can tell, lapack is a private library to dlib (its types are not used in dlib's header files).

If shared linking is requested, it is enough that that the libdlib.so dynamically links against liblapack.so; a downstream compiler using -ldlib should only pass that flag, not -llapack.

-L/path/to/lapack-3/lib -llapack is only required for static linking, so these flags should be in Libs.private instead of Libs, see https://people.freedesktop.org/~dbn/pkg-config-guide.html#faq

My library z uses libx internally, but does not expose libx data types in its public API. What do I put in my z.pc file?

[...] If libx does not support pkg-config, add the necessary linker flags to Libs.private.

This was recently slightly incorrectly implemented in:


As an aside, the .pc file should probably instead generate Requires / Requires.private listead of Libs* when blas/lapack were found via pkg-config in

https://github.com/davisking/dlib/blob/51c7a3597935ed2cb969521a0ada0b7209f78ec2/dlib/cmake_utils/find_blas.cmake#L67-L70

(explanation in the same pkg-config explanations section I linked above.

Steps to Reproduce

cmake with DDLIB_IN_PROJECT_BUILD=on.

Anything else?

No response

nh2 commented 3 months ago

Hm, I might be wrong here.

Is e.g. https://github.com/davisking/dlib/blob/51c7a3597935ed2cb969521a0ada0b7209f78ec2/dlib/matrix/lapack/gesvd.h a header for public consumption?

I suspect that because it declares inline functions such as

https://github.com/davisking/dlib/blob/51c7a3597935ed2cb969521a0ada0b7209f78ec2/dlib/matrix/lapack/gesvd.h#L42-L51

-llapack actually has to be given even in the downstream project.

Let me know what's right!

davisking commented 3 months ago

Right, all the linear algebra stuff in dlib is templated and in headers. You don't even have to compile any of the .cpp files in dlib to use that stuff, so the client has to link to lapack.

nh2 commented 3 months ago

All clear, then there is no bug here. Closing!