-
Notifications
You must be signed in to change notification settings - Fork 15.5k
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
Roll forward Ruby upb changes now that protobuf Ruby build is fixed #5866
Conversation
Revert "Revert "Updated upb from defcleanup branch and modified Ruby to use it (protocolbuffers#5539)" (protocolbuffers#5848)" This reverts commit 1568dea.
Any update? |
There is a sporadic test failure in the parallelism test. It's very hard to reproduce locally, and I haven't figured out what is causing it yet. |
Some of our Kokoro tests seem to run with this level of warnings, and the source strives to be gnu90 compatible. Enforcing it for every build removes the possibility of some errors showing up in Kokoro/Travis tests only.
I tried to match warning flags with what Ruby appears to do in our Kokoro tests.
We need to initialize this marked value before creating the instance.
@TeBoring This CL should be ready with the exception of the macOS Timestamp parsing stuff. |
Could you also merge the fix for Timestamp in this change? |
All tests are passing, but this seems to still be an intermittent, hard-to-reproduce error:
I can't figure out if this is a new problem introduced by this change or if it existed previously. I've tried the following in an attempt to reproduce the bug locally:
None of these strategies has reproduced the problem. |
@@ -17,7 +17,6 @@ module BasicTest | |||
add_message "BadFieldNames" do | |||
optional :dup, :int32, 1 | |||
optional :class, :int32, 2 | |||
optional :"a.b", :int32, 3 |
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.
Why this is not needed?
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's an illegal field name. This is checked more robustly now, so the add_message
call itself will fail now.
Updates the Ruby protobuf library to a new version of upb, the C library used for proto parsing. This removes some APIs that allowed setting fields directly on descriptor objects, but these were internal-only. Any code generated by the protocol compiler, and any code that uses the Ruby/Protobuf DSL should continue to work.