-
Notifications
You must be signed in to change notification settings - Fork 4k
ARROW-13760: [C++] Bump required Protobuf when using Flight #11006
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
|
See #10906 (comment). |
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.
Could you also update this comment?
|
@github-actions crossbow -g submit nightly |
|
|
@github-actions crossbow submit -g nightly |
|
Revision: 2a1f6e074200b0948fed6bdd3f900676752311c0 Submitted crossbow builds: ursacomputing/crossbow @ actions-794 |
|
Failures: |
|
@github-actions crossbow submit test-fedora-33-cpp test-fedora-33-python-3 ubuntu-hirsute-amd64 test-ubuntu-20.10-docs |
|
Revision: 79fdd7e7fd1286c382fc4d15dd5273465e809e55 Submitted crossbow builds: ursacomputing/crossbow @ actions-796
|
|
Hmm, it looks like the bundled protoc on hirsute always just crashes. I'll dig with a docker instance when I get a chance. |
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 improve our CMake configuration to fallback to bundled version automatically instead of specifying BUNDLED explicitly.
Can we see the error message without them?
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.
Ah, this was just my (naive) attempt at fixing things. The original error is here: https://github.com/ursacomputing/crossbow/runs/3433327207?check_suite_focus=true#step:8:856
I added it because I thought the crash was coming from combining the bundled Protobuf with the system gRPC. But that doesn't seem to be the case - it still crashes with the same error - so I want to manually build locally and see what's going on. (Though, I think combining the two would still be an issue regardless at some later stage.)
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.
Thanks. I see.
These =BUNDLED can be reverted, right?
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.
Yes, I just reverted them. I'll try to figure out what's going on here tomorrow.
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 something about the build causes Protobuf to not be linked against pthreads. Protoc then crashes instantly when it tries to use std::call_once. Despite the -lpthread below. Forcing it to load pthreads at runtime via LD_PRELOAD causes the later protoc invocation to succeed.
[200/201] : && /usr/bin/c++ -g -O0 -ffile-prefix-map=/build/apache-arrow-6.0.0~dev20210827=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fdiagnostics-color=always -O3 -DNDEBUG -O3 -DNDEBUG -fPIC -g -O0 -ffile-prefix-map=/build/apache-arrow-6.0.0~dev20210827=. -flto=auto -ffat-lto-objects -fstack-protector-strong -Wformat -Werror=format-security -fdiagnostics-color=always -O3 -DNDEBUG -O3 -DNDEBUG -fPIC -Wl,-Bsymbolic-functions -flto=auto -Wl,-z,relro -rdynamic 'CMakeFiles/protoc.dir/build/apache-arrow-6.0.0~dev20210827/cpp_build/protobuf_ep-prefix/src/protobuf_ep/src/google/protobuf/compiler/main.cc.o' -o protoc-3.17.3.0 -Wl,-rpath,:::::::::::::: libprotoc.a libprotobuf.a -lpthread /usr/lib/x86_64-linux-gnu/libz.so && :
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.
Seemingly related: protocolbuffers/protobuf#7092 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55394
Though running the final linker command without the LTO flags does not help. Running with -Wl,--no-as-needed -lpthread -Wl,--as-needed does.
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.
The seemingly responsible lto flags leak in from the top level due to CMAKE_C_FLAGS and CMAKE_CXX_FLAGS, though I'm still trying to figure out what sets them in this case.
|
@github-actions crossbow submit ubuntu-hirsute-amd64 |
|
Revision: 1f5b181daf6a386fab0ec37a383a8576076f4391 Submitted crossbow builds: ursacomputing/crossbow @ actions-799
|
|
@github-actions crossbow submit -g nightly |
|
Revision: 9d3c510282d39bf25dd9ad56f9ad585e64fda233 Submitted crossbow builds: ursacomputing/crossbow @ actions-803 |
|
Things look to generally be passing here, minus some flakes and some failures from master. |
kou
left a comment
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
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.
Is this just for debug?
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.
Ah, yeah, sorry. I've taken this out.
|
Thanks for checking this over! |
This is to support the FlightSQL proposal in #10906.