open-simulation-platform / libcosim

OSP C++ co-simulation library
https://open-simulation-platform.github.io/libcosim
Mozilla Public License 2.0
61 stars 10 forks source link

Use conan.cmake #662

Closed markaren closed 2 years ago

markaren commented 3 years ago

Controversial (?) PR incoming. Make use of conan CMake wrapper so that users only have to specify -DLIBCOSIM_USING_CONAN=TRUE and CMake handles everything conan related.

Makes stuff much easier.

If OK, README should be updated as well.

Closes #599 as this would make it unnecessary.

kyllingstad commented 3 years ago

Controversial indeed. :) How does it make things easier?

Today, we have a situation where Conan drives the CMake build process. That is, you can run conan create, conan build, etc., and they will trigger various parts of the CMake build process.

With this proposal, it seems like we add another layer of complexity on top of this, where CMake is driving Conan, which is again driving CMake...

According to the comments in cmake/conan.cmake it looks like this was designed for a different use case (emphasis mine):

It is intended to facilitate developers building projects that have conan dependencies [...]. It is not necessary to create conan packages, in fact it shouldn't be use for that.

This indicates it is rather designed to simplify building of "non-package" projects (like cosim-cli).

markaren commented 3 years ago

This indicates it is rather designed to simplify building of "non-package" projects (like cosim-cli).

That is why we do:

if(CONAN_EXPORTED) # in conan local cache
        # standard conan installation, deps will be defined in conanfile.py
        # and not necessary to call conan again, conan is already running
        include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
        conan_basic_setup()
else()
markaren commented 3 years ago

How does it make things easier?

Everything works as before, but the user does not need to manually call conan before building the CMake project. This is a pain, that #599 tried to improve, but that PR is far from perfect.

markaren commented 2 years ago

It should be mentioned that this PR makes it trivial to use WSL as long as #678 is fixed.

markaren commented 2 years ago

WSL is able to compile, but can't find runtime libraries (boost)

markaren commented 2 years ago

I still really want this PR to be merged. It makes life so much easier not having to deal manually with Conan and it makes WSL integration a breeze. I just made the feature optional, so in order to reap the benefits one should enable the CMake option LIBCOSIM_USING_CONAN_AUTO_CONFIG. Otherwise, everyhing will be as before.

kyllingstad commented 2 years ago

Out of curiosity, what is it about WSL that currently prevents compilation? Isn't WSL supposed to give you a full-fledged Linux instance now?

markaren commented 2 years ago

I still don't understand what's so painful about conan install

I must be running this command more often than others and on multiple systems. Each time I have to spend time figuring out the command, and run for multiple builds etc. With this PR you do a load CMake. Done.

Out of curiosity, what is it about WSL that currently prevents compilation

The difficulty comes when using WSL as a toolchain through CLion. Making conan work there without this is beyond me.

kyllingstad commented 2 years ago

Seems like you're dealing with way more complicated setups than me. I just do conan install path/to/source and then it's ready for the standard CMake commands after that. I'didn't even know cross-compilation with WSL-as-a-toolchain was a thing. Sounds complicated indeed. ;)