-
Notifications
You must be signed in to change notification settings - Fork 4k
MINOR: [C++] Use bundled gtest #13063
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
Conversation
|
@github-actions crossbow submit test-build-vcpkg-win |
|
Revision: a29f8ab Submitted crossbow builds: ursacomputing/crossbow @ actions-2011
|
|
@github-actions crossbow submit test-build-vcpkg-win |
|
Revision: f74d85e Submitted crossbow builds: ursacomputing/crossbow @ actions-2012
|
|
@assignUser Thanks for the PR! As a general guideline, can you try to make the PR title and description a bit more explicit? |
|
(also, please include the component in the PR title using the same convention as on JIRA, for example |
| -DARROW_WITH_SNAPPY=ON ^ | ||
| -DARROW_WITH_ZLIB=ON ^ | ||
| -DARROW_WITH_ZSTD=ON ^ | ||
| -DGTest_SOURCE=SYSTEM ^ |
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.
Hmm, how does this work on Windows? "system" would mean system-provided packages on Linux, but I don't think those exist at all on Windows.
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.
cc @kou for advice.
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.
It looks like it worked (vcpkg is still installing 1.10 but the error is gone)? It now fails due to missing tzdata, which should be fixable via the download script
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.
We should not do this. We should fix https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L80-L86 instead.
(This is not a MINOR task.)
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.
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.
It seems that ARROW-14560 is another problem and it has been fixed in upstream.
Could you open a new JIRA issue for it?
How about just removing https://github.com/apache/arrow/blob/master/cpp/cmake_modules/ThirdpartyToolchain.cmake#L80-L86 ? The latest may solve the problem described in the comment.
If the problem isn't solved yet, we should report it to the conda's GTest package. We can use if(ARROW_DEPENDENCY_SOURCE STREQUAL "CONDA" AND MSVC AND "${GTest_SOURCE}" STREQUAL "") condition as a workaround for this case.
|
@github-actions crossbow submit test-build-vcpkg-win |
|
Revision: 10e6832 Submitted crossbow builds: ursacomputing/crossbow @ actions-2016
|
This should fix the nightly test failure
test-build-vcpkg-win