kou commented on code in PR #39824:
URL: https://github.com/apache/arrow/pull/39824#discussion_r1473669370
##########
cpp/src/arrow/acero/CMakeLists.txt:
##########
@@ -154,41 +153,91 @@ function(add_arrow_acero_test REL_TEST_NAME)
add_arrow_test(${REL_TEST_NAME}
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
+ ${EXTRA_LINK_LIBS}
PREFIX
${PREFIX}
LABELS
${LABELS}
${ARG_UNPARSED_ARGUMENTS})
endfunction()
+if(ARROW_TESTING)
+ add_library(arrow_acero_test_nodes OBJECT test_nodes.cc)
+ target_link_libraries(arrow_acero_test_nodes PRIVATE
${ARROW_ACERO_TEST_LINK_LIBS})
+ if(ARROW_WITH_OPENTELEMETRY)
+ target_link_libraries(arrow_acero_test_nodes PRIVATE
${ARROW_OPENTELEMETRY_LIBS})
+ endif()
+endif()
add_arrow_acero_test(plan_test
SOURCES
plan_test.cc
test_nodes_test.cc
- test_nodes.cc)
-add_arrow_acero_test(source_node_test SOURCES source_node_test.cc
test_nodes.cc)
-add_arrow_acero_test(fetch_node_test SOURCES fetch_node_test.cc test_nodes.cc)
-add_arrow_acero_test(order_by_node_test SOURCES order_by_node_test.cc
test_nodes.cc)
-add_arrow_acero_test(hash_join_node_test SOURCES hash_join_node_test.cc
- bloom_filter_test.cc)
-add_arrow_acero_test(pivot_longer_node_test SOURCES pivot_longer_node_test.cc
- test_nodes.cc)
+ EXTRA_LINK_LIBS
+ ${ARROW_ACERO_TEST_LINK_LIBS}
Review Comment:
We can do it by:
```diff
diff --git a/cpp/src/arrow/acero/CMakeLists.txt
b/cpp/src/arrow/acero/CMakeLists.txt
index 36afad4972..7f40d1bd19 100644
--- a/cpp/src/arrow/acero/CMakeLists.txt
+++ b/cpp/src/arrow/acero/CMakeLists.txt
@@ -139,10 +139,9 @@ function(add_arrow_acero_test REL_TEST_NAME)
set(PREFIX "arrow-acero")
endif()
+ set(EXTRA_LINK_LIBS ${ARROW_ACERO_TEST_LINK_LIBS})
if(ARG_EXTRA_LINK_LIBS)
- set(EXTRA_LINK_LIBS ${ARG_EXTRA_LINK_LIBS})
- else()
- set(EXTRA_LINK_LIBS ${ARROW_ACERO_TEST_LINK_LIBS})
+ list(APPEND EXTRA_LINK_LIBS ${ARG_EXTRA_LINK_LIBS})
endif()
if(ARG_LABELS)
```
but we use "override" instead of "append" when we specify a value explicitly
in our functions.
So I used the same behavior for consistency here.
How about defining a variable to remove the duplication and keep consistency?
```diff
diff --git a/cpp/src/arrow/acero/CMakeLists.txt
b/cpp/src/arrow/acero/CMakeLists.txt
index 36afad4972..2b1683b99a 100644
--- a/cpp/src/arrow/acero/CMakeLists.txt
+++ b/cpp/src/arrow/acero/CMakeLists.txt
@@ -161,51 +161,47 @@ function(add_arrow_acero_test REL_TEST_NAME)
${ARG_UNPARSED_ARGUMENTS})
endfunction()
+set(ARROW_ACERO_NODE_TEST_LINK_LIBS ${ARROW_ACERO_TEST_LINK_LIBS})
if(ARROW_TESTING)
add_library(arrow_acero_test_nodes OBJECT test_nodes.cc)
target_link_libraries(arrow_acero_test_nodes PRIVATE
${ARROW_ACERO_TEST_LINK_LIBS})
if(ARROW_WITH_OPENTELEMETRY)
target_link_libraries(arrow_acero_test_nodes PRIVATE
${ARROW_OPENTELEMETRY_LIBS})
endif()
+ list(APPEND ARROW_ACERO_NODE_TEST_LINK_LIBS arrow_acero_test_nodes)
endif()
add_arrow_acero_test(plan_test
SOURCES
plan_test.cc
test_nodes_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(source_node_test
SOURCES
source_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(fetch_node_test
SOURCES
fetch_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(order_by_node_test
SOURCES
order_by_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(hash_join_node_test
SOURCES
hash_join_node_test.cc
bloom_filter_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(pivot_longer_node_test
SOURCES
pivot_longer_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
# asof_join_node and sorted_merge_node use std::thread internally
# and doesn't use ThreadPool so it will
@@ -215,14 +211,12 @@ if(ARROW_ENABLE_THREADING)
SOURCES
asof_join_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
add_arrow_acero_test(sorted_merge_node_test
SOURCES
sorted_merge_node_test.cc
EXTRA_LINK_LIBS
- ${ARROW_ACERO_TEST_LINK_LIBS}
- arrow_acero_test_nodes)
+ ${ARROW_ACERO_NODE_TEST_LINK_LIBS})
endif()
add_arrow_acero_test(tpch_node_test SOURCES tpch_node_test.cc)
```
--
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]
