Merge pull request #764 from nico/winconsole

Implement pool=console support on Windows.

6 files changed, 25 insertions, 24 deletions

diff --git a/doc/manual.asciidoc b/doc/manual.asciidoc
index 5b0c1fe..18760dd 100644
--- a/doc/manual.asciidoc
+++ b/doc/manual.asciidoc
@@ -664,8 +664,6 @@ While a task in the `console` pool is running, Ninja's regular output (such
 as progress status and output from concurrent tasks) is buffered until
 it completes.
 
-This feature is not yet available on Windows.
-
 Ninja file reference
 --------------------
 
diff --git a/src/build_test.cc b/src/build_test.cc
index 119521e..c414c88 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -245,7 +245,6 @@ TEST_F(PlanTest, PoolWithDepthOne) {
 "build out2: poolcat in\n");
 }
 
-#ifndef _WIN32
 TEST_F(PlanTest, ConsolePool) {
   TestPoolWithDepthOne(
 "rule poolcat\n"
@@ -254,7 +253,6 @@ TEST_F(PlanTest, ConsolePool) {
 "build out1: poolcat in\n"
 "build out2: poolcat in\n");
 }
-#endif
 
 TEST_F(PlanTest, PoolsWithDepthTwo) {
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
@@ -1944,12 +1942,12 @@ TEST_F(BuildTest, Console) {
 "rule console\n"
 "  command = console\n"
 "  pool = console\n"
-"build con: console in.txt\n"));
+"build cons: console in.txt\n"));
 
   fs_.Create("in.txt", "");
 
   string err;
-  EXPECT_TRUE(builder_.AddTarget("con", &err));
+  EXPECT_TRUE(builder_.AddTarget("cons", &err));
   ASSERT_EQ("", err);
   EXPECT_TRUE(builder_.Build(&err));
   EXPECT_EQ("", err);
diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index a566eda..6fa4f7c 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -317,10 +317,6 @@ bool ManifestParser::ParseEdge(string* err) {
     Pool* pool = state_->LookupPool(pool_name);
     if (pool == NULL)
       return lexer_.Error("unknown pool name '" + pool_name + "'", err);
-#ifdef _WIN32
-    if (pool == &State::kConsolePool)
-      return lexer_.Error("console pool unsupported on Windows", err);
-#endif
     edge->pool_ = pool;
   }
 
diff --git a/src/subprocess-posix.cc b/src/subprocess-posix.cc
index 793d48f..743e406 100644
--- a/src/subprocess-posix.cc
+++ b/src/subprocess-posix.cc
@@ -65,6 +65,7 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
         break;
 
       if (!use_console_) {
+        // Put the child in its own process group, so ctrl-c won't reach it.
         if (setpgid(0, 0) < 0)
           break;
 
@@ -84,6 +85,8 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
         error_pipe = 2;
         close(output_pipe[1]);
       }
+      // In the console case, output_pipe is still inherited by the child and
+      // closed when the subprocess finishes, which then notifies ninja.
 
       execl("/bin/sh", "/bin/sh", "-c", command.c_str(), (char *) NULL);
     } while (false);
diff --git a/src/subprocess-win32.cc b/src/subprocess-win32.cc
index c9607e1..fad66e8 100644
--- a/src/subprocess-win32.cc
+++ b/src/subprocess-win32.cc
@@ -21,7 +21,9 @@
 
 #include "util.h"
 
-Subprocess::Subprocess() : child_(NULL) , overlapped_(), is_reading_(false) {
+Subprocess::Subprocess(bool use_console) : child_(NULL) , overlapped_(),
+                                           is_reading_(false),
+                                           use_console_(use_console) {
 }
 
 Subprocess::~Subprocess() {
@@ -87,18 +89,25 @@ bool Subprocess::Start(SubprocessSet* set, const string& command) {
   STARTUPINFOA startup_info;
   memset(&startup_info, 0, sizeof(startup_info));
   startup_info.cb = sizeof(STARTUPINFO);
-  startup_info.dwFlags = STARTF_USESTDHANDLES;
-  startup_info.hStdInput = nul;
-  startup_info.hStdOutput = child_pipe;
-  startup_info.hStdError = child_pipe;
+  if (!use_console_) {
+    startup_info.dwFlags = STARTF_USESTDHANDLES;
+    startup_info.hStdInput = nul;
+    startup_info.hStdOutput = child_pipe;
+    startup_info.hStdError = child_pipe;
+  }
+  // In the console case, child_pipe is still inherited by the child and closed
+  // when the subprocess finishes, which then notifies ninja.
 
   PROCESS_INFORMATION process_info;
   memset(&process_info, 0, sizeof(process_info));
 
+  // Ninja handles ctrl-c, except for subprocesses in console pools.
+  DWORD process_flags = use_console_ ? 0 : CREATE_NEW_PROCESS_GROUP;
+
   // Do not prepend 'cmd /c' on Windows, this breaks command
   // lines greater than 8,191 chars.
   if (!CreateProcessA(NULL, (char*)command.c_str(), NULL, NULL,
-                      /* inherit handles */ TRUE, CREATE_NEW_PROCESS_GROUP,
+                      /* inherit handles */ TRUE, process_flags,
                       NULL, NULL,
                       &startup_info, &process_info)) {
     DWORD error = GetLastError();
@@ -215,9 +224,7 @@ BOOL WINAPI SubprocessSet::NotifyInterrupted(DWORD dwCtrlType) {
 }
 
 Subprocess *SubprocessSet::Add(const string& command, bool use_console) {
-  assert(!use_console); // We don't support this yet on Windows.
-
-  Subprocess *subprocess = new Subprocess;
+  Subprocess *subprocess = new Subprocess(use_console);
   if (!subprocess->Start(this, command)) {
     delete subprocess;
     return 0;
@@ -269,7 +276,9 @@ Subprocess* SubprocessSet::NextFinished() {
 void SubprocessSet::Clear() {
   for (vector<Subprocess*>::iterator i = running_.begin();
        i != running_.end(); ++i) {
-    if ((*i)->child_) {
+    // Since the foreground process is in our process group, it will receive a
+    // CTRL_C_EVENT or CTRL_BREAK_EVENT at the same time as us.
+    if ((*i)->child_ && !(*i)->use_console_) {
       if (!GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT,
                                     GetProcessId((*i)->child_))) {
         Win32Fatal("GenerateConsoleCtrlEvent");
diff --git a/src/subprocess.h b/src/subprocess.h
index 6ea6f62..b7a1a4c 100644
--- a/src/subprocess.h
+++ b/src/subprocess.h
@@ -44,14 +44,13 @@ struct Subprocess {
   const string& GetOutput() const;
 
  private:
+  Subprocess(bool use_console);
   bool Start(struct SubprocessSet* set, const string& command);
   void OnPipeReady();
 
   string buf_;
 
 #ifdef _WIN32
-  Subprocess();
-
   /// Set up pipe_ as the parent-side pipe of the subprocess; return the
   /// other end of the pipe, usable in the child process.
   HANDLE SetupPipe(HANDLE ioport);
@@ -62,8 +61,6 @@ struct Subprocess {
   char overlapped_buf_[4 << 10];
   bool is_reading_;
 #else
-  Subprocess(bool use_console);
-
   int fd_;
   pid_t pid_;
 #endif