From f479f9a05de4a3681b0fe15e7575a7946852726f Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Tue, 16 Aug 2022 13:03:59 +1000 Subject: [PATCH 1/3] Sort the Dependency Manifest file --- src/google/protobuf/compiler/command_line_interface.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 652ce04495853..d8ffc57a3e8b9 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -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(), From ad1a0ea5e1768e5ec085421f7af65763b921a390 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Tue, 16 Aug 2022 13:56:14 +1000 Subject: [PATCH 2/3] Add a test --- .../command_line_interface_unittest.cc | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index b2cf3d5d10a73..3b544b0d83f3b 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -1779,6 +1779,39 @@ 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) { From 1bbe862bb722dbbec7a83d5c95bbf9e23eb7b993 Mon Sep 17 00:00:00 2001 From: Trent Houliston Date: Wed, 17 Aug 2022 11:19:03 +1000 Subject: [PATCH 3/3] split expected content to multiple lines --- .../protobuf/compiler/command_line_interface_unittest.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 3b544b0d83f3b..f82710fe9db1d 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -1799,7 +1799,8 @@ TEST_F(CommandLineInterfaceTest, ExpectNoErrors(); ExpectFileContent("manifest_a", - "$tmpdir/a.pb \\\n$tmpdir/bar.proto.MockCodeGenerator.test_generator: " + "$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 " @@ -1809,7 +1810,8 @@ TEST_F(CommandLineInterfaceTest, ExpectNoErrors(); ExpectFileContent("manifest_z", - "$tmpdir/bar.proto.MockCodeGenerator.test_generator \\\n$tmpdir/z.pb: " + "$tmpdir/bar.proto.MockCodeGenerator.test_generator \\\n" + "$tmpdir/z.pb: " "$tmpdir/foo.proto\\\n $tmpdir/bar.proto"); } #endif // !_WIN32