Fresh CMakeLists.txt

[pfeatherstone]

This is an attempt at refactoring dlib's CMakeLists.txt files.
The objectives are:

• Modern cmake, so no globals, very few variables, use the target_ functions
• Use best practices where possible (i'm not a cmake expert so please shout out if you see a better way of doing something)
• Reduce lines of code as much as possible
• All deps are imported using find_package()
• Use CMake's FindCUDAToolkit and others.

[pfeatherstone]

2 problems:

• The implementation of BLAS in external/cblas is broken. Missing symbols
• Cmake's findBLAS isn't detecting any blas libraries. So it's trying to use dlib's own version. Go to last point
  I'll have a look tomorrow.

[pfeatherstone]

So added the missing BLAS files in external/blas. I didn't know there was a difference between cblas and blas. I have no idea how this was working before. We would have got errors like: "undefined reference to dgemm_" in file cblas_dgemm.c

Otherwise, I think the FindBLAS module in cmake is broken. Shame. It doesn't always detect perfectly valid BLAS libraries. So I'll quickly write a custom one tomorrow (steal quite a bit from dlib's old file)

Otherwise, the Python tests are failing in test_global_optimization.py
It has to do with translating exceptions between c++ and Python. No idea how to attack that one and don't know what's caused that.

[pfeatherstone]

The problem with test_global_optimization.py is DLIB_CASSERT(). The second time the assert gets hit, the program aborts. That's expected behaviour.

So I'm not sure how:

with raises(Exception):
    find_max_global(lambda a, b: 0, [0, 0, 0], [1, 1, 1], 10)
with raises(Exception):
    find_min_global(lambda a, b: 0, [0, 0, 0], [1, 1, 1], 10)
with raises(Exception):
    find_max_global(lambda a, b, c, d, *args: 0, [0, 0, 0], [1, 1, 1], 10)
with raises(Exception):
    find_min_global(lambda a, b, c, d, *args: 0, [0, 0, 0], [1, 1, 1], 10)

in test_global_optimization.py is supposed to work with DLIB_CASSERT() being used.

I've replaced a couple DLIB_CASSERT() statements with if(!(condition)) throw std::invalid_argument(msg);

[arrufat]

Yes, a bit confusing. It used to, though. The dependency was removed in #2273.

[pfeatherstone]

Also, build_cpp.yml is getting a bit bloated. There must be a good way to reuse code. It looks like there are a couple options:

• https://docs.github.com/en/actions/using-workflows/reusing-workflows
• https://docs.github.com/en/actions/creating-actions/creating-a-composite-action

@arrufat are you familiar with those?

[arrufat]

No, but I can have a look :)

[pfeatherstone]

I've looked at some online tutorials on how to write cmake Config files for installable targets. It's crazy how complicated it is. It's laughable. There should be a better tool. I'm surprised there isn't a repo with some cmake helper scripts out there that figure out how to package a target.

[davisking]

What's currently there doesn't work? I mean it works on master. But you are finding that something about the new cmake setup breaks it I guess? The current thing is fairly simple 🤷‍♂️

[pfeatherstone]

Yeah if you look at the GitHub Action Runners, you'll see I've added one which demonstrates what I see. It builds and installs dlib then builds one of the examples using the installed version of dlib, i.e using find_package(dlib)
It doesn't recognise the imported targets.

[pfeatherstone]

It's because we are using imported targets now, not XXX_LINK_LIBRARIES and XXX_INCLUDE_DIRECTORIES

[pfeatherstone]

pkg-config is broken too since it uses the old dlib_needed_libraries variables and we've done away with that in this library in favour of imported targets. Probably need another runner that checks pkg-config functionality too.

[pfeatherstone]

Right, made some progress, while reading https://cmake.org/cmake/help/latest/command/install.html#export, I found the EXPORT_PACKAGE_DEPENDENCIES option which nearly fixes it by automatically adding statements like:

include(CMakeFindDependencyMacro)
find_dependency(Threads REQUIRED)
find_dependency(JPEG REQUIRED)
find_dependency(FFMPEG REQUIRED)
find_dependency(CUDAToolkit REQUIRED)
find_dependency(CUDNN REQUIRED)
find_dependency(MKL REQUIRED)

to the generated dlib.cmake file while running install(EXPORT dlib ...)
It's quite clever. It scans the dlib target and finds all its imported target dependencies.
However, this option is only available in cmake version 3.29. So this is bleeding edge.
Also, some of those options like find_dependency(MKL REQUIRED) won't work because it assumes there is a globally available FindMKL.cmake file somewhere, whereas that was in dlib's repository used at build time.

So I think any custom cmake modules, like FindMKL.cmake etc need to be installed along with dlib.cmake and dlibConfig.cmake.

Also i need to add find_dependency() statements to dlibConfig.cmake.in file.

Again, this isn't a problem on master, since it doesn't use imported targets, just plain old compile options and linker flags.
This PR is all about using modern cmake, which means using imported targets, which means all the above should be done.

Unless someone can come up with a better option.

I don't really like how we now have to maintain two lists of dependencies, one using find_package(...) in CMakeLists.txt and one using find_dependency(...) in `dlibConfig.cmake.in.

Is Cmake really this bad?

[pfeatherstone]

https://buildmedia.readthedocs.org/media/pdf/bcm/stable/bcm.pdf
this looks cool.

[pfeatherstone]

I have a working solution but now dlibConfig.cmake.in duplicates all the find_package() statements. I'm not keen on this.

[pfeatherstone]

I've added a test to CI/CD which attempts to build an example using pkg-config. So this needs fixing.
I'm of two minds:

• Don't use cmake imported targets and go back to using XXX_INCLUDE_DIR and XXX_LIBRARIES for everything. This should fix all our problems with the generated dlib.cmake and dlib.pc files. But it's old school cmake. Apparently that's not best practice anymore. I don't know.
• Use some sophisticated cmake to make all the imported targets work during install. Supposedly best practice, but you have to write either duplicated code, or disgusting cmake macros, or use third-party libraries that help with creating packages.

My opinion now is cmake is a pile of ***t

EDIT:
Also, generator expressions will be a problem when building dlib.pc. So it's possible all the generator expressions would have to be replaced with if-else expressions, as before. Unless there's something I've missed, I don't see how "modern cmake" actually solves anything anymore.

[davisking]

I've added a test to CI/CD which attempts to build an example using pkg-config. So this needs fixing. I'm of two minds:

• Don't use cmake imported targets and go back to using XXX_INCLUDE_DIR and XXX_LIBRARIES for everything. This should fix all our problems with the generated dlib.cmake and dlib.pc files. But it's old school cmake. Apparently that's not best practice anymore. I don't know.
• Use some sophisticated cmake to make all the imported targets work during install. Supposedly best practice, but you have to write either duplicated code, or disgusting cmake macros, or use third-party libraries that help with creating packages.

My opinion now is cmake is a pile of ***t

EDIT: Also, generator expressions will be a problem when building dlib.pc. So it's possible all the generator expressions would have to be replaced with if-else expressions, as before. Unless there's something I've missed, I don't see how "modern cmake" actually solves anything anymore.

Well, every year "modern cmake" is different. Which is fine. The only big change to cmake that I really think of as "the modern one" was when you started to be able to do target_link_libraries(my_app dlib::dlib) and that was all you needed. Whereas before you had to screw around with a bunch of other stuff as a client of a library that cmake itself was building.

I mean there are other things too. But the other big one is different platforms and their related tools keep changing so older versions of cmake accumulate a bunch of magic in user cmake scripts to deal with that stuff. And those things eventually get folded into cmake proper and everyone goes "see, new cmake is simple, you don't need that stuff!". But that is a never ending battle since other people keep screwing up/changing/over-complicating/whatevering their systems and tools in new and never ending ways.

Anyway, do you mean that stuff like target_link_libraries(dlib PUBLIC CUDA::cudart CUDA::cublas CUDA::curand CUDA::cusolver CUDNN::CUDNN) is not working with the install scripts? Because that sucks if so. That's been part of cmake for years and years now (well, not for cuda but the whole blah::blah target stuff). Got to be a way for that to work nicely.

[davisking]

Totally keep the instructions. People see this stuff and then go "oh right, I'll do that" rather than do a bunch of confusing hacks.

[davisking]

Yeah it's screwy that these deps are repeated but it's simple so it's not the end of the world.

[pfeatherstone]

I'm just so dissatisfied with this. You would think cmake understands enough about a target that by the time you've built it it understands how to install it with one line of code.
Libraries like https://github.com/boost-cmake/bcm.git and https://github.com/robotology/ycm sort of provide this if you use their custom functions.

[davisking]

Yeah, it is annoying. It's also a bit of a regression compared to how it was before. But it's not a big deal at the end of the day, especially compared to how much other stuff is simpler so ¯_(ツ)_/¯

[pfeatherstone]

Though it doesn't fix dlib.pc for pkg-config. That needs specific compile and linker options. It there was a magic cmake macro/function which took all your imported targets, generator expressions, and resolved everything down to concrete compile and linker options, then everything would be good. It must do this anyway since it has to generate make/ninja files. If there was a way to reuse those functions then it would be great.

[davisking]

Ah, yeah you haven't figured out how to make it generate a dlib.pc file yet?

[pfeatherstone]

No sorry. I mean it generates it fine but it references libraries like blah::blah

[pfeatherstone]

I think I'll have to use something like file(GENERATE) as a hack

[pfeatherstone]

Doesn't work.

[pfeatherstone]

Shame this can't make it into the release but until dlib.pc.in is fixed...
I've searched everywhere and there is no simple solution.
The only thing to do is never use cmake imported targets and just use variables. Bit crap.
So not really worth merging if that's the solution. Let's just keep this open until a good one pops up.

[davisking]

Shame this can't make it into the release but until dlib.pc.in is fixed... I've searched everywhere and there is no simple solution. The only thing to do is never use cmake imported targets and just use variables. Bit crap. So not really worth merging if that's the solution. Let's just keep this open until a good one pops up.

Yeah that's annoying. Thanks for trying it though. At some point cmake will presumably make this possible. I'm also not super sure how popular pkg-config is. I don't use it, but I get asked about it often enough and before this stuff was in dlib there were complaints about it. So maybe popular? It's hard to tell the difference between a loud tiny minority and a bigger thing sometimes. Maybe not enough people care about pkg-config anymore for cmake to support this in a reasonable way?

[pfeatherstone]

What's nice about pkg-config is that it's portable. Multiple IDEs can us it (Eclipse, Netbeans), other c++ builders like Meson support it. But yeah, cmake is making it hard.
I imagine 90% of your users only care about the face detector, particularly in python, and pkg-config is irrelevant there.

[davisking]

What's nice about pkg-config is that it's portable. Multiple IDEs can us it (Eclipse, Netbeans), other c++ builders like Meson support it. But yeah, cmake is making it hard. I imagine 90% of your users only care about the face detector, particularly in python, and pkg-config is irrelevant there.

Heh, yeah, that face detector example is unreasonably popular 😅

[arrufat]

Heh, yeah, that face detector example is unreasonably popular 😅

I don't think it's unreasonable. Here we're complaining about how hard it is to make foolproof C++ builds, but it's still better than all the venv, conda, etc. nightmare.

Honestly. I am always surprised about how many people have problems building dlib. I guess that a lot of the issues are caused by conda replacing system libraries and leaving the system in a broken state.

[pfeatherstone]

Heh, yeah, that face detector example is unreasonably popular 😅

I don't think it's unreasonable. Here we're complaining about how hard it is to make foolproof C++ builds, but it's still better than all the venv, conda, etc. nightmare.

Honestly. I am always surprised about how many people have problems building dlib. I guess that a lot of the issues are caused by conda replacing system libraries and leaving the system in a broken state.

I don't know why people use conda. The only times i've used it it's caused me problems. What's wrong with pip install ... ?

[arrufat]

I don't know why people use conda. The only times i've used it it's caused me problems. What's wrong with pip install ... ?

Yeah, same here, I hate conda. I usually do python -m venv venv and then pip install everything there. Never had issues with that.

[davisking]

Yeah my experience with pip is pretty solid and that's what I always use. My main experience with conda is in all the people messaging me and telling me how conda broke their computer's ability to compile software.

[pfeatherstone]

Pinching the code from https://bcm.readthedocs.io/en/latest/ might be the best bet. It looks really cool.

[davisking]

Yeah but then that's one more dependency that gets out of date and needs updating. Might as well just keep what dlib has then.

…

On Wed, Mar 13, 2024 at 6:42 PM pfeatherstone ***@***.***> wrote: Pinching the code from https://bcm.readthedocs.io/en/latest/ might be the best bet. It looks really cool. — Reply to this email directly, view it on GitHub <#2912 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABPYFRZDYFB5PJCWRGJCQODYYDI4NAVCNFSM6AAAAABC4W44TWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJWGAYTGNZZG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[davisking]

Like if we aren't putting the responsibility on the cmake team to handle this stuff but just having cmake scripts in dlib's repo, what's the point? Since that's already the state of how things are now.

…

On Thu, Mar 14, 2024 at 8:12 AM Davis King ***@***.***> wrote: Yeah but then that's one more dependency that gets out of date and needs updating. Might as well just keep what dlib has then. On Wed, Mar 13, 2024 at 6:42 PM pfeatherstone ***@***.***> wrote: > Pinching the code from https://bcm.readthedocs.io/en/latest/ might be > the best bet. It looks really cool. > > — > Reply to this email directly, view it on GitHub > <#2912 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABPYFRZDYFB5PJCWRGJCQODYYDI4NAVCNFSM6AAAAABC4W44TWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJWGAYTGNZZG4> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

[pfeatherstone]

Cmake and pkg-config issue tracker

https://gitlab.kitware.com/cmake/cmake/-/issues/22621