Copilot commented on code in PR #3618:
URL: https://github.com/apache/brpc/pull/3618#discussion_r4301882234
##########
example/cmake/BrpcExample.cmake:
##########
@@ -29,7 +29,7 @@ endfunction()
macro(brpc_example_find_common_deps out_libs)
execute_process(
- COMMAND bash -c "find ${PROJECT_SOURCE_DIR}/../.. -type d -regex
\".*output/include$\" | head -n1 | xargs dirname | tr -d '\n'"
+ COMMAND bash -c "find ${CMAKE_CURRENT_SOURCE_DIR}/../.. -type d -regex
\".*output/include$\" | head -n1 | xargs dirname | tr -d '\n'"
Review Comment:
This helper still discovers the brpc installation by searching for
`output/include`, but this PR removes the `output/include` staging step from
the top-level CMake build. A fresh in-tree build therefore leaves `OUTPUT_PATH`
empty and the examples fall back to unrelated system headers/libraries (or fail
with `Fail to find brpc`). Switch this lookup to an explicit installed prefix
(or otherwise pass the install location) so the examples remain usable after
the staging directory is removed.
##########
example/ubring_performance/CMakeLists.txt:
##########
@@ -21,7 +21,7 @@ project(ubring_performance C CXX)
option(LINK_SO "Whether examples are linked dynamically" OFF)
execute_process(
- COMMAND bash -c "find ${PROJECT_SOURCE_DIR}/../.. -type d -regex
'.*output/include$' | head -n1 | xargs dirname | tr -d '\n'"
+ COMMAND bash -c "find ${CMAKE_CURRENT_SOURCE_DIR}/../.. -type d -regex
'.*output/include$' | head -n1 | xargs dirname | tr -d '\n'"
Review Comment:
This standalone example has the same stale `output/include` discovery as the
shared example helper. Since the refactored CMake build no longer creates that
directory, configuring this example after building brpc in-tree cannot find the
just-built headers/library and may silently select system installations. Accept
an explicit install prefix (or update the lookup to the installed tree) instead
of relying on the removed staging directory.
##########
src/mcpack2pb/CMakeLists.txt:
##########
@@ -0,0 +1,51 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set(MCPACK2PB_SOURCES
+ ${CMAKE_CURRENT_SOURCE_DIR}/field_type.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/mcpack2pb.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/parser.cpp
+ ${CMAKE_CURRENT_SOURCE_DIR}/serializer.cpp)
+
+add_library(MCPACK2PB_LIB OBJECT ${MCPACK2PB_SOURCES})
+target_link_libraries(MCPACK2PB_LIB PRIVATE butil_build_config)
+set_property(TARGET MCPACK2PB_LIB PROPERTY POSITION_INDEPENDENT_CODE 1)
+
+if(TARGET protobuf::libprotoc)
+ set(PROTOC_LIB protobuf::libprotoc)
+else()
+ find_library(PROTOC_LIB NAMES protoc)
+endif()
+if(NOT PROTOC_LIB)
+ message(FATAL_ERROR "Fail to find protoc lib")
+endif()
+
+# Legacy bundles/tests retain protoc linkage; the runtime component does not.
+set_property(TARGET MCPACK2PB_LIB PROPERTY BRPC_BUNDLE_LIBRARIES
"${PROTOC_LIB}")
+set_property(TARGET MCPACK2PB_LIB PROPERTY BRPC_PRIVATE_LIBS "-lprotoc")
+
+add_executable(protoc-gen-mcpack generator.cpp)
+set_target_properties(protoc-gen-mcpack PROPERTIES
+ RUNTIME_OUTPUT_DIRECTORY ${PROJECT_BINARY_DIR}/output/bin)
+target_link_libraries(protoc-gen-mcpack PRIVATE butil_build_config)
+if(BUILD_SHARED_LIBS)
+ target_link_libraries(protoc-gen-mcpack PRIVATE brpc-shared
brpc_bundle_dependencies)
+else()
+ target_link_libraries(protoc-gen-mcpack PRIVATE brpc-static
brpc_bundle_dependencies)
Review Comment:
`src/mcpack2pb` is processed before `src/CMakeLists.txt` creates
`brpc-static` and `brpc-shared` (those targets are added only after all module
subdirectories). CMake requires the target named by `target_link_libraries` to
already exist, so configuration fails here with “Cannot specify link libraries
for target ... which is not built by this project.” Move these generator link
statements after the bundle targets are created, or otherwise link the
generator through targets available at this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
