From 36a4ca8b113056b20435cd38b00551c1c9275def Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 3 Nov 2016 16:03:10 +0100 Subject: 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 --- lib/spack/llnl/util/tty/log.py | 56 ++++++++++++++++++++++++------------ lib/spack/spack/build_environment.py | 21 +++++++++----- lib/spack/spack/package.py | 21 +++++++------- 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 -- cgit v1.2.3-60-g2f50