Skip to content

Commit 83507c7

Browse files
mkruskal-googlecopybara-github
authored andcommitted
Implement a retain_options flag in protoc.
This will allow users to force the retention of RETENTION_SOURCE options, which get stripped out by default. PiperOrigin-RevId: 520555632
1 parent 4ee65df commit 83507c7

File tree

3 files changed

+124
-50
lines changed

3 files changed

+124
-50
lines changed

src/google/protobuf/compiler/command_line_interface.cc

Lines changed: 81 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,55 @@ std::string PluginName(absl::string_view plugin_prefix,
298298
directive.substr(2, directive.size() - 6));
299299
}
300300

301+
302+
// Get all transitive dependencies of the given file (including the file
303+
// itself), adding them to the given list of FileDescriptorProtos. The
304+
// protos will be ordered such that every file is listed before any file that
305+
// depends on it, so that you can call DescriptorPool::BuildFile() on them
306+
// in order. Any files in *already_seen will not be added, and each file
307+
// added will be inserted into *already_seen. If include_source_code_info is
308+
// true then include the source code information in the FileDescriptorProtos.
309+
// If include_json_name is true, populate the json_name field of
310+
// FieldDescriptorProto for all fields.
311+
struct TransitiveDependencyOptions {
312+
bool include_json_name = false;
313+
bool include_source_code_info = false;
314+
bool retain_options = false;
315+
};
316+
317+
void GetTransitiveDependencies(
318+
const FileDescriptor* file,
319+
absl::flat_hash_set<const FileDescriptor*>* already_seen,
320+
RepeatedPtrField<FileDescriptorProto>* output,
321+
const TransitiveDependencyOptions& options =
322+
TransitiveDependencyOptions()) {
323+
if (!already_seen->insert(file).second) {
324+
// Already saw this file. Skip.
325+
return;
326+
}
327+
328+
// Add all dependencies.
329+
for (int i = 0; i < file->dependency_count(); i++) {
330+
GetTransitiveDependencies(file->dependency(i), already_seen, output,
331+
options);
332+
}
333+
334+
// Add this file.
335+
FileDescriptorProto* new_descriptor = output->Add();
336+
if (options.retain_options) {
337+
file->CopyTo(new_descriptor);
338+
if (options.include_source_code_info) {
339+
file->CopySourceCodeInfoTo(new_descriptor);
340+
}
341+
} else {
342+
*new_descriptor =
343+
StripSourceRetentionOptions(*file, options.include_source_code_info);
344+
}
345+
if (options.include_json_name) {
346+
file->CopyJsonNameTo(new_descriptor);
347+
}
348+
}
349+
301350
} // namespace
302351

303352
// A MultiFileErrorCollector that prints errors to stderr.
@@ -1454,6 +1503,7 @@ void CommandLineInterface::Clear() {
14541503
print_mode_ = PRINT_NONE;
14551504
imports_in_descriptor_set_ = false;
14561505
source_info_in_descriptor_set_ = false;
1506+
retain_options_in_descriptor_set_ = false;
14571507
disallow_services_ = false;
14581508
direct_dependencies_explicitly_set_ = false;
14591509
deterministic_output_ = false;
@@ -1709,6 +1759,11 @@ CommandLineInterface::ParseArgumentStatus CommandLineInterface::ParseArguments(
17091759
"--descriptor_set_out."
17101760
<< std::endl;
17111761
}
1762+
if (retain_options_in_descriptor_set_ && descriptor_set_out_name_.empty()) {
1763+
std::cerr << "--retain_options only makes sense when combined with "
1764+
"--descriptor_set_out."
1765+
<< std::endl;
1766+
}
17121767

17131768
return PARSE_ARGUMENT_DONE_AND_CONTINUE;
17141769
}
@@ -1759,7 +1814,8 @@ bool CommandLineInterface::ParseArgument(const char* arg, std::string* name,
17591814

17601815
if (*name == "-h" || *name == "--help" || *name == "--disallow_services" ||
17611816
*name == "--include_imports" || *name == "--include_source_info" ||
1762-
*name == "--version" || *name == "--decode_raw" ||
1817+
*name == "--retain_options" || *name == "--version" ||
1818+
*name == "--decode_raw" ||
17631819
*name == "--print_free_field_numbers" ||
17641820
*name == "--experimental_allow_proto3_optional" ||
17651821
*name == "--deterministic_output" || *name == "--fatal_warnings") {
@@ -1954,6 +2010,13 @@ CommandLineInterface::InterpretArgument(const std::string& name,
19542010
}
19552011
source_info_in_descriptor_set_ = true;
19562012

2013+
} else if (name == "--retain_options") {
2014+
if (retain_options_in_descriptor_set_) {
2015+
std::cerr << name << " may only be passed once." << std::endl;
2016+
return PARSE_ARGUMENT_FAIL;
2017+
}
2018+
retain_options_in_descriptor_set_ = true;
2019+
19572020
} else if (name == "-h" || name == "--help") {
19582021
PrintHelpText();
19592022
return PARSE_ARGUMENT_DONE_AND_EXIT; // Exit without running compiler.
@@ -2190,6 +2253,11 @@ Parse PROTO_FILES and generate output based on the options given:
21902253
include information about the original
21912254
location of each decl in the source file as
21922255
well as surrounding comments.
2256+
--retain_options When using --descriptor_set_out, do not strip
2257+
any options from the FileDescriptorProto.
2258+
This results in potentially larger descriptors
2259+
that include information about options that were
2260+
only meant to be useful during compilation.
21932261
--dependency_out=FILE Write a dependency output file in the format
21942262
expected by make. This writes the transitive
21952263
set of input file paths to FILE
@@ -2328,7 +2396,7 @@ bool CommandLineInterface::GenerateDependencyManifestFile(
23282396

23292397
absl::flat_hash_set<const FileDescriptor*> already_seen;
23302398
for (int i = 0; i < parsed_files.size(); i++) {
2331-
GetTransitiveDependencies(parsed_files[i], false, false, &already_seen,
2399+
GetTransitiveDependencies(parsed_files[i], &already_seen,
23322400
file_set.mutable_file());
23332401
}
23342402

@@ -2410,10 +2478,11 @@ bool CommandLineInterface::GeneratePluginOutput(
24102478
absl::flat_hash_set<const FileDescriptor*> already_seen;
24112479
for (int i = 0; i < parsed_files.size(); i++) {
24122480
request.add_file_to_generate(parsed_files[i]->name());
2413-
GetTransitiveDependencies(parsed_files[i],
2414-
true, // Include json_name for plugins.
2415-
true, // Include source code info.
2416-
&already_seen, request.mutable_proto_file());
2481+
GetTransitiveDependencies(parsed_files[i], &already_seen,
2482+
request.mutable_proto_file(),
2483+
{/*.include_json_name =*/true,
2484+
/*.include_source_code_info =*/true,
2485+
/*.retain_options =*/true});
24172486
}
24182487

24192488
google::protobuf::compiler::Version* version =
@@ -2575,11 +2644,13 @@ bool CommandLineInterface::WriteDescriptorSet(
25752644
}
25762645
}
25772646
}
2647+
TransitiveDependencyOptions options;
2648+
options.include_json_name = true;
2649+
options.include_source_code_info = source_info_in_descriptor_set_;
2650+
options.retain_options = retain_options_in_descriptor_set_;
25782651
for (int i = 0; i < parsed_files.size(); i++) {
2579-
GetTransitiveDependencies(parsed_files[i],
2580-
true, // Include json_name
2581-
source_info_in_descriptor_set_, &already_seen,
2582-
file_set.mutable_file());
2652+
GetTransitiveDependencies(parsed_files[i], &already_seen,
2653+
file_set.mutable_file(), options);
25832654
}
25842655

25852656
int fd;
@@ -2617,31 +2688,6 @@ bool CommandLineInterface::WriteDescriptorSet(
26172688
return true;
26182689
}
26192690

2620-
void CommandLineInterface::GetTransitiveDependencies(
2621-
const FileDescriptor* file, bool include_json_name,
2622-
bool include_source_code_info,
2623-
absl::flat_hash_set<const FileDescriptor*>* already_seen,
2624-
RepeatedPtrField<FileDescriptorProto>* output) {
2625-
if (!already_seen->insert(file).second) {
2626-
// Already saw this file. Skip.
2627-
return;
2628-
}
2629-
2630-
// Add all dependencies.
2631-
for (int i = 0; i < file->dependency_count(); i++) {
2632-
GetTransitiveDependencies(file->dependency(i), include_json_name,
2633-
include_source_code_info, already_seen, output);
2634-
}
2635-
2636-
// Add this file.
2637-
FileDescriptorProto* new_descriptor = output->Add();
2638-
*new_descriptor =
2639-
StripSourceRetentionOptions(*file, include_source_code_info);
2640-
if (include_json_name) {
2641-
file->CopyJsonNameTo(new_descriptor);
2642-
}
2643-
}
2644-
26452691
const CommandLineInterface::GeneratorInfo*
26462692
CommandLineInterface::FindGeneratorByFlag(const std::string& name) const {
26472693
auto it = generators_by_flag_name_.find(name);

src/google/protobuf/compiler/command_line_interface.h

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -309,21 +309,6 @@ class PROTOC_EXPORT CommandLineInterface {
309309
const GeneratorContextMap& output_directories,
310310
DiskSourceTree* source_tree);
311311

312-
// Get all transitive dependencies of the given file (including the file
313-
// itself), adding them to the given list of FileDescriptorProtos. The
314-
// protos will be ordered such that every file is listed before any file that
315-
// depends on it, so that you can call DescriptorPool::BuildFile() on them
316-
// in order. Any files in *already_seen will not be added, and each file
317-
// added will be inserted into *already_seen. If include_source_code_info is
318-
// true then include the source code information in the FileDescriptorProtos.
319-
// If include_json_name is true, populate the json_name field of
320-
// FieldDescriptorProto for all fields.
321-
static void GetTransitiveDependencies(
322-
const FileDescriptor* file, bool include_json_name,
323-
bool include_source_code_info,
324-
absl::flat_hash_set<const FileDescriptor*>* already_seen,
325-
RepeatedPtrField<FileDescriptorProto>* output);
326-
327312
// Implements the --print_free_field_numbers. This function prints free field
328313
// numbers into stdout for the message and it's nested message types in
329314
// post-order, i.e. nested types first. Printed range are left-right
@@ -452,6 +437,11 @@ class PROTOC_EXPORT CommandLineInterface {
452437
// SourceCodeInfo from the DescriptorSet.
453438
bool source_info_in_descriptor_set_ = false;
454439

440+
// True if --retain_options was given, meaning that we shouldn't strip any
441+
// options from the DescriptorSet, even if they have RETENTION_SOURCE
442+
// specified.
443+
bool retain_options_in_descriptor_set_ = false;
444+
455445
// Was the --disallow_services flag used?
456446
bool disallow_services_ = false;
457447

src/google/protobuf/compiler/command_line_interface_unittest.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1731,6 +1731,44 @@ TEST_F(CommandLineInterfaceTest, DescriptorSetOptionRetention) {
17311731
EXPECT_EQ(unknown_fields.field(0).varint(), 2);
17321732
}
17331733

1734+
TEST_F(CommandLineInterfaceTest, DescriptorSetOptionRetentionOverride) {
1735+
// clang-format off
1736+
CreateTempFile(
1737+
"foo.proto",
1738+
absl::Substitute(R"pb(
1739+
syntax = "proto2";
1740+
import "$0";
1741+
extend google.protobuf.FileOptions {
1742+
optional int32 runtime_retention_option = 50001
1743+
[retention = RETENTION_RUNTIME];
1744+
optional int32 source_retention_option = 50002
1745+
[retention = RETENTION_SOURCE];
1746+
}
1747+
option (runtime_retention_option) = 2;
1748+
option (source_retention_option) = 3;)pb",
1749+
DescriptorProto::descriptor()->file()->name()));
1750+
// clang-format on
1751+
std::string descriptor_proto_base_dir = "src";
1752+
Run(absl::Substitute(
1753+
"protocol_compiler --descriptor_set_out=$$tmpdir/descriptor_set "
1754+
"--proto_path=$$tmpdir --retain_options --proto_path=$0 foo.proto",
1755+
descriptor_proto_base_dir));
1756+
ExpectNoErrors();
1757+
1758+
FileDescriptorSet descriptor_set;
1759+
ReadDescriptorSet("descriptor_set", &descriptor_set);
1760+
ASSERT_EQ(descriptor_set.file_size(), 1);
1761+
const UnknownFieldSet& unknown_fields =
1762+
descriptor_set.file(0).options().unknown_fields();
1763+
1764+
// We expect all options to be present.
1765+
ASSERT_EQ(unknown_fields.field_count(), 2);
1766+
EXPECT_EQ(unknown_fields.field(0).number(), 50001);
1767+
EXPECT_EQ(unknown_fields.field(1).number(), 50002);
1768+
EXPECT_EQ(unknown_fields.field(0).varint(), 2);
1769+
EXPECT_EQ(unknown_fields.field(1).varint(), 3);
1770+
}
1771+
17341772
#ifdef _WIN32
17351773
// TODO(teboring): Figure out how to write test on windows.
17361774
#else

0 commit comments

Comments
 (0)