MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0016077CMakeModulespublic2016-04-22 17:092016-06-10 14:31
Reporterskebanga 
Assigned ToKitware Robot 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionmoved 
PlatformLinuxOSUbuntuOS Version14.04
Product VersionCMake 3.5 
Target VersionFixed in Version 
Summary0016077: FindProtobuf.cmake doesn't have required flexibility to configure protoc usage for all use cases
DescriptionI have here a very simple test project which has 2 protobuf files, one of which is included in the other.

Using cmake, I will create a static library for each generated protobuf message.

##Protobuf files:

**`src/foo/message.proto`:**

    package test.foo;
    
    message FooMsg
    {
        required string s = 1;
    }

**`src/bar/message.proto`:**

    package test.bar;
    import "foo/message.proto";
    
    message BarMsg
    {
        optional foo.FooMsg f = 1;
    }

##CMake files:

I build `lib_foo` from generated `foo/message.proto` files.

**`src/foo/CMakeLists.txt`:**

    protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS message.proto)
    
    add_library(lib_foo STATIC ${PROTO_SRCS})

I build `lib_bar` from the generated `bar/message.proto` files, and link in `lib_foo`:

**`src/bar/CMakeLists.txt`:**

    protobuf_generate_cpp(PROTO_SRCS PROTO_HDRS message.proto )
    
    add_library(lib_bar STATIC ${PROTO_SRCS})
    
    target_link_libraries(lib_bar lib_foo)

**`src/CMakeLists.txt`:**

    cmake_minimum_required (VERSION 3.5)
    project (cmake_proto_test CXX)
    
    find_package(Protobuf REQUIRED)
    
    # proto files import from the source root directory, so add the required -I flag
    set(PROTOBUF_IMPORT_DIRS ${CMAKE_SOURCE_DIR})
    
    # genererated proto files go into the CMake binary output dir
    include_directories("${CMAKE_BINARY_DIR}")
    
    add_subdirectory(foo)
    add_subdirectory(bar)

##Build error:

When I try to build this, I get the following error:

    $ make .. VERBOSE=1
    cd src/cmake_proto/build/bar && /usr/bin/c++
        -I src/cmake_proto/build
        -o CMakeFiles/lib_bar.dir/message.pb.cc.o
        -c src/cmake_proto/build/bar/message.pb.cc

    In file included from src/cmake_proto/build/bar/message.pb.cc:5:0:

    src/cmake_proto/build/bar/message.pb.h:99:24:
        error: ‘foo’ in namespace ‘test’ does not name a type

           inline const ::test::foo::FooMsg& f() const;
                                ^

##Reason:

The error is due to the header guard created by `protoc` being the same for the 2 generated files:

    #ifndef PROTOBUF_message_2eproto__INCLUDED
    #define PROTOBUF_message_2eproto__INCLUDED

    ...

    #endif

The reason is that the header guard is derived from a combination of the output directory and the generated file's path.

The current command issued by `FindProtobuf.cmake` results in the header guard only using the filename:

    cd src/cmake_proto/build/foo && /usr/local/bin/protoc --cpp_out src/cmake_proto/build/foo -I src/cmake_proto/foo -I src/cmake_proto src/cmake_proto/foo/message.proto
    cd src/cmake_proto/build/bar && /usr/local/bin/protoc --cpp_out src/cmake_proto/build/bar -I src/cmake_proto/bar -I src/cmake_proto src/cmake_proto/bar/message.proto

This command, however, will result in the files being generated in the same location, but with a different header guard:

    cd src/cmake_proto/build && /usr/local/bin/protoc --cpp_out src/cmake_proto/build -I src/cmake_proto src/cmake_proto/foo/message.proto
    cd src/cmake_proto/build && /usr/local/bin/protoc --cpp_out src/cmake_proto/build -I src/cmake_proto src/cmake_proto/bar/message.proto

Header guards:

    PROTOBUF_foo_2fmessage_2eproto__INCLUDED
    PROTOBUF_bar_2fmessage_2eproto__INCLUDED

There are three key differences here:

- The `WORKING_DIRECTORY` from which `protoc` is run from is `${CMAKE_BINARY_DIR}`
- The `--cpp_out` directory passed to `protoc` is `${CMAKE_BINARY_DIR}`
- The `-I` include path passed to `protoc` does **not** include the folder where the proto file is found

Being able to control these 3 items would give the flexibility required to use this tool in the above setup.

Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2016-04-22 17:09skebangaNew Issue
2016-04-25 11:11skebangaNote Added: 0040927
2016-04-25 11:28Brad KingNote Added: 0040928
2016-06-10 14:29Kitware RobotNote Added: 0042988
2016-06-10 14:29Kitware RobotStatusnew => resolved
2016-06-10 14:29Kitware RobotResolutionopen => moved
2016-06-10 14:29Kitware RobotAssigned To => Kitware Robot
2016-06-10 14:31Kitware RobotStatusresolved => closed

Notes
(0040927)
skebanga   
2016-04-25 11:11   
Here is an implementation which I've come up with - may be of use?

function(PROTOC SRCS HDRS)

    set(options)
    set(values CPP_OUT CWD)
    set(lists INCLUDE PROTO)
    cmake_parse_arguments(PROTOC "${options}" "${values}" "${lists}" "${ARGN}")

    if (NOT PROTOC_CPP_OUT)
        set(PROTOC_CPP_OUT ${CMAKE_CURRENT_BINARY_DIR})
    endif()

    if (NOT PROTOC_CWD)
        set(PROTOC_CWD ${CMAKE_CURRENT_BINARY_DIR})
    endif()

    if (NOT PROTOC_INCLUDE)
        set(PROTOC_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR})
    endif()

    foreach(PATH ${PROTOC_INCLUDE})
        list(FIND _protobuf_include_path ${PATH} _contains_already)
        if(${_contains_already} EQUAL -1)
                list(APPEND _protobuf_include_path -I ${PATH})
        endif()
    endforeach()

    set(${SRCS})
    set(${HDRS})
    
    foreach(FILE ${PROTOC_PROTO})

        get_filename_component(ABS_FILE ${FILE} ABSOLUTE)

        # convert path of input file into path of output file
        string(REPLACE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR} BIN_DEST ${ABS_FILE})

        get_filename_component(FILE_DEST ${BIN_DEST} DIRECTORY)
        get_filename_component(FILE_WE ${BIN_DEST} NAME_WE)

        list(APPEND ${SRCS} "${FILE_DEST}/${FILE_WE}.pb.cc")
        list(APPEND ${HDRS} "${FILE_DEST}/${FILE_WE}.pb.h")

        add_custom_command(
            OUTPUT
                "${FILE_DEST}/${FILE_WE}.pb.cc"
                "${FILE_DEST}/${FILE_WE}.pb.h"
            COMMAND
                ${PROTOBUF_PROTOC_EXECUTABLE}
            ARGS
                --cpp_out ${PROTOC_CPP_OUT} ${_protobuf_include_path} ${ABS_FILE}
            WORKING_DIRECTORY
                ${PROTOC_CWD}
            DEPENDS
                ${ABS_FILE}
                ${PROTOBUF_PROTOC_EXECUTABLE}
            COMMENT
                "Running C++ protocol buffer compiler on ${FILE}"
            VERBATIM
            )
    endforeach()

    set_source_files_properties(${${SRCS}} ${${HDRS}} PROPERTIES GENERATED TRUE)
    set(${SRCS} ${${SRCS}} PARENT_SCOPE)
    set(${HDRS} ${${HDRS}} PARENT_SCOPE)
endfunction()
(0040928)
Brad King   
2016-04-25 11:28   
Re 0016077:0040927: Thanks. Rather than introducing a whole new function for this, please look at extending protobuf_generate_cpp to parse ARGN and look for options activating such behavior. The defaults can simply fall back to current behavior.
(0042988)
Kitware Robot   
2016-06-10 14:29   
Resolving issue as `moved`.

This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.