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: allow shared on Macos + add protobuf::protoc imported target #4776

Merged
merged 29 commits into from Mar 23, 2021
Merged

protobuf: allow shared on Macos + add protobuf::protoc imported target #4776

Changes from 6 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f320810
really provide protobuf::protoc target
SpaceIm Mar 3, 2021
f13d5b7
cosmetic change
SpaceIm Mar 3, 2021
d636224
simplify check of runtime
SpaceIm Mar 3, 2021
c4037a2
properly handle protobuf with zlib
SpaceIm Mar 3, 2021
5df0f08
append to builddirs
SpaceIm Mar 3, 2021
0913f5f
cleanup
SpaceIm Mar 3, 2021
725d9d4
fix semicolon replacement
SpaceIm Mar 3, 2021
d879ac2
more robust DYLD_LIBRARY_PATH injection
SpaceIm Mar 4, 2021
c2c6033
cosmetic change
SpaceIm Mar 4, 2021
ab34548
improve runtime handling
SpaceIm Mar 4, 2021
0408def
no protobuf_BUILD_LIBPROTOC option before 3.14.0
SpaceIm Mar 4, 2021
f5a965d
honor upstream default value for with_zlib
SpaceIm Mar 4, 2021
a7d6760
add protobuf/3.15.4
SpaceIm Mar 4, 2021
10ac43a
provide Protobuf_PROTOC_EXECUTABLE CACHE variable
SpaceIm Mar 4, 2021
abb921f
trick to no break pkg_config generator internal handling in conan
SpaceIm Mar 4, 2021
dc170c4
workaround for invalid-constexpr with clang
SpaceIm Mar 4, 2021
610126d
clang < 4 not supported in recent protobuf
SpaceIm Mar 5, 2021
d877dc5
cleanup
SpaceIm Mar 5, 2021
af51bdb
do not disable rtti by default
SpaceIm Mar 5, 2021
55a8007
Revert "workaround for invalid-constexpr with clang"
SpaceIm Mar 5, 2021
72f94bf
remove useless test of find_program
SpaceIm Mar 5, 2021
427035b
use TARGET_FILE in apple patch
SpaceIm Mar 5, 2021
b3d5a0c
add 3.15.5 instead of 3.15.4
SpaceIm Mar 5, 2021
3a56544
add rtti option if protobuf >= 3.15.4
SpaceIm Mar 7, 2021
69d7baa
Merge branch 'master' into fix/protobu-protoc-target
SpaceIm Mar 11, 2021
4de7716
removed duplicated version
SpaceIm Mar 11, 2021
cd6f6a6
Merge branch 'master' into fix/protobu-protoc-target
SpaceIm Mar 16, 2021
a46fe2d
empty commit
SpaceIm Mar 19, 2021
a0f4c76
keep find_program protoc logic for cross compilation
SpaceIm Mar 21, 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
112 changes: 53 additions & 59 deletions recipes/protobuf/all/conanfile.py
View file
Open in desktop
Expand Up @@ -34,12 +34,6 @@ def _build_subfolder(self):
def _is_clang_x86(self):
return self.settings.compiler == "clang" and self.settings.arch == "x86"

def source(self):
tools.get(**self.conan_data["sources"][self.version])
extracted_folder = self.name + "-" + self.version
os.rename(extracted_folder, self._source_subfolder)


def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC
Expand All @@ -48,20 +42,23 @@ def configure(self):
if self.options.shared:
del self.options.fPIC

if self.settings.os == "Windows" and self.settings.compiler in ["Visual Studio", "clang"] and "MT" in self.settings.compiler.runtime:
if self.settings.compiler.get_safe("runtime") in ["MT", "MTd"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we already care about this, but this seems not to be future proof with the settings for the new MSVC compiler

https://github.com/conan-io/conan/blob/03b3d71890c537df1154b3d04cc35ed2654a5894/conans/client/conf/__init__.py#L87

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a regression in any case, as the old code did not support the MSVC compiler settings either

Copy link
Contributor Author

@SpaceIm SpaceIm Mar 3, 2021 •
edited

Choose a reason for hiding this comment

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

I can add "static". A new method in tools would be welcome to avoid fragile verifications (something like tools.is_static_runtime(self.settings.compiler). There are many recipes like that.

raise ConanInvalidConfiguration("Protobuf can't be built with shared + MT(d) runtimes")

if tools.is_apple_os(self.settings.os):
raise ConanInvalidConfiguration("Protobuf could not be built as shared library for Mac.")

if self.settings.compiler == "Visual Studio":
if Version(self.settings.compiler.version) < "14":
raise ConanInvalidConfiguration("On Windows Protobuf can only be built with "
"Visual Studio 2015 or higher.")

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")
Expand All @@ -83,57 +80,62 @@ def _patch_sources(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)

find_protoc = """

# Find the protobuf compiler within the paths added by Conan, for use below.
find_program(PROTOC_PROGRAM protoc)
if(NOT PROTOC_PROGRAM)
set(PROTOC_PROGRAM "protoc")
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear to me how this is not conflicting with:
369121c#diff-5dfb647bf8fb82769ddb94143d6b47475c5d4d2db2346115ee6b1991a6db6445R112
What will the merge look like?

Copy link
Contributor Author

@SpaceIm SpaceIm Mar 19, 2021 •
edited

Choose a reason for hiding this comment

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

It's removed. This PR adds protobuf::protoc imported target and Protobuf_PROTOC_EXECUTABLE CACHE variable, but it's not very clear for me if it works with two profiles. I would like some feedbacks.
I should turn this PR to draft for the moment, since it could break cross compilation.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have pasted some commands below with one way you can test it.
The dual profile build doesn't seem to work.
CMake will find the wrong protoc (the one from host context instead of the one from build context).

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=protobuf

android-ndk-r22-api-21-arm64-v8a-clang-c++_static.txt:

[settings]
arch=armv8
build_type=Release
compiler=clang
compiler.libcxx=c++_static
compiler.version=11
os=Android
os.api_level=21
[build_requires]
android-ndk/r22
[options]
[env]

Result:

protobuf/3.15.5@foo/testing (test package): Calling build()
...
[1/4] Running cpp protocol buffer compiler on addressbook.proto
FAILED: addressbook.pb.h addressbook.pb.cc
...
/bin/sh: 1: x/.conan/data/protobuf/3.15.5/dirac/testing/package/475278d37419b9e365c649e594b7c9a419a343bb/bin/protoc: Exec format error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok so maybe we could keep this find_program, and use this path for protobuf::protoc properties.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, could work. I'm happy to try if you push something later :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you test with a0f4c76 please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems to work just fine. I'm happy with it :)

endif()
"""
# Inject relocatable protobuf::protoc target in protobuf-config.cmake.in
# 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(
os.path.join(self._source_subfolder, "cmake", "protobuf-config.cmake.in"),
protobuf_config_cmake,
"@_protobuf_FIND_ZLIB@",
"# CONAN PATCH _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)
tools.replace_in_file(
os.path.join(self._source_subfolder, "cmake", "protobuf-config.cmake.in"),
protobuf_config_cmake,
"include(\"${CMAKE_CURRENT_LIST_DIR}/protobuf-targets.cmake\")",
"# CONAN PATCH include(\"${CMAKE_CURRENT_LIST_DIR}/protobuf-targets.cmake\")" + find_protoc
("if(NOT TARGET protobuf::protoc)\n"
" add_executable(protobuf::protoc IMPORTED)\n"
" get_filename_component(PROTOC_FULL_PATH \"${{CMAKE_CURRENT_LIST_DIR}}/{protoc_rel_path}\" ABSOLUTE)\n"
" set_property(TARGET protobuf::protoc PROPERTY IMPORTED_LOCATION ${{PROTOC_FULL_PATH}})\n"
"endif()"
).format(protoc_rel_path=protoc_rel_path)
)

if tools.Version(self.version) < "3.12.0":
# 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(
os.path.join(self._source_subfolder, "cmake", "protobuf-config.cmake.in"),
"""COMMAND protobuf::protoc
ARGS --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_protobuf_include_path} ${_abs_file}
DEPENDS ${_abs_file} protobuf::protoc""",
"""COMMAND "${CMAKE_COMMAND}"
ARGS -E env "DYLD_LIBRARY_PATH=${Protobuf_LIB_DIRS}:${CONAN_LIB_DIRS}:${Protobuf_LIB_DIRS_RELEASE}:${Protobuf_LIB_DIRS_DEBUG}:${Protobuf_LIB_DIRS_RELWITHDEBINFO}:${Protobuf_LIB_DIRS_MINSIZEREL}" ${PROTOC_PROGRAM} --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_protobuf_include_path} ${_abs_file}
DEPENDS ${_abs_file} USES_TERMINAL"""
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(")
)
else:
tools.replace_in_file(
os.path.join(self._source_subfolder, "cmake", "protobuf-config.cmake.in"),
"""COMMAND protobuf::protoc
ARGS --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_plugin} ${_protobuf_include_path} ${_abs_file}
DEPENDS ${_abs_file} protobuf::protoc""",
"""COMMAND "${CMAKE_COMMAND}"
ARGS -E env "DYLD_LIBRARY_PATH=${Protobuf_LIB_DIRS}:${CONAN_LIB_DIRS}:${Protobuf_LIB_DIRS_RELEASE}:${Protobuf_LIB_DIRS_DEBUG}:${Protobuf_LIB_DIRS_RELWITHDEBINFO}:${Protobuf_LIB_DIRS_MINSIZEREL}" ${PROTOC_PROGRAM} --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_plugin} ${_protobuf_include_path} ${_abs_file}
DEPENDS ${_abs_file} USES_TERMINAL"""
protobuf_config_cmake,
"COMMAND protobuf::protoc",
("COMMAND export DYLD_LIBRARY_PATH=${CUSTOM_DYLD_LIBRARY_PATH}\n"
"COMMAND 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(
os.path.join(self._source_subfolder, "cmake", "protobuf-module.cmake.in"),
'if(DEFINED Protobuf_SRC_ROOT_FOLDER)',
"""if(0)
if(DEFINED Protobuf_SRC_ROOT_FOLDER)""",
protobuf_module_cmake,
"if(DEFINED Protobuf_SRC_ROOT_FOLDER)",
"if(0)\nif(DEFINED Protobuf_SRC_ROOT_FOLDER)",
)
tools.replace_in_file(
os.path.join(self._source_subfolder, "cmake", "protobuf-module.cmake.in"),
'# Define upper case versions of output variables',
'endif()',
protobuf_module_cmake,
"# Define upper case versions of output variables",
"endif()",
)

def build(self):
Expand Down Expand Up @@ -180,25 +182,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 = [
self._cmake_install_base_path,
]

self.cpp_info.components["libprotobuf"].builddirs = [self._cmake_install_base_path]
self.cpp_info.components["libprotobuf"].build_modules.extend([
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"]

self.cpp_info.components["protoc"].name = "protoc"
self.cpp_info.components["protoc"].requires.extend(["libprotoc", "libprotobuf"])

bindir = os.path.join(self.package_folder, "bin")
self.output.info("Appending PATH environment variable: {}".format(bindir))
self.env_info.PATH.append(bindir)
Expand All @@ -216,9 +210,9 @@ def package_info(self):
if self.options.shared:
self.cpp_info.components["libprotobuf-lite"].defines = ["PROTOBUF_USE_DLLS"]

self.cpp_info.components["libprotobuf-lite"].builddirs = [self._cmake_install_base_path]
self.cpp_info.components["libprotobuf-lite"].build_modules.extend([
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"),
])
]