Skip to content

Commit

Permalink
Sort dependency manifest (#10414)
Browse files Browse the repository at this point in the history
* Sort the Dependency Manifest file

* Add a test

* split expected content to multiple lines
  • Loading branch information
TrentHouliston authored Aug 17, 2022
1 parent 3919d23 commit 735221f
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/google/protobuf/compiler/command_line_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2215,6 +2215,12 @@ bool CommandLineInterface::GenerateDependencyManifestFile(
output_filenames.push_back(descriptor_set_out_name_);
}

// Some consumers consider first output as the "primary output" (ninja)
// If this doesn't match their primary output it ignores the manifest.
// The unordered map for output_directories means we can't guarantee the order of files.
// Sorting here makes the "primary output" can be whichever is first alphabetically
std::sort(output_filenames.begin(), output_filenames.end());

int fd;
do {
fd = open(dependency_out_name_.c_str(),
Expand Down
35 changes: 35 additions & 0 deletions src/google/protobuf/compiler/command_line_interface_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,41 @@ TEST_F(CommandLineInterfaceTest,
"$tmpdir/bar.pb: "
"$tmpdir/foo.proto\\\n $tmpdir/bar.proto");
}

TEST_F(CommandLineInterfaceTest,
DependencyManifestFileIsOrderedByFilePath) {
CreateTempFile("foo.proto",
"syntax = \"proto2\";\n"
"message Foo {}\n");
CreateTempFile("bar.proto",
"syntax = \"proto2\";\n"
"import \"foo.proto\";\n"
"message Bar {\n"
" optional Foo foo = 1;\n"
"}\n");

Run("protocol_compiler --dependency_out=$tmpdir/manifest_a "
"--test_out=$tmpdir "
"--descriptor_set_out=$tmpdir/a.pb --proto_path=$tmpdir bar.proto");

ExpectNoErrors();

ExpectFileContent("manifest_a",
"$tmpdir/a.pb \\\n"
"$tmpdir/bar.proto.MockCodeGenerator.test_generator: "
"$tmpdir/foo.proto\\\n $tmpdir/bar.proto");

Run("protocol_compiler --dependency_out=$tmpdir/manifest_z "
"--test_out=$tmpdir "
"--descriptor_set_out=$tmpdir/z.pb --proto_path=$tmpdir bar.proto");

ExpectNoErrors();

ExpectFileContent("manifest_z",
"$tmpdir/bar.proto.MockCodeGenerator.test_generator \\\n"
"$tmpdir/z.pb: "
"$tmpdir/foo.proto\\\n $tmpdir/bar.proto");
}
#endif // !_WIN32

TEST_F(CommandLineInterfaceTest, TestArgumentFile) {
Expand Down

0 comments on commit 735221f

Please sign in to comment.