Add protobuf 3.5.1.1 version #15202
Add protobuf 3.5.1.1 version #15202
Conversation
|
|
This comment has been minimized.
This comment has been minimized.
|
I detected other pull requests that are modifying protobuf/all recipe: This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there. |
This comment has been minimized.
This comment has been minimized.
|
You need to wait for access request before it will be merged :) |
This comment has been minimized.
This comment has been minimized.
Hooks produced the following warnings for commit 00c7e93protobuf/3.5.1.1 |
There was a problem hiding this comment.
Hi @guillaume-michel, thanks for opening this PR.
We typically don't add older versions unless sufficiently justified, and Protobuf 3.5.1.1 is a little over 5 years old.
We have recently removed logic in the recipe to support older versions, which simplifies the logic in the recipe. This would be moving in the opposite direction, and may introduce corner cases in which certain configurations don't work.
In particular with protobuf, my understanding is that Protobuf 3 is backwards compatible and for users it should generally be safe to upgrade to newer versions without experiencing breakage - so long as they have the ability to rebuild/relink protobuf-dependent code (as there are no guarantees on binary compatibility).
Is that the case here? Would love to understand what's holding back being able to move to a newer version, so that we can take appropriate action, as we'd love to support this use case :)
|
We can put the older version in a separate recipe folder so not complicate the newer ones |
Hi, We are building a medical robot which is operated via a vendor provided binary only library which exposes protobuf 3.5.1.1 in its public interface. We tried to ask for an upgraded version but did not work out in the end. Our only option right now is to use protobuf 3.5.1.1. In general, I agree with removing old legacy code but in practice it is not always possible. I am also worried that 2 years from now I could not find the dependencies I am looking for in conan and could not build old version. I am aware that I can use my own artifactory to keep my binaries but how am I supposed to maintain recipes which disappears from conan as time goes on? My naive approach would be to keep a compulsive cumulative strategy but I know it is a large burden to maintain. |
You can see our FAQ for more details but we never delete anything from ConanCenter. That does mean version stop getting updates. Unless there's a specific use-case (which this does seem to meet) then we drop support them when they are too burdensome. The packages are always available but the support and maintenance is not 😸 hopeful that clarifies the policy If you contribute a working version that is not too intrusive I dont see why it could not be accepted but thank you for filling in the explanation as to why ❤️ |
Hi @guillaume-michel - thanks for your reply. Indeed I can see your predicament. If you are a consuming closed-source binaries that impose a specific version, this could be a problem. Out of curiosity, are the Protobuf binaries also provided with the Kinova library? Given that Protobuf does not guarantee binary compatibility (unlike Qt for example, that has some guarantees), even if you independently build it I'm not sure it's guaranteed to work. As @prince-chrismc points out, it should be clarified that the Conan Center remote will serve all versions that have ever been available - even if the recipe in the repository has stopped supporting older versions, so if Protobuf 3.5 were to be published, it would be available in the future even if the recipe stops supporting it. As he mentions, if we can get this to pass our CI checks on the expected platforms, we're happy to proceed with this PR. |
|
I am trying to fix the CI failures for protobuf/3.5.1.1 with shared library but I don't know how to fix it. profile failure1 is: When I am building the package locally on my machine with the following command: When I am building the package with the following command: Repeatting the same with 3.21.9 work in both cases. Comparing protoc for both versions shows that protoc 3.5.1.1 is missing: which is there because there is the following in the RUNPATH in libprotocd.so v3.21.9: Any idea why the test_package fails when using a single profile? Thanks for your help |
|
In the meantime, I will have a look at the 3.5.1.1 CMakeLists.txt and try to patch the file so the RUNPATH is correctly added. |
Fix missing rpath
@jcar87 Kinova provides 2 libraries for their robot:
We chose to use the 1 without included protobuf because we are using protobuf elsewhere in our codebase and we don't want to link with whole kinova library for that part. Hope it clarifies the situation. |
Conan v1 pipeline ✔️All green in build 5 (
Conan v2 pipeline (informative, not required for merge) ✔️
All green in build 4 (
|
| --- a/src/google/protobuf/compiler/js/embed.cc | ||
| +++ /dev/null |
There was a problem hiding this comment.
Why don't you keep this file if it is not used anymore? Patch would be more readable.
| diff --git a/cmake/install.cmake b/cmake/install.cmake | ||
| index 441bf55..ee17601 100644 | ||
| --- a/cmake/install.cmake | ||
| +++ b/cmake/install.cmake | ||
| @@ -13,6 +13,13 @@ foreach(_library | ||
| PROPERTY INTERFACE_INCLUDE_DIRECTORIES | ||
| $<BUILD_INTERFACE:${protobuf_source_dir}/src> | ||
| $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>) | ||
| + if (UNIX AND NOT APPLE) | ||
| + set_property(TARGET ${_library} | ||
| + PROPERTY INSTALL_RPATH "$ORIGIN") | ||
| + elseif (APPLE) | ||
| + set_property(TARGET ${_library} | ||
| + PROPERTY INSTALL_RPATH "@loader_path") | ||
| + endif() | ||
| install(TARGETS ${_library} EXPORT protobuf-targets | ||
| RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT ${_library} | ||
| LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} COMPONENT ${_library} | ||
| @@ -21,6 +28,13 @@ endforeach() | ||
|
|
||
| install(TARGETS protoc EXPORT protobuf-targets | ||
| RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT protoc) | ||
| +if (UNIX AND NOT APPLE) | ||
| + set_property(TARGET protoc | ||
| + PROPERTY INSTALL_RPATH "$ORIGIN/../lib") | ||
| +elseif (APPLE) | ||
| + set_property(TARGET protoc | ||
| + PROPERTY INSTALL_RPATH "@loader_path/../lib") | ||
| +endif() | ||
|
|
||
| install(FILES ${CMAKE_CURRENT_BINARY_DIR}/protobuf.pc ${CMAKE_CURRENT_BINARY_DIR}/protobuf-lite.pc DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig") | ||
|
|
There was a problem hiding this comment.
It can be avoided, conanfile.py of test_v1_package should be fixed instead:
from conans import ConanFile, CMake, tools
import os
class TestPackageConan(ConanFile):
settings = "os", "arch", "compiler", "build_type"
generators = "cmake", "cmake_find_package_multi"
test_type = "explicit"
def requirements(self):
self.requires(self.tested_reference_str)
def build_requirements(self):
if hasattr(self, "settings_build"):
self.build_requires(self.tested_reference_str)
def build(self):
with tools.no_op() if hasattr(self, "settings_build") else tools.run_environment(self):
cmake = CMake(self)
cmake.definitions["protobuf_LITE"] = self.options["protobuf"].lite
cmake.configure()
cmake.build()
def test(self):
if not tools.cross_building(self):
self.run("protoc --version", run_environment=True)
self.run(os.path.join("bin", "test_package"), run_environment=True)|
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. |
|
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. |
|
This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions. |
Specify library name and version: protobuf/3.5.1.1
This library is a dependency for Kinova robot's proprietary library.