Skip to content

Commit

Permalink
Ported Ruby extension to upb_msg (#8184)
Browse files Browse the repository at this point in the history 
* WIP.

* WIP.

* WIP.

* WIP.

* WIP.

* WIP.

* Added some missing files.

* WIP.

* WIP.

* Updated upb.

* Extension loads, but crashes immediately.

* Gets through the test suite without SEGV!

Still a lot of bugs to fix, but it is a major step!

214 tests, 378 assertions, 37 failures, 147 errors, 0 pendings, 0 omissions, 0 notifications
14.0187% passed

* Test and build for Ruby 3.0

* Fixed a few more bugs, efficient #inspect is almost done.

214 tests, 134243 assertions, 30 failures, 144 errors, 0 pendings, 0 omissions, 0 notifications
18.6916% passed

* Fixed message hash initialization and encode depth checking.

214 tests, 124651 assertions, 53 failures, 70 errors, 0 pendings, 0 omissions, 0 notifications
42.5234% passed

* A bunch of fixes to failing tests, now 70% passing.

214 tests, 202091 assertions, 41 failures, 23 errors, 0 pendings, 0 omissions, 0 notifications
70.0935% passed

* More than 80% of tests are passing now.

214 tests, 322331 assertions, 30 failures, 9 errors, 0 pendings, 0 omissions, 0 notifications
81.7757% passed

Unfortunately there is also a sporadic bug/segfault hanging around
that appears to be GC-related.

* Add linux/ruby30 and macos/ruby30

* Use rvm master for 3.0.0-preview2

* Over 90% of tests are passing!

214 tests, 349898 assertions, 15 failures, 1 errors, 0 pendings, 0 omissions, 0 notifications
92.5234% passed

* Passes all tests!

214 tests, 369388 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed

* A bunch of cleanup.

1. Removed a bunch of internal-only symbols from headers.
2. Required a frozen check to get a non-const pointer to a map or array.
3. De-duplicated the code to get a type argument for Map/RepeatedField.

* Removed a bunch more stuff from protobuf.h.  There is an intermittent assert failure.

Intermittent failure:

ruby: ../../../../ext/google/protobuf_c/protobuf.c:263: ObjectCache_Add: Assertion `rb_funcall(obj_cache2, (__builtin_constant_p("[]") ? __extension__ ({ static ID rb_intern_id_cache; if (!rb_intern_id_cache) rb_intern_id_cache = rb_intern2((("[]")
), (long)strlen(("[]"))); (ID) rb_intern_id_cache; }) : rb_intern("[]")), 1, key_rb) == val' failed

* Removed a few more things from protobuf.h.

* Ruby 3.0.0-preview2 to 3.0.0

* Require rake-compiler-dock >= 1.1.0

* More progress, fighting with the object cache.

* Passes on all Ruby versions!

* Updated and clarified comment regarding WeakMap.

* Fixed the wyhash compile.

* Fixed conformance tests for Ruby.

Conformance results now look like:

RUBYLIB=../ruby/lib:. ./conformance-test-runner --enforce_recommended --failure_list failure_list_ruby.txt --text_format_failure_list text_format_failure_list_ruby.txt ./conformance_ruby.rb

CONFORMANCE TEST BEGIN ====================================

CONFORMANCE SUITE PASSED: 1955 successes, 0 skipped, 58 expected failures, 0 unexpected failures.

CONFORMANCE TEST BEGIN ====================================

CONFORMANCE SUITE PASSED: 0 successes, 111 skipped, 8 expected failures, 0 unexpected failures.

Fixes include:

- Changed Ruby compiler to no longer reject proto2 maps.
- Changed Ruby compiler to emit a warning when proto2 extensions are
  present instead of rejecting the .proto file completely.
- Fixed conformance tests to allow proto2 and look up message by name
  instead of hardcoding a specific list of messages.
- Fixed conformance test to support the "ignore unknown" option for
  JSON.
- Fixed conformance test to properly report serialization errors.

* Removed debug printf and fixed #inspect for floats.

* Fixed compatibility test to have proper semantics for #to_json.

* Updated Makefile.am with new file list.

* Don't try to copy wyhash when inside Docker.

* Fixed bug where we would forget that a sub-object is frozen in Ruby >=2.7.

* Avoid exporting unneeded symbols and refactored a bit of code.

* Some more refactoring.

* Simplified and added more comments.

* Some more comments and simplification. Added a missing license block.

Co-authored-by: Masaki Hara <hara@wantedly.com>
  • Loading branch information
@haberman @qnighy
haberman and qnighy committed Jan 13, 2021
1 parent 468bc19 commit 9abf6e2
Show file tree
Hide file tree
Showing 42 changed files with 14,071 additions and 23,328 deletions.
12 changes: 8 additions & 4 deletions Makefile.am
View file
Expand Up @@ -1094,17 +1094,21 @@ ruby_EXTRA_DIST= \
ruby/compatibility_tests/v3.0.0/test.sh \
ruby/compatibility_tests/v3.0.0/Rakefile \
ruby/compatibility_tests/v3.0.0/README.md \
ruby/ext/google/protobuf_c/convert.c \
ruby/ext/google/protobuf_c/convert.h \
ruby/ext/google/protobuf_c/defs.c \
ruby/ext/google/protobuf_c/encode_decode.c \
ruby/ext/google/protobuf_c/defs.h \
ruby/ext/google/protobuf_c/extconf.rb \
ruby/ext/google/protobuf_c/map.c \
ruby/ext/google/protobuf_c/map.h \
ruby/ext/google/protobuf_c/message.c \
ruby/ext/google/protobuf_c/message.h \
ruby/ext/google/protobuf_c/protobuf.c \
ruby/ext/google/protobuf_c/protobuf.h \
ruby/ext/google/protobuf_c/repeated_field.c \
ruby/ext/google/protobuf_c/storage.c \
ruby/ext/google/protobuf_c/upb.c \
ruby/ext/google/protobuf_c/upb.h \
ruby/ext/google/protobuf_c/repeated_field.h \
ruby/ext/google/protobuf_c/ruby-upb.c \
ruby/ext/google/protobuf_c/ruby-upb.h \
ruby/ext/google/protobuf_c/wrap_memcpy.c \
ruby/google-protobuf.gemspec \
ruby/lib/google/protobuf/message_exts.rb \
Expand Down
6 changes: 3 additions & 3 deletions conformance/Makefile.am
View file
Expand Up @@ -5,7 +5,7 @@ conformance_protoc_inputs = \
$(top_srcdir)/src/google/protobuf/test_messages_proto3.proto

# proto2 input files, should be separated with proto3, as we
# can't generate proto2 files for ruby, php and objc
# can't generate proto2 files for php.
conformance_proto2_protoc_inputs = \
$(top_srcdir)/src/google/protobuf/test_messages_proto2.proto

Expand Down Expand Up @@ -261,7 +261,7 @@ if USE_EXTERNAL_PROTOC
# Some implementations include pre-generated versions of well-known types.
protoc_middleman: $(conformance_protoc_inputs) $(conformance_proto2_protoc_inputs) $(well_known_type_protoc_inputs) google-protobuf
$(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --ruby_out=. --objc_out=. --python_out=. --php_out=. --js_out=import_style=commonjs,binary:. $(conformance_protoc_inputs)
$(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --objc_out=. --python_out=. --js_out=import_style=commonjs,binary:. $(conformance_proto2_protoc_inputs)
$(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --ruby_out=. --objc_out=. --python_out=. --js_out=import_style=commonjs,binary:. $(conformance_proto2_protoc_inputs)
$(PROTOC) -I$(srcdir) -I$(top_srcdir) --cpp_out=. --java_out=. --ruby_out=. --python_out=. --js_out=import_style=commonjs,binary:google-protobuf $(well_known_type_protoc_inputs)
## $(PROTOC) -I$(srcdir) -I$(top_srcdir) --java_out=lite:lite $(conformance_protoc_inputs) $(well_known_type_protoc_inputs)
touch protoc_middleman
Expand All @@ -273,7 +273,7 @@ else
# building out-of-tree.
protoc_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(conformance_protoc_inputs) $(conformance_proto2_protoc_inputs) $(well_known_type_protoc_inputs) google-protobuf
oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd --objc_out=$$oldpwd --python_out=$$oldpwd --php_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_protoc_inputs) )
oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --objc_out=. --python_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_proto2_protoc_inputs) )
oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd --objc_out=$$oldpwd --python_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd $(conformance_proto2_protoc_inputs) )
oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd --python_out=$$oldpwd --js_out=import_style=commonjs,binary:$$oldpwd/google-protobuf $(well_known_type_protoc_inputs) )
## @mkdir -p lite
## oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. -I$(top_srcdir)/src --java_out=lite:$$oldpwd/lite $(conformance_protoc_inputs) $(well_known_type_protoc_inputs) )
Expand Down
53 changes: 31 additions & 22 deletions conformance/conformance_ruby.rb
View file
Expand Up @@ -32,46 +32,47 @@

require 'conformance_pb'
require 'google/protobuf/test_messages_proto3_pb'
require 'google/protobuf/test_messages_proto2_pb'

$test_count = 0
$verbose = false

def do_test(request)
test_message = ProtobufTestMessages::Proto3::TestAllTypesProto3.new
response = Conformance::ConformanceResponse.new
descriptor = Google::Protobuf::DescriptorPool.generated_pool.lookup(request.message_type)

unless descriptor
response.skipped = "Unknown message type: " + request.message_type
end

begin
case request.payload
when :protobuf_payload
if request.message_type.eql?('protobuf_test_messages.proto3.TestAllTypesProto3')
begin
test_message = ProtobufTestMessages::Proto3::TestAllTypesProto3.decode(
request.protobuf_payload)
rescue Google::Protobuf::ParseError => err
response.parse_error = err.message.encode('utf-8')
return response
end
elsif request.message_type.eql?('protobuf_test_messages.proto2.TestAllTypesProto2')
response.skipped = "Ruby doesn't support proto2"
begin
test_message = descriptor.msgclass.decode(request.protobuf_payload)
rescue Google::Protobuf::ParseError => err
response.parse_error = err.message.encode('utf-8')
return response
else
fail "Protobuf request doesn't have specific payload type"
end

when :json_payload
begin
test_message = ProtobufTestMessages::Proto3::TestAllTypesProto3.decode_json(
request.json_payload)
options = {}
if request.test_category == :JSON_IGNORE_UNKNOWN_PARSING_TEST
options[:ignore_unknown_fields] = true
end
test_message = descriptor.msgclass.decode_json(request.json_payload, options)
rescue Google::Protobuf::ParseError => err
response.parse_error = err.message.encode('utf-8')
return response
end
when :text_payload
begin
response.skipped = "Ruby doesn't support proto2"
return response
end

when :text_payload
begin
response.skipped = "Ruby doesn't support text format"
return response
end

when nil
fail "Request didn't have payload"
Expand All @@ -82,10 +83,18 @@ def do_test(request)
fail 'Unspecified output format'

when :PROTOBUF
response.protobuf_payload = test_message.to_proto
begin
response.protobuf_payload = test_message.to_proto
rescue Google::Protobuf::ParseError => err
response.serialize_error = err.message.encode('utf-8')
end

when :JSON
response.json_payload = test_message.to_json
begin
response.json_payload = test_message.to_json
rescue Google::Protobuf::ParseError => err
response.serialize_error = err.message.encode('utf-8')
end

when nil
fail "Request didn't have requested output format"
Expand Down

0 comments on commit 9abf6e2

Please sign in to comment.