-
Notifications
You must be signed in to change notification settings - Fork 4.2k
GH-46827: [C++] Update Meson Configuration for compute shared lib #46830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -80,12 +80,12 @@ arrow_acero_srcs = [ | |||||
| arrow_acero_lib = library( | ||||||
| 'arrow-acero', | ||||||
| sources: arrow_acero_srcs, | ||||||
| dependencies: [arrow_dep], | ||||||
| dependencies: [arrow_dep, arrow_compute_dep], | ||||||
| ) | ||||||
|
|
||||||
| arrow_acero_dep = declare_dependency(link_with: [arrow_acero_lib]) | ||||||
|
|
||||||
| arrow_acero_testing_sources = ['test_nodes.cc', 'test_util_internal.cc'] + arrow_compute_testing_srcs | ||||||
| arrow_acero_testing_sources = ['test_nodes.cc', 'test_util_internal.cc'] | ||||||
|
|
||||||
| arrow_acero_tests = { | ||||||
| 'plan-test': {'sources': ['plan_test.cc', 'test_nodes_test.cc']}, | ||||||
|
|
@@ -110,7 +110,7 @@ foreach key, val : arrow_acero_tests | |||||
| exc = executable( | ||||||
| 'arrow-acero-@0@'.format(key), | ||||||
| sources: val['sources'] + arrow_acero_testing_sources, | ||||||
| dependencies: [arrow_acero_dep, arrow_test_dep], | ||||||
| dependencies: [arrow_acero_dep, arrow_compute_test_dep], | ||||||
| ) | ||||||
| test(key, exc) | ||||||
| endforeach | ||||||
|
|
@@ -133,7 +133,19 @@ foreach key, val : arrow_acero_benchmarks | |||||
| exc = executable( | ||||||
| key, | ||||||
| sources: val['sources'] + arrow_acero_testing_sources, | ||||||
| dependencies: [arrow_acero_dep, arrow_benchmark_dep, gmock_dep], | ||||||
| dependencies: [ | ||||||
| arrow_acero_dep, | ||||||
| arrow_compute_test_dep, | ||||||
| arrow_benchmark_dep, | ||||||
| gmock_dep, | ||||||
| ], | ||||||
| ) | ||||||
| benchmark(key, exc) | ||||||
| endforeach | ||||||
|
|
||||||
| pkg.generate( | ||||||
| filebase: 'arrow-acero', | ||||||
| name: 'Apache Arrow Acero Engine', | ||||||
| description: 'Apache Arrow\'s Acero Engine', | ||||||
| requires: ['arrow', 'arrow-compute'], | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need
Suggested change
|
||||||
| ) | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,7 +50,7 @@ if(ARROW_TESTING AND ARROW_COMPUTE) | |
| # arrow_compute_core_testing so that is also included correctly | ||
| target_link_libraries(arrow_compute_testing | ||
| PUBLIC $<TARGET_OBJECTS:arrow_compute_core_testing> | ||
| PUBLIC ${ARROW_GTEST_GTEST_MAIN}) | ||
| PUBLIC ${ARROW_GTEST_GTEST}) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Meson configuration was actually failing to register all of the compute kernels (at least locally). When looking further into this, it appears that googletest actually suggests providing your own main function when registering a custom environment like this, to ensure that initialization happens in proper order. |
||
| endif() | ||
|
|
||
| set(ARROW_COMPUTE_TEST_PREFIX "arrow-compute") | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -41,15 +41,48 @@ install_headers( | |||||
| if needs_compute | ||||||
| pkg.generate( | ||||||
| filebase: 'arrow-compute', | ||||||
| name: 'Apache Arrow Compute', | ||||||
| description: 'All compute kernels for Apache Arrow', | ||||||
| name: 'Apache Arrow Compute Kernels', | ||||||
| description: 'Apache Arrow\'s Compute Kernels', | ||||||
| requires: ['arrow'], | ||||||
| ) | ||||||
| endif | ||||||
|
|
||||||
| arrow_compute_testing_srcs = [] | ||||||
| # Define arrow_compute_core_testing object library for common test files requiring | ||||||
| # only core compute. No extra kernels are required. | ||||||
| if needs_testing | ||||||
| arrow_compute_testing_srcs += files('test_util_internal.cc') | ||||||
| arrow_compute_core_test_lib = library( | ||||||
| 'arrow-compute-core-testing', | ||||||
| sources: files('test_util_internal.cc'), | ||||||
| dependencies: arrow_test_dep, | ||||||
| ) | ||||||
| arrow_compute_core_test_dep = declare_dependency( | ||||||
| link_with: arrow_compute_core_test_lib, | ||||||
| ) | ||||||
| else | ||||||
| arrow_compute_core_test_dep = disabler() | ||||||
| endif | ||||||
|
|
||||||
| # Define arrow_compute_testing object library for test files requiring extra kernels. | ||||||
| if needs_testing and needs_compute | ||||||
| arrow_compute_testing_lib = library( | ||||||
| 'arrow-compute-testing', | ||||||
| sources: files('test_env.cc'), | ||||||
| dependencies: [ | ||||||
| arrow_compute_dep, | ||||||
| arrow_compute_core_test_dep, | ||||||
| arrow_test_dep_no_main, | ||||||
| ], | ||||||
| ) | ||||||
| arrow_compute_test_dep = declare_dependency( | ||||||
| link_with: arrow_compute_testing_lib, | ||||||
| dependencies: [ | ||||||
| arrow_compute_dep, | ||||||
| arrow_compute_core_test_dep, | ||||||
| arrow_test_dep_no_main, | ||||||
| ], | ||||||
| ) | ||||||
| else | ||||||
| arrow_compute_test_dep = disabler() | ||||||
| endif | ||||||
|
|
||||||
| exc = executable( | ||||||
|
|
@@ -59,15 +92,13 @@ exc = executable( | |||||
| 'exec_test.cc', | ||||||
| 'kernel_test.cc', | ||||||
| 'registry_test.cc', | ||||||
| ] + arrow_compute_testing_srcs, | ||||||
| dependencies: [arrow_test_dep], | ||||||
| ], | ||||||
| dependencies: [arrow_test_dep, arrow_compute_core_test_dep], | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| ) | ||||||
| test('arrow-internals-test', exc) | ||||||
|
|
||||||
| compute_tests = { | ||||||
| 'arrow-compute-expression-test': { | ||||||
| 'sources': ['expression_test.cc'] + arrow_compute_testing_srcs, | ||||||
| }, | ||||||
| 'arrow-compute-expression-test': {'sources': ['expression_test.cc']}, | ||||||
| 'arrow-compute-row-test': { | ||||||
| 'sources': [ | ||||||
| 'key_hash_test.cc', | ||||||
|
|
@@ -77,7 +108,7 @@ compute_tests = { | |||||
| 'row/row_encoder_internal_test.cc', | ||||||
| 'row/row_test.cc', | ||||||
| 'util_internal_test.cc', | ||||||
| ] + arrow_compute_testing_srcs, | ||||||
| ], | ||||||
| }, | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -96,16 +127,14 @@ compute_tests = { | |||||
| # - value_counts | ||||||
| # | ||||||
| # Also see: GH-34388, GH-34615 | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we update this comment block?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't step through all of the kernels but this comment still holds true, no?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, removing the |
||||||
| if needs_compute | ||||||
| foreach key, val : compute_tests | ||||||
| exc = executable( | ||||||
| key, | ||||||
| sources: val['sources'], | ||||||
| dependencies: [arrow_test_dep], | ||||||
| ) | ||||||
| test(key, exc) | ||||||
| endforeach | ||||||
| endif | ||||||
| foreach key, val : compute_tests | ||||||
| exc = executable( | ||||||
| key, | ||||||
| sources: val['sources'], | ||||||
| dependencies: [arrow_compute_test_dep], | ||||||
| ) | ||||||
| test(key, exc) | ||||||
| endforeach | ||||||
|
|
||||||
| exc = executable( | ||||||
| 'arrow-compute-function-benchmark', | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arrow_compute_depmust be placed beforearrow_depbecausearrow_compute_depdepends onarrow_dep: