up-cpp: new recipe #23733
up-cpp: new recipe #23733
Conversation
ghost
mentioned this pull request
This comment has been minimized.
This comment has been minimized.
PLeVasseur
force-pushed
the
new/up-cpp
branch
2 times, most recently
from
b6b5a1a
to
7698cf4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
* Added build_requirements() method which specifies that CMake must be 3.18 or higher
* added src folder back under layout()
* Attempt to isolate the clang + libc++ case and mark it as an invalid configuration
* Had to turn off msvc compilation since currently CMake files can only handle / and not \ for paths
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
* Cleaned up some of the formatting for explanations as to why a configuration wasn't valid
* Some further clean-up
Conan v1 pipeline ✔️All green in build 9 (
Conan v2 pipeline ✔️
All green in build 9 (
|
|
I noticed in a previous attempt to package up-cpp when we were using a git submodule that was seen as unacceptable. I read that it's possible to instead use the Conan Git helper and clone in a certain tag based on our version in Would that suffice? |
|
Hey folks 👋 Is it possible to receive a review to know what I'll need to fix / update? Thanks for your support! -Pete |
There was a problem hiding this comment.
@PLeVasseur Thank you for your first contribution, it looks an interesting project. Please, take a look in my review and please, try to clarify the proto part in this project, is not clear why is it needed and if should be added as a separated package.
There was a problem hiding this comment.
@PLeVasseur Could you please explain why do you need 2 different packages here? What are they (e.g library and compiler)?
We don't use Git feature for download, only the tag directly, because the source should be immutable.
| return { | ||
| "apple-clang": "13", | ||
| "clang": "13", | ||
| "gcc": "11", |
There was a problem hiding this comment.
GCC 6 supports C++14 already, and other compilers also can be lowered: https://en.cppreference.com/w/cpp/compiler_support/14
Or, is there a real feature required?
| def export_sources(self): | ||
| export_conandata_patches(self) | ||
|
|
There was a problem hiding this comment.
| def export_sources(self): | |
| export_conandata_patches(self) |
No patches to be applied, so not needed :)
| self.folders.build = os.path.join("build", str(self.settings.build_type)) | ||
| self.folders.install = "install" |
There was a problem hiding this comment.
| self.folders.build = os.path.join("build", str(self.settings.build_type)) | |
| self.folders.install = "install" |
Please, keep the default layout.
| if self.options.build_testing: | ||
| self.requires("gtest/1.14.0") |
There was a problem hiding this comment.
| if self.options.build_testing: | |
| self.requires("gtest/1.14.0") | |
| if self.options.build_testing: | |
| self.requires("gtest/1.14.0") |
Testing is not something really supported, because the idea is packaging the project, not acting like a build tool. Still, in case you really want to be able to run unit tests, please, use the follow approach: https://github.com/conan-io/conan-center-index/blob/master/recipes/tree-gen/all/conanfile.py#L33 It will not affect the package ID, which is really important.
| f"{self.ref} need to have paths in CMake files updated to handle both \ and /." | ||
| ) | ||
| if self.settings.compiler == "clang" and stdcpp_library(self) == "c++": | ||
| raise ConanInvalidConfiguration( |
There was a problem hiding this comment.
Do you have a build log with that error? It would be nice pointing it as a comment in the recipe. For the message, it can be simplified like:
f"{self.ref} can not be built with clang and libc++ due math.h errors"
| git = Git(self) | ||
| git_method = os.getenv("GIT_METHOD", "https") | ||
| git.clone(self.conan_data["proto"][self.version][git_method], target=self.proto_containing_folder) | ||
| git.folder=self.proto_containing_folder | ||
| git.checkout(self.conan_data["proto"][self.version]["tag"]) |
There was a problem hiding this comment.
| git = Git(self) | |
| git_method = os.getenv("GIT_METHOD", "https") | |
| git.clone(self.conan_data["proto"][self.version][git_method], target=self.proto_containing_folder) | |
| git.folder=self.proto_containing_folder | |
| git.checkout(self.conan_data["proto"][self.version]["tag"]) | |
| git = Git(self) | |
| git_method = os.getenv("GIT_METHOD", "https") | |
| git.clone(self.conan_data["proto"][self.version][git_method], target=self.proto_containing_folder) | |
| git.folder=self.proto_containing_folder | |
| git.checkout(self.conan_data["proto"][self.version]["tag"]) |
Need more detail about it, we don't use Git in ConanCenterIndex, instead, we download the tar.gz to be sure about the immutability of the package.
| tc.variables["PROTO_PATH"] = os.path.join(self.proto_containing_folder, os.path.join(self.proto_subfolder, self.proto_folder)) | ||
| tc.variables["BUILD_TESTING"] = self.options.build_testing | ||
| tc.variables["BUILD_UNBUNDLED"] = self.options.build_unbundled | ||
| tc.variables["BUILD_SHARED_LIBS"] = self.options.shared |
There was a problem hiding this comment.
| tc.variables["BUILD_SHARED_LIBS"] = self.options.shared |
Automatically handled by CMakeToolchain: https://docs.conan.io/2/reference/tools/cmake/cmaketoolchain.html#extending-and-advanced-customization
| self.cpp_info.set_property("cmake_file_name", "up-cpp") | ||
| self.cpp_info.set_property("cmake_target_name", "up-cpp::up-cpp") | ||
| self.cpp_info.set_property("pkg_config_name", "up-cpp") | ||
| self.cpp_info.libs = collect_libs(self) | ||
|
|
||
| self.cpp_info.set_property("cmake_target_name", "up-cpp::up-cpp") | ||
| self.cpp_info.requires = ["spdlog::spdlog", "protobuf::protobuf"] | ||
|
|
||
| self.cpp_info.names["cmake_find_package"] = "up-cpp" | ||
| self.cpp_info.names["cmake_find_package_multi"] = "up-cpp" |
There was a problem hiding this comment.
| self.cpp_info.set_property("cmake_file_name", "up-cpp") | |
| self.cpp_info.set_property("cmake_target_name", "up-cpp::up-cpp") | |
| self.cpp_info.set_property("pkg_config_name", "up-cpp") | |
| self.cpp_info.libs = collect_libs(self) | |
| self.cpp_info.set_property("cmake_target_name", "up-cpp::up-cpp") | |
| self.cpp_info.requires = ["spdlog::spdlog", "protobuf::protobuf"] | |
| self.cpp_info.names["cmake_find_package"] = "up-cpp" | |
| self.cpp_info.names["cmake_find_package_multi"] = "up-cpp" |
All these configuration is generated by default ;)
| self.requires("protobuf/3.21.12") | ||
| self.requires("spdlog/1.13.0") |
There was a problem hiding this comment.
| self.requires("protobuf/3.21.12") | |
| self.requires("spdlog/1.13.0") |
Transitive dependencies are managed by Conan already.
|
Hey @uilianries 👋 I wanted to follow up here. My employer has decided to part ways with many staff, myself included. I was working on Eclipse uProtocol as my "day job". My ability to contribute to Eclipse uProtocol is reduced at the moment. I'd propose closing this for now and revisiting when myself or another maintainer has bandwidth. |
|
Hello @PLeVasseur! I'm currently on vacation 🌴, please, ping |
Specify library name and version: up-cpp/0.1.2-dev
Hi there 👋
I contribute to the Eclipse uProtocol project for developing a Software Defined Vehicle (SDV) middleware to knit together compute within the vehicle, cloud, mobile. We'd like to be able to host the
up-cpppackage on CCI since it seems like the reasonable first choice.Forgive me as I've not contributed before I'm not entirely sure I've done everything correctly 😅
I left a few questions and TODOs in the conanfile.py as guidance to where I'm unsure in case the recipe is unable to build in certain spots. Will remove those once I can confirm the recipe passes as-is.