-
Notifications
You must be signed in to change notification settings - Fork 624
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
#2 Conan package support #19
Conversation
- Add conan recipe for BehaviorTree.CPP Signed-off-by: Uilian Ries <uilianries@gmail.com>
default_options = {"shared": False, "zmq": True} | ||
generators = "cmake" | ||
exports = "LICENSE" | ||
exports_sources = ("cmake/*", "include/*", "src/*", "3rdparty/*", "CMakeLists.txt") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the 3rdparty, it would be nice to be able to avoid it and use conan packages.
I think there are packages for flatbuffers and for tinyXML2.
This could be done later too, not critical, though it might save some building time too, and avoid conflicts for users using those libraries too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. If you notice: https://github.com/BehaviorTree/BehaviorTree.CPP/blob/master/CMakeLists.txt#L30
When ZMQ is detected the definition ZMQ_FOUND is added, otherwise, it isn't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both flatbuffers and tinyXML2 cpp files are compiled directly with BehaviorTree. We could change the cmake file to use both cpp files from Conan but could result in different ABI when building with/without Conan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI:
tinyXML2, minitracer and ZMQ are all is "hidden" both in terms of API and ABI.
I removed any reference to these libraries in the header files and I used the Pimpl idiom.
There is a single class, PublisherZMQ, whihc requires ZMQ.
A possible solution might be to release BehaviorTree.CPP without ZMQ and PublisherZMQ as a separate projects.
"""Install ZMQ when required | ||
""" | ||
if self.options.zmq: | ||
self.requires("cppzmq/4.3.0@bincrafters/stable") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this have some effect on the build? Like some option USE_ZMQ to CMakeLists?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well, find_package for ZMQ is not mandatory so we could build BehaviorTree without cppzmq. I think it could be an option for tiny version of package.
I will try to review and merge this tomorrow |
I finally decided to make ZMQ not an optional but a mandatory requirement. I slightly modifed your file and included it. Thanks |
Thanks! I'll open a new PR including CI support. BehaviorTree could be built by Conan and distributed on Conan Center |
Changes batch 2
Hi!
This PR is related to issue #2
Here I added the complete Conan recipe to build BehaviorTreeCPP. The recipe has some attributes, including options (shared and zmq support) and some functions to build your project.
Conan provides an excellent CMake integration. As you can see, both examples and tests are disabled when creating the package. Also, there is a little patch on line 41 where Conan is included in your cmake file. The patch is needed to solve ZMQ and cmake definitions, as
BUILD_SHARED_LIBS
when the optionshared
is True.To build this project using Conan:
The command conan create will copy all files listed by
exports_sources
to build directory and will execute all cmake steps, including configure, build and install. The requirement ZMQ C++ will be resolved by Conan automatically. There is the optionzmq
to exclude it as a dependency when requested. By default, will produce BehaviorTree.CPP as static library and linking cppzmq. Your project folder will not be changed by Conan after to build.To build BehaviorTree.CPP as shared library and standalone (without zmq):
The package reference
BehaviorTree.CPP/<version>@BehaviorTree/stable
is just a suggestion. You can rename as you prefer.This PR is the first one of two. After to accept (or not) this PR, I'll create a new one to introduce Conan in your CI (travis and appveyor) to distribute BehaviorTree.CPP as Conan package (recipe and pre-built packages) on Bintray
Adding Conan experts to review this PR together:
/cc @memsharded @danimtb @SSE4
closes #2
Regards!