Skip to content

Commit

Permalink
cmake: make DEPENDENCIES in protobuf_generate() a multi-value keyword
Browse files Browse the repository at this point in the history
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:

```cmake
diff --git a/cmake/protobuf-generate.cmake b/cmake/protobuf-generate.cmake
index 244407ee2..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 7976546fe..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.
  • Loading branch information
71 committed Nov 7, 2024
1 parent 429c9bc commit c998afa
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions cmake/protobuf-generate.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ function(protobuf_generate)
include(CMakeParseArguments)

set(_options APPEND_PATH)
set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR PLUGIN PLUGIN_OPTIONS DEPENDENCIES PROTOC_EXE)
set(_singleargs LANGUAGE OUT_VAR EXPORT_MACRO PROTOC_OUT_DIR PLUGIN PLUGIN_OPTIONS PROTOC_EXE)
if(COMMAND target_sources)
list(APPEND _singleargs TARGET)
endif()
set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS PROTOC_OPTIONS)
set(_multiargs PROTOS IMPORT_DIRS GENERATE_EXTENSIONS PROTOC_OPTIONS DEPENDENCIES)

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

Expand Down

0 comments on commit c998afa

Please sign in to comment.