mavlink
Mavlink header install using ExternalProject_Add
When building MAVSDK using CMake's ExternalProject_Add, with SUPERBUILD on, it will properly pull and build all of MAVSDK's dependencies. However, I get errors for missing mavlink headers and indeed this commit changed where the files were installed to. It would be great if we could either have the mavlink headers installed alongside the mavsdk ones automatically, or at least have a CMake option to change where dependencies are installed to.
I would suggest possibly setting DEPS_INSTALL_PATH to CMAKE_INSTALL_PREFIX on this line . Alternatively, you could make DEPS_INSTALL_PATH an optional argument, so users can place it to their needs, and it would be accessible via ExternalProject_Add.
I wonder if -DDEPS_INSTALL_PATH when you build MAVSDK doesn't just allow you to set it. But probably it should rather be an option() to make this clear...
Note that in your project that depends on MAVSDK (let's call it my-project), you can set the CMAKE_PREFIX_PATH to the install path of the MAVSDK third parties. Something like this:
cmake -DCMAKE_PREFIX_PATH="your/cmake/prefix/path;path/to/mavsdk/build/third_party/install" -Bbuild -S.
This said, after some experience with CMake superbuilds, my more recent opinion is that we should provide helpers, but not a superbuild (see my blog post here). That's much less confusing in the sense that it makes it clear to the user that they should handle their dependencies.
But well, doesn't change the fact that MAVSDK uses this superbuild, and many users don't want to know about how they should handle their dependencies :see_no_evil:. So probably your best bet is what I wrote in my previous message.
Yeah, for the time being I am just using:
target_include_directories(my-project PUBLIC
${CMAKE_BINARY_DIR}/mavsdk/src/mavsdk_proj-build/third_party/install/include
)
An option would be nice to have still, just so it's not quite so hacky.
Just build your project with CMAKE_PREFIX_PATH=path/to/mavsdk/src/mavsdk_proj-build/third_party/install, right? That would be clean.
It would even allow you to build mavlink yourself separately, install it somewhere, and point CMAKE_PREFIX_PATH to that (instead of the mavsdk one) if you like it better.
The default for find_package is to look for the package on the system. CMAKE_PREFIX_PATH just says "btw, look here first because I have some packages installed here". It's perfectly legal and very convenient.
To a degree, yeah, if I wanted to put mavlink specifically into my cmake code. I was trying to keep it generic. When using ExternalProject_Add, the code is not yet in CMake at generation time, so I can't use find_package on a target which is not installed yet. If I pulled the full repo and had it as a submodule instead (before calling any CMake), then yes I could use the CMAKE_PREFIX_PATH.
To a degree, yeah, if I wanted to put mavlink specifically into my cmake code. I was trying to keep it generic.
Well MAVSDK transitively depends on MAVLink in its public interface (before #2084), and therefore find_package must find MAVLink somewhere. It's not about "putting mavlink specifically into your cmake code", it won't actually change your CMake code. You just have to tell your CMake project where to find the dependencies with find_package. Either you install MAVLink on your system, or you point CMAKE_PREFIX_PATH to it.
When you install the MAVSDK .deb package, it depends on stuff like openssl, and it will require you to install them on your system. Still you don't consider that you are "putting openssl specifically into your cmake code", right? That's exactly the same for MAVLink, except that MAVLink is not available with apt install libmavlink-dev.
I somewhat reverted this to the previous behaviour: https://github.com/mavlink/MAVSDK/pull/2084.
Yeah I guess that solves the confusion around the transitive dependency :+1:
I had tried adding to the CMAKE_PREFIX_PATH like you suggested @JonasVautherin, but still got the same header include errors. That's why I resorted to the more manual target_include_directories entry.
Meant to comment, I tried it and works well, with no janky include paths :)