Merge pull request #9047 from JasonLunn/jruby_test_fixes_redux

Adopt pure ruby DSL implementation for JRuby
protocolbuffers · Oct 5, 2021 · d546ec7 · d546ec7
2 parents ec494a0 + 05ddf82
commit d546ec7
Show file tree

Hide file tree

Showing 24 changed files with 289 additions and 1,875 deletions.
diff --git a/Makefile.am b/Makefile.am
@@ -1150,19 +1150,14 @@ ruby_EXTRA_DIST=                                                             \
   ruby/lib/google/protobuf/well_known_types.rb                               \
   ruby/lib/google/protobuf.rb                                                \
   ruby/pom.xml                                                               \
-  ruby/src/main/java/com/google/protobuf/jruby/RubyBuilder.java              \
   ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptor.java           \
   ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptorPool.java       \
   ruby/src/main/java/com/google/protobuf/jruby/RubyEnum.java                 \
-  ruby/src/main/java/com/google/protobuf/jruby/RubyEnumBuilderContext.java   \
   ruby/src/main/java/com/google/protobuf/jruby/RubyEnumDescriptor.java       \
   ruby/src/main/java/com/google/protobuf/jruby/RubyFieldDescriptor.java      \
-  ruby/src/main/java/com/google/protobuf/jruby/RubyFileBuilderContext.java   \
   ruby/src/main/java/com/google/protobuf/jruby/RubyFileDescriptor.java       \
   ruby/src/main/java/com/google/protobuf/jruby/RubyMap.java                  \
   ruby/src/main/java/com/google/protobuf/jruby/RubyMessage.java              \
-  ruby/src/main/java/com/google/protobuf/jruby/RubyMessageBuilderContext.java \
-  ruby/src/main/java/com/google/protobuf/jruby/RubyOneofBuilderContext.java  \
   ruby/src/main/java/com/google/protobuf/jruby/RubyOneofDescriptor.java      \
   ruby/src/main/java/com/google/protobuf/jruby/RubyProtobuf.java             \
   ruby/src/main/java/com/google/protobuf/jruby/RubyRepeatedField.java        \

diff --git a/conformance/failure_list_jruby.txt b/conformance/failure_list_jruby.txt
diff --git a/ruby/Rakefile b/ruby/Rakefile
@@ -67,16 +67,18 @@ unless ENV['IN_DOCKER'] == 'true'
 end
 
 if RUBY_PLATFORM == "java"
-  if `which mvn` == ''
-    raise ArgumentError, "maven needs to be installed"
-  end
-  task :clean do
+  task :clean => :require_mvn do
     system("mvn --batch-mode clean")
   end
 
-  task :compile do
+  task :compile => :require_mvn do
     system("mvn --batch-mode package")
   end
+
+  task :require_mvn do
+    raise ArgumentError, "maven needs to be installed" if `which mvn` == ''
+  end
+
 else
   Rake::ExtensionTask.new("protobuf_c", spec) do |ext|
     unless RUBY_PLATFORM =~ /darwin/
@@ -95,7 +97,22 @@ else
     ]
   end
 
+  task 'gem:java' do
+    sh "rm Gemfile.lock"
+    require 'rake_compiler_dock'
+    # Specify the repo root as the working and mount directory to provide access
+    # to the java directory
+    repo_root = File.realdirpath File.join(Dir.pwd, '..')
+    RakeCompilerDock.sh <<-"EOT", platform: 'jruby', rubyvm: :jruby, mountdir: repo_root, workdir: repo_root
+      sudo apt-get install maven -y && \
+      cd java && mvn install -Dmaven.test.skip=true && cd ../ruby && \
+      bundle && \
+      IN_DOCKER=true rake compile gem
+    EOT
+  end
+
   task 'gem:windows' do
+    sh "rm Gemfile.lock"
     require 'rake_compiler_dock'
     ['x86-mingw32', 'x64-mingw32', 'x86_64-linux', 'x86-linux'].each do |plat|
       RakeCompilerDock.sh <<-"EOT", platform: plat
@@ -111,7 +128,7 @@ else
       system "rake cross native gem RUBY_CC_VERSION=3.0.0:2.7.0:2.6.0:2.5.1:2.4.0:2.3.0"
     end
   else
-    task 'gem:native' => [:genproto, 'gem:windows']
+    task 'gem:native' => [:genproto, 'gem:windows', 'gem:java']
   end
 end
 

diff --git a/ruby/compatibility_tests/v3.0.0/test.sh b/ruby/compatibility_tests/v3.0.0/test.sh
@@ -14,4 +14,4 @@ wget https://repo1.maven.org/maven2/com/google/protobuf/protoc/3.0.0/${PROTOC_BI
 chmod +x protoc
 
 # Run tests
-rake test
+RUBYLIB=../../lib:. rake test
diff --git a/ruby/lib/google/protobuf.rb b/ruby/lib/google/protobuf.rb
@@ -51,9 +51,9 @@ class TypeError < ::TypeError; end
     require 'google/protobuf_c'
   end
 
-  require 'google/protobuf/descriptor_dsl'
 end
 
+require 'google/protobuf/descriptor_dsl'
 require 'google/protobuf/repeated_field'
 
 module Google

diff --git a/ruby/pom.xml b/ruby/pom.xml
@@ -1,7 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<project xmlns="http://maven.apache.org/POM/4.0.0"
-         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
     <modelVersion>4.0.0</modelVersion>
     <parent>
       <groupId>com.google</groupId>
@@ -11,7 +9,7 @@
 
     <groupId>com.google.protobuf.jruby</groupId>
     <artifactId>protobuf-jruby</artifactId>
-    <version>1.0-SNAPSHOT</version>
+    <version>3.18.0</version>
     <name>Protocol Buffer JRuby native extension</name>
     <description>
       Protocol Buffers are a way of encoding structured data in an efficient yet
@@ -74,6 +72,12 @@
         </plugins>
     </build>
     <dependencies>
+        <!-- Ordering of dependencies matters to update_version.py -->
+        <dependency>
+          <groupId>com.google.protobuf</groupId>
+          <artifactId>protobuf-java-util</artifactId>
+          <version>3.18.0</version>
+        </dependency>
         <dependency>
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-core</artifactId>
@@ -85,10 +89,5 @@
             <version>9.2.11.1</version>
             <scope>provided</scope>
         </dependency>
-        <dependency>
-            <groupId>com.google.protobuf</groupId>
-            <artifactId>protobuf-java-util</artifactId>
-            <version>3.13.0</version>
-        </dependency>
     </dependencies>
 </project>
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyBuilder.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyBuilder.java
diff --git a/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptorPool.java b/ruby/src/main/java/com/google/protobuf/jruby/RubyDescriptorPool.java
@@ -37,9 +37,11 @@
 import com.google.protobuf.Descriptors.DescriptorValidationException;
 import com.google.protobuf.Descriptors.EnumDescriptor;
 import com.google.protobuf.Descriptors.FileDescriptor;
+import com.google.protobuf.InvalidProtocolBufferException;
 import org.jruby.*;
 import org.jruby.anno.JRubyClass;
 import org.jruby.anno.JRubyMethod;
+import org.jruby.exceptions.RaiseException;
 import org.jruby.runtime.*;
 import org.jruby.runtime.builtin.IRubyObject;
 
@@ -61,7 +63,6 @@ public IRubyObject allocate(Ruby runtime, RubyClass klazz) {
 
         cDescriptorPool.defineAnnotatedMethods(RubyDescriptorPool.class);
         descriptorPool = (RubyDescriptorPool) cDescriptorPool.newInstance(runtime.getCurrentContext(), Block.NULL_BLOCK);
-        cBuilder = (RubyClass) runtime.getClassFromPath("Google::Protobuf::Internal::Builder");
         cDescriptor = (RubyClass) runtime.getClassFromPath("Google::Protobuf::Descriptor");
         cEnumDescriptor = (RubyClass) runtime.getClassFromPath("Google::Protobuf::EnumDescriptor");
     }
@@ -74,9 +75,10 @@ public RubyDescriptorPool(Ruby runtime, RubyClass klazz) {
 
     @JRubyMethod
     public IRubyObject build(ThreadContext context, Block block) {
-        RubyBuilder ctx = (RubyBuilder) cBuilder.newInstance(context, this, Block.NULL_BLOCK);
+        RubyClass cBuilder = (RubyClass) context.runtime.getClassFromPath("Google::Protobuf::Internal::Builder");
+        RubyBasicObject ctx = (RubyBasicObject) cBuilder.newInstance(context, this, Block.NULL_BLOCK);
         ctx.instance_eval(context, block);
-        ctx.build(context); // Needs to be called to support the deprecated syntax
+        ctx.callMethod(context, "build"); // Needs to be called to support the deprecated syntax
         return context.nil;
     }
 
@@ -109,6 +111,18 @@ public static IRubyObject generatedPool(ThreadContext context, IRubyObject recv)
         return descriptorPool;
     }
 
+    @JRubyMethod(required = 1)
+    public IRubyObject add_serialized_file (ThreadContext context, IRubyObject data ) {
+        byte[] bin = data.convertToString().getBytes();
+        try {
+            FileDescriptorProto.Builder builder = FileDescriptorProto.newBuilder().mergeFrom(bin);
+            registerFileDescriptor(context, builder);
+        } catch (InvalidProtocolBufferException e) {
+            throw RaiseException.from(context.runtime, (RubyClass) context.runtime.getClassFromPath("Google::Protobuf::ParseError"), e.getMessage());
+        }
+        return context.nil;
+    }
+
     protected void registerFileDescriptor(ThreadContext context, FileDescriptorProto.Builder builder) {
         final FileDescriptor fd;
         try {
@@ -157,7 +171,6 @@ private FileDescriptor[] existingFileDescriptors() {
         return fileDescriptors.toArray(new FileDescriptor[fileDescriptors.size()]);
     }
 
-    private static RubyClass cBuilder;
     private static RubyClass cDescriptor;
     private static RubyClass cEnumDescriptor;
     private static RubyDescriptorPool descriptorPool;