Merge pull request #1292 from haberman/ruby-allow-descriptor

Generate well-known types in Ruby extension and prune unneeded proto2 dependencies.

Makefile.am

@@ -653,7 +653,6 @@ ruby_EXTRA_DIST=                                                             \
   ruby/tests/repeated_field_test.rb                                          \
   ruby/tests/stress.rb                                                       \
   ruby/tests/generated_code.proto                                            \
-  ruby/tests/generated_code.rb                                               \
   ruby/tests/generated_code_test.rb                                          \
   ruby/travis-test.sh
 

ruby/Rakefile

@@ -34,6 +34,49 @@ else
   end
 end
 
+well_known_protos = %w[
+  google/protobuf/any.proto
+  google/protobuf/api.proto
+  google/protobuf/duration.proto
+  google/protobuf/empty.proto
+  google/protobuf/field_mask.proto
+  google/protobuf/source_context.proto
+  google/protobuf/struct.proto
+  google/protobuf/timestamp.proto
+  google/protobuf/type.proto
+  google/protobuf/wrappers.proto
+]
+
+# These are omitted for now because we don't support proto2.
+proto2_protos = %w[
+  google/protobuf/descriptor.proto
+  google/protobuf/compiler/plugin.proto
+]
+
+genproto_output = []
+
+well_known_protos.each do |proto_file|
+  input_file = "../src/" + proto_file
+  output_file = "lib/" + proto_file.sub(/\.proto$/, ".rb")
+  genproto_output << output_file
+  file output_file => input_file do |file_task|
+    sh "../src/protoc -I../src --ruby_out=lib #{input_file}"
+  end
+end
+
+
+# Proto for tests.
+genproto_output << "tests/generated_code.rb"
+file "tests/generated_code.rb" => "tests/generated_code.proto" do |file_task|
+  sh "../src/protoc --ruby_out=. tests/generated_code.proto"
+end
+
+task :genproto => genproto_output
+
+task :clean do
+  sh "rm -f #{genproto_output.join(' ')}"
+end
+
 Gem::PackageTask.new(spec) do |pkg|
 end
 
@@ -41,7 +84,7 @@ Rake::TestTask.new(:test => :build) do |t|
   t.test_files = FileList["tests/*.rb"]
 end
 
-task :build => [:clean, :compile]
+task :build => [:clean, :compile, :genproto]
 task :default => [:build]
 
 # vim:sw=2:et

ruby/tests/generated_code.rb

-# Generated by the protocol buffer compiler.  DO NOT EDIT!
-# source: generated_code.proto
-
-require 'google/protobuf'
-
-Google::Protobuf::DescriptorPool.generated_pool.build do
-  add_message "A.B.C.TestMessage" do
-    optional :optional_int32, :int32, 1
-    optional :optional_int64, :int64, 2
-    optional :optional_uint32, :uint32, 3
-    optional :optional_uint64, :uint64, 4
-    optional :optional_bool, :bool, 5
-    optional :optional_double, :double, 6
-    optional :optional_float, :float, 7
-    optional :optional_string, :string, 8
-    optional :optional_bytes, :string, 9
-    optional :optional_enum, :enum, 10, "A.B.C.TestEnum"
-    optional :optional_msg, :message, 11, "A.B.C.TestMessage"
-    repeated :repeated_int32, :int32, 21
-    repeated :repeated_int64, :int64, 22
-    repeated :repeated_uint32, :uint32, 23
-    repeated :repeated_uint64, :uint64, 24
-    repeated :repeated_bool, :bool, 25
-    repeated :repeated_double, :double, 26
-    repeated :repeated_float, :float, 27
-    repeated :repeated_string, :string, 28
-    repeated :repeated_bytes, :string, 29
-    repeated :repeated_enum, :enum, 30, "A.B.C.TestEnum"
-    repeated :repeated_msg, :message, 31, "A.B.C.TestMessage"
-    map :map_int32_string, :int32, :string, 61
-    map :map_int64_string, :int64, :string, 62
-    map :map_uint32_string, :uint32, :string, 63
-    map :map_uint64_string, :uint64, :string, 64
-    map :map_bool_string, :bool, :string, 65
-    map :map_string_string, :string, :string, 66
-    map :map_string_msg, :string, :message, 67, "A.B.C.TestMessage"
-    map :map_string_enum, :string, :enum, 68, "A.B.C.TestEnum"
-    map :map_string_int32, :string, :int32, 69
-    map :map_string_bool, :string, :bool, 70
-    optional :nested_message, :message, 80, "A.B.C.TestMessage.NestedMessage"
-    oneof :my_oneof do
-      optional :oneof_int32, :int32, 41
-      optional :oneof_int64, :int64, 42
-      optional :oneof_uint32, :uint32, 43
-      optional :oneof_uint64, :uint64, 44
-      optional :oneof_bool, :bool, 45
-      optional :oneof_double, :double, 46
-      optional :oneof_float, :float, 47
-      optional :oneof_string, :string, 48
-      optional :oneof_bytes, :string, 49
-      optional :oneof_enum, :enum, 50, "A.B.C.TestEnum"
-      optional :oneof_msg, :message, 51, "A.B.C.TestMessage"
-    end
-  end
-  add_message "A.B.C.TestMessage.NestedMessage" do
-    optional :foo, :int32, 1
-  end
-  add_enum "A.B.C.TestEnum" do
-    value :Default, 0
-    value :A, 1
-    value :B, 2
-    value :C, 3
-  end
-end
-
-module A
-  module B
-    module C
-      TestMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage").msgclass
-      TestMessage::NestedMessage = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestMessage.NestedMessage").msgclass
-      TestEnum = Google::Protobuf::DescriptorPool.generated_pool.lookup("A.B.C.TestEnum").enummodule
-    end
-  end
-end

src/google/protobuf/compiler/ruby/ruby_generator.cc

@@ -75,6 +75,10 @@ std::string StripDotProto(const std::string& proto_file) {
   return proto_file.substr(0, lastindex);
 }
 
+std::string GetOutputFilename(const std::string& proto_file) {
+  return StripDotProto(proto_file) + ".rb";
+}
+
 std::string LabelForField(const google::protobuf::FieldDescriptor* field) {
   switch (field->label()) {
     case FieldDescriptor::LABEL_OPTIONAL: return "optional";
@@ -331,8 +335,69 @@ void EndPackageModules(
   }
 }
 
-void GenerateFile(const google::protobuf::FileDescriptor* file,
-                  google::protobuf::io::Printer* printer) {
+bool UsesTypeFromFile(const Descriptor* message, const FileDescriptor* file,
+                      string* error) {
+  for (int i = 0; i < message->field_count(); i++) {
+    const FieldDescriptor* field = message->field(i);
+    if ((field->cpp_type() == FieldDescriptor::CPPTYPE_MESSAGE &&
+         field->message_type()->file() == file) ||
+        (field->type() == FieldDescriptor::TYPE_ENUM &&
+         field->enum_type()->file() == file)) {
+      *error = "proto3 message field " + field->full_name() + " in file " +
+               file->name() + " has a dependency on a type from proto2 file " +
+               file->name() +
+               ".  Ruby doesn't support proto2 yet, so we must fail.";
+      return true;
+    }
+  }
+
+  for (int i = 0; i < message->nested_type_count(); i++) {
+    if (UsesTypeFromFile(message->nested_type(i), file, error)) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
+// Ruby doesn't currently support proto2.  This causes a failure even for proto3
+// files that import proto2.  But in some cases, the proto2 file is only being
+// imported to extend another proto2 message.  The prime example is declaring
+// custom options by extending FileOptions/FieldOptions/etc.
+//
+// If the proto3 messages don't have any proto2 submessages, it is safe to omit
+// the dependency completely.  Users won't be able to use any proto2 extensions,
+// but they already couldn't because proto2 messages aren't supported.
+//
+// If/when we add proto2 support, we should remove this.
+bool MaybeEmitDependency(const FileDescriptor* import,
+                         const FileDescriptor* from,
+                         io::Printer* printer,
+                         string* error) {
+  if (import->syntax() == FileDescriptor::SYNTAX_PROTO2) {
+    for (int i = 0; i < from->message_type_count(); i++) {
+      if (UsesTypeFromFile(from->message_type(i), import, error)) {
+        // Error text was already set by UsesTypeFromFile().
+        return false;
+      }
+    }
+
+    // Ok to omit this proto2 dependency -- so we won't print anything.
+    GOOGLE_LOG(WARNING) << "Omitting proto2 dependency '" << import->name()
+                        << "' from proto3 output file '"
+                        << GetOutputFilename(from->name())
+                        << "' because we don't support proto2 and no proto2 "
+                           "types from that file are being used.";
+    return true;
+  } else {
+    printer->Print(
+      "require '$name$'\n", "name", StripDotProto(import->name()));
+    return true;
+  }
+}
+
+bool GenerateFile(const FileDescriptor* file, io::Printer* printer,
+                  string* error) {
   printer->Print(
     "# Generated by the protocol buffer compiler.  DO NOT EDIT!\n"
     "# source: $filename$\n"
@@ -343,9 +408,9 @@ void GenerateFile(const google::protobuf::FileDescriptor* file,
     "require 'google/protobuf'\n\n");
 
   for (int i = 0; i < file->dependency_count(); i++) {
-    const std::string& name = file->dependency(i)->name();
-    printer->Print(
-      "require '$name$'\n", "name", StripDotProto(name));
+    if (!MaybeEmitDependency(file->dependency(i), file, printer, error)) {
+      return false;
+    }
   }
 
   printer->Print(
@@ -369,6 +434,7 @@ void GenerateFile(const google::protobuf::FileDescriptor* file,
     GenerateEnumAssignment("", file->enum_type(i), printer);
   }
   EndPackageModules(levels, printer);
+  return true;
 }
 
 bool Generator::Generate(
@@ -384,15 +450,11 @@ bool Generator::Generate(
     return false;
   }
 
-  std::string filename =
-      StripDotProto(file->name()) + ".rb";
   scoped_ptr<io::ZeroCopyOutputStream> output(
-      generator_context->Open(filename));
+      generator_context->Open(GetOutputFilename(file->name())));
   io::Printer printer(output.get(), '$');
 
-  GenerateFile(file, &printer);
-
-  return true;
+  return GenerateFile(file, &printer, error);
 }
 
 }  // namespace ruby