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.

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

[package] protobuf/any: access to protobuf_generate_cpp function #1956

Closed
davidtazy opened this issue Jun 17, 2020 · 12 comments
Closed

[package] protobuf/any: access to protobuf_generate_cpp function #1956

davidtazy opened this issue Jun 17, 2020 · 12 comments
Labels
bug Something isn't working

Comments

@davidtazy
Copy link
Contributor

Current good practice is to use cmake_find_package generator.
on an other hand, i want conan to be as discreet as possible in my CMakelists.txt.
So this generator can be very helpful when no standard "Find<Package_name>.cmake exists.

For the protobuf library case, this generator hide a very important feature : the protobuf_generate_cpp function. And as a user cmake_find_package seem broken!

As a conan developer, i understand the fact that cmake guys should separate "target detection" and function utilities.
but cmake is a giant.... conan is not...yet.

furthermore, protobuf test_package is so sad ... download a binary (not built by conan!!!) and reinvent the wheel to generate cpp file.

Do u have a plan for this kind or issue?

Thx.

Package and Environment Details (include every applicable attribute)

  • Package Name/Version: protobuf/any
  • Operating System+version: any
  • Compiler+version: GCC 8
  • Docker image: conanio/gcc8
  • Conan version: conan 1.26.0
  • Python version: Python 3.7.4

Conan profile

[settings]
os=Linux
os_build=Linux
arch=x86_64
arch_build=x86_64
compiler=gcc
compiler.version=8
compiler.libcxx=libstdc++11
build_type=Release
[options]
[build_requires]
[env]
CC=gcc-8
CXX=g++-8

Steps to reproduce (Include if Applicable)

conanfile.txt

[requires]
protobuf/3.11.4
[generators]
cmake_find_package
cmake_paths

CMakefiles.txt

...
include(${CMAKE_CURRENT_BINARY_DIR}/conan_paths.cmake)
find_package(Protobuf REQUIRED)

include_directories(${CMAKE_CURRENT_BINARY_DIR})
protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS foo.proto)
protobuf_generate_python(PROTO_PY foo.proto)
add_executable(bar bar.cc ${PROTO_SRCS} ${PROTO_HDRS})
target_link_libraries(bar protobuf::libprotobuf)
@davidtazy davidtazy added the bug Something isn't working label Jun 17, 2020
@uilianries
Copy link
Member

@davidtazy !

It should be fixed by #1179

Please, follow that PR.

@davidtazy
Copy link
Contributor Author

#1179 is better. nice work
but my configuration(see the first post) fails because of two things:

  • "official protobuf target" protobuf::libprotobuf is not available, available target is Protobuf::Protobuf
  • had to append virtualrunenv generator in conanfile.txt to locate protoc during "make" step.

make command without virtualrunenv generator:

[ 25%] Running cpp protocol buffer compiler on addressbook.proto
/usr/local/bin/cmake -E env DYLD_LIBRARY_PATH=/home/david/.conan/data/protobuf/3.11.4/david/testing/package/56e0cf6d16ee57367a0661ab743f4e43b29223f8/lib::::: protoc --cpp_out /home/david/conan/conan-center-index/recipes/protobuf/all/test_package/build -I /home/david/conan/conan-center-index/recipes/protobuf/all/test_package /home/david/conan/conan-center-index/recipes/protobuf/all/test_package/addressbook.proto
No such file or directory

@uilianries uilianries mentioned this issue Jun 24, 2020
4 tasks
@sourcedelica
Copy link
Contributor

sourcedelica commented Jul 20, 2020

I have the same problem. I tried to figure out why the recipe test_package worked and it's because the conanfile.py has:

 with tools.environment_append(RunEnvironment(self).vars):
            if self._protoc_available:
                # Build with protoc
                cmake = CMake(self)
                cmake.definitions["protobuf_VERBOSE"] = True
                cmake.definitions["protobuf_MODULE_COMPATIBLE"] = True
                cmake.definitions["PROTOC_AVAILABLE"] = True
                cmake.configure(build_folder="with_protoc")
                cmake.build()

It appears that is adding the recipe bin package to the path before it builds.

If I try to run the test_package build manually, it fails:

epederson@usdlpfal203 ~/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc (master)
$ rm addressbook.pb.*

epederson@usdlpfal203 ~/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc (master)
$ cmake ../../..
-- Conan: called by CMake conan helper
-- Conan: Adjusting output directories
-- Conan: Using cmake global configuration
-- Conan: Adjusting default RPATHs Conan policies
-- Conan: Adjusting language standard
-- Conan: Compiler GCC>=5, checking major version 5
-- Conan: Checking correct version: 5
-- Conan: C++ stdlib: libstdc++
-- Conan: Using autogenerated FindProtobuf.cmake
-- Library protocd found /userhome/epederson/.conan/data/protobuf/3.11.4/_/_/package/435f6d82215d51c7350154060d2d6634250003de/lib/libprotocd.a
-- Found: /userhome/epederson/.conan/data/protobuf/3.11.4/_/_/package/435f6d82215d51c7350154060d2d6634250003de/lib/libprotocd.a
-- Library protobufd found /userhome/epederson/.conan/data/protobuf/3.11.4/_/_/package/435f6d82215d51c7350154060d2d6634250003de/lib/libprotobufd.a
-- Found: /userhome/epederson/.conan/data/protobuf/3.11.4/_/_/package/435f6d82215d51c7350154060d2d6634250003de/lib/libprotobufd.a
-- protobuf_generate_cpp test_package
-- PROTO_SRCS: /userhome/epederson/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc//addressbook.pb.cc
-- PROTO_HDRS: /userhome/epederson/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc//addressbook.pb.h
-- Configuring done
-- Generating done
-- Build files have been written to: /userhome/epederson/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc
epederson@usdlpfal203 ~/work/conan-center-index/recipes/protobuf/all/test_package/build/4963bbe949a5a06e7cbe262e1161147ad9146ff9/with_protoc (master)

$ make
[ 25%] Running cpp protocol buffer compiler on addressbook.proto
addressbook.proto:10:10: Unrecognized syntax identifier "proto3".  This parser only recognizes "proto2".
make[2]: *** [addressbook.pb.h] Error 1
make[1]: *** [CMakeFiles/test_package.dir/all] Error 2
make: *** [all] Error 2

That error indicates that it found an old 2.x protoc in my Path.

The FindProtobuf.cmake generated by cmake_find_package doesn't have any logic for protoc. The protobuf-generate.cmake from the package has:

    add_custom_command(
      OUTPUT ${_generated_srcs}
      COMMAND "${CMAKE_COMMAND}"  #FIXME: use conan binary component
      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 --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_protobuf_include_path} ${_abs_file}
      DEPENDS ${_abs_file} USES_TERMINAL
      COMMENT "Running ${protobuf_generate_LANGUAGE} protocol buffer compiler on ${_proto}"
      VERBATIM )

Possibly that FIXME is the issue?

@dheater
Copy link
Contributor

dheater commented Aug 9, 2020

I posted a stupid simple repo that replicates the problem for me: https://github.com/dheater/conan-protobuf-example

As @sourcedelica alludes to, protoc is build in the package, but is not in my path so I get a No such file or directory error.

$ make
-- Conan: Adjusting output directories
-- Conan: Using cmake global configuration
-- Conan: Adjusting default RPATHs Conan policies
-- Conan: Adjusting language standard
-- Current conanbuildinfo.cmake directory: /home/dheater/src/proto-test/build
-- Conan: Compiler GCC>=5, checking major version 9
-- Conan: Checking correct version: 9
-- Conan: Using autogenerated FindProtobuf.cmake
-- Library protoc found /home/dheater/.conan/data/protobuf/3.11.4///package/50a5030bbbb13ae56bc4be41915ecd48549cb895/lib/libprotoc.a
-- Found: /home/dheater/.conan/data/protobuf/3.11.4///package/50a5030bbbb13ae56bc4be41915ecd48549cb895/lib/libprotoc.a
-- Library protobuf found /home/dheater/.conan/data/protobuf/3.11.4///package/50a5030bbbb13ae56bc4be41915ecd48549cb895/lib/libprotobuf.a
-- Found: /home/dheater/.conan/data/protobuf/3.11.4///package/50a5030bbbb13ae56bc4be41915ecd48549cb895/lib/libprotobuf.a
-- Configuring done
-- Generating done
-- Build files have been written to: /home/dheater/src/proto-test/build
[ 20%] Running cpp protocol buffer compiler on userinput.proto
No such file or directory
make[2]: *** [CMakeFiles/lr-proto.dir/build.make:69: userinput.pb.h] Error 1
make[1]: *** [CMakeFiles/Makefile2:75: CMakeFiles/lr-proto.dir/all] Error 2
make: *** [Makefile:84: all] Error 2

I can make the project work if I modify FindProtobuf.cmake to contain the full path to protoc on the ARGS line:

  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 --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_protobuf_include_path} ${_abs_file}

Suggestions?

@dheater
Copy link
Contributor

dheater commented Aug 9, 2020

I posted a stupid simple repo that replicates the problem for me: https://github.com/dheater/conan-protobuf-example

I created a branch called try-py which works with conan build but not with conan install .. && cmake .. && make

@rockdreamer
Copy link
Contributor

rockdreamer commented Aug 11, 2020

I was able to build the conan-protobuf-example repo on macos by changing the ARGS line to
ARGS -E env "PATH=${CONAN_BIN_DIRS}/" "LD_LIBRARY_PATH=${Protobuf_LIB_DIRS}:${CONAN_LIB_DIRS}:${Protobuf_LIB_DIRS_RELEASE}:${Protobuf_LIB_DIRS_DEBUG}:${Protobuf_LIB_DIRS_RELWITHDEBINFO}:${Protobuf_LIB_DIRS_MINSIZEREL}" "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 --${protobuf_generate_LANGUAGE}_out ${_dll_export_decl}${protobuf_generate_PROTOC_OUT_DIR} ${_protobuf_include_path} ${_abs_file}
I.e. setting the path to protoc's position relative to the lib directory.
Unsure if this is a useful thing for cmake_multi based targets or if it's the minimum change required.

As a sidenote, I wonder what the reasoning is for not exporting bin dirs in the cmake_find_package generators? I'm trying to integrate > 50 packages right now and I find myself wanting that more and more...

@dheater
Copy link
Contributor

dheater commented Aug 12, 2020

Works brilliantly for me @rockdreamer ! I submitted a PR.

@rockdreamer
Copy link
Contributor

I'm finding out that this works fine if the only dependency is protobuf. If you add something else, like ninja in build_dependencies, CONAN_BIN_DIRS is split with ; instead of : on linux, making the path setting useless.

@rockdreamer
Copy link
Contributor

We could use CMake's SHELL_PATH generator expression, but it's for CMake > 3.4 only.

@dheater
Copy link
Contributor

dheater commented Aug 12, 2020

Replacing PATH=${CONAN_BIN_DIRS}/ with PATH=${Protobuf_LIB_DIRS}/../bin looks a bit crude, but it works. Even with Ninja as a dependency

@rockdreamer
Copy link
Contributor

Yes, that should work, with a static zlib :)

@perseoGI
Copy link
Contributor

perseoGI commented Sep 2, 2024

Hi there!
I'm closing this issue as protobuf recipe has suffer plenty of modifications since this PR was opened.
If the problem remains, do not hesitate to open a new ticket with updated logs!

Happy coding 🐸

@perseoGI perseoGI closed this as completed Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants