JesseLovelace opened a new pull request, #47503:
URL: https://github.com/apache/arrow/pull/47503

   ### Rationale for this change
   
   The currently bundled version of GRPC is several years out of date
   
   ### What changes are included in this PR?
   
   There's a few things going on here so let me explain:
   
   - I've upgraded grpc
   - Upgrading grpc requires updating protobuf, so I've done that as well
   - Updating protbuf requires updating abseil, so I've done that as well
   - The upgraded version of abseil has an updated dependency tree, so I've 
updated the build_absl macro
   - The new version of protobuf introduces a dependency on abseil. So, I had 
to move the build_absl macro up farther in the cmake file so that it's defined 
when protobuf references it
   - The new version of protobuf also exports the utf8_range and utf8_validity 
libraries, as well as the upb library, which are needed at runtime, so I added 
those to the static library list
   - The new version of grpc no longer exports a library called "upb", instead 
it exports a bunch of smaller upb libraries. However, these libraries are 
identical to the one exported by protobuf, so including them both would create 
conflicts. Instead I just linked the grpc libraries to the upb library exported 
by protobuf, which works. The same goes for utf8_range
   - I added `-DCMAKE_CXX_EXTENSIONS=OFF` to the common cmake args. I think 
this is actually intended to be the case anyway, based on 
[this](https://github.com/apache/arrow/blob/403ba70f838dff125c46945948b544a61d12b27f/cpp/cmake_modules/SetupCxxFlags.cmake#L153),
 but that wasn't propagating to the ThirdPartyToolchain build. This was causing 
all bundled libraries to default to building with `std=gnu++17`, whereas the 
arrow libraries were being build with `std=c++17`, which was causing runtime 
issues with the new version of absl
   - I removed the check for protobuf versions greater than 3.22 from the 
`build_opentelemetry` macro. It isn't needed anymore as of 
https://github.com/open-telemetry/opentelemetry-cpp/pull/2163, and 
https://github.com/apache/arrow/pull/46509/files#diff-39a645630afbfb1702af73bcb3fcdb13d87be3d78fcf501f2dcf93000f4aa738
 bumped it to the version with that fix
   - I changed the protobuf version check in 
https://github.com/apache/arrow/pull/47408/files from 32.0 to 22.0. I'm *pretty 
sure* it was a typo and should have been 22.0 in the first place, and I was 
running into the issue without it. I also specifically added a check for when 
protobuf is vendored, since Protobuf_VERSION is not populated at build time if 
that's the case
   
   I think that's everything. Github is weirdly showing a bunch of things that 
I didn't actually touch in the diff, I think it maybe has something to do with 
the fact that I moved some of the macros higher up in the file. If you open it 
in VScode it shows a much more sensible diff 
   
   ### Are these changes tested?
   
   I ran the ORC tests, Flight tests, and Telemetry tests, since those seem to 
the parts of the repo that have dependencies on grpc, protobuf, and abseil. 
Everything passed.  Let me know if I need to do anything else
   
   ### Are there any user-facing changes?
   
   No
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to