GH-46827: [C++] Update Meson Configuration for compute shared lib

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.

[WillAyd]

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

• GitHub Issue: [C++] Meson Configuration Broken After Compute Module Refactor #46827

[WillAyd]

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

[WillAyd]

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:

 RUN ] Substrait.ExecReadRelWithLocalFiles
C:/projects/arrow/cpp/src/arrow/engine/substrait/serde_test.cc(1132): error: Failed
'_error_or_value131.status()' failed with Invalid: Cannot parse URI: 'file://C:projectsarrowcppsubmodulesparquet-testingdata/byte_stream_split.zstd.parquet' due to syntax error at character '/' (position 54)

In the AppVeyor build we set a variable like:

set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data

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

[kou]

Do we need arrow_test_dep here?

Suggested change

	arrow_c_bridge_deps = []
	arrow_c_bridge_deps = [arrow_test_dep]

[WillAyd]

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?

[kou]

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.

[WillAyd]

Oh I see what you mean - nice catch!

[kou]

Suggested change

[kou]

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?

[WillAyd]

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:

[WillAyd]

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

[WillAyd]

We are all green - any other feedback on this?

[raulcd]

Thanks for following up this! I've taken a look and looks good to me, I am not an expert on this but matches what was done at CMake level.
I'll let @kou take a look and merge

[kou]

+1

[conbench-apache-arrow]

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.