You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@arrow.apache.org by "austin3dickey (via GitHub)" <gi...@apache.org> on 2023/04/03 15:39:29 UTC
[GitHub] [arrow] austin3dickey opened a new issue, #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
austin3dickey opened a new issue, #34861:
URL: https://github.com/apache/arrow/issues/34861
### Describe the bug, including details regarding any error messages, version, and platform.
Since the PR https://github.com/apache/arrow/pull/34765 was merged, the `cpp-micro` benchmarks are failing with the following error:
```
CMake Error at cmake_modules/ThirdpartyToolchain.cmake:2236 (message):
System GTest is built with a C++ standard lower than 17. Use bundled GTest
via passing in CMake flag
-DGTest_SOURCE="BUNDLED"
```
[Here](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-arm64-m6g-linux-compute/builds/2413) is an example Buildkite build.
This error message was explicitly added in that PR, so we'll need to take action to correct it.
cc @HaochengLIU
### Component(s)
Benchmarking, C++
--
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: issues-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1495252267
Here is the PR to clean it up. Anyone mind reviewing it? Ty
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494582316
Oddly, it looks like from the output in buildkie, GTest_SOURCE is being set to bundled: https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-arm64-m6g-linux-compute/builds/2413#018736cc-c891-4214-9536-87de3d50e702/6-1069
I'm also curious if this else block is indicating that GTest_SOURCE=BUNDLED works?
https://github.com/apache/arrow/blob/efb0be0d146715663fbfe9162f91da2e8e05b58f/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2257-L2261
Also is it accurate | right that we have the `message(FATAL_ERROR ...` there above the if/else checking `GTest_SOURCE`?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1495077281
> We may want to move the change to `if(GTest_SOURCE STREQUAL "SYSTEM")`:
>
> ```diff
> diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
> index caf33f224b..c0b289cea8 100644
> --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
> +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
> @@ -2229,15 +2229,15 @@ if(ARROW_TESTING)
> 1.10.0
> USE_CONFIG
> ${GTEST_USE_CONFIG})
> - get_target_property(gtest_cxx_standard GTest::gtest INTERFACE_COMPILE_FEATURES)
> + if(GTest_SOURCE STREQUAL "SYSTEM")
> + get_target_property(gtest_cxx_standard GTest::gtest INTERFACE_COMPILE_FEATURES)
>
> - if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL
> - "cxx_std_14"))
> - message(FATAL_ERROR "System GTest is built with a C++ standard lower than 17. Use bundled GTest via passing in CMake flag
> --DGTest_SOURCE=\"BUNDLED\"")
> - endif()
> + if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL
> + "cxx_std_14"))
> + message(FATAL_ERROR "System GTest is built with a C++ standard lower than 17. Use bundled GTest via passing in CMake flag
> + -DGTest_SOURCE=\"BUNDLED\"")
> + endif()
>
> - if(GTest_SOURCE STREQUAL "SYSTEM")
> find_package(PkgConfig QUIET)
> pkg_check_modules(gtest_PC
> gtest
> ```
I'm totally fine with above change
Q: In `BUNDLE` mode, should gTest compiled with whatever CXX_STANDARD Arrow is using? If so, the existing code should work out of box.... It smells like a configuration issue in CI setup..? Curious to learn what Jacob's findings are.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] austin3dickey commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "austin3dickey (via GitHub)" <gi...@apache.org>.
austin3dickey commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1500241972
Thanks @kou ! I've confirmed that benchmarks are passing again.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494654914
> - So this should just work already. @assignUser Do you have a chance to take a look at this?
Do you mean to assign the issue to that user or it's a typo?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494626105
Another follow up here: the build that buildkite is running uses this file for flags like these — and we do have `GTest_SOURCE=BUNDLED` there: https://github.com/apache/arrow/blob/efb0be0d146715663fbfe9162f91da2e8e05b58f/dev/conbench_envs/benchmarks.env#L45
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494607185
Hi, I do not have access to the output you linked. What PR https://github.com/apache/arrow/pull/34765 does is to error out when discover discrepancy between CXX standard between gtest and Arrow. If it's indeed set to `BUNDLED`, it will hits this [CMake block](https://github.com/apache/arrow/blob/main/cpp/cmake_modules/ThirdpartyToolchain.cmake#L277-L278) and build GTest from source(it should be built with C++17 if i read [code here](https://github.com/apache/arrow/blob/main/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2128-L2135) correctly? )
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1495370376
Ah, it seems that some benchmarks specify `-DGTest_SOURCE=BUNDLED` and others don't:
```text
subprocess.CalledProcessError: Command '['cmake', '-GNinja', '-DCMAKE_EXPORT_COMPILE_COMMANDS=ON', '-DCMAKE_BUILD_TYPE=release', '-D
BUILD_WARNING_LEVEL=production', '-DCMAKE_INSTALL_PREFIX=/var/lib/buildkite-agent/.conda/envs/arrow-commit', '-DARROW_BUILD_STATIC=O
N', '-DARROW_BUILD_SHARED=ON', '-DCMAKE_UNITY_BUILD=ON', '-DARROW_BUILD_TESTS=OFF', '-DARROW_BUILD_BENCHMARKS=ON', '-DARROW_BUILD_EX
AMPLES=OFF', '-DARROW_BUILD_INTEGRATION=OFF', '-DARROW_USE_ASAN=OFF', '-DARROW_USE_TSAN=OFF', '-DARROW_USE_UBSAN=OFF', '-DARROW_FUZZ
ING=OFF', '-DARROW_COMPUTE=ON', '-DARROW_CSV=ON', '-DARROW_CUDA=OFF', '-DARROW_DATASET=ON', '-DARROW_FILESYSTEM=ON', '-DARROW_FLIGHT
=OFF', '-DARROW_GANDIVA=OFF', '-DARROW_GCS=OFF', '-DARROW_HDFS=OFF', '-DARROW_IPC=ON', '-DARROW_JSON=ON', '-DARROW_MIMALLOC=OFF', '-
DARROW_JEMALLOC=OFF', '-DARROW_PARQUET=ON', '-DARROW_S3=OFF', '-DARROW_WITH_BROTLI=ON', '-DARROW_WITH_BZ2=ON', '-DARROW_WITH_LZ4=ON'
, '-DARROW_WITH_SNAPPY=ON', '-DARROW_WITH_ZLIB=ON', '-DARROW_WITH_ZSTD=ON', '-DARROW_LINT_ONLY=OFF', '-DARROW_USE_LD_GOLD=ON', '-DAR
ROW_SIMD_LEVEL=DEFAULT', '-DCMAKE_AR=/var/lib/buildkite-agent/.conda/envs/arrow-commit/bin/aarch64-conda-linux-gnu-ar', '-DCMAKE_RAN
LIB=/var/lib/buildkite-agent/.conda/envs/arrow-commit/bin/aarch64-conda-linux-gnu-ranlib', '/var/lib/buildkite-agent/builds/aws-arm6
4-m6g-linux-compute-i-0759a1137c729e8ed-1/apache-arrow/arrow-bci-benchmark-on-arm64-m6g-linux-compute/arrow/cpp']' returned non-zero
exit status 1.
```
How about removing `gtest` in https://github.com/apache/arrow/blob/efb0be0d146715663fbfe9162f91da2e8e05b58f/dev/conbench_envs/hooks.sh#L23-L34 until we implement #34813?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou closed issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou closed issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
URL: https://github.com/apache/arrow/issues/34861
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494968807
We may want to move the change to `if(GTest_SOURCE STREQUAL "SYSTEM")`:
```diff
diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
index caf33f224b..c0b289cea8 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -2229,15 +2229,15 @@ if(ARROW_TESTING)
1.10.0
USE_CONFIG
${GTEST_USE_CONFIG})
- get_target_property(gtest_cxx_standard GTest::gtest INTERFACE_COMPILE_FEATURES)
+ if(GTest_SOURCE STREQUAL "SYSTEM")
+ get_target_property(gtest_cxx_standard GTest::gtest INTERFACE_COMPILE_FEATURES)
- if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL
- "cxx_std_14"))
- message(FATAL_ERROR "System GTest is built with a C++ standard lower than 17. Use bundled GTest via passing in CMake flag
--DGTest_SOURCE=\"BUNDLED\"")
- endif()
+ if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL
+ "cxx_std_14"))
+ message(FATAL_ERROR "System GTest is built with a C++ standard lower than 17. Use bundled GTest via passing in CMake flag
+ -DGTest_SOURCE=\"BUNDLED\"")
+ endif()
- if(GTest_SOURCE STREQUAL "SYSTEM")
find_package(PkgConfig QUIET)
pkg_check_modules(gtest_PC
gtest
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494681434
> Do you mean to assign the issue to that user or it's a typo?
I did indeed mean to tag Jacob (@assignUser) who knows a bunch about our CI stuff (as well as has some experience with the benchmarking setup). But I totally see how that username looks funny there 😂
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494624866
A quick fix is to change `if((${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL` to `if(AAPLE AND (${gtest_cxx_standard} STREQUAL "cxx_std_11") OR (${gtest_cxx_standard} STREQUAL` in [this line](https://github.com/apache/arrow/blob/main/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2234), limiting the scope to APPLE only. Add discuss the long term fix in this follow up issue https://github.com/apache/arrow/issues/34813. @austin3dickey Can you please give it a shot?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494701371
:D I'll have a look later today!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1499768937
I've implemented #34813. So this must be fixed too.
If this is still happen. We can reopen this.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] HaochengLIU commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "HaochengLIU (via GitHub)" <gi...@apache.org>.
HaochengLIU commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494752110
haha, I was firstly shocked by how smart github is to map `assignUser` to issue assignee :)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] assignUser commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1495213453
I am unable to reproduce this error and after looking deeply at our cmake I am unsure why this would happen with `GTEST_SOURCE` set to BUNDLED with AUTO it could pick up an incompatible system version that was compiled with the old std but detection of the system lib will be skipped on bundled. We also have CMAKE_CXX_STANDARD set to 17 so any default values should be initialized correctly.
Though I think kou's patch might work.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou closed issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou closed issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
URL: https://github.com/apache/arrow/issues/34861
--
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: issues-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on issue #34861: [Benchmarks] All cpp microbenchmarks are failing with new GTest CMake Error
Posted by "jonkeane (via GitHub)" <gi...@apache.org>.
jonkeane commented on issue #34861:
URL: https://github.com/apache/arrow/issues/34861#issuecomment-1494635278
Some more notes, that benchmark.env (and the hooks.sh next to it in the hierarchy, which calls it) is called by https://github.com/voltrondata-labs/arrow-benchmarks-ci/blob/0d74a7a525d631f26057ad6a6b4248df96e96ec3/buildkite/benchmark/utils.sh#L34-L35 during our benchmark builds.
So this _should_ just work already. @assignUser Do you have a chance to take a look at this?
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org