Merge pull request #1292 from haberman/ruby-allow-descriptor
Generate well-known types in Ruby extension and prune unneeded proto2 dependencies.
diff --git a/Makefile.am b/Makefile.am
index fbd27fc..3e21db8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -653,7 +653,6 @@
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
diff --git a/ruby/Rakefile b/ruby/Rakefile
index 81c3119..8eb7a2d 100644
--- a/ruby/Rakefile
+++ b/ruby/Rakefile
@@ -34,6 +34,49 @@
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 @@
t.test_files = FileList["tests/*.rb"]
end
-task :build => [:clean, :compile]
+task :build => [:clean, :compile, :genproto]
task :default => [:build]
# vim:sw=2:et
diff --git a/ruby/tests/generated_code.rb b/ruby/tests/generated_code.rb
deleted file mode 100644
index 5a68543..0000000
--- a/ruby/tests/generated_code.rb
+++ /dev/null
@@ -1,74 +0,0 @@
-# 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
diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc
index 9692f1b..92c76fb 100644
--- a/src/google/protobuf/compiler/ruby/ruby_generator.cc
+++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc
@@ -75,6 +75,10 @@
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 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 @@
"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 @@
GenerateEnumAssignment("", file->enum_type(i), printer);
}
EndPackageModules(levels, printer);
+ return true;
}
bool Generator::Generate(
@@ -384,15 +450,11 @@
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