spack install: forward sys.stdin to child processes (#2158)

* spack install: forward sys.stdin to child processes fixes #2140

 - [ ] redirection process is spawned in __enter__ instead of __init__
 - [ ] sys.stdin is forwarded to child processes

* log: wrapped __init__ definition

3 files changed, 62 insertions, 36 deletions

diff --git a/lib/spack/llnl/util/tty/log.py b/lib/spack/llnl/util/tty/log.py
index a4ba2a9bdf..3d4972b3ae 100644
--- a/lib/spack/llnl/util/tty/log.py
+++ b/lib/spack/llnl/util/tty/log.py
@@ -120,7 +120,14 @@ class log_output(object):
     daemon joining. If echo is True, also prints the output to stdout.
     """
 
-    def __init__(self, filename, echo=False, force_color=False, debug=False):
+    def __init__(
+            self,
+            filename,
+            echo=False,
+            force_color=False,
+            debug=False,
+            input_stream=sys.stdin
+    ):
         self.filename = filename
         # Various output options
         self.echo = echo
@@ -132,46 +139,57 @@ class log_output(object):
         self.directAssignment = False
         self.read, self.write = os.pipe()
 
-        # Sets a daemon that writes to file what it reads from a pipe
-        self.p = multiprocessing.Process(
-            target=self._spawn_writing_daemon,
-            args=(self.read,),
-            name='logger_daemon'
-        )
-        self.p.daemon = True
         # Needed to un-summon the daemon
         self.parent_pipe, self.child_pipe = multiprocessing.Pipe()
+        # Input stream that controls verbosity interactively
+        self.input_stream = input_stream
 
     def __enter__(self):
-        self.p.start()
+        # Sets a daemon that writes to file what it reads from a pipe
+        try:
+            fwd_input_stream = os.fdopen(
+                os.dup(self.input_stream.fileno())
+            )
+            self.p = multiprocessing.Process(
+                target=self._spawn_writing_daemon,
+                args=(self.read, fwd_input_stream),
+                name='logger_daemon'
+            )
+            self.p.daemon = True
+            self.p.start()
+        finally:
+            fwd_input_stream.close()
         return log_output.OutputRedirection(self)
 
     def __exit__(self, exc_type, exc_val, exc_tb):
         self.parent_pipe.send(True)
         self.p.join(60.0)  # 1 minute to join the child
 
-    def _spawn_writing_daemon(self, read):
+    def _spawn_writing_daemon(self, read, input_stream):
         # Parent: read from child, skip the with block.
         read_file = os.fdopen(read, 'r', 0)
         with open(self.filename, 'w') as log_file:
-            with keyboard_input(sys.stdin):
+            with keyboard_input(input_stream):
                 while True:
-                    rlist, _, _ = select.select([read_file, sys.stdin], [], [])
-                    if not rlist:
-                        break
+                    # Without the last parameter (timeout) select will wait
+                    # until at least one of the two streams are ready. This
+                    # may cause the function to hang.
+                    rlist, _, _ = select.select(
+                        [read_file, input_stream], [], [], 0
+                    )
 
                     # Allow user to toggle echo with 'v' key.
                     # Currently ignores other chars.
-                    if sys.stdin in rlist:
-                        if sys.stdin.read(1) == 'v':
+                    if input_stream in rlist:
+                        if input_stream.read(1) == 'v':
                             self.echo = not self.echo
 
                     # Handle output from the with block process.
                     if read_file in rlist:
+                        # If we arrive here it means that
+                        # read_file was ready for reading : it
+                        # should never happen that line is false-ish
                         line = read_file.readline()
-                        if not line:
-                            # For some reason we never reach this point...
-                            break
 
                         # Echo to stdout if requested.
                         if self.echo:
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 81f893f736..1e6c473efd 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -528,10 +528,10 @@ def fork(pkg, function, dirty=False):
     carries on.
     """
 
-    def child_execution(child_connection):
+    def child_execution(child_connection, input_stream):
         try:
             setup_package(pkg, dirty=dirty)
-            function()
+            function(input_stream)
             child_connection.send(None)
         except:
             # catch ANYTHING that goes wrong in the child process
@@ -559,11 +559,18 @@ def fork(pkg, function, dirty=False):
             child_connection.close()
 
     parent_connection, child_connection = multiprocessing.Pipe()
-    p = multiprocessing.Process(
-        target=child_execution,
-        args=(child_connection,)
-    )
-    p.start()
+    try:
+        # Forward sys.stdin to be able to activate / deactivate
+        # verbosity pressing a key at run-time
+        input_stream = os.fdopen(os.dup(sys.stdin.fileno()))
+        p = multiprocessing.Process(
+            target=child_execution,
+            args=(child_connection, input_stream)
+        )
+        p.start()
+    finally:
+        # Close the input stream in the parent process
+        input_stream.close()
     child_exc = parent_connection.recv()
     p.join()
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 8ce8da1ff2..0d3c3c0ad5 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -95,8 +95,6 @@ class InstallPhase(object):
         # install phase, thus return a properly set wrapper
         phase = getattr(instance, self.name)
 
-        print phase
-
         @functools.wraps(phase)
         def phase_wrapper(spec, prefix):
             # Check instance attributes at the beginning of a phase
@@ -394,7 +392,8 @@ class PackageBase(object):
     The install function is designed so that someone not too terribly familiar
     with Python could write a package installer.  For example, we put a number
     of commands in install scope that you can use almost like shell commands.
-    These include make, configure, cmake, rm, rmtree, mkdir, mkdirp, and others.
+    These include make, configure, cmake, rm, rmtree, mkdir, mkdirp, and
+    others.
 
     You can see above in the cmake script that these commands are used to run
     configure and make almost like they're used on the command line.  The
@@ -409,9 +408,9 @@ class PackageBase(object):
     pollute other namespaces, and it allows you to more easily implement an
     install function.
 
-    For a full list of commands and variables available in module scope, see the
-    add_commands_to_module() function in this class. This is where most of
-    them are created and set on the module.
+    For a full list of commands and variables available in module scope, see
+    the add_commands_to_module() function in this class. This is where most
+    of them are created and set on the module.
 
     **Parallel Builds**
 
@@ -1197,7 +1196,7 @@ class PackageBase(object):
         self.make_jobs = make_jobs
 
         # Then install the package itself.
-        def build_process():
+        def build_process(input_stream):
             """Forked for each build. Has its own process and python
                module space set up by build_environment.fork()."""
 
@@ -1239,9 +1238,11 @@ class PackageBase(object):
                     # Spawn a daemon that reads from a pipe and redirects
                     # everything to log_path
                     redirection_context = log_output(
-                        log_path, verbose,
-                        sys.stdout.isatty(),
-                        True
+                        log_path,
+                        echo=verbose,
+                        force_color=sys.stdout.isatty(),
+                        debug=True,
+                        input_stream=input_stream
                     )
                     with redirection_context as log_redirection:
                         for phase_name, phase in zip(self.phases, self._InstallPhase_phases):  # NOQA: ignore=E501