Ship all option dependencies to plugins along with regular ones.

mkruskal-google · copybara-github · commit abeb1306914e · 2025-09-05T20:22:47.000-07:00
For most plugins, these will still be ignored because they're disjoint from dependencies in the descriptors
themselves.  For java (and similar languages), optional dependencies
will need to be extracted specially and made to handle the case where they aren't available
smoothly.

PiperOrigin-RevId: 803702133
diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc
@@ -333,6 +333,14 @@ void CommandLineInterface::GetTransitiveDependencies(
     GetTransitiveDependencies(file->dependency(i), already_seen, output,
                               options);
   }
+  for (int i = 0; i < file->option_dependency_count(); ++i) {
+    const FileDescriptor* dep =
+        file->pool()->FindFileByName(file->option_dependency_name(i));
+    ABSL_CHECK(dep != nullptr)
+        << "Option dependency " << file->option_dependency_name(i)
+        << " not found in pool.  This should never happen.";
+    GetTransitiveDependencies(dep, already_seen, output, options);
+  }
 
   // Add this file.
   FileDescriptorProto* new_descriptor = output->Add();
diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc
@@ -3104,7 +3104,7 @@ TEST_F(CommandLineInterfaceTest, WriteTransitiveDescriptorSetWithSourceInfo) {
   EXPECT_TRUE(descriptor_set.file(1).has_source_code_info());
 }
 
-TEST_F(CommandLineInterfaceTest, NoWriteTransitiveOptionImportDescriptorSet) {
+TEST_F(CommandLineInterfaceTest, WriteTransitiveOptionImportDescriptorSet) {
   CreateTempFile("google/protobuf/descriptor.proto",
                  google::protobuf::DescriptorProto::descriptor()->file()->DebugString());
   CreateTempFile("custom_option.proto",
@@ -3140,13 +3140,41 @@ TEST_F(CommandLineInterfaceTest, NoWriteTransitiveOptionImportDescriptorSet) {
   FileDescriptorSet descriptor_set;
   ReadDescriptorSet("descriptor_set", &descriptor_set);
   if (HasFatalFailure()) return;
-  EXPECT_EQ(2, descriptor_set.file_size());
+  EXPECT_EQ(4, descriptor_set.file_size());
   if (descriptor_set.file(0).name() == "bar.proto") {
     std::swap(descriptor_set.mutable_file()->mutable_data()[0],
               descriptor_set.mutable_file()->mutable_data()[1]);
   }
   EXPECT_EQ("foo.proto", descriptor_set.file(0).name());
-  EXPECT_EQ("bar.proto", descriptor_set.file(1).name());
+  EXPECT_EQ("google/protobuf/descriptor.proto", descriptor_set.file(1).name());
+  EXPECT_EQ("custom_option.proto", descriptor_set.file(2).name());
+  EXPECT_EQ("bar.proto", descriptor_set.file(3).name());
+}
+
+TEST_F(CommandLineInterfaceTest, DisallowMissingOptionImportsDescriptorSetIn) {
+  FileDescriptorSet file_descriptor_set;
+
+  FileDescriptorProto* file = file_descriptor_set.add_file();
+  file->set_syntax("editions");
+  file->set_edition(Edition::EDITION_2024);
+  file->set_name("foo.proto");
+  file->add_option_dependency("bar.proto");
+  file->add_message_type()->set_name("Foo");
+
+  // Add an unknown field to the file options to make it look like a custom
+  // option.
+  file->mutable_message_type(0)
+      ->mutable_options()
+      ->mutable_unknown_fields()
+      ->AddVarint(123, 456);
+
+  WriteDescriptorSet("foo.bin", &file_descriptor_set);
+
+  Run("protocol_compiler --descriptor_set_out=$tmpdir/descriptor_set "
+      "--include_imports --descriptor_set_in=$tmpdir/foo.bin "
+      "--proto_path=$tmpdir --experimental_editions foo.proto");
+
+  ExpectErrorSubstring("foo.proto: Import \"bar.proto\" was not found");
 }
 
 TEST_F(CommandLineInterfaceTest, DescriptorSetOptionRetention) {
diff --git a/src/google/protobuf/compiler/java/file.cc b/src/google/protobuf/compiler/java/file.cc
@@ -105,6 +105,15 @@ bool CollectExtensions(const Message& message, FieldDescriptorSet* extensions) {
   return true;
 }
 
+void CollectPublicDependencies(
+    const FileDescriptor* file,
+    absl::flat_hash_set<const FileDescriptor*>* dependencies) {
+  if (file == nullptr || !dependencies->insert(file).second) return;
+  for (int i = 0; file != nullptr && i < file->public_dependency_count(); i++) {
+    CollectPublicDependencies(file->public_dependency(i), dependencies);
+  }
+}
+
 // Finds all extensions for custom options in the given file descriptor with the
 // builder pool which resolves Java features instead of the generated pool.
 void CollectExtensions(const FileDescriptor& file,
@@ -129,6 +138,17 @@ void CollectExtensions(const FileDescriptor& file,
   extensions->clear();
   // Unknown extensions are ok and expected in the case of option imports.
   CollectExtensions(*dynamic_file_proto, extensions);
+
+  // TODO Check against dependencies to remove option dependencies
+  // polluting the pool.  These will be handled as optional dependencies.
+  absl::flat_hash_set<const FileDescriptor*> dependencies;
+  dependencies.insert(&file);
+  for (int i = 0; i < file.dependency_count(); i++) {
+    CollectPublicDependencies(file.dependency(i), &dependencies);
+  }
+  absl::erase_if(*extensions, [&](const FieldDescriptor* fieldDescriptor) {
+    return !dependencies.contains(fieldDescriptor->file());
+  });
 }
 
 // Our static initialization methods can become very, very large.
