Support special characters in file names. (Charles Crain)

8 files changed, 290 insertions, 49 deletions

diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index 06d8dd9..255f329 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -50,12 +50,6 @@ exitvalmap = {
 
 default_ENV = None
 
-def quote(x):
-    if ' ' in x or '\t' in x:
-        return '"'+x+'"'
-    else:
-        return x
-
 def rfile(n):
     try:
         return n.rfile()
@@ -64,16 +58,16 @@ def rfile(n):
 
 if os.name == 'posix':
 
-    def escape(arg):
+    def defaultEscape(arg):
         "escape shell special characters"
         slash = '\\'
-        special = '"\'`&;><| \t#()*?$~!'
+        special = '"$'
 
         arg = string.replace(arg, slash, slash+slash)
         for c in special:
             arg = string.replace(arg, c, slash+c)
 
-        return arg
+        return '"' + arg + '"'
 
     # If the env command exists, then we can use os.system()
     # to spawn commands, otherwise we fall back on os.fork()/os.exec().
@@ -84,11 +78,11 @@ if os.name == 'posix':
             if env:
                 s = 'env -i '
                 for key in env.keys():
-                    s = s + '%s=%s '%(key, escape(env[key]))
+                    s = s + '%s=%s '%(key, defaultEscape(env[key]))
                 s = s + 'sh -c '
-                s = s + escape(string.join(map(quote, args)))
+                s = s + defaultEscape(string.join(args))
             else:
-                s = string.join(map(quote, args))
+                s = string.join(args)
 
             return os.system(s) >> 8
     else:
@@ -97,7 +91,7 @@ if os.name == 'posix':
             if not pid:
                 # Child process.
                 exitval = 127
-                args = ['sh', '-c', string.join(map(quote, args))]
+                args = ['sh', '-c', string.join(args)]
                 try:
                     os.execvpe('sh', args, env)
                 except OSError, e:
@@ -181,12 +175,17 @@ elif os.name == 'nt':
             return 127
         else:
             try:
-                args = [cmd_interp, '/C', quote(string.join(map(quote, args)))]
+                args = [cmd_interp, '/C', quote(string.join(args)) ]
                 ret = os.spawnve(os.P_WAIT, cmd_interp, args, env)
             except OSError, e:
                 ret = exitvalmap[e[0]]
                 sys.stderr.write("scons: %s: %s\n" % (cmd, e[1]))
             return ret
+
+    # Windows does not allow special characters in file names
+    # anyway, so no need for an escape function, we will just quote
+    # the arg.
+    defaultEscape = lambda x: '"' + x + '"'
 else:
     def defaultSpawn(cmd, args, env):
         sys.stderr.write("scons: Unknown os '%s', cannot spawn command interpreter.\n" % os.name)
@@ -194,15 +193,32 @@ else:
         return 127
 
 spawn = defaultSpawn
-
-def SetCommandHandler(func):
-    global spawn
+escape_cmd = defaultEscape
+
+def SetCommandHandler(func, escape = lambda x: x):
+    """Sets the command handler and escape function for the
+    system.  All command actions are passed through
+    the command handler, which should be a function that accepts
+    3 arguments: a string command, a list of arguments (the first
+    of which is the command itself), and a dictionary representing
+    the execution environment.  The function should then pass
+    the string to a suitable command interpreter.
+
+    The escape function should take a string and return the same
+    string with all special characters escaped such that the command
+    interpreter will interpret the string literally."""
+    global spawn, escape_cmd
     spawn = func
-
+    escape_cmd = escape
+    
 def GetCommandHandler():
     global spawn
     return spawn
 
+def GetEscapeHandler():
+    global escape_cmd
+    return escape_cmd
+
 class CommandGenerator:
     """
     Wraps a command generator function so the Action() factory
@@ -316,7 +332,7 @@ def _string_from_cmd_list(cmd_list):
     """Takes a list of command line arguments and returns a pretty
     representation for printing."""
     cl = []
-    for arg in cmd_list:
+    for arg in map(str, cmd_list):
         if ' ' in arg or '\t' in arg:
             arg = '"' + arg + '"'
         cl.append(arg)
@@ -328,9 +344,7 @@ _remove = re.compile(r'\$\(([^\$]|\$[^\(])*?\$\)')
 class CommandAction(ActionBase):
     """Class for command-execution actions."""
     def __init__(self, cmd):
-        import SCons.Util
-
-        self.cmd_list = map(SCons.Util.to_String, cmd)
+        self.cmd_list = cmd
 
     def execute(self, target, source, env):
         dict = self.subst_dict(target, source, env)
@@ -349,6 +363,10 @@ class CommandAction(ActionBase):
                             import SCons.Environment
                             default_ENV = SCons.Environment.Environment()['ENV']
                         ENV = default_ENV
+                    # Escape the command line for the command
+                    # interpreter we are using
+                    map(lambda x: x.escape(escape_cmd), cmd_line)
+                    cmd_line = map(str, cmd_line)
                     ret = spawn(cmd_line[0], cmd_line, ENV)
                     if ret:
                         return ret
@@ -376,7 +394,7 @@ class CommandAction(ActionBase):
         This strips $(-$) and everything in between the string,
         since those parts don't affect signatures.
         """
-        return SCons.Util.scons_subst(string.join(self.cmd_list),
+        return SCons.Util.scons_subst(string.join(map(str, self.cmd_list)),
                                       self._sig_dict(target, source, env), {}, _remove)
 
 class CommandGeneratorAction(ActionBase):
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 2e53bbd..1ef71f0 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -158,13 +158,30 @@ class CommandActionTestCase(unittest.TestCase):
                 self.executed = 0
         t=Test()
         def func(cmd, args, env, test=t):
-            test.executed = 1
+            test.executed = args
             return 0
+        def escape_func(cmd):
+            return '**' + cmd + '**'
+
+        class LiteralStr:
+            def __init__(self, x):
+                self.data = x
+            def __str__(self):
+                return self.data
+            def is_literal(self):
+                return 1
+            
         SCons.Action.SetCommandHandler(func)
         assert SCons.Action.spawn is func
         a = SCons.Action.CommandAction(["xyzzy"])
         a.execute([],[],Environment({}))
-        assert t.executed == 1
+        assert t.executed == [ 'xyzzy' ]
+
+        SCons.Action.SetCommandHandler(func,escape_func)
+        assert SCons.Action.GetEscapeHandler() == escape_func
+        a = SCons.Action.CommandAction([ LiteralStr("xyzzy") ])
+        a.execute([],[],Environment({ }))
+        assert t.executed == [ '**xyzzy**' ], t.executed
 
     def test_get_raw_contents(self):
         """Test fetching the contents of a command Action
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index 2211bf8..0d0d7da 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -471,12 +471,15 @@ class EnvironmentTestCase(unittest.TestCase):
 	env = Environment(AAA = 'a', BBB = 'b')
 	str = env.subst("$AAA ${AAA}A $BBBB $BBB")
 	assert str == "a aA b", str
-	env = Environment(AAA = '$BBB', BBB = 'b', BBBA = 'foo')
+
+        # Changed the tests below to reflect a bug fix in
+        # subst()
+        env = Environment(AAA = '$BBB', BBB = 'b', BBBA = 'foo')
 	str = env.subst("$AAA ${AAA}A ${AAA}B $BBB")
-	assert str == "b foo b", str
+	assert str == "b bA bB b", str
 	env = Environment(AAA = '$BBB', BBB = '$CCC', CCC = 'c')
 	str = env.subst("$AAA ${AAA}A ${AAA}B $BBB")
-	assert str == "c c", str
+	assert str == "c cA cB c", str
 
         env = Environment(AAA = '$BBB', BBB = '$CCC', CCC = [ 'a', 'b\nc' ])
         lst = env.subst_list([ "$AAA", "B $CCC" ])
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index 8ae7165..cd2d2e3 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -682,7 +682,11 @@ class NodeTestCase(unittest.TestCase):
         assert not hasattr(nodes[1], 'b'), nodes[1]
         assert not hasattr(nodes[1], 'bbbb'), nodes[0]
         assert nodes[1].c == 1, nodes[1]
-
+        
+    def test_literal(self):
+        """Test the is_literal() function."""
+        n=SCons.Node.Node()
+        assert n.is_literal()
 
 
 if __name__ == "__main__":
diff --git a/src/engine/SCons/Node/__init__.py b/src/engine/SCons/Node/__init__.py
index 22bf949..1afc79f 100644
--- a/src/engine/SCons/Node/__init__.py
+++ b/src/engine/SCons/Node/__init__.py
@@ -394,6 +394,11 @@ class Node:
     def rstr(self):
         return str(self)
 
+    def is_literal(self):
+        """Always pass the string representation of a Node to
+        the command interpreter literally."""
+        return 1
+
 def get_children(node, parent): return node.children()
 def ignore_cycle(node, stack): pass
 def do_nothing(node, parent): pass
diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py
index 3d4a6f7..45d05b9 100644
--- a/src/engine/SCons/Script/SConscript.py
+++ b/src/engine/SCons/Script/SConscript.py
@@ -329,6 +329,7 @@ def BuildDefaultGlobals():
     globals['Help']              = Help
     globals['Import']            = Import
     globals['Library']           = SCons.Defaults.StaticLibrary
+    globals['Literal']           = SCons.Util.Literal
     globals['Local']             = Local
     globals['Object']            = SCons.Defaults.StaticObject
     globals['Options']           = Options
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 5c2fd26..adba42a 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -76,6 +76,20 @@ def updrive(path):
         path = string.upper(drive) + rest
     return path
 
+class Literal:
+    """A wrapper for a string.  If you use this object wrapped
+    around a string, then it will be interpreted as literal.
+    When passed to the command interpreter, all special
+    characters will be escaped."""
+    def __init__(self, lstr):
+        self.lstr = lstr
+
+    def __str__(self):
+        return self.lstr
+
+    def is_literal(self):
+        return 1
+
 class PathList(UserList.UserList):
     """This class emulates the behavior of a list, but also implements
     the special "path dissection" attributes we can use to find
@@ -153,6 +167,9 @@ class PathList(UserList.UserList):
                          "dir" : __getDir,
                          "suffix" : __getSuffix,
                          "abspath" : __getAbsPath}
+
+    def is_literal(self):
+        return 1
     
     def __str__(self):
         return string.join(self.data)
@@ -189,8 +206,118 @@ def quote_spaces(arg):
     else:
         return arg
 
+# Several functions below deal with Environment variable
+# substitution.  Part of this process involves inserting
+# a bunch of special escape sequences into the string
+# so that when we are all done, we know things like
+# where to split command line args, what strings to
+# interpret literally, etc.  A dictionary of these
+# sequences follows:
+#
+# \0\1          signifies a division between arguments in
+#               a command line.
+#
+# \0\2          signifies a division between multiple distinct
+#               commands
+#
+# \0\3          This string should be interpreted literally.
+#               This code occurring anywhere in the string means
+#               the whole string should have all special characters
+#               escaped.
+#
+# \0\4          A literal dollar sign '$'
+#
+# \0\5          Placed before and after interpolated variables
+#               so that we do not accidentally smush to variables
+#               together during the recursive interpolation process.
+
 _cv = re.compile(r'\$([_a-zA-Z]\w*|{[^}]*})')
 _space_sep = re.compile(r'[\t ]+(?![^{]*})')
+_newline = re.compile(r'[\r\n]+')
+
+def _convertArg(x):
+    """This function converts an individual argument.  If the
+    argument is to be interpreted literally, with all special
+    characters escaped, then we insert a special code in front
+    of it, so that the command interpreter will know this."""
+    literal = 0
+
+    try:
+        if x.is_literal():
+            literal = 1
+    except AttributeError:
+        pass
+    
+    if not literal:
+        # escape newlines as '\0\2', '\0\1' denotes an argument split
+        # Also escape double-dollar signs to mean the literal dollar sign.
+        return string.replace(_newline.sub('\0\2', to_String(x)), '$$', '\0\4')
+    else:
+        # Interpret non-string args as literals.
+        # The special \0\3 code will tell us to encase this string
+        # in a Literal instance when we are all done
+        # Also escape out any $ signs because we don't want
+        # to continue interpolating a literal.
+        return '\0\3' + string.replace(str(x), '$', '\0\4')
+
+def _convert(x):
+    """This function is used to convert construction variable
+    values or the value of strSubst to a string for interpolation.
+    This function follows the rules outlined in the documentaion
+    for scons_subst_list()"""
+    if x is None:
+        return ''
+    elif is_String(x):
+        # escape newlines as '\0\2', '\0\1' denotes an argument split
+        return _convertArg(_space_sep.sub('\0\1', x))
+    elif is_List(x):
+        # '\0\1' denotes an argument split
+        return string.join(map(_convertArg, x), '\0\1')
+    else:
+        return _convertArg(x)
+
+class CmdStringHolder:
+    """This is a special class used to hold strings generated
+    by scons_subst_list().  It defines a special method escape().
+    When passed a function with an escape algorithm for a
+    particular platform, it will return the contained string
+    with the proper escape sequences inserted."""
+
+    def __init__(self, cmd):
+        """This constructor receives a string.  The string
+        can contain the escape sequence \0\3.
+        If it does, then we will escape all special characters
+        in the string before passing it to the command interpreter."""
+        self.data = cmd
+        
+        # Populate flatdata (the ting returned by str()) with the
+        # non-escaped string
+        self.escape(lambda x: x, lambda x: x)
+
+    def __str__(self):
+        """Return the string in its current state."""
+        return self.flatdata
+
+    def escape(self, escape_func, quote_func=quote_spaces):
+        """Escape the string with the supplied function.  The
+        function is expected to take an arbitrary string, then
+        return it with all special characters escaped and ready
+        for passing to the command interpreter.
+
+        After calling this function, the next call to str() will
+        return the escaped string.
+        """
+
+        if string.find(self.data, '\0\3') >= 0:
+            self.flatdata = escape_func(string.replace(self.data, '\0\3', ''))
+        elif ' ' in self.data or '\t' in self.data:
+            self.flatdata = quote_func(self.data)
+        else:
+            self.flatdata = self.data
+
+    def __cmp__(self, rhs):
+        return cmp(self.flatdata, str(rhs))
+        
 
 def scons_subst_list(strSubst, globals, locals, remove=None):
     """
@@ -231,18 +358,21 @@ def scons_subst_list(strSubst, globals, locals, remove=None):
         else:
             return to_String(x)
 
-    def repl(m, globals=globals, locals=locals, convert=convert):
+    def repl(m, globals=globals, locals=locals):
         key = m.group(1)
         if key[0] == '{':
             key = key[1:-1]
         try:
             e = eval(key, globals, locals)
-            return convert(e)
+            # The \0\5 escape code keeps us from smushing two or more
+            # variables together during recusrive substitution, i.e.
+            # foo=$bar bar=baz barbaz=blat => $foo$bar->blat (bad)
+            return "\0\5" + _convert(e) + "\0\5"
         except NameError:
-            return ''
+            return '\0\5'
 
     # Convert the argument to a string:
-    strSubst = convert(strSubst)
+    strSubst = _convert(strSubst)
 
     # Do the interpolation:
     n = 1
@@ -250,14 +380,17 @@ def scons_subst_list(strSubst, globals, locals, remove=None):
         strSubst, n = _cv.subn(repl, strSubst)
         
     # Convert the interpolated string to a list of lines:
-    listLines = string.split(strSubst, '\n')
+    listLines = string.split(strSubst, '\0\2')
 
     # Remove the patterns that match the remove argument: 
     if remove:
         listLines = map(lambda x,re=remove: re.sub('', x), listLines)
 
+    # Process escaped $'s and remove placeholder \0\5's
+    listLines = map(lambda x: string.replace(string.replace(x, '\0\4', '$'), '\0\5', ''), listLines)
+
     # Finally split each line up into a list of arguments:
-    return map(lambda x: filter(lambda y: y, string.split(x, '\0')),
+    return map(lambda x: map(CmdStringHolder, filter(lambda y:y, string.split(x, '\0\1'))),
                listLines)
 
 def scons_subst(strSubst, globals, locals, remove=None):
@@ -287,16 +420,23 @@ def scons_subst(strSubst, globals, locals, remove=None):
                 s = to_String(e)
         except NameError:
             s = ''
-        return s
+        # Insert placeholders to avoid accidentally smushing
+        # separate variables together.
+        return "\0\5" + s + "\0\5"
 
     # Now, do the substitution
     n = 1
     while n != 0:
+        # escape double dollar signs
+        strSubst = string.replace(strSubst, '$$', '\0\4')
         strSubst,n = _cv.subn(repl, strSubst)
     # and then remove remove
     if remove:
         strSubst = remove.sub('', strSubst)
-    
+
+    # Un-escape the string
+    strSubst = string.replace(string.replace(strSubst, '\0\4', '$'),
+                              '\0\5', '')
     # strip out redundant white-space
     return string.strip(_space_sep.sub(' ', strSubst))
 
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 628de30..e966253 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -128,6 +128,20 @@ class UtilTestCase(unittest.TestCase):
         newcom = scons_subst("test $a $b $c $d test", glob, loc)
         assert newcom == "test 3 2 4 test", newcom
 
+        # Test against a former bug in scons_subst_list()
+        glob = { "FOO" : "$BAR",
+                 "BAR" : "BAZ",
+                 "BLAT" : "XYX",
+                 "BARXYX" : "BADNEWS" }
+        newcom = scons_subst("$FOO$BLAT", glob, {})
+        assert newcom == "BAZXYX", newcom
+
+        # Test for double-dollar-sign behavior
+        glob = { "FOO" : "BAR",
+                 "BAZ" : "BLAT" }
+        newcom = scons_subst("$$FOO$BAZ", glob, {})
+        assert newcom == "$FOOBLAT", newcom
+
     def test_splitext(self):
         assert splitext('foo') == ('foo','')
         assert splitext('foo.bar') == ('foo','.bar')
@@ -141,6 +155,8 @@ class UtilTestCase(unittest.TestCase):
                 self.name = name
             def __str__(self):
                 return self.name
+            def is_literal(self):
+                return 1
         
         loc = {}
         loc['TARGETS'] = PathList(map(os.path.normpath, [ "./foo/bar.exe",
@@ -185,19 +201,13 @@ class UtilTestCase(unittest.TestCase):
         assert cmd_list[0][1] == '--in=foo.in', cmd_list[0][1]
         assert cmd_list[0][2] == '--out=bar with spaces.out', cmd_list[0][2]
 
-        # XXX: The newline in crazy really should be interpreted as
-        #      part of the file name, and not as delimiting a new command
-        #      line
-        #      In other words the following test fragment is illustrating
-        #      a bug in variable interpolation.
+        # This test is now fixed, and works like it should.
         cmd_list = scons_subst_list("$DO --in=$CRAZY --out=$BAR", loc, {})
-        assert len(cmd_list) == 2, cmd_list
-        assert len(cmd_list[0]) == 2, cmd_list
-        assert len(cmd_list[1]) == 2, cmd_list
+        assert len(cmd_list) == 1, map(str, cmd_list[0])
+        assert len(cmd_list[0]) == 3, cmd_list
         assert cmd_list[0][0] == 'do something', cmd_list[0][0]
-        assert cmd_list[0][1] == '--in=crazy', cmd_list[0][1]
-        assert cmd_list[1][0] == 'file.in', cmd_list[1][0]
-        assert cmd_list[1][1] == '--out=bar with spaces.out', cmd_list[1][1]
+        assert cmd_list[0][1] == '--in=crazy\nfile.in', cmd_list[0][1]
+        assert cmd_list[0][2] == '--out=bar with spaces.out', cmd_list[0][2]
         
         # Test inputting a list to scons_subst_list()
         cmd_list = scons_subst_list([ "$SOURCES$NEWLINE", "$TARGETS",
@@ -212,7 +222,36 @@ class UtilTestCase(unittest.TestCase):
         loc = {'a' : 3, 'c' : 4 }
         cmd_list = scons_subst_list("test $a $b $c $d test", glob, loc)
         assert len(cmd_list) == 1, cmd_list
-        assert cmd_list[0] == ['test', '3', '2', '4', 'test'], cmd_list
+        assert map(str, cmd_list[0]) == ['test', '3', '2', '4', 'test'], map(str, cmd_list[0])
+
+        # Test against a former bug in scons_subst_list()
+        glob = { "FOO" : "$BAR",
+                 "BAR" : "BAZ",
+                 "BLAT" : "XYX",
+                 "BARXYX" : "BADNEWS" }
+        cmd_list = scons_subst_list("$FOO$BLAT", glob, {})
+        assert cmd_list[0][0] == "BAZXYX", cmd_list[0][0]
+
+        # Test for double-dollar-sign behavior
+        glob = { "FOO" : "BAR",
+                 "BAZ" : "BLAT" }
+        cmd_list = scons_subst_list("$$FOO$BAZ", glob, {})
+        assert cmd_list[0][0] == "$FOOBLAT", cmd_list[0][0]
+
+        # Now test escape functionality
+        def escape_func(foo):
+            return '**' + foo + '**'
+        def quote_func(foo):
+            return foo
+        glob = { "FOO" : PathList([ 'foo\nwith\nnewlines',
+                                    'bar\nwith\nnewlines' ]) }
+        cmd_list = scons_subst_list("$FOO", glob, {})
+        assert cmd_list[0][0] == 'foo\nwith\nnewlines', cmd_list[0][0]
+        cmd_list[0][0].escape(escape_func)
+        assert cmd_list[0][0] == '**foo\nwith\nnewlines**', cmd_list[0][0]
+        assert cmd_list[0][1] == 'bar\nwith\nnewlines', cmd_list[0][0]
+        cmd_list[0][1].escape(escape_func)
+        assert cmd_list[0][1] == '**bar\nwith\nnewlines**', cmd_list[0][0]
 
     def test_quote_spaces(self):
         """Testing the quote_spaces() method..."""
@@ -424,6 +463,20 @@ class UtilTestCase(unittest.TestCase):
 
         assert p.baz == 5, p.baz
 
+    def test_Literal(self):
+        """Test the Literal() function."""
+        cmd_list = [ '$FOO', Literal('$BAR') ]
+        cmd_list = scons_subst_list(cmd_list,
+                                    { 'FOO' : 'BAZ',
+                                      'BAR' : 'BLAT' }, {})
+        def escape_func(cmd):
+            return '**' + cmd + '**'
+
+        map(lambda x, e=escape_func: x.escape(e), cmd_list[0])
+        cmd_list = map(str, cmd_list[0])
+        assert cmd_list[0] == 'BAZ', cmd_list[0]
+        assert cmd_list[1] == '**$BAR**', cmd_list[1]
+
 if __name__ == "__main__":
     suite = unittest.makeSuite(UtilTestCase, 'test_')
     if not unittest.TextTestRunner().run(suite).wasSuccessful():