From 3f98af287b8b1919e1efb65d5f5851ff1e7628ce Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 11 Jul 2023 11:21:22 -0700 Subject: [PATCH] Updated Ruby min version to 2.7, and removed some compat code PiperOrigin-RevId: 547245820 --- .github/workflows/test_ruby.yml | 7 +-- ruby/ext/google/protobuf_c/protobuf.c | 2 +- ruby/ext/google/protobuf_c/protobuf.h | 6 +++ ruby/google-protobuf.gemspec | 2 +- ruby/lib/google/protobuf/object_cache.rb | 20 ++++---- ruby/lib/google/protobuf/well_known_types.rb | 10 +--- ruby/tests/basic.rb | 11 ++--- ruby/tests/common_tests.rb | 49 +++++++++----------- ruby/tests/type_errors.rb | 9 ++-- 9 files changed, 50 insertions(+), 66 deletions(-) diff --git a/.github/workflows/test_ruby.yml b/.github/workflows/test_ruby.yml index 24b4122475..a401c32dc5 100644 --- a/.github/workflows/test_ruby.yml +++ b/.github/workflows/test_ruby.yml @@ -17,14 +17,13 @@ jobs: fail-fast: false matrix: include: - - { name: Ruby 2.6, ruby: ruby-2.6.0, bazel: 5.1.1} - { name: Ruby 2.7, ruby: ruby-2.7.0, bazel: 5.1.1} - { name: Ruby 3.0, ruby: ruby-3.0.2, bazel: 5.1.1} - { name: Ruby 3.1, ruby: ruby-3.1.0, bazel: 5.1.1} - { name: Ruby 3.2, ruby: ruby-3.2.0, bazel: 5.1.1} - { name: JRuby 9.2, ruby: jruby-9.2.20.1, bazel: 5.1.1} - { name: JRuby 9.3, ruby: jruby-9.3.4.0, bazel: 5.1.1} - - { name: Ruby 2.6 (Bazel6), ruby: ruby-2.6.0, bazel: 6.0.0} + - { name: Ruby 2.7 (Bazel6), ruby: ruby-2.7.0, bazel: 6.0.0} - { name: JRuby 9.2 (Bazel6), ruby: jruby-9.2.20.1, bazel: 6.0.0} name: Linux ${{ matrix.name }} @@ -109,14 +108,13 @@ jobs: fail-fast: false matrix: include: - - { name: Ruby 2.6, ruby: ruby-2.6.0, bazel: 5.1.1} - { name: Ruby 2.7, ruby: ruby-2.7.0, bazel: 5.1.1} - { name: Ruby 3.0, ruby: ruby-3.0.2, bazel: 5.1.1} - { name: Ruby 3.1, ruby: ruby-3.1.0, bazel: 5.1.1} - { name: Ruby 3.2, ruby: ruby-3.2.0, bazel: 5.1.1} - { name: JRuby 9.2, ruby: jruby-9.2.20.1, bazel: 5.1.1} - { name: JRuby 9.3, ruby: jruby-9.3.4.0, bazel: 5.1.1} - - { name: Ruby 2.6 (Bazel6), ruby: ruby-2.6.0, bazel: 6.0.0} + - { name: Ruby 2.7 (Bazel6), ruby: ruby-2.7.0, bazel: 6.0.0} - { name: JRuby 9.2 (Bazel6), ruby: jruby-9.2.20.1, bazel: 6.0.0} name: Install ${{ matrix.name }} runs-on: ubuntu-latest @@ -139,4 +137,3 @@ jobs: bazel-bin/protoc --proto_path=src --proto_path=ruby/tests --proto_path=ruby --ruby_out=ruby tests/test_import_proto2.proto; bazel-bin/protoc --proto_path=src --proto_path=ruby/tests --proto_path=ruby --ruby_out=ruby tests/basic_test.proto; ruby ruby/tests/basic.rb - diff --git a/ruby/ext/google/protobuf_c/protobuf.c b/ruby/ext/google/protobuf_c/protobuf.c index 46f6436bab..c9354db4e3 100644 --- a/ruby/ext/google/protobuf_c/protobuf.c +++ b/ruby/ext/google/protobuf_c/protobuf.c @@ -264,7 +264,7 @@ static void ObjectCache_Init(VALUE protobuf) { item_try_add = rb_intern("try_add"); rb_gc_register_address(&weak_obj_cache); -#if RUBY_API_VERSION_CODE >= 20700 && SIZEOF_LONG >= SIZEOF_VALUE +#if SIZEOF_LONG >= SIZEOF_VALUE VALUE cache_class = rb_const_get(protobuf, rb_intern("ObjectCache")); #else VALUE cache_class = rb_const_get(protobuf, rb_intern("LegacyObjectCache")); diff --git a/ruby/ext/google/protobuf_c/protobuf.h b/ruby/ext/google/protobuf_c/protobuf.h index a27bdb8a15..334dedf34a 100644 --- a/ruby/ext/google/protobuf_c/protobuf.h +++ b/ruby/ext/google/protobuf_c/protobuf.h @@ -39,6 +39,12 @@ #undef NDEBUG #endif +#include + +#if RUBY_API_VERSION_CODE < 20700 +#error Protobuf requires Ruby >= 2.7 +#endif + #include // Must be included after the NDEBUG logic above. #include #include diff --git a/ruby/google-protobuf.gemspec b/ruby/google-protobuf.gemspec index 14f334a0ae..fbf0631671 100644 --- a/ruby/google-protobuf.gemspec +++ b/ruby/google-protobuf.gemspec @@ -18,7 +18,7 @@ Gem::Specification.new do |s| s.files += Dir.glob('ext/**/*') s.extensions= ["ext/google/protobuf_c/extconf.rb"] s.add_development_dependency "rake-compiler-dock", "= 1.2.1" end - s.required_ruby_version = '>= 2.3' + s.required_ruby_version = '>= 2.5' s.add_development_dependency "rake-compiler", "~> 1.1.0" s.add_development_dependency "test-unit", '~> 3.0', '>= 3.0.9' end diff --git a/ruby/lib/google/protobuf/object_cache.rb b/ruby/lib/google/protobuf/object_cache.rb index 583cabf98c..432fd22419 100644 --- a/ruby/lib/google/protobuf/object_cache.rb +++ b/ruby/lib/google/protobuf/object_cache.rb @@ -36,17 +36,14 @@ module Google # different wrapper objects for the same C object, which saves memory and # preserves object identity. # - # We use WeakMap for the cache. For Ruby <2.7 we also need a secondary Hash - # to store WeakMap keys because Ruby <2.7 WeakMap doesn't allow non-finalizable - # keys. - # - # We also need the secondary Hash if sizeof(long) < sizeof(VALUE), because this - # means it may not be possible to fit a pointer into a Fixnum. Keys are - # pointers, and if they fit into a Fixnum, Ruby doesn't collect them, but if - # they overflow and require allocating a Bignum, they could get collected - # prematurely, thus removing the cache entry. This happens on 64-bit Windows, - # on which pointers are 64 bits but longs are 32 bits. In this case, we enable - # the secondary Hash to hold the keys and prevent them from being collected. + # We use WeakMap for the cache. If sizeof(long) > sizeof(VALUE), we also + # need a secondary Hash to store WeakMap keys, because our pointer keys may + # need to be stored as Bignum instead of Fixnum. Since WeakMap is weak for + # both keys and values, a Bignum key will cause the WeakMap entry to be + # collected immediately unless there is another reference to the Bignum. + # This happens on 64-bit Windows, on which pointers are 64 bits but longs + # are 32 bits. In this case, we enable the secondary Hash to hold the keys + # and prevent them from being collected. class ObjectCache def initialize @map = ObjectSpace::WeakMap.new @@ -121,4 +118,3 @@ module Google end end end - diff --git a/ruby/lib/google/protobuf/well_known_types.rb b/ruby/lib/google/protobuf/well_known_types.rb index 2d06ca2742..5b8a5d8744 100755 --- a/ruby/lib/google/protobuf/well_known_types.rb +++ b/ruby/lib/google/protobuf/well_known_types.rb @@ -72,14 +72,8 @@ module Google end Timestamp.class_eval do - if RUBY_VERSION < "2.5" - def to_time - Time.at(self.to_f) - end - else - def to_time - Time.at(seconds, nanos, :nanosecond) - end + def to_time + Time.at(seconds, nanos, :nanosecond) end def self.from_time(time) diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 2da06df76a..b70f6304a8 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -618,9 +618,6 @@ module BasicTest assert_equal :proto3, file_descriptor.syntax end - # Ruby 2.5 changed to raise FrozenError instead of RuntimeError - FrozenErrorType = Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.5') ? RuntimeError : FrozenError - def test_map_freeze m = proto_module::MapMessage.new m.map_string_int32['a'] = 5 @@ -632,10 +629,10 @@ module BasicTest assert m.map_string_int32.frozen? assert m.map_string_msg.frozen? - assert_raises(FrozenErrorType) { m.map_string_int32['foo'] = 1 } - assert_raises(FrozenErrorType) { m.map_string_msg['bar'] = proto_module::TestMessage2.new } - assert_raises(FrozenErrorType) { m.map_string_int32.delete('a') } - assert_raises(FrozenErrorType) { m.map_string_int32.clear } + assert_raises(FrozenError) { m.map_string_int32['foo'] = 1 } + assert_raises(FrozenError) { m.map_string_msg['bar'] = proto_module::TestMessage2.new } + assert_raises(FrozenError) { m.map_string_int32.delete('a') } + assert_raises(FrozenError) { m.map_string_int32.clear } end def test_map_length diff --git a/ruby/tests/common_tests.rb b/ruby/tests/common_tests.rb index afa3da59d1..52e0845985 100644 --- a/ruby/tests/common_tests.rb +++ b/ruby/tests/common_tests.rb @@ -9,9 +9,6 @@ require 'google/protobuf/wrappers_pb.rb' require 'bigdecimal' module CommonTests - # Ruby 2.5 changed to raise FrozenError instead of RuntimeError - FrozenErrorType = Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.5') ? RuntimeError : FrozenError - def test_defaults m = proto_module::TestMessage.new assert_equal 0, m.optional_int32 @@ -210,7 +207,7 @@ module CommonTests # strings are immutable so we can't do this, but serialize should catch it. m.optional_string = "asdf".encode!('UTF-8') - assert_raises(FrozenErrorType) { m.optional_string.encode!('ASCII-8BIT') } + assert_raises(FrozenError) { m.optional_string.encode!('ASCII-8BIT') } end def test_rptfield_int32 @@ -1711,31 +1708,31 @@ module CommonTests m.optional_int32 = 10 m.freeze - frozen_error = assert_raises(FrozenErrorType) { m.optional_int32 = 20 } + frozen_error = assert_raises(FrozenError) { m.optional_int32 = 20 } assert_match "can't modify frozen #{proto_module}::TestMessage", frozen_error.message assert_equal 10, m.optional_int32 assert m.frozen? - assert_raises(FrozenErrorType) { m.optional_int64 = 2 } - assert_raises(FrozenErrorType) { m.optional_uint32 = 3 } - assert_raises(FrozenErrorType) { m.optional_uint64 = 4 } - assert_raises(FrozenErrorType) { m.optional_bool = true } - assert_raises(FrozenErrorType) { m.optional_float = 6.0 } - assert_raises(FrozenErrorType) { m.optional_double = 7.0 } - assert_raises(FrozenErrorType) { m.optional_string = '8' } - assert_raises(FrozenErrorType) { m.optional_bytes = nil } - assert_raises(FrozenErrorType) { m.optional_msg = proto_module::TestMessage2.new } - assert_raises(FrozenErrorType) { m.optional_enum = :A } - assert_raises(FrozenErrorType) { m.repeated_int32 = 1 } - assert_raises(FrozenErrorType) { m.repeated_int64 = 2 } - assert_raises(FrozenErrorType) { m.repeated_uint32 = 3 } - assert_raises(FrozenErrorType) { m.repeated_uint64 = 4 } - assert_raises(FrozenErrorType) { m.repeated_bool = true } - assert_raises(FrozenErrorType) { m.repeated_float = 6.0 } - assert_raises(FrozenErrorType) { m.repeated_double = 7.0 } - assert_raises(FrozenErrorType) { m.repeated_string = '8' } - assert_raises(FrozenErrorType) { m.repeated_bytes = nil } - assert_raises(FrozenErrorType) { m.repeated_msg = proto_module::TestMessage2.new } - assert_raises(FrozenErrorType) { m.repeated_enum = :A } + assert_raises(FrozenError) { m.optional_int64 = 2 } + assert_raises(FrozenError) { m.optional_uint32 = 3 } + assert_raises(FrozenError) { m.optional_uint64 = 4 } + assert_raises(FrozenError) { m.optional_bool = true } + assert_raises(FrozenError) { m.optional_float = 6.0 } + assert_raises(FrozenError) { m.optional_double = 7.0 } + assert_raises(FrozenError) { m.optional_string = '8' } + assert_raises(FrozenError) { m.optional_bytes = nil } + assert_raises(FrozenError) { m.optional_msg = proto_module::TestMessage2.new } + assert_raises(FrozenError) { m.optional_enum = :A } + assert_raises(FrozenError) { m.repeated_int32 = 1 } + assert_raises(FrozenError) { m.repeated_int64 = 2 } + assert_raises(FrozenError) { m.repeated_uint32 = 3 } + assert_raises(FrozenError) { m.repeated_uint64 = 4 } + assert_raises(FrozenError) { m.repeated_bool = true } + assert_raises(FrozenError) { m.repeated_float = 6.0 } + assert_raises(FrozenError) { m.repeated_double = 7.0 } + assert_raises(FrozenError) { m.repeated_string = '8' } + assert_raises(FrozenError) { m.repeated_bytes = nil } + assert_raises(FrozenError) { m.repeated_msg = proto_module::TestMessage2.new } + assert_raises(FrozenError) { m.repeated_enum = :A } end def test_eq diff --git a/ruby/tests/type_errors.rb b/ruby/tests/type_errors.rb index 6f6eb06178..76c591c0a5 100755 --- a/ruby/tests/type_errors.rb +++ b/ruby/tests/type_errors.rb @@ -8,16 +8,13 @@ require 'google/protobuf/well_known_types' require 'generated_code_pb' class TestTypeErrors < Test::Unit::TestCase - # Ruby 2.4 unified Fixnum with Integer - IntegerType = Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.4') ? Fixnum : Integer - def test_bad_string check_error Google::Protobuf::TypeError, - "Invalid argument for string field 'optional_string' (given #{IntegerType.name})." do + "Invalid argument for string field 'optional_string' (given Integer)." do A::B::C::TestMessage.new(optional_string: 4) end check_error Google::Protobuf::TypeError, - "Invalid argument for string field 'oneof_string' (given #{IntegerType.name})." do + "Invalid argument for string field 'oneof_string' (given Integer)." do A::B::C::TestMessage.new(oneof_string: 4) end check_error ArgumentError, @@ -154,7 +151,7 @@ class TestTypeErrors < Test::Unit::TestCase def test_bad_msg check_error Google::Protobuf::TypeError, - "Invalid type #{IntegerType.name} to assign to submessage field 'optional_msg'." do + "Invalid type Integer to assign to submessage field 'optional_msg'." do A::B::C::TestMessage.new(optional_msg: 2) end check_error Google::Protobuf::TypeError,