protobuf: allow shared on Macos + add protobuf::protoc imported target #4776
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| from conans import ConanFile, CMake, tools | ||
| from conans.errors import ConanInvalidConfiguration | ||
| import os | ||
| import textwrap | ||
|
|
||
|
|
||
| class ProtobufConan(ConanFile): | ||
|
|
@@ -12,25 +12,27 @@ class ProtobufConan(ConanFile): | |
| url = "https://github.com/conan-io/conan-center-index" | ||
| homepage = "https://github.com/protocolbuffers/protobuf" | ||
| license = "BSD-3-Clause" | ||
|
|
||
| settings = "os", "arch", "compiler", "build_type" | ||
| options = { | ||
| "shared": [True, False], | ||
| "fPIC": [True, False], | ||
| "with_zlib": [True, False], | ||
| "with_rtti": [True, False], | ||
| "lite": [True, False], | ||
| } | ||
| default_options = { | ||
| "shared": False, | ||
| "fPIC": True, | ||
| "with_zlib": True, | ||
| "with_rtti": True, | ||
| "lite": False, | ||
| } | ||
|
|
||
| short_paths = True | ||
|
|
||
| exports_sources = ["CMakeLists.txt", "patches/*"] | ||
| generators = "cmake" | ||
| _cmake = None | ||
|
|
||
| @property | ||
|
|
@@ -49,11 +51,6 @@ def _is_clang_x86(self): | |
| def _can_disable_rtti(self): | ||
| return tools.Version(self.version) >= "3.15.4" | ||
|
|
||
| def config_options(self): | ||
| if self.settings.os == "Windows": | ||
| del self.options.fPIC | ||
|
|
@@ -64,14 +61,11 @@ def configure(self): | |
| if self.options.shared: | ||
| del self.options.fPIC | ||
|
|
||
| if str(self.settings.compiler.get_safe("runtime")) in ["MT", "MTd", "static"]: | ||
|
There was a problem hiding this comment. why did it change?! https://github.com/conan-io/conan/blob/03b3d71890c537df1154b3d04cc35ed2654a5894/conans/client/conf/__init__.py#L91 Nice catch you two! There was a problem hiding this comment. It is a new compiler setting, so strictly speaking it isn't changed :) |
||
| raise ConanInvalidConfiguration("Protobuf can't be built with shared + MT(d) runtimes") | ||
|
|
||
| if self.settings.compiler == "Visual Studio": | ||
| if tools.Version(self.settings.compiler.version) < "14": | ||
| raise ConanInvalidConfiguration("On Windows Protobuf can only be built with " | ||
| "Visual Studio 2015 or higher.") | ||
|
|
||
|
|
@@ -83,6 +77,11 @@ def requirements(self): | |
| if self.options.with_zlib: | ||
| self.requires("zlib/1.2.11") | ||
|
|
||
| def source(self): | ||
| tools.get(**self.conan_data["sources"][self.version]) | ||
| extracted_folder = self.name + "-" + self.version | ||
| os.rename(extracted_folder, self._source_subfolder) | ||
|
|
||
| @property | ||
| def _cmake_install_base_path(self): | ||
| return os.path.join("lib", "cmake", "protobuf") | ||
|
|
@@ -94,69 +93,81 @@ def _configure_cmake(self): | |
| self._cmake.definitions["protobuf_WITH_ZLIB"] = self.options.with_zlib | ||
| self._cmake.definitions["protobuf_BUILD_TESTS"] = False | ||
| self._cmake.definitions["protobuf_BUILD_PROTOC_BINARIES"] = True | ||
| if tools.Version(self.version) >= "3.14.0": | ||
| self._cmake.definitions["protobuf_BUILD_LIBPROTOC"] = True | ||
| if self._can_disable_rtti: | ||
| self._cmake.definitions["protobuf_DISABLE_RTTI"] = not self.options.with_rtti | ||
| if self.settings.compiler.get_safe("runtime"): | ||
| self._cmake.definitions["protobuf_MSVC_STATIC_RUNTIME"] = str(self.settings.compiler.runtime) in ["MT", "MTd", "static"] | ||
| self._cmake.configure(build_folder=self._build_subfolder) | ||
| return self._cmake | ||
|
|
||
| def _patch_sources(self): | ||
| for patch in self.conan_data.get("patches", {}).get(self.version, []): | ||
| tools.patch(**patch) | ||
|
|
||
|
There was a problem hiding this comment. It's not clear to me how this is not conflicting with: There was a problem hiding this comment. It's removed. This PR adds There was a problem hiding this comment. I have pasted some commands below with one way you can test it. Command: conan create . 3.15.5@foo/testing \
--profile:host android-ndk-r22-api-21-arm64-v8a-clang-c++_static.txt \
--profile:build default \
--build=protobufandroid-ndk-r22-api-21-arm64-v8a-clang-c++_static.txt: Result: There was a problem hiding this comment. Ok so maybe we could keep this There was a problem hiding this comment. Yeah, could work. I'm happy to try if you push something later :) There was a problem hiding this comment. Could you test with a0f4c76 please? There was a problem hiding this comment. Seems to work just fine. I'm happy with it :) |
||
| # Provide relocatable protobuf::protoc target and Protobuf_PROTOC_EXECUTABLE cache variable | ||
| # TODO: some of the following logic might be disabled when conan will | ||
| # allow to create executable imported targets in package_info() | ||
| protobuf_config_cmake = os.path.join(self._source_subfolder, "cmake", "protobuf-config.cmake.in") | ||
|
|
||
| tools.replace_in_file( | ||
| protobuf_config_cmake, | ||
| "@_protobuf_FIND_ZLIB@", | ||
| "# BEGIN CONAN PATCH\n#_protobuf_FIND_ZLIB@\n# END CONAN PATCH" | ||
| ) | ||
|
|
||
| exe_ext = ".exe" if self.settings.os == "Windows" else "" | ||
| protoc_filename = "protoc" + exe_ext | ||
| module_folder_depth = len(os.path.normpath(self._cmake_install_base_path).split(os.path.sep)) | ||
| protoc_rel_path = "{}bin/{}".format("".join(["../"] * module_folder_depth), protoc_filename) | ||
| protoc_target = textwrap.dedent("""\ | ||
| if(NOT TARGET protobuf::protoc) | ||
| find_program(PROTOC_PROGRAM protoc PATHS ENV PATH NO_DEFAULT_PATH) | ||
| if(NOT PROTOC_PROGRAM) | ||
| set(PROTOC_PROGRAM \"${{CMAKE_CURRENT_LIST_DIR}}/{protoc_rel_path}\") | ||
| endif() | ||
| get_filename_component(PROTOC_PROGRAM \"${{PROTOC_PROGRAM}}\" ABSOLUTE) | ||
| set(Protobuf_PROTOC_EXECUTABLE ${{PROTOC_PROGRAM}} CACHE FILEPATH \"The protoc compiler\") | ||
| add_executable(protobuf::protoc IMPORTED) | ||
| set_property(TARGET protobuf::protoc PROPERTY IMPORTED_LOCATION ${{Protobuf_PROTOC_EXECUTABLE}}) | ||
| endif() | ||
| """.format(protoc_rel_path=protoc_rel_path)) | ||
| tools.replace_in_file( | ||
| protobuf_config_cmake, | ||
| "include(\"${CMAKE_CURRENT_LIST_DIR}/protobuf-targets.cmake\")", | ||
| protoc_target | ||
| ) | ||
|
|
||
| # Set DYLD_LIBRARY_PATH in command line to avoid issues with shared protobuf | ||
| # (even with virtualrunenv, this fix might be required due to SIP) | ||
| # Only works with cmake, cmake_find_package or cmake_find_package_multi generators | ||
| if tools.is_apple_os(self.settings.os): | ||
| tools.replace_in_file( | ||
| protobuf_config_cmake, | ||
| "add_custom_command(", | ||
| ("set(CUSTOM_DYLD_LIBRARY_PATH ${CONAN_LIB_DIRS} ${Protobuf_LIB_DIRS} ${Protobuf_LIB_DIRS_RELEASE} ${Protobuf_LIB_DIRS_DEBUG} ${Protobuf_LIB_DIRS_RELWITHDEBINFO} ${Protobuf_LIB_DIRS_MINSIZEREL})\n" | ||
| "string(REPLACE \";\" \":\" CUSTOM_DYLD_LIBRARY_PATH \"${CUSTOM_DYLD_LIBRARY_PATH}\")\n" | ||
| "add_custom_command(") | ||
| ) | ||
| tools.replace_in_file( | ||
| protobuf_config_cmake, | ||
| "COMMAND protobuf::protoc", | ||
| "COMMAND ${CMAKE_COMMAND} -E env \"DYLD_LIBRARY_PATH=${CUSTOM_DYLD_LIBRARY_PATH}\" $<TARGET_FILE:protobuf::protoc>" | ||
| ) | ||
|
|
||
| # Disable a potential warning in protobuf-module.cmake.in | ||
| # TODO: remove this patch? Is it really useful? | ||
| protobuf_module_cmake = os.path.join(self._source_subfolder, "cmake", "protobuf-module.cmake.in") | ||
| tools.replace_in_file( | ||
| protobuf_module_cmake, | ||
| "if(DEFINED Protobuf_SRC_ROOT_FOLDER)", | ||
| "if(0)\nif(DEFINED Protobuf_SRC_ROOT_FOLDER)", | ||
| ) | ||
| tools.replace_in_file( | ||
| protobuf_module_cmake, | ||
| "# Define upper case versions of output variables", | ||
| "endif()", | ||
| ) | ||
|
|
||
| def build(self): | ||
|
|
@@ -186,6 +197,7 @@ def package_info(self): | |
|
|
||
| self.cpp_info.names["cmake_find_package"] = "protobuf" | ||
| self.cpp_info.names["cmake_find_package_multi"] = "protobuf" | ||
| self.cpp_info.names["pkg_config"] = "protobuf_full_package" # unofficial, but required to avoid side effects (libprotobuf component "steals" the default global pkg_config name) | ||
|
|
||
| lib_prefix = "lib" if self.settings.compiler == "Visual Studio" else "" | ||
| lib_suffix = "d" if self.settings.build_type == "Debug" else "" | ||
|
|
@@ -205,25 +217,17 @@ def package_info(self): | |
| if self.settings.os == "Windows": | ||
| if self.options.shared: | ||
| self.cpp_info.components["libprotobuf"].defines = ["PROTOBUF_USE_DLLS"] | ||
| self.cpp_info.components["libprotobuf"].builddirs.append(self._cmake_install_base_path) | ||
| self.cpp_info.components["libprotobuf"].build_modules = [ | ||
| os.path.join(self._cmake_install_base_path, "protobuf-generate.cmake"), | ||
| os.path.join(self._cmake_install_base_path, "protobuf-module.cmake"), | ||
| os.path.join(self._cmake_install_base_path, "protobuf-options.cmake"), | ||
| ] | ||
|
|
||
| self.cpp_info.components["libprotoc"].name = "libprotoc" | ||
| self.cpp_info.components["libprotoc"].libs = [lib_prefix + "protoc" + lib_suffix] | ||
| self.cpp_info.components["libprotoc"].requires = ["libprotobuf"] | ||
|
|
||
| bindir = os.path.join(self.package_folder, "bin") | ||
| self.output.info("Appending PATH environment variable: {}".format(bindir)) | ||
| self.env_info.PATH.append(bindir) | ||
|
|
@@ -243,9 +247,9 @@ def package_info(self): | |
| if self.settings.os == "Android": | ||
| self.cpp_info.components["libprotobuf-lite"].system_libs.append("log") | ||
|
|
||
| self.cpp_info.components["libprotobuf-lite"].builddirs.append(self._cmake_install_base_path) | ||
| self.cpp_info.components["libprotobuf-lite"].build_modules = [ | ||
| os.path.join(self._cmake_install_base_path, "protobuf-generate.cmake"), | ||
| os.path.join(self._cmake_install_base_path, "protobuf-module.cmake"), | ||
| os.path.join(self._cmake_install_base_path, "protobuf-options.cmake"), | ||
| ] | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| from conans import ConanFile, CMake, tools | ||
| import os | ||
|
|
||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,11 @@ | ||
| versions: | ||
| "3.9.1": | ||
| folder: all | ||
| "3.11.4": | ||
| folder: all | ||
| "3.12.4": | ||
| folder: all | ||
| "3.13.0": | ||
| folder: all | ||
| "3.15.5": | ||
| folder: all |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Changing option 🙈
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.
@SpaceIm why
with_zlibshould be True by default?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.
@prince-chrismc you are reviewing the merge of master branch into this PR. Actually, I don't change the option name introduced in #4682, I resolve a conflict because I also added this option in this PR prior to #4682 merge, but with a different name.
@uilianries
with_zlibshould be True because it's enabled by default in upstream build files: https://github.com/protocolbuffers/protobuf/blob/146d579738acf7c990611365abf09e1f71d8bfc1/cmake/CMakeLists.txt#L58-L59You can also look at
configure.ac, the logic is less obvious, but it expects zlib by default:https://github.com/protocolbuffers/protobuf/blob/146d579738acf7c990611365abf09e1f71d8bfc1/configure.ac#L55-L58
https://github.com/protocolbuffers/protobuf/blob/146d579738acf7c990611365abf09e1f71d8bfc1/configure.ac#L139-L171
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.
derp, thank you I see it now that I turned off the filter
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.
@SpaceIm Thank you!