diff --git a/ruby/Rakefile b/ruby/Rakefile index 60ec6ea66583..c7187a6a8989 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -61,7 +61,7 @@ unless ENV['IN_DOCKER'] == 'true' output_file = proto_file.sub(/\.proto$/, "_pb.rb") genproto_output << output_file file output_file => proto_file do |file_task| - sh "#{protoc_command} -I../src -I. --ruby_out=. #{proto_file}" + sh "#{protoc_command} -I../src -I. -I./tests --ruby_out=. #{proto_file}" end end end diff --git a/ruby/tests/basic.rb b/ruby/tests/basic.rb index 6beb4b75713e..ed15bde0eb67 100755 --- a/ruby/tests/basic.rb +++ b/ruby/tests/basic.rb @@ -168,6 +168,17 @@ def test_set_clear_defaults assert_equal nil, m.singular_msg end + def test_import_proto2 + m = TestMessage.new + assert !m.has_optional_proto2_submessage? + m.optional_proto2_submessage = ::FooBar::Proto2::TestImportedMessage.new + assert m.has_optional_proto2_submessage? + assert TestMessage.descriptor.lookup('optional_proto2_submessage').has?(m) + + m.clear_optional_proto2_submessage + assert !m.has_optional_proto2_submessage? + end + def test_clear_repeated_fields m = TestMessage.new @@ -487,6 +498,7 @@ def test_to_h :optional_int64=>0, :optional_msg=>nil, :optional_msg2=>nil, + :optional_proto2_submessage=>nil, :optional_string=>"foo", :optional_uint32=>0, :optional_uint64=>0, diff --git a/ruby/tests/basic_test.proto b/ruby/tests/basic_test.proto index 096ded876d22..bca172a722ec 100644 --- a/ruby/tests/basic_test.proto +++ b/ruby/tests/basic_test.proto @@ -6,6 +6,7 @@ import "google/protobuf/wrappers.proto"; import "google/protobuf/timestamp.proto"; import "google/protobuf/duration.proto"; import "google/protobuf/struct.proto"; +import "test_import_proto2.proto"; message Foo { Bar bar = 1; @@ -32,6 +33,7 @@ message TestMessage { optional bytes optional_bytes = 9; optional TestMessage2 optional_msg = 10; optional TestEnum optional_enum = 11; + optional foo_bar.proto2.TestImportedMessage optional_proto2_submessage = 24; repeated int32 repeated_int32 = 12; repeated int64 repeated_int64 = 13; diff --git a/src/Makefile.am b/src/Makefile.am index d4f11ce79959..48e0aafcb49b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -546,6 +546,7 @@ EXTRA_DIST = \ google/protobuf/compiler/package_info.h \ google/protobuf/compiler/ruby/ruby_generated_code.proto \ google/protobuf/compiler/ruby/ruby_generated_code_pb.rb \ + google/protobuf/compiler/ruby/ruby_generated_code_proto2_import.proto \ google/protobuf/compiler/ruby/ruby_generated_code_proto2.proto \ google/protobuf/compiler/ruby/ruby_generated_code_proto2_pb.rb \ google/protobuf/compiler/ruby/ruby_generated_pkg_explicit.proto \ diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_code.proto b/src/google/protobuf/compiler/ruby/ruby_generated_code.proto index 42d82a6bac7e..70ec9f17a0fc 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generated_code.proto +++ b/src/google/protobuf/compiler/ruby/ruby_generated_code.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package A.B.C; +import "ruby_generated_code_proto2_import.proto"; + message TestMessage { int32 optional_int32 = 1; int64 optional_int64 = 2; @@ -14,6 +16,7 @@ message TestMessage { bytes optional_bytes = 9; TestEnum optional_enum = 10; TestMessage optional_msg = 11; + TestImportedMessage optional_proto2_submessage = 12; repeated int32 repeated_int32 = 21; repeated int64 repeated_int64 = 22; diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_code_pb.rb b/src/google/protobuf/compiler/ruby/ruby_generated_code_pb.rb index 7e66d1ecc8fd..4cf4866cfa38 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generated_code_pb.rb +++ b/src/google/protobuf/compiler/ruby/ruby_generated_code_pb.rb @@ -1,6 +1,7 @@ # Generated by the protocol buffer compiler. DO NOT EDIT! # source: ruby_generated_code.proto +require 'ruby_generated_code_proto2_import_pb' require 'google/protobuf' Google::Protobuf::DescriptorPool.generated_pool.build do @@ -17,6 +18,7 @@ optional :optional_bytes, :bytes, 9 optional :optional_enum, :enum, 10, "A.B.C.TestEnum" optional :optional_msg, :message, 11, "A.B.C.TestMessage" + optional :optional_proto2_submessage, :message, 12, "A.B.C.TestImportedMessage" repeated :repeated_int32, :int32, 21 repeated :repeated_int64, :int64, 22 repeated :repeated_uint32, :uint32, 23 diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2.proto b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2.proto index 8d3cc13e03d2..ea7f78363447 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2.proto +++ b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2.proto @@ -2,6 +2,8 @@ syntax = "proto2"; package A.B.C; +import "ruby_generated_code_proto2_import.proto"; + message TestMessage { optional int32 optional_int32 = 1 [default = 1]; optional int64 optional_int64 = 2 [default = 2]; @@ -14,6 +16,7 @@ message TestMessage { optional bytes optional_bytes = 9 [default = "\0\1\2\100fubar"]; optional TestEnum optional_enum = 10 [default = A]; optional TestMessage optional_msg = 11; + optional TestImportedMessage optional_proto2_submessage = 12; repeated int32 repeated_int32 = 21; repeated int64 repeated_int64 = 22; diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_import.proto b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_import.proto new file mode 100644 index 000000000000..9ec07381be54 --- /dev/null +++ b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_import.proto @@ -0,0 +1,5 @@ +syntax = "proto2"; + +package A.B.C; + +message TestImportedMessage {} diff --git a/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_pb.rb b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_pb.rb index c89738fe83da..e331e9b2b3eb 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_pb.rb +++ b/src/google/protobuf/compiler/ruby/ruby_generated_code_proto2_pb.rb @@ -1,6 +1,7 @@ # Generated by the protocol buffer compiler. DO NOT EDIT! # source: ruby_generated_code_proto2.proto +require 'ruby_generated_code_proto2_import_pb' require 'google/protobuf' Google::Protobuf::DescriptorPool.generated_pool.build do @@ -17,6 +18,7 @@ optional :optional_bytes, :bytes, 9, default: "\x00\x01\x02\x40\x66\x75\x62\x61\x72".force_encoding("ASCII-8BIT") optional :optional_enum, :enum, 10, "A.B.C.TestEnum", default: 1 optional :optional_msg, :message, 11, "A.B.C.TestMessage" + optional :optional_proto2_submessage, :message, 12, "A.B.C.TestImportedMessage" repeated :repeated_int32, :int32, 21 repeated :repeated_int64, :int64, 22 repeated :repeated_uint32, :uint32, 23 diff --git a/src/google/protobuf/compiler/ruby/ruby_generator.cc b/src/google/protobuf/compiler/ruby/ruby_generator.cc index 2cee902d713b..f45c0f3c8946 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator.cc @@ -490,43 +490,6 @@ bool UsesTypeFromFile(const Descriptor* message, const FileDescriptor* file, 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, - std::string* error) { - if (from->syntax() == FileDescriptor::SYNTAX_PROTO3 && - 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", GetRequireName(import->name())); - return true; - } -} - bool GenerateDslDescriptor(const FileDescriptor* file, io::Printer* printer, std::string* error) { printer->Print( @@ -572,9 +535,7 @@ bool GenerateFile(const FileDescriptor* file, io::Printer* printer, "filename", file->name()); for (int i = 0; i < file->dependency_count(); i++) { - if (!MaybeEmitDependency(file->dependency(i), file, printer, error)) { - return false; - } + printer->Print("require '$name$'\n", "name", GetRequireName(file->dependency(i)->name())); } // TODO: Remove this when ruby supports extensions for proto2 syntax. diff --git a/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc b/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc index 27439a737bba..040b6c981cb2 100644 --- a/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc +++ b/src/google/protobuf/compiler/ruby/ruby_generator_unittest.cc @@ -57,7 +57,7 @@ std::string FindRubyTestDir() { // Some day, we may integrate build systems between protoc and the language // extensions to the point where we can do this test in a more automated way. -void RubyTest(string proto_file) { +void RubyTest(string proto_file, string import_proto_file = "") { std::string ruby_tests = FindRubyTestDir(); google::protobuf::compiler::CommandLineInterface cli; @@ -77,9 +77,23 @@ void RubyTest(string proto_file) { test_input, true)); + // Copy generated_code_import.proto to the temporary test directory. + std::string test_import; + if (!import_proto_file.empty()) { + GOOGLE_CHECK_OK(File::GetContents( + ruby_tests + import_proto_file + ".proto", + &test_import, + true)); + GOOGLE_CHECK_OK(File::SetContents( + TestTempDir() + import_proto_file + ".proto", + test_import, + true)); + } + // Invoke the proto compiler (we will be inside TestTempDir() at this point). std::string ruby_out = "--ruby_out=" + TestTempDir(); std::string proto_path = "--proto_path=" + TestTempDir(); + std::string proto_target = TestTempDir() + proto_file + ".proto"; const char* argv[] = { "protoc", @@ -105,11 +119,11 @@ void RubyTest(string proto_file) { } TEST(RubyGeneratorTest, Proto3GeneratorTest) { - RubyTest("/ruby_generated_code"); + RubyTest("/ruby_generated_code", "/ruby_generated_code_proto2_import"); } TEST(RubyGeneratorTest, Proto2GeneratorTest) { - RubyTest("/ruby_generated_code_proto2"); + RubyTest("/ruby_generated_code_proto2", "/ruby_generated_code_proto2_import"); } TEST(RubyGeneratorTest, Proto3ImplicitPackageTest) {