You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/04/06 03:28:17 UTC
[GitHub] [arrow] kou opened a new pull request, #34920: GH-34813: [C++] Improve GoogleTest detection
kou opened a new pull request, #34920:
URL: https://github.com/apache/arrow/pull/34920
### Rationale for this change
#34765 approach has some corner cases. They causes CI failures.
### What changes are included in this PR?
If incompatible GoogleTest is detected, we can fallback to bundled GoogleTest automatically.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1499556346
> Confirming that the latest version of the PR seems to work for me (with a gtest from conda):
Thanks for confirming this!
> On a previous version I got an error like "CMake Error: The source directory "SOURCES/CMakeFiles/CMakeTmp" does not exist" from FindGTestAlt.cmake:53 (try_compile)
Yes. I needed to write a code to work with newer CMake and older CMake. On a previous version this implementation worked only with newer CMake.
--
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 a diff in pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #34920:
URL: https://github.com/apache/arrow/pull/34920#discussion_r1160202166
##########
cpp/CMakeLists.txt:
##########
@@ -839,8 +839,8 @@ if(NOT MSVC_TOOLCHAIN)
list(APPEND ARROW_SHARED_LINK_LIBS ${CMAKE_DL_LIBS})
endif()
-set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers GTest::gtest_main GTest::gtest
- GTest::gmock)
+set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GTEST_MAIN}
Review Comment:
We can use `add_library(ALIAS)` with CMake 3.11: https://cmake.org/cmake/help/latest/command/add_library.html#alias-libraries
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1501036788
`cmake --debug-trycompile ...` will help us: https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-debug-trycompile
--
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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498446391
Revision: a48266e6e0b3ab8499448e8895d5770af009a02a
Submitted crossbow builds: [ursacomputing/crossbow @ actions-6fe8b41aa2](https://github.com/ursacomputing/crossbow/branches/all?query=actions-6fe8b41aa2)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/4625194203/jobs/8180724615)|
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498445152
@github-actions crossbow submit test-alpine-linux-cpp
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1499768008
+1
--
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] ursabot commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1500747374
Benchmark runs are scheduled for baseline = 6f3d96ec4ca3e425e541128c32a8629998ef068a and contender = f7644ae88cafc4b86d7eb294c238f34b35791388. f7644ae88cafc4b86d7eb294c238f34b35791388 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/80e9f88f73c4415fac46d4bd15ba57e9...2c780182efd8430196a3a554c994e54a/)
[Failed :arrow_down:3.09% :arrow_up:1.16%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/12df7e4c3b43401fbd9293849b0d4b99...1e808868ad364539ba727c017932cd45/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/9a8989d2a33f4156a679fadc8a75f5b8...8222fe26318c48aa85faacd42bc85e6d/)
[Failed :arrow_down:1.14% :arrow_up:0.98%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/6c8916961cc44f3091f4322659330fd7...8709593596c0484287c3d9fd90f3b10d/)
Buildkite builds:
[Finished] [`f7644ae8` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2651)
[Finished] [`f7644ae8` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2684)
[Finished] [`f7644ae8` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2649)
[Finished] [`f7644ae8` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2675)
[Finished] [`6f3d96ec` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2650)
[Failed] [`6f3d96ec` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2683)
[Finished] [`6f3d96ec` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2648)
[Failed] [`6f3d96ec` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2674)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498548511
Revision: 3440365ac36110029cbd65fc3dab24eaf3390f38
Submitted crossbow builds: [ursacomputing/crossbow @ actions-ca7a76dd0a](https://github.com/ursacomputing/crossbow/branches/all?query=actions-ca7a76dd0a)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/4626031358/jobs/8182353004)|
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498545451
@github-actions crossbow submit test-alpine-linux-cpp
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498578492
@github-actions crossbow submit test-alpine-linux-cpp
--
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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498441800
* Closes: #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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498520700
Revision: a585e3564236f752a49806a2944b4392b1acaadc
Submitted crossbow builds: [ursacomputing/crossbow @ actions-fc43e340df](https://github.com/ursacomputing/crossbow/branches/all?query=actions-fc43e340df)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/4625833526/jobs/8181953681)|
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498519231
@github-actions crossbow submit test-alpine-linux-cpp
--
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 a diff in pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #34920:
URL: https://github.com/apache/arrow/pull/34920#discussion_r1159797944
##########
cpp/CMakeLists.txt:
##########
@@ -839,8 +839,8 @@ if(NOT MSVC_TOOLCHAIN)
list(APPEND ARROW_SHARED_LINK_LIBS ${CMAKE_DL_LIBS})
endif()
-set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers GTest::gtest_main GTest::gtest
- GTest::gmock)
+set(ARROW_TEST_LINK_TOOLCHAIN arrow::flatbuffers ${ARROW_GTEST_GTEST_MAIN}
Review Comment:
IMHO this is a regression as using variables instead of targets is an anti-pattern but I currently don't see a cleaner approach either.
I think we should rethink the entire dependency management once we have bumped the cmake version to 3.16 as we will have much better tools available. (this is of course out of scope for this PR)
--
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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498580504
Revision: 381532803c29fcdd003d10f0821289c21bea7d72
Submitted crossbow builds: [ursacomputing/crossbow @ actions-e991323147](https://github.com/ursacomputing/crossbow/branches/all?query=actions-e991323147)
|Task|Status|
|----|------|
|test-alpine-linux-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/4626273862/jobs/8182838998)|
--
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 merged pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #34920:
URL: https://github.com/apache/arrow/pull/34920
--
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] jorisvandenbossche commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498689943
Confirming that the latest version of the PR seems to work for me (with a gtest from conda):
```
...
-- Found GTest: /home/joris/miniconda3/envs/arrow-dev/lib/cmake/GTest/GTestConfig.cmake (found suitable version "1.13.0", minimum required is "1.10.0")
...
```
On a previous version I got an error like "CMake Error: The source directory "SOURCES/CMakeFiles/CMakeTmp" does not exist" from FindGTestAlt.cmake:53 (try_compile)
--
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] zfoobar commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "zfoobar (via GitHub)" <gi...@apache.org>.
zfoobar commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1501027742
Sure. Any specific flags you want me to pass to the build process for
debugging/error log purposes?
On Sat, Apr 8, 2023 at 7:12 PM Sutou Kouhei ***@***.***>
wrote:
> Could you open a new issue with how to reproduce and full error log?
>
> At least I can't reproduce it with macOS/arm64/Homebrew(not conda).
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/arrow/pull/34920#issuecomment-1501020946>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AC74BGOJK7CBX6ZWKIJDOGDXAILJ5ANCNFSM6AAAAAAWU3LK3E>
> .
> You are receiving this because you commented.Message ID:
> ***@***.***>
>
--
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 pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1501020946
Could you open a new issue with how to reproduce and full error log?
At least I can't reproduce it with macOS/arm64/Homebrew(not conda).
--
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] github-actions[bot] commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1498441817
:warning: GitHub issue #34813 **has been automatically assigned in GitHub** to PR creator.
--
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] zfoobar commented on pull request #34920: GH-34813: [C++] Improve GoogleTest detection
Posted by "zfoobar (via GitHub)" <gi...@apache.org>.
zfoobar commented on PR #34920:
URL: https://github.com/apache/arrow/pull/34920#issuecomment-1500979161
This is a regression - build fails for me (macos/arm64/conda).
The only reason I could get it building is because I've been tracking this issue and knew to supply the proper BUNDLED flag. At least before the cmake module prompted you to put the flag in place if it detected the installed binaries weren't c++17.
It's back to the old behavior, build fails and you just see the symbols it can't resolve.
--
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