Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the Conan wrapper into the main CMakeLists.txt? #53

Closed
offa opened this issue Oct 27, 2020 · 6 comments
Closed

Include the Conan wrapper into the main CMakeLists.txt? #53

offa opened this issue Oct 27, 2020 · 6 comments

Comments

@offa
Copy link
Owner

offa commented Oct 27, 2020

Does it make sense to include the Conan wrapper cmake code into the main CMakeLists.txt?

The relevant part consists only of two lines, but the wrapper complicates the build unnecessarily.

Another possible approach: Test whether conanbuildinfo.cmake exists; if so, include it and call conan_basic_setup().

Benefit:

It simplifies the Conan build and it remains fully optional.

Drawback:

It introduces Conan related code – it's still optional though.

CC @steinerthomas

@offa offa added this to the next milestone Oct 27, 2020
@offa offa self-assigned this Oct 27, 2020
@steinerthomas
Copy link
Contributor

As you mentioned as a drawback, some people maybe don't want Conan related code if they use your library. I saw this wrapper injection in the google benchmark library.

Of course the goal should be, to add the library to the conan-center-index.

@offa
Copy link
Owner Author

offa commented Oct 31, 2020

Conan should still be fully optional of course, so it should always be possible to compile without it. Something like here. Are there any drawbacks using this way?

@steinerthomas
Copy link
Contributor

No there are no drawbacks. Pls describe your use case: I think you need it for getting boost binaries from conan-center and you want to call conan install .?
I personally don't need this in the CMakeLists, because I'm a consumer of your library and I need the binaries for this lib on our artifactory.
For me as a consumer of this lib, as already mentioned, it would be more important to bring a recipe to the conan-center-index and there you'll still have to wrap the conan stuff into a CMake wrapper.

@offa
Copy link
Owner Author

offa commented Nov 6, 2020

No there are no drawbacks. Pls describe your use case: I think you need it for getting boost binaries from conan-center and you want to call conan install .?

Exactly, the main reason for the idea is to simplify the build. However I'm not quite sure if this may interfere with the other cmake generators.

For me as a consumer of this lib, as already mentioned, it would be more important to bring a recipe to the conan-center-index and there you'll still have to wrap the conan stuff into a CMake wrapper.

That makes sense to me, but before submitting it, I want to improve some things. Unfortunately I can't get Conan to work with Boost and Catch at the moment too.

@steinerthomas
Copy link
Contributor

Exactly, the main reason for the idea is to simplify the build. However I'm not quite sure if this may interfere with the other cmake generators.

But the build should also work without conan and with conan you can call conan create . and then the conan_basic_setup will get injected by the recipe.

That makes sense to me, but before submitting it, I want to improve some things. Unfortunately I can't get Conan to work with Boost and Catch at the moment too.

I'm looking forward to find it on conan-center-index soon 😉

@offa
Copy link
Owner Author

offa commented Nov 7, 2020

I see, thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants