From 684d45b2feac114a0152cf4458846fbeca61b5d9 Mon Sep 17 00:00:00 2001 From: "kenton@google.com" Date: Sat, 19 Dec 2009 04:50:00 +0000 Subject: [PATCH] Fix build on MinGW/Win32 (including implementing Subprocess using CreateProcess()). --- .../compiler/command_line_interface.cc | 9 + .../command_line_interface_unittest.cc | 19 ++ src/google/protobuf/compiler/plugin.cc | 10 + src/google/protobuf/compiler/subprocess.cc | 232 +++++++++++++++++- src/google/protobuf/compiler/subprocess.h | 28 ++- src/google/protobuf/descriptor.cc | 25 +- src/google/protobuf/io/gzip_stream.cc | 2 +- .../protobuf/repeated_field_unittest.cc | 2 + 8 files changed, 318 insertions(+), 9 deletions(-) diff --git a/src/google/protobuf/compiler/command_line_interface.cc b/src/google/protobuf/compiler/command_line_interface.cc index 39bd370e48..a611fceef6 100644 --- a/src/google/protobuf/compiler/command_line_interface.cc +++ b/src/google/protobuf/compiler/command_line_interface.cc @@ -529,10 +529,19 @@ CommandLineInterface::InsertionOutputStream::~InsertionOutputStream() { // If everything was successful, overwrite the original file with the temp // file. if (!had_error) { +#ifdef _WIN32 + // rename() on Windows fails if the file exists. + if (!MoveFileEx(temp_filename_.c_str(), filename_.c_str(), + MOVEFILE_REPLACE_EXISTING)) { + cerr << filename_ << ": MoveFileEx: " + << Subprocess::Win32ErrorMessage(GetLastError()) << endl; + } +#else // _WIN32 if (rename(temp_filename_.c_str(), filename_.c_str()) < 0) { cerr << filename_ << ": rename: " << strerror(errno) << endl; had_error = true; } +#endif // !_WIN32 } if (had_error) { diff --git a/src/google/protobuf/compiler/command_line_interface_unittest.cc b/src/google/protobuf/compiler/command_line_interface_unittest.cc index b43f781c12..abe58af008 100644 --- a/src/google/protobuf/compiler/command_line_interface_unittest.cc +++ b/src/google/protobuf/compiler/command_line_interface_unittest.cc @@ -248,7 +248,11 @@ void CommandLineInterfaceTest::Run(const string& command) { if (!disallow_plugins_) { cli_.AllowPlugins("prefix-"); +#ifdef _WIN32 + args.push_back("--plugin=prefix-gen-plug=test_plugin.exe"); +#else args.push_back("--plugin=prefix-gen-plug=test_plugin"); +#endif } scoped_array argv(new const char*[args.size()]); @@ -1026,9 +1030,16 @@ TEST_F(CommandLineInterfaceTest, GeneratorPluginCrash) { ExpectErrorSubstring("Saw message type MockCodeGenerator_Abort."); +#ifdef _WIN32 + // Windows doesn't have signals. It looks like abort()ing causes the process + // to exit with status code 3, but let's not depend on the exact number here. + ExpectErrorSubstring( + "--plug_out: prefix-gen-plug: Plugin failed with status code"); +#else // Don't depend on the exact signal number. ExpectErrorSubstring( "--plug_out: prefix-gen-plug: Plugin killed by signal"); +#endif } TEST_F(CommandLineInterfaceTest, GeneratorPluginNotFound) { @@ -1042,11 +1053,19 @@ TEST_F(CommandLineInterfaceTest, GeneratorPluginNotFound) { "--plugin=prefix-gen-badplug=no_such_file " "--proto_path=$tmpdir error.proto"); +#ifdef _WIN32 + ExpectErrorSubstring( + "--badplug_out: prefix-gen-badplug: The system cannot find the file " + "specified."); +#else + // Error written to stdout by child process after exec() fails. ExpectErrorSubstring( "no_such_file: program not found or is not executable"); + // Error written by parent process when child fails. ExpectErrorSubstring( "--badplug_out: prefix-gen-badplug: Plugin failed with status code 1."); +#endif } TEST_F(CommandLineInterfaceTest, GeneratorPluginNotAllowed) { diff --git a/src/google/protobuf/compiler/plugin.cc b/src/google/protobuf/compiler/plugin.cc index b64a841396..4757652f2f 100644 --- a/src/google/protobuf/compiler/plugin.cc +++ b/src/google/protobuf/compiler/plugin.cc @@ -36,6 +36,11 @@ #include #include +#ifdef _WIN32 +#include +#include +#endif + #include #include #include @@ -80,6 +85,11 @@ int PluginMain(int argc, char* argv[], const CodeGenerator* generator) { return 1; } +#ifdef _WIN32 + _setmode(STDIN_FILENO, _O_BINARY); + _setmode(STDOUT_FILENO, _O_BINARY); +#endif + CodeGeneratorRequest request; if (!request.ParseFromFileDescriptor(STDIN_FILENO)) { cerr << argv[0] << ": protoc sent unparseable request to plugin." << endl; diff --git a/src/google/protobuf/compiler/subprocess.cc b/src/google/protobuf/compiler/subprocess.cc index 33f5bd6b6d..b70c600dc4 100644 --- a/src/google/protobuf/compiler/subprocess.cc +++ b/src/google/protobuf/compiler/subprocess.cc @@ -32,9 +32,12 @@ #include -#include +#ifndef _WIN32 #include #include +#endif + +#include #include #include #include @@ -43,6 +46,231 @@ namespace google { namespace protobuf { namespace compiler { +#ifdef _WIN32 + +static void CloseHandleOrDie(HANDLE handle) { + if (!CloseHandle(handle)) { + GOOGLE_LOG(FATAL) << "CloseHandle: " + << Subprocess::Win32ErrorMessage(GetLastError()); + } +} + +Subprocess::Subprocess() + : process_start_error_(ERROR_SUCCESS), + child_handle_(NULL), child_stdin_(NULL), child_stdout_(NULL) {} + +Subprocess::~Subprocess() { + if (child_stdin_ != NULL) { + CloseHandleOrDie(child_stdin_); + } + if (child_stdout_ != NULL) { + CloseHandleOrDie(child_stdout_); + } +} + +void Subprocess::Start(const string& program, SearchMode search_mode) { + // Create the pipes. + HANDLE stdin_pipe_read; + HANDLE stdin_pipe_write; + HANDLE stdout_pipe_read; + HANDLE stdout_pipe_write; + + if (!CreatePipe(&stdin_pipe_read, &stdin_pipe_write, NULL, 0)) { + GOOGLE_LOG(FATAL) << "CreatePipe: " << Win32ErrorMessage(GetLastError()); + } + if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, NULL, 0)) { + GOOGLE_LOG(FATAL) << "CreatePipe: " << Win32ErrorMessage(GetLastError()); + } + + // Make child side of the pipes inheritable. + if (!SetHandleInformation(stdin_pipe_read, + HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)) { + GOOGLE_LOG(FATAL) << "SetHandleInformation: " + << Win32ErrorMessage(GetLastError()); + } + if (!SetHandleInformation(stdout_pipe_write, + HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT)) { + GOOGLE_LOG(FATAL) << "SetHandleInformation: " + << Win32ErrorMessage(GetLastError()); + } + + // Setup STARTUPINFO to redirect handles. + STARTUPINFO startup_info; + ZeroMemory(&startup_info, sizeof(startup_info)); + startup_info.cb = sizeof(startup_info); + startup_info.dwFlags = STARTF_USESTDHANDLES; + startup_info.hStdInput = stdin_pipe_read; + startup_info.hStdOutput = stdout_pipe_write; + startup_info.hStdError = GetStdHandle(STD_ERROR_HANDLE); + + if (startup_info.hStdError == INVALID_HANDLE_VALUE) { + GOOGLE_LOG(FATAL) << "GetStdHandle: " + << Win32ErrorMessage(GetLastError()); + } + + // CreateProcess() mutates its second parameter. WTF? + char* name_copy = strdup(program.c_str()); + + // Create the process. + PROCESS_INFORMATION process_info; + + if (CreateProcess((search_mode == SEARCH_PATH) ? NULL : program.c_str(), + (search_mode == SEARCH_PATH) ? name_copy : NULL, + NULL, // process security attributes + NULL, // thread security attributes + TRUE, // inherit handles? + 0, // obscure creation flags + NULL, // environment (inherit from parent) + NULL, // current directory (inherit from parent) + &startup_info, + &process_info)) { + child_handle_ = process_info.hProcess; + CloseHandleOrDie(process_info.hThread); + child_stdin_ = stdin_pipe_write; + child_stdout_ = stdout_pipe_read; + } else { + process_start_error_ = GetLastError(); + CloseHandleOrDie(stdin_pipe_write); + CloseHandleOrDie(stdout_pipe_read); + } + + CloseHandleOrDie(stdin_pipe_read); + CloseHandleOrDie(stdout_pipe_write); + free(name_copy); +} + +bool Subprocess::Communicate(const Message& input, Message* output, + string* error) { + if (process_start_error_ != ERROR_SUCCESS) { + *error = Win32ErrorMessage(process_start_error_); + return false; + } + + GOOGLE_CHECK(child_handle_ != NULL) << "Must call Start() first."; + + string input_data = input.SerializeAsString(); + string output_data; + + int input_pos = 0; + + while (child_stdout_ != NULL) { + HANDLE handles[2]; + int handle_count = 0; + + if (child_stdin_ != NULL) { + handles[handle_count++] = child_stdin_; + } + if (child_stdout_ != NULL) { + handles[handle_count++] = child_stdout_; + } + + DWORD wait_result = + WaitForMultipleObjects(handle_count, handles, FALSE, INFINITE); + + HANDLE signaled_handle; + if (wait_result >= WAIT_OBJECT_0 && + wait_result < WAIT_OBJECT_0 + handle_count) { + signaled_handle = handles[wait_result - WAIT_OBJECT_0]; + } else if (wait_result == WAIT_FAILED) { + GOOGLE_LOG(FATAL) << "WaitForMultipleObjects: " + << Win32ErrorMessage(GetLastError()); + } else { + GOOGLE_LOG(FATAL) << "WaitForMultipleObjects: Unexpected return code: " + << wait_result; + } + + if (signaled_handle == child_stdin_) { + DWORD n; + if (!WriteFile(child_stdin_, + input_data.data() + input_pos, + input_data.size() - input_pos, + &n, NULL)) { + // Child closed pipe. Presumably it will report an error later. + // Pretend we're done for now. + input_pos = input_data.size(); + } else { + input_pos += n; + } + + if (input_pos == input_data.size()) { + // We're done writing. Close. + CloseHandleOrDie(child_stdin_); + child_stdin_ = NULL; + } + } else if (signaled_handle == child_stdout_) { + char buffer[4096]; + DWORD n; + + if (!ReadFile(child_stdout_, buffer, sizeof(buffer), &n, NULL)) { + // We're done reading. Close. + CloseHandleOrDie(child_stdout_); + child_stdout_ = NULL; + } else { + output_data.append(buffer, n); + } + } + } + + if (child_stdin_ != NULL) { + // Child did not finish reading input before it closed the output. + // Presumably it exited with an error. + CloseHandleOrDie(child_stdin_); + child_stdin_ = NULL; + } + + DWORD wait_result = WaitForSingleObject(child_handle_, INFINITE); + + if (wait_result == WAIT_FAILED) { + GOOGLE_LOG(FATAL) << "WaitForSingleObject: " + << Win32ErrorMessage(GetLastError()); + } else if (wait_result != WAIT_OBJECT_0) { + GOOGLE_LOG(FATAL) << "WaitForSingleObject: Unexpected return code: " + << wait_result; + } + + DWORD exit_code; + if (!GetExitCodeProcess(child_handle_, &exit_code)) { + GOOGLE_LOG(FATAL) << "GetExitCodeProcess: " + << Win32ErrorMessage(GetLastError()); + } + + CloseHandleOrDie(child_handle_); + child_handle_ = NULL; + + if (exit_code != 0) { + *error = strings::Substitute( + "Plugin failed with status code $0.", exit_code); + return false; + } + + if (!output->ParseFromString(output_data)) { + *error = "Plugin output is unparseable: " + CEscape(output_data); + return false; + } + + return true; +} + +string Subprocess::Win32ErrorMessage(DWORD error_code) { + char* message; + + // WTF? + FormatMessage(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + NULL, error_code, 0, + (LPTSTR)&message, // NOT A BUG! + 0, NULL); + + string result = message; + LocalFree(message); + return result; +} + +// =================================================================== + +#else // _WIN32 + Subprocess::Subprocess() : child_pid_(-1), child_stdin_(-1), child_stdout_(-1) {} @@ -222,6 +450,8 @@ bool Subprocess::Communicate(const Message& input, Message* output, return true; } +#endif // !_WIN32 + } // namespace compiler } // namespace protobuf } // namespace google diff --git a/src/google/protobuf/compiler/subprocess.h b/src/google/protobuf/compiler/subprocess.h index d7e6ed58b2..f9e8ae8d81 100644 --- a/src/google/protobuf/compiler/subprocess.h +++ b/src/google/protobuf/compiler/subprocess.h @@ -33,12 +33,16 @@ #ifndef GOOGLE_PROTOBUF_COMPILER_SUBPROCESS_H__ #define GOOGLE_PROTOBUF_COMPILER_SUBPROCESS_H__ -#include -#include +#ifdef _WIN32 +#define WIN32_LEAN_AND_MEAN // right... +#include +#else // _WIN32 #include #include -#include +#endif // !_WIN32 +#include +#include namespace google { namespace protobuf { @@ -69,13 +73,31 @@ class Subprocess { // *error to a description of the problem. bool Communicate(const Message& input, Message* output, string* error); +#ifdef _WIN32 + // Given an error code, returns a human-readable error message. This is + // defined here so that CommandLineInterface can share it. + static string Subprocess::Win32ErrorMessage(DWORD error_code); +#endif + private: +#ifdef _WIN32 + DWORD process_start_error_; + HANDLE child_handle_; + + // The file handles for our end of the child's pipes. We close each and + // set it to NULL when no longer needed. + HANDLE child_stdin_; + HANDLE child_stdout_; + +#else // _WIN32 pid_t child_pid_; // The file descriptors for our end of the child's pipes. We close each and // set it to -1 when no longer needed. int child_stdin_; int child_stdout_; + +#endif // !_WIN32 }; } // namespace compiler diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index fe2caad30f..81c4ac0f37 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -3000,12 +3001,28 @@ void DescriptorBuilder::BuildFieldOrExtension(const FieldDescriptorProto& proto, strtou64(proto.default_value().c_str(), &end_pos, 0); break; case FieldDescriptor::CPPTYPE_FLOAT: - result->default_value_float_ = - NoLocaleStrtod(proto.default_value().c_str(), &end_pos); + if (proto.default_value() == "inf") { + result->default_value_float_ = numeric_limits::infinity(); + } else if (proto.default_value() == "-inf") { + result->default_value_float_ = -numeric_limits::infinity(); + } else if (proto.default_value() == "nan") { + result->default_value_float_ = numeric_limits::quiet_NaN(); + } else { + result->default_value_float_ = + NoLocaleStrtod(proto.default_value().c_str(), &end_pos); + } break; case FieldDescriptor::CPPTYPE_DOUBLE: - result->default_value_double_ = - NoLocaleStrtod(proto.default_value().c_str(), &end_pos); + if (proto.default_value() == "inf") { + result->default_value_double_ = numeric_limits::infinity(); + } else if (proto.default_value() == "-inf") { + result->default_value_double_ = -numeric_limits::infinity(); + } else if (proto.default_value() == "nan") { + result->default_value_double_ = numeric_limits::quiet_NaN(); + } else { + result->default_value_double_ = + NoLocaleStrtod(proto.default_value().c_str(), &end_pos); + } break; case FieldDescriptor::CPPTYPE_BOOL: if (proto.default_value() == "true") { diff --git a/src/google/protobuf/io/gzip_stream.cc b/src/google/protobuf/io/gzip_stream.cc index e1a35ea2c5..84d277f483 100644 --- a/src/google/protobuf/io/gzip_stream.cc +++ b/src/google/protobuf/io/gzip_stream.cc @@ -315,6 +315,6 @@ bool GzipOutputStream::Close() { } // namespace io } // namespace protobuf +} // namespace google #endif // HAVE_ZLIB -} // namespace google diff --git a/src/google/protobuf/repeated_field_unittest.cc b/src/google/protobuf/repeated_field_unittest.cc index 798b05ee19..cd9ca8a92a 100644 --- a/src/google/protobuf/repeated_field_unittest.cc +++ b/src/google/protobuf/repeated_field_unittest.cc @@ -261,7 +261,9 @@ TEST(RepeatedField, Truncate) { // Truncations that don't change the size are allowed, but growing is not // allowed. field.Truncate(field.size()); +#ifdef GTEST_HAS_DEATH_TEST EXPECT_DEBUG_DEATH(field.Truncate(field.size() + 1), "new_size"); +#endif }