Refactor spawning command-line actions to clean up the interface between Action and SConf.

4 files changed, 100 insertions, 91 deletions

diff --git a/doc/man/scons.1 b/doc/man/scons.1
index 28f7029..43cc318 100644
--- a/doc/man/scons.1
+++ b/doc/man/scons.1
@@ -6177,7 +6177,7 @@ that may not be set or used in a construction environment.
 
 .IP SPAWN
 A command interpreter function that will be called to execute command line
-strings. The function must expect 4 arguments:
+strings. The function must expect the following arguments:
 
 .ES
 def spawn(shell, escape, cmd, args, env):
@@ -6191,7 +6191,7 @@ the command line.
 .I cmd
 is the path to the command to be executed.
 .I args
-is that arguments to the command.
+is the arguments to the command.
 .I env
 is a dictionary of the environment variables
 in which the command should be executed.
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 69ba78c..ec6a3b0 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -307,77 +307,54 @@ class CommandAction(ActionBase):
         handle lists of commands, even though that's not how we use it
         externally.
         """
-        import SCons.Util
+        from SCons.Util import is_String, is_List, flatten, escape_list
 
-        escape = env.get('ESCAPE', lambda x: x)
-
-        if env.has_key('SHELL'):
+        try:
             shell = env['SHELL']
-        else:
+        except KeyError:
             raise SCons.Errors.UserError('Missing SHELL construction variable.')
 
-        # for SConf support (by now): check, if we want to pipe the command
-        # output to somewhere else
-        if env.has_key('PIPE_BUILD'):
-            pipe_build = 1
-            if env.has_key('PSPAWN'):
-                pspawn = env['PSPAWN']
-            else:
-                raise SCons.Errors.UserError('Missing PSPAWN construction variable.')
-            if env.has_key('PSTDOUT'):
-                pstdout = env['PSTDOUT']
-            else:
-                raise SCons.Errors.UserError('Missing PSTDOUT construction variable.')
-            if env.has_key('PSTDERR'):
-                pstderr = env['PSTDERR']
-            else:
-                raise SCons.Errors.UserError('Missing PSTDOUT construction variable.')
-        else:
-            pipe_build = 0
-            if env.has_key('SPAWN'):
-                spawn = env['SPAWN']
-            else:
-                raise SCons.Errors.UserError('Missing SPAWN construction variable.')
+        try:
+            spawn = env['SPAWN']
+        except KeyError:
+            raise SCons.Errors.UserError('Missing SPAWN construction variable.')
 
-        cmd_list = env.subst_list(self.cmd_list, 0, target, source)
-        for cmd_line in cmd_list:
-            if len(cmd_line):
-                try:
-                    ENV = env['ENV']
-                except KeyError:
-                    global default_ENV
-                    if not default_ENV:
-                        import SCons.Environment
-                        default_ENV = SCons.Environment.Environment()['ENV']
-                    ENV = default_ENV
-
-                # ensure that the ENV values are all strings:
-                for key, value in ENV.items():
-                    if SCons.Util.is_List(value):
-                        # If the value is a list, then we assume
-                        # it is a path list, because that's a pretty
-                        # common list like value to stick in an environment
-                        # variable:
-                        value = SCons.Util.flatten(value)
-                        ENV[key] = string.join(map(str, value), os.pathsep)
-                    elif not SCons.Util.is_String(value):
-                        # If it isn't a string or a list, then
-                        # we just coerce it to a string, which
-                        # is proper way to handle Dir and File instances
-                        # and will produce something reasonable for
-                        # just about everything else:
-                        ENV[key] = str(value)
-
-                # Escape the command line for the command
-                # interpreter we are using
-                cmd_line = SCons.Util.escape_list(cmd_line, escape)
-                if pipe_build:
-                    ret = pspawn( shell, escape, cmd_line[0], cmd_line,
-                                  ENV, pstdout, pstderr )
+        escape = env.get('ESCAPE', lambda x: x)
+
+        try:
+            ENV = env['ENV']
+        except KeyError:
+            global default_ENV
+            if not default_ENV:
+                import SCons.Environment
+                default_ENV = SCons.Environment.Environment()['ENV']
+            ENV = default_ENV
+
+        # Ensure that the ENV values are all strings:
+        for key, value in ENV.items():
+            if not is_String(value):
+                if is_List(value):
+                    # If the value is a list, then we assume it is a
+                    # path list, because that's a pretty common list-like
+                    # value to stick in an environment variable:
+                    value = flatten(value)
+                    ENV[key] = string.join(map(str, value), os.pathsep)
                 else:
-                    ret = spawn(shell, escape, cmd_line[0], cmd_line, ENV)
-                if ret:
-                    return ret
+                    # If it isn't a string or a list, then we just coerce
+                    # it to a string, which is the proper way to handle
+                    # Dir and File instances and will produce something
+                    # reasonable for just about everything else:
+                    ENV[key] = str(value)
+
+        cmd_list = env.subst_list(self.cmd_list, 0, target, source)
+
+        # Use len() to filter out any "command" that's zero-length.
+        for cmd_line in filter(len, cmd_list):
+            # Escape the command line for the interpreter we are using.
+            cmd_line = escape_list(cmd_line, escape)
+            result = spawn(shell, escape, cmd_line[0], cmd_line, ENV)
+            if result:
+                return result
         return 0
 
     def get_contents(self, target, source, env, dict=None):
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index be0b418..fa2bfe7 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -783,8 +783,15 @@ class CommandActionTestCase(unittest.TestCase):
         r = act([], [], env.Copy(out = outfile))
         assert r == expect_nonexecutable, "r == %d" % r
 
-    def test_pipe_execute(self):
+    def _DO_NOT_EXECUTE_test_pipe_execute(self):
         """Test capturing piped output from an action
+
+        We used to have PIPE_BUILD support built right into
+        Action.execute() for the benefit of the SConf subsystem, but we've
+        moved that logic back into SConf itself.  We'll leave this code
+        here, just in case we ever want to resurrect this functionality
+        in the future, but change the name of the test so it doesn't
+        get executed as part of the normal test suite.
         """
         pipe = open( pipe_file, "w" )
         self.env = Environment(ENV = {'ACTPY_PIPE' : '1'}, PIPE_BUILD = 1,
diff --git a/src/engine/SCons/SConf.py b/src/engine/SCons/SConf.py
index 3aac882..910d47f 100644
--- a/src/engine/SCons/SConf.py
+++ b/src/engine/SCons/SConf.py
@@ -247,6 +247,18 @@ class SConf:
                 sys.stderr = oldStderr
         return ret
 
+    def pspawn_wrapper(self, sh, escape, cmd, args, env):
+        """Wrapper function for handling piped spawns.
+
+        This looks to the calling interface (in Action.py) like a "normal"
+        spawn, but associates the call with the PSPAWN variable from
+        the construction environment and with the streams to which we
+        want the output logged.  This gets slid into the construction
+        environment as the SPAWN variable so Action.py doesn't have to
+        know or care whether it's spawning a piped command or not.
+        """
+        return self.pspawn(sh, escape, cmd, args, env, self.logstream, self.logstream)
+
 
     def TryBuild(self, builder, text = None, extension = ""):
         """Low level TryBuild implementation. Normally you don't need to
@@ -254,43 +266,56 @@ class SConf:
         """
         global _ac_build_counter
 
+        # Make sure we have a PSPAWN value, and save the current
+        # SPAWN value.
+        try:
+            self.pspawn = self.env['PSPAWN']
+        except KeyError:
+            raise SCons.Errors.UserError('Missing PSPAWN construction variable.')
+        try:
+            save_spawn = self.env['SPAWN']
+        except KeyError:
+            raise SCons.Errors.UserError('Missing SPAWN construction variable.')
+
         nodesToBeBuilt = []
 
         f = "conftest_" + str(_ac_build_counter)
         pref = self.env.subst( builder.builder.prefix )
         suff = self.env.subst( builder.builder.suffix )
         target = self.confdir.File(pref + f + suff)
-        self.env['SCONF_TEXT'] = text
-        self.env['PIPE_BUILD'] = 1
-        self.env['PSTDOUT'] = self.logstream
-        self.env['PSTDERR'] = self.logstream
-        if text != None:
-            source = self.confdir.File(f + extension)
-            sourceNode = self.env.SConfSourceBuilder(target=source,
-                                                     source=None)
-            nodesToBeBuilt.extend(sourceNode)
-        else:
-            source = None
 
-        nodes = builder(target = target, source = source)
-        if not SCons.Util.is_List(nodes):
-            nodes = [nodes]
-        nodesToBeBuilt.extend(nodes)
-        ret = self.BuildNodes(nodesToBeBuilt)
+        try:
+            # Slide our wrapper into the construction environment as
+            # the SPAWN function.
+            self.env['SPAWN'] = self.pspawn_wrapper
+            self.env['SCONF_TEXT'] = text
+
+            if text != None:
+                source = self.confdir.File(f + extension)
+                sourceNode = self.env.SConfSourceBuilder(target=source,
+                                                         source=None)
+                nodesToBeBuilt.extend(sourceNode)
+            else:
+                source = None
 
-        # clean up environment
-        del self.env['PIPE_BUILD']
-        del self.env['PSTDOUT']
-        del self.env['PSTDERR']
-        del self.env['SCONF_TEXT']
+            nodes = builder(target = target, source = source)
+            if not SCons.Util.is_List(nodes):
+                nodes = [nodes]
+            nodesToBeBuilt.extend(nodes)
+            result = self.BuildNodes(nodesToBeBuilt)
+
+        finally:
+            # Clean up the environment, restoring the SPAWN value.
+            self.env['SPAWN'] = save_spawn
+            del self.env['SCONF_TEXT']
 
         _ac_build_counter = _ac_build_counter + 1
-        if ret:
+        if result:
             self.lastTarget = nodes[0]
         else:
             self.lastTarget = None
 
-        return ret
+        return result
 
     def TryAction(self, action, text = None, extension = ""):
         """Tries to execute the given action with optional source file