From 5914ce7a160a82db1490e99c45c95c69417b20ea Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 3 Feb 2015 21:35:50 -0800 Subject: [PATCH] Implement a feature to generate a dependency file. By giving protoc the flag "--dependency_manifest_out=FILE", protoc will write dependencies of input proto files into FILE. In FILE, the format will be : \\\n ... This cl is based on https://github.com/google/protobuf/pull/178 --- .../compiler/command_line_interface.cc | 55 ++++++++-------- .../compiler/command_line_interface.h | 2 +- .../command_line_interface_unittest.cc | 65 +++++++++++++++++++ src/google/protobuf/testing/file.cc | 4 ++ src/google/protobuf/testing/file.h | 3 + test-driver | 20 ++++-- 6 files changed, 115 insertions(+), 34 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 649aa42d77..1a182f7356 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -726,7 +726,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } } - if (!manifest_name_.empty()) { + if (!dependency_manifest_name_.empty()) { if (!GenerateDependencyManifestFile(parsed_files, &source_tree)) { return 1; } @@ -782,7 +782,7 @@ void CommandLineInterface::Clear() { output_directives_.clear(); codec_type_.clear(); descriptor_set_name_.clear(); - manifest_name_.clear(); + dependency_manifest_name_.clear(); mode_ = MODE_COMPILE; print_mode_ = PRINT_NONE; @@ -1020,8 +1020,8 @@ CommandLineInterface::InterpretArgument(const string& name, } descriptor_set_name_ = value; - } else if (name == "--manifest-file") { - if (!manifest_name_.empty()) { + } else if (name == "--dependency_manifest_out") { + if (!dependency_manifest_name_.empty()) { cerr << name << " may only be passed once." << endl; return PARSE_ARGUMENT_FAIL; } @@ -1034,7 +1034,7 @@ CommandLineInterface::InterpretArgument(const string& name, "same time." << endl; return PARSE_ARGUMENT_FAIL; } - manifest_name_ = value; + dependency_manifest_name_ = value; } else if (name == "--include_imports") { if (imports_in_descriptor_set_) { @@ -1323,48 +1323,45 @@ bool CommandLineInterface::GenerateDependencyManifestFile( int fd; do { - fd = open(manifest_name_.c_str(), + fd = open(dependency_manifest_name_.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666); } while (fd < 0 && errno == EINTR); if (fd < 0) { - perror(manifest_name_.c_str()); + perror(dependency_manifest_name_.c_str()); return false; } - stringstream ss; - string output_filename = manifest_name_; - if (output_filename.compare(0, 2, "./") == 0) { - output_filename = output_filename.substr(2); + io::FileOutputStream out(fd); + io::Printer printer(&out, '$'); + + string output_filename = dependency_manifest_name_; + if (output_filename.compare(0, 1, "/") != 0) { + // Convert relative path to absolute path before print. + printer.Print("$working_directory$/$output_filename$:", + "working_directory", get_current_dir_name(), + "output_filename",output_filename); + } else { + printer.Print("$output_filename$:", + "output_filename",output_filename); } - ss << output_filename << ": "; - for (set::const_iterator it = already_seen.begin(); it != already_seen.end(); ++it ) { - string virtual_file = (*it)->name(); + for (int i = 0; i < file_set.file_size(); i++) { + const FileDescriptorProto& file = file_set.file(i); + string virtual_file = file.name(); string disk_file; - if (source_tree && source_tree->VirtualFileToDiskFile(virtual_file, &disk_file) ) { - ss << " " << disk_file << " \\" << endl; + if (source_tree && + source_tree->VirtualFileToDiskFile(virtual_file, &disk_file)) { + printer.Print(" $disk_file$", "disk_file", disk_file); + if (i < file_set.file_size() - 1) printer.Print("\\\n"); } else { cerr << "Unable to identify path for file " << virtual_file << endl; return false; } } - - string manifest_contents = ss.str(); - if ( write(fd, manifest_contents.c_str(), manifest_contents.size()) != manifest_contents.size() ) { - cerr << "Error when writing to " << manifest_name_ << endl; - return false; - } - - int rv = ::close(fd); - if ( rv != 0 ) { - cerr << manifest_name_ << ": " << strerror(rv) << endl; - return false; - } return true; } - bool CommandLineInterface::GeneratePluginOutput( const vector& parsed_files, const string& plugin_name, diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 59d57fbf49..1b657f5edb 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -360,7 +360,7 @@ class LIBPROTOC_EXPORT CommandLineInterface { // If --manifest_file was given, this is the filename to which the input // file should be written. Otherwise, empty. - string manifest_name_; + string dependency_manifest_name_; // True if --include_imports was given, meaning that we should // write all transitive dependencies to the DescriptorSet. Otherwise, only diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index dbaaa40509..e7cfff6bef 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -116,6 +116,10 @@ class CommandLineInterfaceTest : public testing::Test { cli_.SetInputsAreProtoPathRelative(enable); } + string GetTempDirectory() { + return temp_directory_; + } + // ----------------------------------------------------------------- // Methods to check the test results (called after Run()). @@ -176,6 +180,9 @@ class CommandLineInterfaceTest : public testing::Test { void ReadDescriptorSet(const string& filename, FileDescriptorSet* descriptor_set); + void ExpectFileContent(const string& filename, + const string& content); + private: // The object we are testing. CommandLineInterface cli_; @@ -456,6 +463,17 @@ void CommandLineInterfaceTest::ExpectCapturedStdout( EXPECT_EQ(expected_text, captured_stdout_); } + +void CommandLineInterfaceTest::ExpectFileContent( + const string& filename, const string& content) { + string path = temp_directory_ + "/" + filename; + string file_contents; + GOOGLE_CHECK_OK(File::GetContents(path, &file_contents, true)); + + EXPECT_EQ(StringReplace(content, "$tmpdir", temp_directory_, true), + file_contents); +} + // =================================================================== TEST_F(CommandLineInterfaceTest, BasicOutput) { @@ -940,6 +958,53 @@ TEST_F(CommandLineInterfaceTest, WriteTransitiveDescriptorSetWithSourceInfo) { EXPECT_TRUE(descriptor_set.file(1).has_source_code_info()); } +TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) { + 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_manifest_out=$tmpdir/manifest " + "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); + + ExpectNoErrors(); + + ExpectFileContent("manifest", + "$tmpdir/manifest: $tmpdir/foo.proto\\\n" + " $tmpdir/bar.proto"); +} + +TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { + 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"); + + string current_working_directory = get_current_dir_name(); + File::ChangeWorkingDirectory(GetTempDirectory()); + + Run("protocol_compiler --dependency_manifest_out=manifest " + "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); + + ExpectNoErrors(); + + ExpectFileContent("manifest", + "$tmpdir/manifest: $tmpdir/foo.proto\\\n" + " $tmpdir/bar.proto"); + + File::ChangeWorkingDirectory(current_working_directory); +} + // ------------------------------------------------------------------- TEST_F(CommandLineInterfaceTest, ParseErrors) { diff --git a/src/google/protobuf/testing/file.cc b/src/google/protobuf/testing/file.cc index 5344ec15a9..3d07b1276c 100644 --- a/src/google/protobuf/testing/file.cc +++ b/src/google/protobuf/testing/file.cc @@ -192,5 +192,9 @@ void File::DeleteRecursively(const string& name, #endif } +bool File::ChangeWorkingDirectory(const string& new_working_directory) { + return chdir(new_working_directory.c_str()) == 0; +} + } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/testing/file.h b/src/google/protobuf/testing/file.h index d2aeabf2b7..2f63f80e7b 100644 --- a/src/google/protobuf/testing/file.h +++ b/src/google/protobuf/testing/file.h @@ -77,6 +77,9 @@ class File { static void DeleteRecursively(const string& name, void* dummy1, void* dummy2); + // Change working directory to given directory. + static bool ChangeWorkingDirectory(const string& new_working_directory); + static bool GetContents( const string& name, string* output, bool /*is_default*/) { return ReadFileToString(name, output); diff --git a/test-driver b/test-driver index 32bf39e837..d30605660a 100755 --- a/test-driver +++ b/test-driver @@ -1,7 +1,7 @@ #! /bin/sh # test-driver - basic testsuite driver script. -scriptversion=2012-06-27.10; # UTC +scriptversion=2013-07-13.22; # UTC # Copyright (C) 2011-2013 Free Software Foundation, Inc. # @@ -44,13 +44,12 @@ print_usage () Usage: test-driver --test-name=NAME --log-file=PATH --trs-file=PATH [--expect-failure={yes|no}] [--color-tests={yes|no}] - [--enable-hard-errors={yes|no}] [--] TEST-SCRIPT + [--enable-hard-errors={yes|no}] [--] + TEST-SCRIPT [TEST-SCRIPT-ARGUMENTS] The '--test-name', '--log-file' and '--trs-file' options are mandatory. END } -# TODO: better error handling in option parsing (in particular, ensure -# TODO: $log_file, $trs_file and $test_name are defined). test_name= # Used for reporting. log_file= # Where to save the output of the test script. trs_file= # Where to save the metadata of the test run. @@ -69,10 +68,23 @@ while test $# -gt 0; do --enable-hard-errors) enable_hard_errors=$2; shift;; --) shift; break;; -*) usage_error "invalid option: '$1'";; + *) break;; esac shift done +missing_opts= +test x"$test_name" = x && missing_opts="$missing_opts --test-name" +test x"$log_file" = x && missing_opts="$missing_opts --log-file" +test x"$trs_file" = x && missing_opts="$missing_opts --trs-file" +if test x"$missing_opts" != x; then + usage_error "the following mandatory options are missing:$missing_opts" +fi + +if test $# -eq 0; then + usage_error "missing argument" +fi + if test $color_tests = yes; then # Keep this in sync with 'lib/am/check.am:$(am__tty_colors)'. red='' # Red.