Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[protobuf] Enable Android/iOS cross building #4556

Merged
merged 2 commits into from Mar 16, 2021
Merged

[protobuf] Enable Android/iOS cross building #4556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d81b812
[protobuf] Enable Android/iOS cross building
anton-danielsson Feb 13, 2021
facc289
[protobuf] Update patches
anton-danielsson Mar 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions recipes/protobuf/all/conandata.yml
View file
Open in desktop
Expand Up @@ -20,6 +20,17 @@ patches:
base_path: "source_subfolder"
- patch_file: "patches/upstream-issue-7567-no-export-template-define.patch"
base_path: "source_subfolder"
- patch_file: "patches/upstream-pr-7288-android-log.patch"
base_path: "source_subfolder"
- patch_file: "patches/upstream-pr-8301-bundle-destination.patch"
base_path: "source_subfolder"
3.13.0:
- patch_file: "patches/upstream-pr-7761-cmake-regex-fix.patch"
base_path: "source_subfolder"
- patch_file: "patches/upstream-pr-7288-android-log.patch"
base_path: "source_subfolder"
- patch_file: "patches/upstream-pr-8301-bundle-destination.patch"
base_path: "source_subfolder"
3.15.5:
- patch_file: "patches/upstream-pr-8301-bundle-destination.patch"
base_path: "source_subfolder"
6 changes: 5 additions & 1 deletion recipes/protobuf/all/conanfile.py
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ def _patch_sources(self):
find_protoc = """

# Find the protobuf compiler within the paths added by Conan, for use below.
find_program(PROTOC_PROGRAM protoc)
find_program(PROTOC_PROGRAM protoc PATHS ENV PATH NO_DEFAULT_PATH)
anton-danielsson marked this conversation as resolved.
Show resolved Hide resolved
if(NOT PROTOC_PROGRAM)
set(PROTOC_PROGRAM "protoc")
endif()
Expand Down Expand Up @@ -200,6 +200,8 @@ def package_info(self):
self.cpp_info.components["libprotobuf"].system_libs.append("pthread")
if self._is_clang_x86 or "arm" in str(self.settings.arch):
self.cpp_info.components["libprotobuf"].system_libs.append("atomic")
if self.settings.os == "Android":
self.cpp_info.components["libprotobuf"].system_libs.append("log")
if self.settings.os == "Windows":
if self.options.shared:
self.cpp_info.components["libprotobuf"].defines = ["PROTOBUF_USE_DLLS"]
Expand Down Expand Up @@ -238,6 +240,8 @@ def package_info(self):
if self.settings.os == "Windows":
if self.options.shared:
self.cpp_info.components["libprotobuf-lite"].defines = ["PROTOBUF_USE_DLLS"]
if self.settings.os == "Android":
self.cpp_info.components["libprotobuf-lite"].system_libs.append("log")

self.cpp_info.components["libprotobuf-lite"].builddirs = [self._cmake_install_base_path]
self.cpp_info.components["libprotobuf-lite"].build_modules.extend([
Expand Down
26 changes: 26 additions & 0 deletions recipes/protobuf/all/patches/upstream-pr-7288-android-log.patch
View file
Open in desktop
@@ -0,0 +1,26 @@
https://github.com/protocolbuffers/protobuf/pull/7288
--- a/cmake/libprotobuf.cmake
anton-danielsson marked this conversation as resolved.
Show resolved Hide resolved
+++ b/cmake/libprotobuf.cmake
@@ -121,6 +121,9 @@ endif()
if(protobuf_LINK_LIBATOMIC)
target_link_libraries(libprotobuf atomic)
endif()
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Android")
+ target_link_libraries(libprotobuf log)
+endif()
Comment on lines +8 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch like this could be implemented not like a patch, but as a small change in this file:
https://github.com/conan-io/conan-center-index/blob/master/recipes/protobuf/all/CMakeLists.txt

This will work forever and easy to maintain for future versions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uilianries @prince-chrismc please, share your opinion on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I general I would agree but since the PR corresponding to this patch is already merge to protobuf (v3.14.0) the future maintenance should not suffer.
protocolbuffers/protobuf#7288

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @mathbunnyru , but as it's a patch related for building support and already accepted by the upstream, I see no problem in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also the complexity of adding protobuf versions which (imo) is always more complicated than it seems.
This case is acceptable. It follow our usually pattern.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, everyone. I suggest we add upstream PR link to the patch and it should be good to go.

target_include_directories(libprotobuf PUBLIC ${protobuf_source_dir}/src)
if(MSVC AND protobuf_BUILD_SHARED_LIBS)
target_compile_definitions(libprotobuf

--- a/cmake/libprotobuf-lite.cmake
+++ b/cmake/libprotobuf-lite.cmake
@@ -67,6 +67,9 @@ target_link_libraries(libprotobuf-lite ${CMAKE_THREAD_LIBS_INIT})
if(protobuf_LINK_LIBATOMIC)
target_link_libraries(libprotobuf-lite atomic)
endif()
+if(${CMAKE_SYSTEM_NAME} STREQUAL "Android")
+ target_link_libraries(libprotobuf-lite log)
+endif()
target_include_directories(libprotobuf-lite PUBLIC ${protobuf_source_dir}/src)
if(MSVC AND protobuf_BUILD_SHARED_LIBS)
target_compile_definitions(libprotobuf-lite
11 changes: 11 additions & 0 deletions recipes/protobuf/all/patches/upstream-pr-8301-bundle-destination.patch
View file
Open in desktop
@@ -0,0 +1,11 @@
--- a/cmake/install.cmake
+++ b/cmake/install.cmake
@@ -30,7 +30,7 @@ endforeach()

if (protobuf_BUILD_PROTOC_BINARIES)
install(TARGETS protoc EXPORT protobuf-targets
- RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT protoc)
+ RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR} BUNDLE DESTINATION ${CMAKE_INSTALL_BINDIR} COMPONENT protoc)
if (UNIX AND NOT APPLE)
set_property(TARGET protoc
PROPERTY INSTALL_RPATH "$ORIGIN/../${CMAKE_INSTALL_LIBDIR}")
5 changes: 0 additions & 5 deletions recipes/protobuf/all/test_package/CMakeLists.txt
View file
Open in desktop
Expand Up @@ -6,11 +6,6 @@ conan_basic_setup(TARGETS)

find_package(protobuf CONFIG REQUIRED)

find_program(PROTOC_PROGRAM protoc)
anton-danielsson marked this conversation as resolved.
Show resolved Hide resolved
if (NOT PROTOC_PROGRAM)
message(WARNING "Protoc was not found")
endif()

add_executable(${PROJECT_NAME} test_package.cpp addressbook.proto)
set_property(TARGET ${PROJECT_NAME} PROPERTY CXX_STANDARD 11)
target_include_directories(${PROJECT_NAME} PRIVATE "${CMAKE_BINARY_DIR}")
Expand Down
13 changes: 8 additions & 5 deletions recipes/protobuf/all/test_package/conanfile.py
View file
Open in desktop
Expand Up @@ -6,12 +6,15 @@ class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package_multi"

def build_requirements(self):
if tools.cross_building(self.settings):
self.build_requires(str(self.requires['protobuf']))
anton-danielsson marked this conversation as resolved.
Show resolved Hide resolved

def build(self):
if not tools.cross_building(self.settings, skip_x64_x86=True):
cmake = CMake(self)
cmake.definitions["protobuf_LITE"] = self.options["protobuf"].lite
cmake.configure()
cmake.build()
cmake = CMake(self)
cmake.definitions["protobuf_LITE"] = self.options["protobuf"].lite
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self.settings):
Expand Down