[C++] Add support for Protobuf 23.2 on non-Windows #35987

kou · 2023-06-08T02:46:36Z

Describe the enhancement requested

We can't resolve google::protobuf::internal::ThreadSafeArena::thread_cache_ with Protobuf 23.2 on non-Windows:

 /Applications/Xcode_14.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../release/libarrow.a(util_avx2.cc.o) has no symbols
ld: illegal thread local variable reference to regular symbol __ZN6google8protobuf8internal15ThreadSafeArena13thread_cache_E for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

I think that protocolbuffers/protobuf#12983 solves this but it's not released yet.

Workarounds:

Specify PROTOBUF_USE_DLLS explicitly on non-Windows too.
Use bundled (old) Protobuf

Component(s)

C++

The text was updated successfully, but these errors were encountered:

kou · 2023-06-08T02:48:52Z

Homebrew is only effected for now.
So I think that we can choose "2. Use bundled (old) Protobuf" (-DProtobuf_SOURCE=BUNDLED) and it's not a 12.0.1 release blocker.

kou · 2023-06-08T03:55:06Z

protocolbuffers/protobuf#12699 is also related.

h-vetinari · 2023-06-08T04:01:38Z

I opened a very similar issue just ahead of you: #35986 ;-)

h-vetinari · 2023-06-08T04:12:29Z

I think that protocolbuffers/protobuf#12983 solves this but it's not released yet.

I have a feeling that this might not be backported to the 23.x series because it's effectively an ABI break. Perhaps I'm wrong though. Anyway, in conda-forge, we set PROTOBUF_USE_DLLS also on unix to solve this.

kou · 2023-06-08T07:32:09Z

I opened a very similar issue just ahead of you: #35986 ;-)

I saw it but I think that it's a different problem. :-)

we set PROTOBUF_USE_DLLS also on unix to solve this.

I think that it's one of workarounds.

raulcd · 2023-06-08T10:56:21Z

Homebrew is only effected for now. So I think that we can choose "2. Use bundled (old) Protobuf" (-DProtobuf_SOURCE=BUNDLED) and it's not a 12.0.1 release blocker.

I am trying to run the verification tasks for MacOS from my fork:

archery crossbow submit --job-prefix release-12.0.1-rc1-with-bundled-protobuf --arrow-branch release-12.0.1-rc1-with-bundled-protobuf --arrow-remote "https://github.com/raulcd/arrow" --arrow-version 12.0.1 verify-rc-source-*macos-*

Adding the following patch on top of the release-12.0.1-rc1 branch:

$ git log -1 -p
commit 6989adf5de211d72199a6a627ea302338a65eb06 (HEAD -> release-12.0.1-rc1-with-bundled-protobuf, origin/release-12.0.1-rc1-with-bundled-protobuf)
Author: Raúl Cumplido <raulcumplido@gmail.com>
Date:   Thu Jun 8 12:23:44 2023 +0200

    Try bundled protobuf for verification on macOS

diff --git a/dev/release/verify-release-candidate.sh b/dev/release/verify-release-candidate.sh
index 12e6d9c..080ffa2 100755
--- a/dev/release/verify-release-candidate.sh
+++ b/dev/release/verify-release-candidate.sh
@@ -641,6 +641,7 @@ test_and_install_cpp() {
     -DPARQUET_BUILD_EXAMPLES=ON \
     -DPARQUET_BUILD_EXECUTABLES=ON \
     -DPARQUET_REQUIRE_ENCRYPTION=ON \
+    -DProtobuf_SOURCE=BUNDLED \
     ${ARROW_CMAKE_OPTIONS:-} \
     ${ARROW_SOURCE_DIR}/cpp
   export CMAKE_BUILD_PARALLEL_LEVEL=${CMAKE_BUILD_PARALLEL_LEVEL:-${NPROC}}

And I am getting some failures on protobuf:

/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:17:2: error: This file was generated by an older version of protoc which is
#error This file was generated by an older version of protoc which is
 ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:18:2: error: incompatible with your Protocol Buffer headers. Please
#error incompatible with your Protocol Buffer headers. Please
 ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:19:2: error: regenerate this file with a newer version of protoc.
#error regenerate this file with a newer version of protoc.
 ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:147:30: error: no type named 'ConstStringParam' in namespace 'google::protobuf'
    ::PROTOBUF_NAMESPACE_ID::ConstStringParam name, FlightDescriptor_DescriptorType* value) {
    ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:256:35: error: no type named 'StringPiece' in namespace 'google::protobuf'
  static ::PROTOBUF_NAMESPACE_ID::StringPiece FullMessageName() {
         ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:221:5: error: use of undeclared identifier 'GOOGLE_DCHECK'
    GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
    ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:257:12: error: cannot initialize return object of type 'int' with an lvalue of type 'const char [39]'
    return "arrow.flight.protocol.HandshakeRequest";
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:420:35: error: no type named 'StringPiece' in namespace 'google::protobuf'
  static ::PROTOBUF_NAMESPACE_ID::StringPiece FullMessageName() {
         ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:385:5: error: use of undeclared identifier 'GOOGLE_DCHECK'
    GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
    ^
[ 38%] Building CXX object src/arrow/acero/CMakeFiles/arrow_acero_objlib.dir/accumulation_queue.cc.o
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:421:12: error: cannot initialize return object of type 'int' with an lvalue of type 'const char [40]'
    return "arrow.flight.protocol.HandshakeResponse";
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:584:35: error: no type named 'StringPiece' in namespace 'google::protobuf'
  static ::PROTOBUF_NAMESPACE_ID::StringPiece FullMessageName() {
         ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:549:5: error: use of undeclared identifier 'GOOGLE_DCHECK'
    GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
    ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:585:12: error: cannot initialize return object of type 'int' with an lvalue of type 'const char [32]'
    return "arrow.flight.protocol.BasicAuth";
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:738:35: error: no type named 'StringPiece' in namespace 'google::protobuf'
  static ::PROTOBUF_NAMESPACE_ID::StringPiece FullMessageName() {
         ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:717:5: error: use of undeclared identifier 'GOOGLE_DCHECK'
    GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
    ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:739:12: error: cannot initialize return object of type 'int' with an lvalue of type 'const char [28]'
    return "arrow.flight.protocol.Empty";
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:871:35: error: no type named 'StringPiece' in namespace 'google::protobuf'
  static ::PROTOBUF_NAMESPACE_ID::StringPiece FullMessageName() {
         ~~~~~~~~~~~~~~~~~~~~~~~~~^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:836:5: error: use of undeclared identifier 'GOOGLE_DCHECK'
    GOOGLE_DCHECK(GetOwningArena() == other->GetOwningArena());
    ^
/var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/src/arrow/flight/Flight.pb.h:872:12: error: cannot initialize return object of type 'int' with an lvalue of type 'const char [33]'
    return "arrow.flight.protocol.ActionType";
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]

Examples:
https://github.com/ursacomputing/crossbow/actions/runs/5210265087/jobs/9401119933
https://github.com/ursacomputing/crossbow/actions/runs/5210265256/jobs/9401120122

assignUser · 2023-06-08T13:13:35Z

The error above is weird as that file is created in the cmake using protoc https://github.com/apache/arrow/blob/main/cpp/src/arrow/flight/CMakeLists.txt#L80

Maybe a system version is also found and used?

kou · 2023-06-08T21:00:28Z

Maybe a system version is also found and used?

I think so too. It looks like system Protobuf and bundled Protobuf are mixed. But it's strange... Our build process uses absolute path for protoc:

https://github.com/ursacomputing/crossbow/actions/runs/5210265087/jobs/9401119933#step:4:322

-- Found protoc: /var/folders/dl/2sqc_b2s20vfy540jn97pz8h0000gn/T/arrow-HEAD.XXXXX.vQ2csunE/cpp-build/protobuf_ep-install/bin/protoc

raulcd · 2023-06-08T22:11:06Z

It seems ti was GRPC. It should work if we use:

    -DProtobuf_SOURCE=BUNDLED \
    -DgRPC_SOURCE=BUNDLED \

And remove them from the Brewfile as seen on this testing PR: #35995
So probably not a blocker for the 12.0.1 release?

kou · 2023-06-09T01:02:35Z

I think that -DgRPC_SOURCE=BUNDLED is needless because we always use the same _SOURCE for Protobuf and gRPC:

arrow/cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines 4080 to 4084 in 0bbfe98

    
             if(NOT Protobuf_SOURCE STREQUAL gRPC_SOURCE) 
        
               # ARROW-15495: Protobuf/gRPC must come from the same source 
        
               message(STATUS "Forcing gRPC_SOURCE to Protobuf_SOURCE (${Protobuf_SOURCE})") 
        
               set(gRPC_SOURCE "${Protobuf_SOURCE}") 
        
             endif()

And remove them from the Brewfile

I think that this is needed. (Or we run brew uninstall protobuf abseil grpc.)

FYI: We can specify -DProtobuf_SOURCE=BUNDLED without changing our verification script by ARROW_CMAKE_OPTIONS="-DProtobuf_SOURCE=BUNDLED dev/release/verify-release-candidate.sh ...`.

So probably not a blocker for the 12.0.1 release?

I think that this is not a blocker.

js8544 · 2023-06-10T14:04:22Z

Homebrew is only effected for now. So I think that we can choose "2. Use bundled (old) Protobuf" (-DProtobuf_SOURCE=BUNDLED) and it's not a 12.0.1 release blocker.

Hi @kou, since only Homebrew is affected. Why not simply change protobuf to protobuf@21 in our brewfile?

kou · 2023-06-10T20:19:14Z

Oh, I didn't notice it!
It's not a real fix but it'll work as a workaround for now.
Could you open a pull request for the workaround?

### Rationale for this change A temporary workaround for link errors caused by protobuf 23. ### What changes are included in this PR? Pin brew protobuf version to 21 for now. ### Are these changes tested? To be tested by CI pipelines. ### Are there any user-facing changes? No * Closes: #35987 Authored-by: Jin Shang <shangjin1997@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

kou · 2023-06-11T20:52:57Z

Reopen this because #36029 is a workaround.

This allows us to use the latest `protobuf` with `libphonenumber`. See Homebrew#133508. It likely also allows us to use a newer `protobuf` and `grpc` in `apache-arrow`. See apache/arrow#35987.

This is a follow-up of apacheGH-36029. This is still a workaround.

raulcd · 2023-06-15T09:25:17Z

@kou @js8544 from my understanding this fix on homebrew should allow us to unpin the versions of protobuf and grpc, right?
Homebrew/homebrew-core#133623

js8544 · 2023-06-15T09:27:38Z

@kou @js8544 from my understanding this fix on homebrew should allow us to unpin the versions of protobuf and grpc, right? Homebrew/homebrew-core#133623

It probably does! I'll open a PR to try this.

js8544 · 2023-06-15T09:52:45Z

@kou @js8544 from my understanding this fix on homebrew should allow us to unpin the versions of protobuf and grpc, right? Homebrew/homebrew-core#133623

Well, #36013 is still causing errors: https://github.com/apache/arrow/actions/runs/5277136967/jobs/9544769123?pr=36087. I'll try again once #36016 is merged.

Unpin brew protobuf version because of an upstream patch. * Closes: #35987 Authored-by: Jin Shang <shangjin1997@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

kou · 2023-06-16T06:56:49Z

We don't need something because the problem was happen in upstream and it was fixed in upstream.
(We may want to reject Protobuf versions that have this problem explicitly.)

kou added the Type: enhancement label Jun 8, 2023

github-actions bot added the Component: C++ label Jun 8, 2023

kou mentioned this issue Jun 8, 2023

GH-35961: [C++][FlightSQL] Accept Protobuf 3.12.0 or later #35962

Merged

kou changed the title ~~[C++] Add support for Protobuf 23.2.0 on non-Windows~~ [C++] Add support for Protobuf 23.2 on non-Windows Jun 8, 2023

h-vetinari mentioned this issue Jun 8, 2023

[C++] Compatibility with protobuf 4.23.x #35986

Closed

kou mentioned this issue Jun 8, 2023

ci macOS: add a missing dependency package groonga/groonga#1574

Closed

lidavidm mentioned this issue Jun 9, 2023

GH-36013: [C++] Disabling bundled OpenTelemetry with Protobuf 3.22+ #36016

Merged

js8544 added a commit to js8544/arrow that referenced this issue Jun 11, 2023

apacheGH-35987: [C++] Pin brew protobuf version to 21

ede37eb

github-actions bot mentioned this issue Jun 11, 2023

GH-35987: [C++] Pin brew protobuf version to 21 #36029

Merged

github-actions bot assigned js8544 Jun 11, 2023

kou closed this as completed in #36029 Jun 11, 2023

kou added this to the 13.0.0 milestone Jun 11, 2023

kou reopened this Jun 11, 2023

scpeters mentioned this issue Jun 12, 2023

All protobuf dependent bottles are broken (gz-msgs*, gz-fuel-tools*, etc) osrf/homebrew-simulation#2274

Closed

46 tasks

carlocab mentioned this issue Jun 14, 2023

protobuf: apply upstream fix for thread-local variables Homebrew/homebrew-core#133623

Merged

6 tasks

raulcd mentioned this issue Jun 14, 2023

WIP: [R] Verify CRAN release-12.0.1-rc1 #36054

Closed

kou added a commit to kou/arrow that referenced this issue Jun 15, 2023

apacheGH-35987: [C++] Use old Protobuf and gRPC on macOS + Homebrew

2c90236

This is a follow-up of apacheGH-36029. This is still a workaround.

github-actions bot mentioned this issue Jun 15, 2023

GH-35987: [C++] Use old Protobuf and gRPC on macOS + Homebrew #36086

Closed

github-actions bot mentioned this issue Jun 15, 2023

GH-35987: [C++] Unpin brew protobuf version #36087

Merged

kou closed this as completed in #36087 Jun 16, 2023

kou pushed a commit that referenced this issue Jun 16, 2023

GH-35987: [C++] Unpin brew protobuf version (#36087)

01745aa

Unpin brew protobuf version because of an upstream patch. * Closes: #35987 Authored-by: Jin Shang <shangjin1997@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

westonpace mentioned this issue Jul 10, 2023

[C++] CI failures on Windows & Substrait (appears to be abseil linking issues) #36598

Closed

nilsnolde mentioned this issue Aug 2, 2023

fix up Cmake/protobuf disaster valhalla/valhalla#4207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[C++] Add support for Protobuf 23.2 on non-Windows #35987

[C++] Add support for Protobuf 23.2 on non-Windows #35987

kou commented Jun 8, 2023 •

edited

Loading

kou commented Jun 8, 2023

kou commented Jun 8, 2023

h-vetinari commented Jun 8, 2023

h-vetinari commented Jun 8, 2023

kou commented Jun 8, 2023

raulcd commented Jun 8, 2023

assignUser commented Jun 8, 2023

kou commented Jun 8, 2023

raulcd commented Jun 8, 2023

kou commented Jun 9, 2023

js8544 commented Jun 10, 2023 •

edited

Loading

kou commented Jun 10, 2023

kou commented Jun 11, 2023

raulcd commented Jun 15, 2023

js8544 commented Jun 15, 2023

js8544 commented Jun 15, 2023

kou commented Jun 16, 2023

[C++] Add support for Protobuf 23.2 on non-Windows #35987

[C++] Add support for Protobuf 23.2 on non-Windows #35987

Comments

kou commented Jun 8, 2023 • edited Loading

Describe the enhancement requested

Component(s)

kou commented Jun 8, 2023

kou commented Jun 8, 2023

h-vetinari commented Jun 8, 2023

h-vetinari commented Jun 8, 2023

kou commented Jun 8, 2023

raulcd commented Jun 8, 2023

assignUser commented Jun 8, 2023

kou commented Jun 8, 2023

raulcd commented Jun 8, 2023

kou commented Jun 9, 2023

js8544 commented Jun 10, 2023 • edited Loading

kou commented Jun 10, 2023

kou commented Jun 11, 2023

raulcd commented Jun 15, 2023

js8544 commented Jun 15, 2023

js8544 commented Jun 15, 2023

kou commented Jun 16, 2023

kou commented Jun 8, 2023 •

edited

Loading

js8544 commented Jun 10, 2023 •

edited

Loading