-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
GH-40181: [C++] Support glog 0.7 build #40230
Conversation
@github-actions crossbow submit -g cpp -g r -g linux -g python -g r |
Revision: 1475046 Submitted crossbow builds: ursacomputing/crossbow @ actions-5f343110fe |
Failures look all unrelated, so this is good to go. |
@@ -116,7 +116,7 @@ static std::unique_ptr<std::string> log_dir_; | |||
#ifdef ARROW_USE_GLOG | |||
|
|||
// Glog's severity map. | |||
static int GetMappedSeverity(ArrowLogLevel severity) { | |||
static google::LogSeverity GetMappedSeverity(ArrowLogLevel severity) { |
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.
Does this break compatibility with previous versions? If so, perhaps use auto
to be more lenient.
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.
Ok, I see that previously it was using LogSeverity = int
, so nothing should break. Thank you.
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.
+1
Thank you. Please note that anything building with ArrowConfig.cmake now fails
This was not the case before. Edit: Ah even worse! Even with glog 0.7 being there, providing /usr/lib64/cmake/glog, the build fails because ArrowConfig.cmake can not find uppercas GLOG. |
Could you open a new issue for it? |
After merging your PR, Conbench analyzed the 7 benchmarking runs that have been run so far on merge-commit 5ce060a. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them. |
Fixes apache#40181 ### Are these changes tested? These changes have been tested as part of the conda feedstocks for Arrow. * GitHub Issue: apache#40181 Authored-by: Uwe L. Korn <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Fixes apache#40181 ### Are these changes tested? These changes have been tested as part of the conda feedstocks for Arrow. * GitHub Issue: apache#40181 Authored-by: Uwe L. Korn <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Fixes #40181
Are these changes tested?
These changes have been tested as part of the conda feedstocks for Arrow.