From f44cca6c98792b1e9a08e7492c570f3bc785c944 Mon Sep 17 00:00:00 2001 From: Christian Blichmann Date: Wed, 18 Mar 2020 18:47:02 +0100 Subject: [PATCH] Fix path to generated proto sources when embedding When embedding SAPI in an external CMake project, the version of `protobuf_generate_cpp` that we lifted from upstream protobuf produces the wrong generated file paths. For example, given this project structure: ``` /parent/ +-- myproject/ +-- myproject_build/ <- CMake build directory +-- sandboxed-api/ <- Checkout from GitHub ``` And a CMake file in `myproject/CMakeLists.txt` that embeds SAPI like this: ``` cmake_minimum_required(VERSION 3.12) project(SandboxedTest LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) add_subdirectory( ${PROJECT_SOURCE_DIR}/../sandboxed-api ${PROJECT_BINARY_DIR}/sandboxed-api ) ``` Then `protobuf_generate_cpp` correctly invokes the protoc compiler to generate `/parent/myproject_build/sandboxed-api/sandboxed_api/proto_arg.proto.pb.cc'. However, the path of the generated source file that is passed to the C++ compiler will be `/parent/myproject_build/sandboxed-api/sandboxed_api/../../myproject_build/sandboxed-api/sandboxed_api/proto_arg.pb.cc`. Note the duplicated project build directory component in the canonicalized version: `/parent/myproject_build/myproject_build/sandboxed-api/sandboxed_api/proto_arg.pb.cc`. This change simple omits the computation of any relative file paths and simply uses `_pb_PROTOC_OUT_DIR` which defauls to `CMAKE_CURRENT_BINARY_DIR`, which should always contain the correct path. Signed-off-by: Christian Blichmann --- cmake/SapiUtil.cmake | 218 +++++++++--------- sandboxed_api/CMakeLists.txt | 2 +- .../examples/stringop/lib/CMakeLists.txt | 3 +- sandboxed_api/examples/sum/lib/CMakeLists.txt | 2 +- sandboxed_api/sandbox2/CMakeLists.txt | 11 +- sandboxed_api/sandbox2/unwind/CMakeLists.txt | 2 +- sandboxed_api/util/CMakeLists.txt | 2 +- 7 files changed, 122 insertions(+), 118 deletions(-) diff --git a/cmake/SapiUtil.cmake b/cmake/SapiUtil.cmake index 9fe712b..8773f59 100644 --- a/cmake/SapiUtil.cmake +++ b/cmake/SapiUtil.cmake @@ -48,10 +48,10 @@ endfunction() # Helper function that behaves just like Protobuf's protobuf_generate_cpp(), # except that it strips import paths. This is necessary, because CMake's # protobuf rules don't work well with imports across different directories. -function(protobuf_generate_cpp SRCS HDRS) +function(sapi_protobuf_generate_cpp SRCS HDRS) cmake_parse_arguments(_pb "" "EXPORT_MACRO" "" ${ARGN}) if(NOT _pb_UNPARSED_ARGUMENTS) - message(FATAL_ERROR "protobuf_generate_cpp() missing proto files") + message(FATAL_ERROR "sapi_protobuf_generate_cpp() missing proto files") return() endif() @@ -65,11 +65,11 @@ function(protobuf_generate_cpp SRCS HDRS) endforeach() set(_outvar) - protobuf_generate(APPEND_PATH - LANGUAGE cpp - EXPORT_MACRO ${_pb_EXPORT_MACRO} - OUT_VAR _outvar - PROTOS ${_pb_files}) + sapi_protobuf_generate(APPEND_PATH + LANGUAGE cpp + EXPORT_MACRO ${_pb_EXPORT_MACRO} + OUT_VAR _outvar + PROTOS ${_pb_files}) set(${SRCS}) set(${HDRS}) foreach(_file IN LISTS _outvar) @@ -83,117 +83,119 @@ function(protobuf_generate_cpp SRCS HDRS) set(${HDRS} ${${HDRS}} PARENT_SCOPE) endfunction() -if(NOT COMMAND protobuf_generate) - # Runs the protocol buffer compiler on the given proto files. Compatible - # with the upstream version and included here so we can add_subdirectory() - # the protobuf source tree. - function(protobuf_generate) - set(_options APPEND_PATH) - set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR TARGET) - set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS) - cmake_parse_arguments(_pb "${_options}" "${_singleargs}" "${_multiargs}" - "${ARGN}") +# Runs the protocol buffer compiler on the given proto files. Compatible +# with the upstream version and included here so we can add_subdirectory() +# the protobuf source tree. +# One difference to the protobuf version is that this function handles +# relative paths differently, which is relevant when Sandboxed API is +# embedded in another project. +# TODO(cblichmann): We should try and upstream this behavior. +function(sapi_protobuf_generate) + set(_options APPEND_PATH) + set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR TARGET) + set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS) + cmake_parse_arguments(_pb "${_options}" "${_singleargs}" "${_multiargs}" + "${ARGN}") - if(NOT _pb_PROTOS AND NOT _pb_TARGET) - message(FATAL_ERROR "protobuf_generate missing targets or sources") - return() - endif() + if(NOT _pb_PROTOS AND NOT _pb_TARGET) + message(FATAL_ERROR "sapi_protobuf_generate missing targets or sources") + return() + endif() - if(NOT _pb_OUT_VAR AND NOT _pb_TARGET) - message(FATAL_ERROR "protobuf_generate missing target or output var") - return() - endif() + if(NOT _pb_OUT_VAR AND NOT _pb_TARGET) + message(FATAL_ERROR "sapi_protobuf_generate missing target or output var") + return() + endif() - if(NOT _pb_LANGUAGE) - set(_pb_LANGUAGE cpp) + if(NOT _pb_LANGUAGE) + set(_pb_LANGUAGE cpp) + else() + string(TOLOWER ${_pb_LANGUAGE} _pb_LANGUAGE) + endif() + + if(NOT _pb_PROTOC_OUT_DIR) + set(_pb_PROTOC_OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}) + endif() + + if(_pb_EXPORT_MACRO AND _pb_LANGUAGE STREQUAL cpp) + set(_dll_export_decl "dllexport_decl=${_pb_EXPORT_MACRO}:") + endif() + + if(NOT _pb_GENERATE_EXTENSIONS) + if(_pb_LANGUAGE STREQUAL cpp) + set(_pb_GENERATE_EXTENSIONS .pb.h .pb.cc) + elseif(_pb_LANGUAGE STREQUAL python) + set(_pb_GENERATE_EXTENSIONS _pb2.py) else() - string(TOLOWER ${_pb_LANGUAGE} _pb_LANGUAGE) - endif() - - if(NOT _pb_PROTOC_OUT_DIR) - set(_pb_PROTOC_OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}) - endif() - - if(_pb_EXPORT_MACRO AND _pb_LANGUAGE STREQUAL cpp) - set(_dll_export_decl "dllexport_decl=${_pb_EXPORT_MACRO}:") - endif() - - if(NOT _pb_GENERATE_EXTENSIONS) - if(_pb_LANGUAGE STREQUAL cpp) - set(_pb_GENERATE_EXTENSIONS .pb.h .pb.cc) - elseif(_pb_LANGUAGE STREQUAL python) - set(_pb_GENERATE_EXTENSIONS _pb2.py) - else() - message(FATAL_ERROR - "protobuf_generate given unknown language ${_pb_LANGUAGE}") - return() - endif() - endif() - - if(_pb_TARGET) - get_target_property(_source_list ${_pb_TARGET} SOURCES) - foreach(_file IN LISTS _source_list) - if(_file MATCHES "proto$") - list(APPEND _pb_PROTOS "${_file}") - endif() - endforeach() - endif() - - if(NOT _pb_PROTOS) - message(FATAL_ERROR "protobuf_generate could not find any .proto files") + message(FATAL_ERROR + "sapi_protobuf_generate given unknown language ${_pb_LANGUAGE}") return() endif() + endif() - # Create an include path for each file specified - foreach(_file ${_pb_PROTOS}) - get_filename_component(_abs_file ${_file} ABSOLUTE) - get_filename_component(_abs_path ${_abs_file} PATH) - list(FIND _protobuf_include_path "${_abs_path}" _contains_already) - if(${_contains_already} EQUAL -1) - list(APPEND _protobuf_include_path -I ${_abs_path}) + if(_pb_TARGET) + get_target_property(_source_list ${_pb_TARGET} SOURCES) + foreach(_file IN LISTS _source_list) + if(_file MATCHES "proto$") + list(APPEND _pb_PROTOS "${_file}") endif() endforeach() + endif() - foreach(_dir IN LISTS _pb_IMPORT_DIRS) - get_filename_component(_abs_path "${_dir}" ABSOLUTE) - list(FIND _protobuf_include_path "${_abs_path}" _contains_already) - if(${_contains_already} EQUAL -1) - list(APPEND _protobuf_include_path -I "${_abs_path}") - endif() - endforeach() + if(NOT _pb_PROTOS) + message(FATAL_ERROR + "sapi_protobuf_generate could not find any .proto files") + return() + endif() - set(_generated_srcs_all) - foreach(_proto IN LISTS _pb_PROTOS) - get_filename_component(_abs_file ${_proto} ABSOLUTE) - get_filename_component(_abs_dir ${_abs_file} DIRECTORY) - get_filename_component(_basename ${_proto} NAME_WE) - file(RELATIVE_PATH _rel_dir "${CMAKE_CURRENT_SOURCE_DIR}" "${_abs_dir}") - - set(_generated_srcs) - foreach(_ext ${_pb_GENERATE_EXTENSIONS}) - list(APPEND _generated_srcs - "${_pb_PROTOC_OUT_DIR}/${_rel_dir}/${_basename}${_ext}") - endforeach() - list(APPEND _generated_srcs_all ${_generated_srcs}) - - add_custom_command(OUTPUT ${_generated_srcs} - COMMAND protobuf::protoc - ARGS --${_pb_LANGUAGE}_out - ${_dll_export_decl}${_pb_PROTOC_OUT_DIR} - ${_protobuf_include_path} - ${_abs_file} - DEPENDS ${_abs_file} protobuf::protoc - COMMENT "Running ${_pb_LANGUAGE} protoc on ${_proto}" - VERBATIM) - endforeach() - - set_source_files_properties(${_generated_srcs_all} - PROPERTIES GENERATED TRUE) - if(_pb_OUT_VAR) - set(${_pb_OUT_VAR} ${_generated_srcs_all} PARENT_SCOPE) + # Create an include path for each file specified + foreach(_file ${_pb_PROTOS}) + get_filename_component(_abs_file ${_file} ABSOLUTE) + get_filename_component(_abs_path ${_abs_file} PATH) + list(FIND _protobuf_include_path "${_abs_path}" _contains_already) + if(${_contains_already} EQUAL -1) + list(APPEND _protobuf_include_path -I ${_abs_path}) endif() - if(_pb_TARGET) - target_sources(${_pb_TARGET} PRIVATE ${_generated_srcs_all}) + endforeach() + + foreach(_dir IN LISTS _pb_IMPORT_DIRS) + get_filename_component(_abs_path "${_dir}" ABSOLUTE) + list(FIND _protobuf_include_path "${_abs_path}" _contains_already) + if(${_contains_already} EQUAL -1) + list(APPEND _protobuf_include_path -I "${_abs_path}") endif() - endfunction() -endif() + endforeach() + + set(_generated_srcs_all) + foreach(_proto IN LISTS _pb_PROTOS) + get_filename_component(_abs_file ${_proto} ABSOLUTE) + get_filename_component(_abs_dir ${_abs_file} DIRECTORY) + get_filename_component(_basename ${_proto} NAME_WE) + + set(_generated_srcs) + foreach(_ext ${_pb_GENERATE_EXTENSIONS}) + # Use _pb_PROTOC_OUT_DIR directly without computing a relative path + list(APPEND _generated_srcs "${_pb_PROTOC_OUT_DIR}/${_basename}${_ext}") + endforeach() + list(APPEND _generated_srcs_all ${_generated_srcs}) + + add_custom_command(OUTPUT ${_generated_srcs} + COMMAND protobuf::protoc + ARGS --${_pb_LANGUAGE}_out + ${_dll_export_decl}${_pb_PROTOC_OUT_DIR} + ${_protobuf_include_path} + ${_abs_file} + DEPENDS ${_abs_file} protobuf::protoc + COMMENT "Running ${_pb_LANGUAGE} protoc on ${_proto}" + VERBATIM) + endforeach() + + set_source_files_properties(${_generated_srcs_all} + PROPERTIES GENERATED TRUE) + if(_pb_OUT_VAR) + set(${_pb_OUT_VAR} ${_generated_srcs_all} PARENT_SCOPE) + endif() + if(_pb_TARGET) + target_sources(${_pb_TARGET} PRIVATE ${_generated_srcs_all}) + endif() +endfunction() diff --git a/sandboxed_api/CMakeLists.txt b/sandboxed_api/CMakeLists.txt index 636f158..3ac74aa 100644 --- a/sandboxed_api/CMakeLists.txt +++ b/sandboxed_api/CMakeLists.txt @@ -18,7 +18,7 @@ add_subdirectory(sandbox2) add_subdirectory(util) # sandboxed_api:proto_arg -protobuf_generate_cpp(_sapi_proto_arg_pb_cc _sapi_proto_arg_pb_h +sapi_protobuf_generate_cpp(_sapi_proto_arg_pb_cc _sapi_proto_arg_pb_h proto_arg.proto ) add_library(sapi_proto_arg_proto STATIC diff --git a/sandboxed_api/examples/stringop/lib/CMakeLists.txt b/sandboxed_api/examples/stringop/lib/CMakeLists.txt index be1b61a..89c2999 100644 --- a/sandboxed_api/examples/stringop/lib/CMakeLists.txt +++ b/sandboxed_api/examples/stringop/lib/CMakeLists.txt @@ -13,7 +13,8 @@ # limitations under the License. # sandboxed_api/examples/stringop/lib:stringop_params_proto -protobuf_generate_cpp(_sapi_stringop_params_pb_cc _sapi_stringop_params_pb_h +sapi_protobuf_generate_cpp( + _sapi_stringop_params_pb_cc _sapi_stringop_params_pb_h stringop_params.proto ) # Object library to avoid having to use -Wl,--whole-archive. This simulates diff --git a/sandboxed_api/examples/sum/lib/CMakeLists.txt b/sandboxed_api/examples/sum/lib/CMakeLists.txt index 1987a3e..49665eb 100644 --- a/sandboxed_api/examples/sum/lib/CMakeLists.txt +++ b/sandboxed_api/examples/sum/lib/CMakeLists.txt @@ -13,7 +13,7 @@ # limitations under the License. # sandboxed_api/examples/sum/lib:sum_params_proto -protobuf_generate_cpp(_sapi_sum_params_pb_cc _sapi_sum_params_pb_h +sapi_protobuf_generate_cpp(_sapi_sum_params_pb_cc _sapi_sum_params_pb_h sum_params.proto ) # Object library to avoid having to use -Wl,--whole-archive. This simulates diff --git a/sandboxed_api/sandbox2/CMakeLists.txt b/sandboxed_api/sandbox2/CMakeLists.txt index 863667e..e813cbd 100644 --- a/sandboxed_api/sandbox2/CMakeLists.txt +++ b/sandboxed_api/sandbox2/CMakeLists.txt @@ -80,7 +80,7 @@ target_link_libraries(sandbox2_result PRIVATE ) # sandboxed_api/sandbox2:logserver_proto -protobuf_generate_cpp(_sandbox2_logserver_pb_h _sandbox2_logserver_pb_cc +sapi_protobuf_generate_cpp(_sandbox2_logserver_pb_h _sandbox2_logserver_pb_cc logserver.proto ) add_library(sandbox2_logserver_proto STATIC @@ -463,7 +463,7 @@ target_link_libraries(sandbox2_buffer PRIVATE ) # sandboxed_api/sandbox2:forkserver_proto -protobuf_generate_cpp(_sandbox2_forkserver_pb_h _sandbox2_forkserver_pb_cc +sapi_protobuf_generate_cpp(_sandbox2_forkserver_pb_h _sandbox2_forkserver_pb_cc forkserver.proto ) add_library(sandbox2_forkserver_proto STATIC @@ -478,7 +478,7 @@ target_link_libraries(sandbox2_forkserver_proto PRIVATE ) # sandboxed_api/sandbox2:mounttree_proto -protobuf_generate_cpp(_sandbox2_mounttree_pb_h _sandbox2_mounttree_pb_cc +sapi_protobuf_generate_cpp(_sandbox2_mounttree_pb_h _sandbox2_mounttree_pb_cc mounttree.proto ) add_library(sandbox2_mounttree_proto STATIC @@ -515,7 +515,7 @@ target_link_libraries(sandbox2_comms ) # sandboxed_api/sandbox2:violation_proto -protobuf_generate_cpp(_sandbox2_violation_pb_cc _sandbox2_violation_pb_h +sapi_protobuf_generate_cpp(_sandbox2_violation_pb_cc _sandbox2_violation_pb_h violation.proto ) add_library(sandbox2_violation_proto STATIC @@ -610,7 +610,8 @@ if(SAPI_ENABLE_TESTS) ) # sandboxed_api/sandbox2:comms_test_proto - protobuf_generate_cpp(_sandbox2_comms_test_pb_h _sandbox2_comms_test_pb_cc + sapi_protobuf_generate_cpp( + _sandbox2_comms_test_pb_h _sandbox2_comms_test_pb_cc comms_test.proto ) add_library(sandbox2_comms_test_proto STATIC diff --git a/sandboxed_api/sandbox2/unwind/CMakeLists.txt b/sandboxed_api/sandbox2/unwind/CMakeLists.txt index 01e10b2..66233f4 100644 --- a/sandboxed_api/sandbox2/unwind/CMakeLists.txt +++ b/sandboxed_api/sandbox2/unwind/CMakeLists.txt @@ -40,7 +40,7 @@ target_link_libraries(sandbox2_unwind PRIVATE ) # sandboxed_api/sandbox2/unwind:unwind_proto -protobuf_generate_cpp(_sandbox2_unwind_pb_h _sandbox2_unwind_pb_cc +sapi_protobuf_generate_cpp(_sandbox2_unwind_pb_h _sandbox2_unwind_pb_cc unwind.proto ) add_library(sandbox2_unwind_proto STATIC diff --git a/sandboxed_api/util/CMakeLists.txt b/sandboxed_api/util/CMakeLists.txt index 4d27506..4bd6d25 100644 --- a/sandboxed_api/util/CMakeLists.txt +++ b/sandboxed_api/util/CMakeLists.txt @@ -13,7 +13,7 @@ # limitations under the License. # sandboxed_api/util:status_proto -protobuf_generate_cpp(_sapi_util_status_pb_cc _sapi_util_status_pb_h +sapi_protobuf_generate_cpp(_sapi_util_status_pb_cc _sapi_util_status_pb_h status.proto ) add_library(sapi_util_status_proto STATIC