Skip to content

Commit 52887e1

Browse files
71copybara-github
authored andcommitted
Fix DEPENDENCIES in protobuf_generate() to accept multiple values instead of silently dropping
cmake: make DEPENDENCIES in protobuf_generate() a multi-value keyword (#19175) It was previously a one-value keyword, so given ```cmake protobuf_generate( ... DEPENDENCIES a b ) ``` `a` would be a dependency, and `b` would be discarded (or, more precisely, added to `protobuf_generate_UNPARSED_ARGUMENTS`). This can be confirmed with the following changes: ```diff diff --git a/cmake/protobuf-generate.cmake b/cmake/protobuf-generate.cmake index 244407e..6dfbcef6d 100644 --- a/cmake/protobuf-generate.cmake +++ b/cmake/protobuf-generate.cmake @@ -10,6 +10,9 @@ function(protobuf_generate) cmake_parse_arguments(protobuf_generate "${_options}" "${_singleargs}" "${_multiargs}" "${ARGN}") + message("Deps: ${protobuf_generate_DEPENDENCIES}") + message("Unparsed args: ${protobuf_generate_UNPARSED_ARGUMENTS}") + if(NOT protobuf_generate_PROTOS AND NOT protobuf_generate_TARGET) message(SEND_ERROR "Error: protobuf_generate called without any targets or source files") return() diff --git a/cmake/tests.cmake b/cmake/tests.cmake index 7976546..1cadf1e5a 100644 --- a/cmake/tests.cmake +++ b/cmake/tests.cmake @@ -27,6 +27,7 @@ foreach(proto_file ${lite_test_protos}) LANGUAGE cpp OUT_VAR pb_generated_files IMPORT_DIRS ${protobuf_SOURCE_DIR}/src + DEPENDENCIES a b ) set(lite_test_proto_files ${lite_test_proto_files} ${pb_generated_files}) endforeach(proto_file) ``` Which printed "Deps: a" and "Unparsed args: b" for lite test files. With this change, it now prints "Deps: a;b" and "Unparsed args: ". AFAIK, making `DEPENDENCIES` a multi-value keyword is mostly backwards-compatible; however, users who specified dependencies that were being ignored may be surprised to see that they are no longer ignored (which could cause build errors). To prevent this kind of problem in the future, validating that `protobuf_generate_UNPARSED_ARGUMENTS` is empty could be useful, but to avoid potentially breaking more builds I did not do this in this PR. Closes #19175 COPYBARA_INTEGRATE_REVIEW=#19175 from 71:protobuf-generate-dependencies c998afa PiperOrigin-RevId: 694186507
1 parent b591280 commit 52887e1

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

cmake/protobuf-generate.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ function(protobuf_generate)
22
include(CMakeParseArguments)
33

44
set(_options APPEND_PATH)
5-
set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR PLUGIN PLUGIN_OPTIONS DEPENDENCIES PROTOC_EXE)
5+
set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR PLUGIN PLUGIN_OPTIONS PROTOC_EXE)
66
if(COMMAND target_sources)
77
list(APPEND _singleargs TARGET)
88
endif()
9-
set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS PROTOC_OPTIONS)
9+
set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS PROTOC_OPTIONS DEPENDENCIES)
1010

1111
cmake_parse_arguments(protobuf_generate "${_options}" "${_singleargs}" "${_multiargs}" "${ARGN}")
1212

0 commit comments

Comments
 (0)