GH-46827: [C++] Update Meson Configuration for compute shared lib#46839
GH-46827: [C++] Update Meson Configuration for compute shared lib#46839raulcd merged 9 commits intoapache:mainfrom
Conversation
|
This was a continuation of #46830 which I thought got borked somehow, but I'm guessing github is just in the middle of some service outages |
c2de059 to
5123728
Compare
5123728 to
cd2f9f4
Compare
|
Unfortunately I don't think my prior PR is recoverable - I think it got borked in an outage, so we maybe need to continue along here @kou @raulcd I think I've addressed all the feedback. With respect to the AppVeyor failure, it looks like the previous failures occurred with Visual Studio 2019, and I've made some macros to handle that accordingly. However, I now see the following AppVeyor error: In the AppVeyor build we set a variable like: and the failing test does a string replace with that against: "uriFile": "file://[DIRECTORY_PLACEHOLDER]/byte_stream_split.zstd.parquet",So I guess Visual Studio is not happy with the mix of forward/backslashes, although I'm still stumped as to why the changes in this PR would bring that to light |
cd2f9f4 to
30afe0a
Compare
aa1e82b to
4d01c2c
Compare
cpp/src/arrow/c/meson.build
Outdated
There was a problem hiding this comment.
Do we need arrow_test_dep here?
| arrow_c_bridge_deps = [] | |
| arrow_c_bridge_deps = [arrow_test_dep] |
There was a problem hiding this comment.
It is not required because the arrow_compute_test_dep includes arrow_test_dep_no_main transitively. Maybe there is better naming we can use for the dependencies?
There was a problem hiding this comment.
Ah, I want to focus on if not needs_compute branch here. In the branch, we want to run bridge_test.cc without compute support.
There was a problem hiding this comment.
Oh I see what you mean - nice catch!
cpp/src/arrow/compute/CMakeLists.txt
Outdated
cpp/src/arrow/compute/CMakeLists.txt
Outdated
There was a problem hiding this comment.
Hmm. Is it really related to Visual Studio version?
It seems that this will be happened with newer Visual Studio.
Could you share the build log URL for this change?
There was a problem hiding this comment.
It appears so. Here is the last build log URL before I added this:
https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/52249396
So the failure messages looked like:
[----------] 18 tests from TestPivotKernel
[ RUN ] TestPivotKernel.Basics
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.Basics (2 ms)
[ RUN ] TestPivotKernel.BinaryKeyTypes
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.BinaryKeyTypes (1 ms)
[ RUN ] TestPivotKernel.IntegerKeyTypes
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.IntegerKeyTypes (1 ms)
[ RUN ] TestPivotKernel.Numbers
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.Numbers (1 ms)
[ RUN ] TestPivotKernel.Binary
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.Binary (0 ms)
[ RUN ] TestPivotKernel.NullType
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
[ FAILED ] TestPivotKernel.NullType (0 ms)
[ RUN ] TestPivotKernel.NullValues
unknown file: error: SEH exception with code 0xc0000005 thrown in the test body.
Stack trace:
There was a problem hiding this comment.
OK without the macro the SEH exceptions are gone, so maybe that was a temporal issue with AppVeyor. However, I still get the error about the invalid file path without this macro, which you can see in the latest run here:
https://ci.appveyor.com/project/ApacheSoftwareFoundation/arrow/builds/52266070
In the interim, I fixed up the macro (changed _MSVC_VER -> _MSC_VER) and it looks like things are all green, so hopefully that works for now
4d01c2c to
06ca403
Compare
3ae0327 to
20dd3df
Compare
This reverts commit 787016b5882f2c3b923b680eacd4c2e6ac9456ff.
927b44d to
d8a3ca0
Compare
|
We are all green - any other feedback on this? |
|
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit e6cef22. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…shared lib (apache#46839)" This reverts commit e6cef22.
Rationale for this change
The major refactor of the compute sources in #46261 addressed updates to the CMake configuration but not Meson. This is a follow up to get the Meson builds working again
What changes are included in this PR?
Meson configuration files are updated to reflect new source structure. gtest has also been bumped to a new WrapDB version, which fixes some undefined behavior that was compounded by updates to the compute test structure
Are these changes tested?
Yes
Are there any user-facing changes?
No