From 532c94145b6605361513682601f1d8e9f97a2497 Mon Sep 17 00:00:00 2001 From: Richard Geary Date: Fri, 23 Jan 2015 12:35:03 -0500 Subject: Add support for outputting dependency manifest files, used by ninja and make Use --manifest-file=somefile.d to output the dependency manifest. This file will contain a list of files which were read by protoc as part of creating the output files. It doesn't include the plugin inputs if plugins are used, that could be a later extension. The manifest file is in the format : . The manifest file format only allows you to specify one output file, which isn't a problem as it's used to detect input changes in order to detect when to rerun the protoc command. The output file used in the manifest is the manifest filename itself; to use this in ninja you should declare the manifest file as the first output as well as the depfile input. --- .../protobuf/compiler/command_line_interface.cc | 83 ++++++++++++++ .../protobuf/compiler/command_line_interface.h | 9 ++ test-driver | 127 +++++++++++++++++++++ 3 files changed, 219 insertions(+) create mode 100755 test-driver diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 13250702..649aa42d 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -46,6 +46,7 @@ #endif #include #include +#include #include #include @@ -725,6 +726,12 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } } + if (!manifest_name_.empty()) { + if (!GenerateDependencyManifestFile(parsed_files, &source_tree)) { + return 1; + } + } + if (mode_ == MODE_ENCODE || mode_ == MODE_DECODE) { if (codec_type_.empty()) { // HACK: Define an EmptyMessage type to use for decoding. @@ -775,6 +782,7 @@ void CommandLineInterface::Clear() { output_directives_.clear(); codec_type_.clear(); descriptor_set_name_.clear(); + manifest_name_.clear(); mode_ = MODE_COMPILE; print_mode_ = PRINT_NONE; @@ -1012,6 +1020,22 @@ CommandLineInterface::InterpretArgument(const string& name, } descriptor_set_name_ = value; + } else if (name == "--manifest-file") { + if (!manifest_name_.empty()) { + cerr << name << " may only be passed once." << endl; + return PARSE_ARGUMENT_FAIL; + } + if (value.empty()) { + cerr << name << " requires a non-empty value." << endl; + return PARSE_ARGUMENT_FAIL; + } + if (mode_ != MODE_COMPILE) { + cerr << "Cannot use --encode or --decode and generate a manifest at the " + "same time." << endl; + return PARSE_ARGUMENT_FAIL; + } + manifest_name_ = value; + } else if (name == "--include_imports") { if (imports_in_descriptor_set_) { cerr << name << " may only be passed once." << endl; @@ -1206,6 +1230,9 @@ void CommandLineInterface::PrintHelpText() { " include information about the original\n" " location of each decl in the source file as\n" " well as surrounding comments.\n" +" --manifest_file=FILE Write a dependency output file in the format\n" +" expected by make. This writes the transitive\n" +" set of input file paths to FILE\n" " --error_format=FORMAT Set the format in which to print errors.\n" " FORMAT may be 'gcc' (the default) or 'msvs'\n" " (Microsoft Visual Studio format).\n" @@ -1282,6 +1309,62 @@ bool CommandLineInterface::GenerateOutput( return true; } +bool CommandLineInterface::GenerateDependencyManifestFile( + const vector parsed_files, + DiskSourceTree * source_tree) { + FileDescriptorSet file_set; + + set already_seen; + for (int i = 0; i < parsed_files.size(); i++) { + GetTransitiveDependencies(parsed_files[i], + false, + &already_seen, file_set.mutable_file()); + } + + int fd; + do { + fd = open(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()); + return false; + } + + stringstream ss; + string output_filename = manifest_name_; + if (output_filename.compare(0, 2, "./") == 0) { + output_filename = output_filename.substr(2); + } + ss << output_filename << ": "; + for (set::const_iterator it = already_seen.begin(); it != already_seen.end(); ++it ) { + string virtual_file = (*it)->name(); + string disk_file; + if (source_tree && source_tree->VirtualFileToDiskFile(virtual_file, &disk_file) ) { + ss << " " << disk_file << " \\" << endl; + } 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 74a0adb4..59d57fbf 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -247,6 +247,11 @@ class LIBPROTOC_EXPORT CommandLineInterface { // Implements the --descriptor_set_out option. bool WriteDescriptorSet(const vector parsed_files); + // Implements the --manifest-file option + bool GenerateDependencyManifestFile( + const vector parsed_files, + DiskSourceTree * source_tree); + // Get all transitive dependencies of the given file (including the file // itself), adding them to the given list of FileDescriptorProtos. The // protos will be ordered such that every file is listed before any file that @@ -353,6 +358,10 @@ class LIBPROTOC_EXPORT CommandLineInterface { // FileDescriptorSet should be written. Otherwise, empty. string descriptor_set_name_; + // If --manifest_file was given, this is the filename to which the input + // file should be written. Otherwise, empty. + string manifest_name_; + // True if --include_imports was given, meaning that we should // write all transitive dependencies to the DescriptorSet. Otherwise, only // the .proto files listed on the command-line are added. diff --git a/test-driver b/test-driver new file mode 100755 index 00000000..32bf39e8 --- /dev/null +++ b/test-driver @@ -0,0 +1,127 @@ +#! /bin/sh +# test-driver - basic testsuite driver script. + +scriptversion=2012-06-27.10; # UTC + +# Copyright (C) 2011-2013 Free Software Foundation, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2, or (at your option) +# any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# As a special exception to the GNU General Public License, if you +# distribute this file as part of a program that contains a +# configuration script generated by Autoconf, you may include it under +# the same distribution terms that you use for the rest of that program. + +# This file is maintained in Automake, please report +# bugs to or send patches to +# . + +# Make unconditional expansion of undefined variables an error. This +# helps a lot in preventing typo-related bugs. +set -u + +usage_error () +{ + echo "$0: $*" >&2 + print_usage >&2 + exit 2 +} + +print_usage () +{ + cat <$log_file 2>&1 +estatus=$? +if test $enable_hard_errors = no && test $estatus -eq 99; then + estatus=1 +fi + +case $estatus:$expect_failure in + 0:yes) col=$red res=XPASS recheck=yes gcopy=yes;; + 0:*) col=$grn res=PASS recheck=no gcopy=no;; + 77:*) col=$blu res=SKIP recheck=no gcopy=yes;; + 99:*) col=$mgn res=ERROR recheck=yes gcopy=yes;; + *:yes) col=$lgn res=XFAIL recheck=no gcopy=yes;; + *:*) col=$red res=FAIL recheck=yes gcopy=yes;; +esac + +# Report outcome to console. +echo "${col}${res}${std}: $test_name" + +# Register the test result, and other relevant metadata. +echo ":test-result: $res" > $trs_file +echo ":global-test-result: $res" >> $trs_file +echo ":recheck: $recheck" >> $trs_file +echo ":copy-in-global-log: $gcopy" >> $trs_file + +# Local Variables: +# mode: shell-script +# sh-indentation: 2 +# eval: (add-hook 'write-file-hooks 'time-stamp) +# time-stamp-start: "scriptversion=" +# time-stamp-format: "%:y-%02m-%02d.%02H" +# time-stamp-time-zone: "UTC" +# time-stamp-end: "; # UTC" +# End: -- cgit v1.2.3 From 5914ce7a160a82db1490e99c45c95c69417b20ea Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 3 Feb 2015 21:35:50 -0800 Subject: 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 --- .../protobuf/compiler/command_line_interface.cc | 55 +++++++++--------- .../protobuf/compiler/command_line_interface.h | 2 +- .../compiler/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 649aa42d..1a182f73 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 59d57fbf..1b657f5e 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 dbaaa405..e7cfff6b 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 5344ec15..3d07b127 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 d2aeabf2..2f63f80e 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 32bf39e8..d3060566 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. -- cgit v1.2.3 From 5fa3956edef4648fa14da81958330b6df59cdb6d Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Tue, 3 Feb 2015 21:51:29 -0800 Subject: Fix comments. --- src/google/protobuf/compiler/command_line_interface.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 1b657f5e..0c951a71 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -358,8 +358,8 @@ class LIBPROTOC_EXPORT CommandLineInterface { // FileDescriptorSet should be written. Otherwise, empty. string descriptor_set_name_; - // If --manifest_file was given, this is the filename to which the input - // file should be written. Otherwise, empty. + // If --dependency_manifest_out was given, this is the filename to which the + // input file should be written. Otherwise, empty. string dependency_manifest_name_; // True if --include_imports was given, meaning that we should -- cgit v1.2.3 From a1b351c55bee819fc69861b3d0d88dab7dce0c4f Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 4 Feb 2015 10:20:20 -0800 Subject: Change flag for generating dependency file to "dependency_out". Delete test-driver --- .../protobuf/compiler/command_line_interface.cc | 4 +- .../protobuf/compiler/command_line_interface.h | 4 +- .../compiler/command_line_interface_unittest.cc | 15 +-- test-driver | 139 --------------------- 4 files changed, 12 insertions(+), 150 deletions(-) delete mode 100755 test-driver diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 1a182f73..4b28cf6b 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -1020,7 +1020,7 @@ CommandLineInterface::InterpretArgument(const string& name, } descriptor_set_name_ = value; - } else if (name == "--dependency_manifest_out") { + } else if (name == "--dependency_out") { if (!dependency_manifest_name_.empty()) { cerr << name << " may only be passed once." << endl; return PARSE_ARGUMENT_FAIL; @@ -1230,7 +1230,7 @@ void CommandLineInterface::PrintHelpText() { " include information about the original\n" " location of each decl in the source file as\n" " well as surrounding comments.\n" -" --manifest_file=FILE Write a dependency output file in the format\n" +" --dependency_out=FILE Write a dependency output file in the format\n" " expected by make. This writes the transitive\n" " set of input file paths to FILE\n" " --error_format=FORMAT Set the format in which to print errors.\n" diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 0c951a71..13e93c54 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -358,8 +358,8 @@ class LIBPROTOC_EXPORT CommandLineInterface { // FileDescriptorSet should be written. Otherwise, empty. string descriptor_set_name_; - // If --dependency_manifest_out was given, this is the filename to which the - // input file should be written. Otherwise, empty. + // If --dependency_out was given, this is the filename to which the input file + // should be written. Otherwise, empty. string dependency_manifest_name_; // True if --include_imports was given, meaning that we should diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index e7cfff6b..f298be75 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -112,12 +112,13 @@ class CommandLineInterfaceTest : public testing::Test { // Create a subdirectory within temp_directory_. void CreateTempDir(const string& name); - void SetInputsAreProtoPathRelative(bool enable) { - cli_.SetInputsAreProtoPathRelative(enable); + // Change working directory to temp directory. + void SwitchToTempDirectory() { + File::ChangeWorkingDirectory(temp_directory_); } - string GetTempDirectory() { - return temp_directory_; + void SetInputsAreProtoPathRelative(bool enable) { + cli_.SetInputsAreProtoPathRelative(enable); } // ----------------------------------------------------------------- @@ -969,7 +970,7 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) { " optional Foo foo = 1;\n" "}\n"); - Run("protocol_compiler --dependency_manifest_out=$tmpdir/manifest " + Run("protocol_compiler --dependency_out=$tmpdir/manifest " "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); ExpectNoErrors(); @@ -991,9 +992,9 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { "}\n"); string current_working_directory = get_current_dir_name(); - File::ChangeWorkingDirectory(GetTempDirectory()); + SwitchToTempDirectory(); - Run("protocol_compiler --dependency_manifest_out=manifest " + Run("protocol_compiler --dependency_out=manifest " "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); ExpectNoErrors(); diff --git a/test-driver b/test-driver deleted file mode 100755 index d3060566..00000000 --- a/test-driver +++ /dev/null @@ -1,139 +0,0 @@ -#! /bin/sh -# test-driver - basic testsuite driver script. - -scriptversion=2013-07-13.22; # UTC - -# Copyright (C) 2011-2013 Free Software Foundation, Inc. -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 2, or (at your option) -# any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -# As a special exception to the GNU General Public License, if you -# distribute this file as part of a program that contains a -# configuration script generated by Autoconf, you may include it under -# the same distribution terms that you use for the rest of that program. - -# This file is maintained in Automake, please report -# bugs to or send patches to -# . - -# Make unconditional expansion of undefined variables an error. This -# helps a lot in preventing typo-related bugs. -set -u - -usage_error () -{ - echo "$0: $*" >&2 - print_usage >&2 - exit 2 -} - -print_usage () -{ - cat <$log_file 2>&1 -estatus=$? -if test $enable_hard_errors = no && test $estatus -eq 99; then - estatus=1 -fi - -case $estatus:$expect_failure in - 0:yes) col=$red res=XPASS recheck=yes gcopy=yes;; - 0:*) col=$grn res=PASS recheck=no gcopy=no;; - 77:*) col=$blu res=SKIP recheck=no gcopy=yes;; - 99:*) col=$mgn res=ERROR recheck=yes gcopy=yes;; - *:yes) col=$lgn res=XFAIL recheck=no gcopy=yes;; - *:*) col=$red res=FAIL recheck=yes gcopy=yes;; -esac - -# Report outcome to console. -echo "${col}${res}${std}: $test_name" - -# Register the test result, and other relevant metadata. -echo ":test-result: $res" > $trs_file -echo ":global-test-result: $res" >> $trs_file -echo ":recheck: $recheck" >> $trs_file -echo ":copy-in-global-log: $gcopy" >> $trs_file - -# Local Variables: -# mode: shell-script -# sh-indentation: 2 -# eval: (add-hook 'write-file-hooks 'time-stamp) -# time-stamp-start: "scriptversion=" -# time-stamp-format: "%:y-%02m-%02d.%02H" -# time-stamp-time-zone: "UTC" -# time-stamp-end: "; # UTC" -# End: -- cgit v1.2.3 From 2e32b8b569f316acc405a36dfad2c742438cb41f Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 4 Feb 2015 10:27:52 -0800 Subject: Remove "include sstream" from command_line_interface.cc --- src/google/protobuf/compiler/command_line_interface.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 4b28cf6b..06aedd22 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -46,7 +46,6 @@ #endif #include #include -#include #include #include -- cgit v1.2.3 From 3edcbaf57a3dc3bc60a9c8e80e9edac813d86b1d Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 4 Feb 2015 13:42:16 -0800 Subject: Clean code --- .../protobuf/compiler/command_line_interface.cc | 32 +++++++++++----------- .../protobuf/compiler/command_line_interface.h | 8 +++--- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 06aedd22..c8e60bc4 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -725,7 +725,7 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } } - if (!dependency_manifest_name_.empty()) { + if (!dependency_out_name_.empty()) { if (!GenerateDependencyManifestFile(parsed_files, &source_tree)) { return 1; } @@ -781,7 +781,7 @@ void CommandLineInterface::Clear() { output_directives_.clear(); codec_type_.clear(); descriptor_set_name_.clear(); - dependency_manifest_name_.clear(); + dependency_out_name_.clear(); mode_ = MODE_COMPILE; print_mode_ = PRINT_NONE; @@ -1020,7 +1020,7 @@ CommandLineInterface::InterpretArgument(const string& name, descriptor_set_name_ = value; } else if (name == "--dependency_out") { - if (!dependency_manifest_name_.empty()) { + if (!dependency_out_name_.empty()) { cerr << name << " may only be passed once." << endl; return PARSE_ARGUMENT_FAIL; } @@ -1029,11 +1029,11 @@ CommandLineInterface::InterpretArgument(const string& name, return PARSE_ARGUMENT_FAIL; } if (mode_ != MODE_COMPILE) { - cerr << "Cannot use --encode or --decode and generate a manifest at the " - "same time." << endl; + cerr << "Cannot use --encode or --decode and --dependency_out=FILE at " + "the same time." << endl; return PARSE_ARGUMENT_FAIL; } - dependency_manifest_name_ = value; + dependency_out_name_ = value; } else if (name == "--include_imports") { if (imports_in_descriptor_set_) { @@ -1309,44 +1309,44 @@ bool CommandLineInterface::GenerateOutput( } bool CommandLineInterface::GenerateDependencyManifestFile( - const vector parsed_files, - DiskSourceTree * source_tree) { + const vector& parsed_files, + DiskSourceTree* source_tree) { FileDescriptorSet file_set; set already_seen; for (int i = 0; i < parsed_files.size(); i++) { GetTransitiveDependencies(parsed_files[i], false, - &already_seen, file_set.mutable_file()); + &already_seen, + file_set.mutable_file()); } int fd; do { - fd = open(dependency_manifest_name_.c_str(), + fd = open(dependency_out_name_.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666); } while (fd < 0 && errno == EINTR); if (fd < 0) { - perror(dependency_manifest_name_.c_str()); + perror(dependency_out_name_.c_str()); return false; } io::FileOutputStream out(fd); io::Printer printer(&out, '$'); - string output_filename = dependency_manifest_name_; - if (output_filename.compare(0, 1, "/") != 0) { + if (dependency_out_name_.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); + "output_filename", dependency_out_name_); } else { printer.Print("$output_filename$:", - "output_filename",output_filename); + "output_filename", dependency_out_name_); } for (int i = 0; i < file_set.file_size(); i++) { const FileDescriptorProto& file = file_set.file(i); - string virtual_file = file.name(); + const string& virtual_file = file.name(); string disk_file; if (source_tree && source_tree->VirtualFileToDiskFile(virtual_file, &disk_file)) { diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 13e93c54..92c578c8 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -247,10 +247,10 @@ class LIBPROTOC_EXPORT CommandLineInterface { // Implements the --descriptor_set_out option. bool WriteDescriptorSet(const vector parsed_files); - // Implements the --manifest-file option + // Implements the --dependency_out option bool GenerateDependencyManifestFile( - const vector parsed_files, - DiskSourceTree * source_tree); + const vector& parsed_files, + DiskSourceTree* source_tree); // Get all transitive dependencies of the given file (including the file // itself), adding them to the given list of FileDescriptorProtos. The @@ -360,7 +360,7 @@ class LIBPROTOC_EXPORT CommandLineInterface { // If --dependency_out was given, this is the filename to which the input file // should be written. Otherwise, empty. - string dependency_manifest_name_; + string dependency_out_name_; // True if --include_imports was given, meaning that we should // write all transitive dependencies to the DescriptorSet. Otherwise, only -- cgit v1.2.3 From 1d627f85c3aef00b24a6a19e5bc2a46ef889f16f Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Wed, 4 Feb 2015 15:48:06 -0800 Subject: Fix comment --- src/google/protobuf/compiler/command_line_interface.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 92c578c8..1df53c4a 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -358,8 +358,8 @@ class LIBPROTOC_EXPORT CommandLineInterface { // FileDescriptorSet should be written. Otherwise, empty. string descriptor_set_name_; - // If --dependency_out was given, this is the filename to which the input file - // should be written. Otherwise, empty. + // If --dependency_out was given, this is the path to the file where the + // dependency file will be written. Otherwise, empty. string dependency_out_name_; // True if --include_imports was given, meaning that we should -- cgit v1.2.3 From e2555e235f867f3d7a0378e95a45109c8fd2dfbe Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Sat, 7 Feb 2015 15:28:54 -0800 Subject: Change target to output filenames --- .../protobuf/compiler/command_line_interface.cc | 69 +++++++++++++++------- .../protobuf/compiler/command_line_interface.h | 3 + .../compiler/command_line_interface_unittest.cc | 26 ++++++-- 3 files changed, 73 insertions(+), 25 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index c8e60bc4..ddd58e2d 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -48,7 +48,6 @@ #include #include -#include #include #ifndef _SHARED_PTR_H #include @@ -254,6 +253,9 @@ class CommandLineInterface::GeneratorContextImpl : public GeneratorContext { // format, unless one has already been written. void AddJarManifest(); + // Get name of all output files. + void GetOutputFilenames(vector* output_filenames); + // implements GeneratorContext -------------------------------------- io::ZeroCopyOutputStream* Open(const string& filename); io::ZeroCopyOutputStream* OpenForAppend(const string& filename); @@ -441,6 +443,14 @@ void CommandLineInterface::GeneratorContextImpl::AddJarManifest() { } } +void CommandLineInterface::GeneratorContextImpl::GetOutputFilenames( + vector* output_filenames) { + for (map::iterator iter = files_.begin(); + iter != files_.end(); ++iter) { + output_filenames->push_back(iter->first); + } +} + io::ZeroCopyOutputStream* CommandLineInterface::GeneratorContextImpl::Open( const string& filename) { return new MemoryOutputStream(this, filename, false); @@ -670,7 +680,6 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { // We construct a separate GeneratorContext for each output location. Note // that two code generators may output to the same location, in which case // they should share a single GeneratorContext so that OpenForInsert() works. - typedef hash_map GeneratorContextMap; GeneratorContextMap output_directories; // Generate output. @@ -717,16 +726,17 @@ int CommandLineInterface::Run(int argc, const char* const argv[]) { } } - STLDeleteValues(&output_directories); - - if (!descriptor_set_name_.empty()) { - if (!WriteDescriptorSet(parsed_files)) { + if (!dependency_out_name_.empty()) { + if (!GenerateDependencyManifestFile(parsed_files, output_directories, + &source_tree)) { return 1; } } - if (!dependency_out_name_.empty()) { - if (!GenerateDependencyManifestFile(parsed_files, &source_tree)) { + STLDeleteValues(&output_directories); + + if (!descriptor_set_name_.empty()) { + if (!WriteDescriptorSet(parsed_files)) { return 1; } } @@ -878,6 +888,15 @@ CommandLineInterface::ParseArguments(int argc, const char* const argv[]) { cerr << "Missing output directives." << endl; return PARSE_ARGUMENT_FAIL; } + if (mode_ != MODE_COMPILE && !dependency_out_name_.empty()) { + cerr << "Can only use --dependency_out=FILE when generating code." << endl; + return PARSE_ARGUMENT_FAIL; + } + if (!dependency_out_name_.empty() && input_files_.size() > 1) { + cerr << "Can only process one input file when using --dependency_out=FILE." + << endl; + return PARSE_ARGUMENT_FAIL; + } if (imports_in_descriptor_set_ && descriptor_set_name_.empty()) { cerr << "--include_imports only makes sense when combined with " "--descriptor_set_out." << endl; @@ -1028,11 +1047,6 @@ CommandLineInterface::InterpretArgument(const string& name, cerr << name << " requires a non-empty value." << endl; return PARSE_ARGUMENT_FAIL; } - if (mode_ != MODE_COMPILE) { - cerr << "Cannot use --encode or --decode and --dependency_out=FILE at " - "the same time." << endl; - return PARSE_ARGUMENT_FAIL; - } dependency_out_name_ = value; } else if (name == "--include_imports") { @@ -1310,6 +1324,7 @@ bool CommandLineInterface::GenerateOutput( bool CommandLineInterface::GenerateDependencyManifestFile( const vector& parsed_files, + const GeneratorContextMap& output_directories, DiskSourceTree* source_tree) { FileDescriptorSet file_set; @@ -1321,6 +1336,18 @@ bool CommandLineInterface::GenerateDependencyManifestFile( file_set.mutable_file()); } + vector output_filenames; + for (GeneratorContextMap::const_iterator iter = output_directories.begin(); + iter != output_directories.end(); ++iter) { + const string& location = iter->first; + GeneratorContextImpl* directory = iter->second; + vector relative_output_filenames; + directory->GetOutputFilenames(&relative_output_filenames); + for (int i = 0; i < relative_output_filenames.size(); i++) { + output_filenames.push_back(location + relative_output_filenames[i]); + } + } + int fd; do { fd = open(dependency_out_name_.c_str(), @@ -1335,15 +1362,15 @@ bool CommandLineInterface::GenerateDependencyManifestFile( io::FileOutputStream out(fd); io::Printer printer(&out, '$'); - if (dependency_out_name_.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", dependency_out_name_); - } else { - printer.Print("$output_filename$:", - "output_filename", dependency_out_name_); + for (int i = 0; i < output_filenames.size(); i++) { + printer.Print(output_filenames[i].c_str()); + if (i == output_filenames.size() - 1) { + printer.Print(":"); + } else { + printer.Print(" \\\n"); + } } + for (int i = 0; i < file_set.file_size(); i++) { const FileDescriptorProto& file = file_set.file(i); const string& virtual_file = file.name(); diff --git a/src/google/protobuf/compiler/command_line_interface.h b/src/google/protobuf/compiler/command_line_interface.h index 1df53c4a..7e611c44 100644 --- a/src/google/protobuf/compiler/command_line_interface.h +++ b/src/google/protobuf/compiler/command_line_interface.h @@ -39,6 +39,7 @@ #define GOOGLE_PROTOBUF_COMPILER_COMMAND_LINE_INTERFACE_H__ #include +#include #include #include #include @@ -190,6 +191,7 @@ class LIBPROTOC_EXPORT CommandLineInterface { class ErrorPrinter; class GeneratorContextImpl; class MemoryOutputStream; + typedef hash_map GeneratorContextMap; // Clear state from previous Run(). void Clear(); @@ -250,6 +252,7 @@ class LIBPROTOC_EXPORT CommandLineInterface { // Implements the --dependency_out option bool GenerateDependencyManifestFile( const vector& parsed_files, + const GeneratorContextMap& output_directories, DiskSourceTree* source_tree); // Get all transitive dependencies of the given file (including the file diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index f298be75..9e02f729 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -959,6 +959,24 @@ TEST_F(CommandLineInterfaceTest, WriteTransitiveDescriptorSetWithSourceInfo) { EXPECT_TRUE(descriptor_set.file(1).has_source_code_info()); } +TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileGivenTwoInputs) { + 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 " + "--test_out=$tmpdir --proto_path=$tmpdir bar.proto foo.proto"); + + ExpectErrorText( + "Can only process one input file when using --dependency_out=FILE.\n"); +} + TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) { CreateTempFile("foo.proto", "syntax = \"proto2\";\n" @@ -976,8 +994,8 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) { ExpectNoErrors(); ExpectFileContent("manifest", - "$tmpdir/manifest: $tmpdir/foo.proto\\\n" - " $tmpdir/bar.proto"); + "$tmpdir/bar.proto.MockCodeGenerator.test_generator: " + "$tmpdir/foo.proto\\\n $tmpdir/bar.proto"); } TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { @@ -1000,8 +1018,8 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { ExpectNoErrors(); ExpectFileContent("manifest", - "$tmpdir/manifest: $tmpdir/foo.proto\\\n" - " $tmpdir/bar.proto"); + "$tmpdir/bar.proto.MockCodeGenerator.test_generator: " + "$tmpdir/foo.proto\\\n $tmpdir/bar.proto"); File::ChangeWorkingDirectory(current_working_directory); } -- cgit v1.2.3 From eb2ce0293138532680fdd3647c01db7587b9367b Mon Sep 17 00:00:00 2001 From: Bo Yang Date: Mon, 9 Feb 2015 11:57:41 -0800 Subject: Fix absolute/relative path in output --- .../protobuf/compiler/command_line_interface.cc | 6 +++++- .../compiler/command_line_interface_unittest.cc | 22 +++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index ddd58e2d..4095b3c9 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -1344,7 +1344,11 @@ bool CommandLineInterface::GenerateDependencyManifestFile( vector relative_output_filenames; directory->GetOutputFilenames(&relative_output_filenames); for (int i = 0; i < relative_output_filenames.size(); i++) { - output_filenames.push_back(location + relative_output_filenames[i]); + string output_filename = location + relative_output_filenames[i]; + if (output_filename.compare(0, 2, "./") == 0) { + output_filename = output_filename.substr(2); + } + output_filenames.push_back(output_filename); } } diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index 9e02f729..2aba68b9 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -988,17 +988,22 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFile) { " optional Foo foo = 1;\n" "}\n"); - Run("protocol_compiler --dependency_out=$tmpdir/manifest " - "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); + string current_working_directory = get_current_dir_name(); + SwitchToTempDirectory(); + + Run("protocol_compiler --dependency_out=manifest --test_out=. " + "bar.proto"); ExpectNoErrors(); ExpectFileContent("manifest", - "$tmpdir/bar.proto.MockCodeGenerator.test_generator: " - "$tmpdir/foo.proto\\\n $tmpdir/bar.proto"); + "bar.proto.MockCodeGenerator.test_generator: " + "foo.proto\\\n bar.proto"); + + File::ChangeWorkingDirectory(current_working_directory); } -TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { +TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForAbsolutePath) { CreateTempFile("foo.proto", "syntax = \"proto2\";\n" "message Foo {}\n"); @@ -1009,10 +1014,7 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { " optional Foo foo = 1;\n" "}\n"); - string current_working_directory = get_current_dir_name(); - SwitchToTempDirectory(); - - Run("protocol_compiler --dependency_out=manifest " + Run("protocol_compiler --dependency_out=$tmpdir/manifest " "--test_out=$tmpdir --proto_path=$tmpdir bar.proto"); ExpectNoErrors(); @@ -1020,8 +1022,6 @@ TEST_F(CommandLineInterfaceTest, WriteDependencyManifestFileForRelativePath) { ExpectFileContent("manifest", "$tmpdir/bar.proto.MockCodeGenerator.test_generator: " "$tmpdir/foo.proto\\\n $tmpdir/bar.proto"); - - File::ChangeWorkingDirectory(current_working_directory); } // ------------------------------------------------------------------- -- cgit v1.2.3