behaviortree.cpp: add versions 3.8.3,4.0.2,4.1.1 #16731

bariscelik · 2023-03-27T15:23:00Z

behaviortree.cpp/4.1.1
behaviortree.cpp/4.0.2
behaviortree.cpp/3.8.3

Introduced 3 new versions 4.1.1, 4.0.2, 3.8.3
Updated the recipe for conan 2.0.0+
Added some logic to prevent building < 4.1.0 versions of the recipe on conan 2.0.0+
Updated dependencies by versions for each platform
Fixed 3.5.6

This PR is based on #16189

I've read the contributing guidelines.
I've used a recent Conan client version close to the currently deployed.
I've tried at least one configuration locally with the conan-center hook activated.

CLAassistant · 2023-03-27T15:23:08Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

facontidavide · 2023-03-29T12:22:28Z

recipes/behaviortree.cpp/all/conanfile.py

+        "with_groot_interface": False,
+        "with_sqlite_logging": False,


These two should be True by default. also, these options make sense only in version 4.1.1 or older.
Additionally with_coroutines should be removed in version 4.1.1 and newer

facontidavide · 2023-03-29T12:23:06Z

recipes/behaviortree.cpp/all/conanfile.py

+        if Version(self.version) < "4.1.0":
+            self.cpp_info.components[libname].requires.extend(["ncurses::ncurses"])
+            if self.options.with_coroutines:
+                self.cpp_info.components[libname].requires.append("boost::coroutine")
+
+        if Version(self.version) > "4.1.0" and self.options.with_sqlite_logging:


change to 4.1.1 ?

I think SQLite is needed for 4.1.1 and more, right? (so > 4.1.0)

facontidavide · 2023-03-29T12:26:17Z

recipes/behaviortree.cpp/all/patches/4.0.2-0002-use-cci.patch

+find_package(lexy REQUIRED CONFIG)
+find_package(minitrace REQUIRED CONFIG)
+find_package(tinyxml2 REQUIRED CONFIG)


These should be removed alson in 4.0.2 and 4.0.1:

cppzmq

lexy

minitrace

tinyxml2

It was never intended to use conan for these specific dependencies, only sqlite and ZeroMQ

The problem is that if the behaviortree library uses these dependencies internally, vendored, or something like that (it seems so https://github.com/BehaviorTree/BehaviorTree.CPP/tree/master/3rdparty), and the behaviortree library is used together with these vendored libraries, there could easily be linking errors, right? In principle, vendored libraries are not allowed in ConanCenter.

I'd like to understand a bit better about this.

facontidavide · 2023-03-29T12:27:02Z

recipes/behaviortree.cpp/all/patches/4.0.1-0002-use-cci.patch

+-add_subdirectory(3rdparty/lexy)
+find_package(cppzmq REQUIRED CONFIG)
+find_package(lexy REQUIRED CONFIG)
+find_package(tinyxml2 REQUIRED CONFIG)


see comment below about dependencies

prince-chrismc · 2023-04-17T16:47:45Z

recipes/behaviortree.cpp/all/conanfile.py

@@ -58,38 +67,49 @@ def config_options(self):
            del self.options.fPIC

    def configure(self):
+        if conan_version >= Version("2.0.0") and Version(self.version) < "4.0.1":
+            raise Exception("Conan >=v2.0.0 is not compatible with behaviortree.cpp < 4.0.1")


What is the reason for this error? We have not made 2.0 required so it seems odd to have this in 🤔
The Conan client also prints a message so it should not be require in each recipe

prince-chrismc · 2023-04-17T16:50:07Z

recipes/behaviortree.cpp/all/patches/4.0.2-0002-use-cci.patch

+-find_package(ZMQ)
+find_package(ZeroMQ)


This can be changed in a recipe with CMakeDeps using the set_property in the generate method

prince-chrismc · 2023-04-17T16:51:12Z

recipes/behaviortree.cpp/all/conanfile.py

+        "with_groot_interface": [True, False],
+        "with_sqlite_logging": [True, False],


If these are specific to a version, they should be removed from config_options or package ID

prince-chrismc · 2023-04-17T16:51:50Z

recipes/behaviortree.cpp/all/conanfile.py

+
+            if Version(self.version) == "4.0.1" or Version(self.version) == "4.0.2":
+                tc.variables["BTCPP_MANUAL_SELECTOR"] = False
+
        tc.cache_variables["CMAKE_POLICY_DEFAULT_CMP0077"] = "NEW"


This can be dropped if the conan required version is bump to 1.54.0 :)

You're right

prince-chrismc · 2023-04-17T16:52:24Z

recipes/behaviortree.cpp/all/conanfile.py

+        replace_in_file(self,
+            path_bt_factory,
+            "#define BT_FACTORY_H",
+            "#define BT_FACTORY_H\n#define BEHAVIORTREE_VERSION 0x%02d%02d%03d" % (int(str(ver.major)), int(str(ver.minor)), int(str(ver.patch))))


Why is this required?

The library doesn't include any version information anywhere. I wanted to provide a version to the developer. Because there are some deps which can't be used at compilation time without version info.

prince-chrismc

Did a quick pass, few questions to understand what you are doing + some suggestions to help clean up the recipe more

Thansk for taking the time to work on this ❤️

stale · 2023-06-18T02:16:28Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

memsharded

Hi @bariscelik @facontidavide

Sorry this was not followed up, the activity and backlog in ConanCenter is huge, 5k PR per year, and it is challenging to manage everything. I am assigning it to myself to try to help to get it to the finish line.

@bariscelik, the CLA needs to be signed again, it was reset and it needs to be signed again, sorry for the inconvenience

memsharded · 2023-07-08T20:01:47Z

recipes/behaviortree.cpp/all/conandata.yml

  "4.0.1":
    - patch_file: "patches/4.0.1-0001-remove-fpic.patch"
-    - patch_file: "patches/4.0.1-0002-find-zmq.patch"


I am a bit confused about this change, how it is possible to remove patches that were there in the past for an already packaged version?

memsharded · 2023-07-08T20:06:10Z

recipes/behaviortree.cpp/all/patches/4.0.2-0002-use-cci.patch

+find_package(lexy REQUIRED CONFIG)
+find_package(minitrace REQUIRED CONFIG)
+find_package(tinyxml2 REQUIRED CONFIG)


The problem is that if the behaviortree library uses these dependencies internally, vendored, or something like that (it seems so https://github.com/BehaviorTree/BehaviorTree.CPP/tree/master/3rdparty), and the behaviortree library is used together with these vendored libraries, there could easily be linking errors, right? In principle, vendored libraries are not allowed in ConanCenter.

I'd like to understand a bit better about this.

conan-center-bot · 2023-07-24T11:26:34Z

Conan v1 pipeline ❌

Sorry, the system is under maintenance and it doesn't accept builds right now.
Please, check https://status.conan.io to obtain more information.
Thanks for your understanding and help with the Conan Center Index!

Conan v2 pipeline ❌

Note: Conan v2 builds may be required once they are on the v2 ready list

The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future.

See details:

Sorry, the system is under maintenance and it doesn't accept builds right now.
Please, check https://status.conan.io to obtain more information.
Thanks for your understanding and help with the Conan Center Index!

stale · 2023-10-15T12:07:20Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions · 2024-02-16T06:36:39Z

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

bariscelik added 2 commits March 27, 2023 18:12

behaviortree.cpp: add version 4.1.0, 3.8.3

65e3b6c

add versions into config.yml

279dd3e

conan-center-bot added the User-approval pending label Mar 27, 2023