Fix Command() when not called through an Environment.

8 files changed, 108 insertions, 47 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 1813fc3..16f76ba 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -35,7 +35,7 @@ RELEASE 0.96 - XXX
     behavior when the variable being appended to is a UserList object.
 
   - Fix a regression that prevented the Command() global function in
-    0.95 from working with command-line strings.
+    0.95 from working with command-line strings as actions.
 
   - Fix checking out a file from a source code management system when
     the env.SourceCode() method was called with an individual file name
diff --git a/src/engine/SCons/Action.py b/src/engine/SCons/Action.py
index c074de4..c3ee84a 100644
--- a/src/engine/SCons/Action.py
+++ b/src/engine/SCons/Action.py
@@ -341,7 +341,7 @@ class CommandAction(ActionBase):
             cmd = string.join(map(str, cmd))
         else:
             cmd = str(cmd)
-        return env.subst(cmd, SCons.Util.SUBST_SIG, target, source, dict)
+        return env.subst_target_source(cmd, SCons.Util.SUBST_SIG, target, source, dict)
 
 class CommandGeneratorAction(ActionBase):
     """Class for command-generator actions."""
diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index 75c82ae..e81af0f 100644
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -126,6 +126,7 @@ class Environment:
     # Just use the underlying scons_subst*() utility methods.
     def subst(self, strSubst, raw=0, target=[], source=[], dict=None):
         return SCons.Util.scons_subst(strSubst, self, raw, target, source, dict)
+    subst_target_source = subst
     def subst_list(self, strSubst, raw=0, target=[], source=[], dict=None):
         return SCons.Util.scons_subst_list(strSubst, self, raw, target, source, dict)
     def __getitem__(self, item):
@@ -696,7 +697,18 @@ class CommandActionTestCase(unittest.TestCase):
                                            baz = CmdGen))
         assert c == "| | FFF BBB 1", c
 
-        # We've discusssed using the real target and source names in a
+        # Make sure that CommandActions use an Environment's
+        # subst_target_source() method for substitution.
+        class SpecialEnvironment(Environment):
+            def subst_target_source(self, strSubst, raw=0, target=[], source=[], dict=None):
+                return 'subst_target_source: ' + strSubst
+
+        c = a.get_contents(target=DummyNode('ttt'), source = DummyNode('sss'),
+                           env=SpecialEnvironment(foo = 'GGG', bar = 'CCC',
+                                                  baz = 'ZZZ'))
+        assert c == 'subst_target_source: | $( $foo | $bar $) | $baz 1', c
+
+        # We've discussed using the real target and source names in a
         # CommandAction's signature contents.  This would have have the
         # advantage of recompiling when a file's name changes (keeping
         # debug info current), but it would currently break repository
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 477149b..e332f66 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -438,6 +438,8 @@ class Base:
             r.append(p)
         return r
 
+    subst_target_source = subst
+
     def _update(self, dict):
         """Update an environment's values directly, bypassing the normal
         checks that occur when users try to set items.
@@ -1200,12 +1202,28 @@ def NoSubstitutionProxy(subject):
             return getattr(self.__dict__['__subject'], name)
         def __setattr__(self, name, value):
             return setattr(self.__dict__['__subject'], name, value)
+        def raw_to_mode(self, dict):
+            try:
+                raw = dict['raw']
+            except KeyError:
+                pass
+            else:
+                del dict['raw']
+                dict['mode'] = raw
         def subst(self, string, *args, **kwargs):
             return string
         def subst_kw(self, kw, *args, **kwargs):
             return kw
         def subst_list(self, string, *args, **kwargs):
-            if not SCons.Util.is_List(string):
-                string = [[string]]
-            return string
+            nargs = (string, self,) + args
+            nkw = kwargs.copy()
+            nkw['gvars'] = {}
+            self.raw_to_mode(nkw)
+            return apply(SCons.Util.scons_subst_list, nargs, nkw)
+        def subst_target_source(self, string, *args, **kwargs):
+            nargs = (string, self,) + args
+            nkw = kwargs.copy()
+            nkw['gvars'] = {}
+            self.raw_to_mode(nkw)
+            return apply(SCons.Util.scons_subst, nargs, nkw)
     return _NoSubstitutionProxy(subject)
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index de313b4..b2b17e1 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -125,6 +125,18 @@ class CLVar(UserList.UserList):
 
 
 
+class DummyNode:
+    def __init__(self, name):
+        self.name = name
+    def __str__(self):
+        return self.name
+    def rfile(self):
+        return self
+    def get_subst_proxy(self):
+        return self
+
+
+
 class EnvironmentTestCase(unittest.TestCase):
 
     def test___init__(self):
@@ -294,16 +306,6 @@ class EnvironmentTestCase(unittest.TestCase):
         mystr = env.subst("$AAA ${AAA}A ${AAA}B $BBB")
         assert mystr == "c cA cB c", mystr
 
-        class DummyNode:
-            def __init__(self, name):
-                self.name = name
-            def __str__(self):
-                return self.name
-            def rfile(self):
-                return self
-            def get_subst_proxy(self):
-                return self
-
         t1 = DummyNode('t1')
         t2 = DummyNode('t2')
         s1 = DummyNode('s1')
@@ -375,16 +377,6 @@ class EnvironmentTestCase(unittest.TestCase):
         lst = env.subst_list([ "$AAA", "B $CCC" ])
         assert lst == [[ "a", "b"], ["c", "B a", "b"], ["c"]], lst
 
-        class DummyNode:
-            def __init__(self, name):
-                self.name = name
-            def __str__(self):
-                return self.name
-            def rfile(self):
-                return self
-            def get_subst_proxy(self):
-                return self
-
         t1 = DummyNode('t1')
         t2 = DummyNode('t2')
         s1 = DummyNode('s1')
@@ -2431,12 +2423,28 @@ class NoSubstitutionProxyTestCase(unittest.TestCase):
         x = env.subst_list('$XXX')
         assert x == [['x']], x
         x = proxy.subst_list('$XXX')
-        assert x == [['$XXX']], x
+        assert x == [[]], x
+
+        x = proxy.subst_list('$YYY', raw=0, target=None, source=None,
+                             dict=None, conv=None)
+        assert x == [[]], x
+
+    def test_subst_target_source(self):
+        """Test the NoSubstitutionProxy.subst_target_source() method"""
+        env = Environment(XXX = 'x', YYY = 'y')
+        assert env['XXX'] == 'x', env['XXX']
+        assert env['YYY'] == 'y', env['YYY']
+
+        proxy = NoSubstitutionProxy(env)
+        assert proxy['XXX'] == 'x', proxy['XXX']
+        assert proxy['YYY'] == 'y', proxy['YYY']
 
-        x = proxy.subst_list('$YYY', raw=7, target=None, source=None,
-                             dict=None, conv=None,
-                             extra_meaningless_keyword_argument=None)
-        assert x == [['$YYY']], x
+        args = ('$XXX $TARGET $SOURCE $YYY',)
+        kw = {'target' : DummyNode('ttt'), 'source' : DummyNode('sss')}
+        x = apply(env.subst_target_source, args, kw)
+        assert x == 'x ttt sss y', x
+        x = apply(proxy.subst_target_source, args, kw)
+        assert x == ' ttt sss ', x
 
 
 
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 2ede614..e07675c 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -501,7 +501,7 @@ _separate_args = re.compile(r'(\$[\$\(\)]|\$[_a-zA-Z][\.\w]*|\${[^}]*}|\s+|[^\s\
 # space characters in the string result from the scons_subst() function.
 _space_sep = re.compile(r'[\t ]+(?![^{]*})')
 
-def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=None, conv=None):
+def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=None, conv=None, gvars=None):
     """Expand a string containing construction variable substitutions.
 
     This is the work-horse function for substitutions in file names
@@ -516,14 +516,13 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=No
         source with two methods (substitute() and expand()) that handle
         the expansion.
         """
-        def __init__(self, env, mode, target, source, conv):
+        def __init__(self, env, mode, target, source, conv, gvars):
             self.env = env
             self.mode = mode
             self.target = target
             self.source = source
-
-            self.gvars = env.Dictionary()
             self.conv = conv
+            self.gvars = gvars
 
         def expand(self, s, lvars):
             """Expand a single "token" as necessary, returning an
@@ -606,8 +605,10 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=No
         dict = subst_dict(target, source)
     if conv is None:
         conv = _strconv[mode]
+    if gvars is None:
+        gvars = env.Dictionary()
 
-    ss = StringSubber(env, mode, target, source, conv)
+    ss = StringSubber(env, mode, target, source, conv, gvars)
     result = ss.substitute(strSubst, dict)
 
     if is_String(result):
@@ -623,7 +624,7 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=No
 
     return result
 
-def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=None, conv=None):
+def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, dict=None, conv=None, gvars=None):
     """Substitute construction variables in a string (or list or other
     object) and separate the arguments into a command list.
 
@@ -647,20 +648,19 @@ def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, di
         and the rest of the object takes care of doing the right thing
         internally.
         """
-        def __init__(self, env, mode, target, source, conv):
+        def __init__(self, env, mode, target, source, conv, gvars):
             UserList.UserList.__init__(self, [])
             self.env = env
             self.mode = mode
             self.target = target
             self.source = source
-
-            self.gvars = env.Dictionary()
+            self.conv = conv
+            self.gvars = gvars
 
             if self.mode == SUBST_RAW:
                 self.add_strip = lambda x, s=self: s.append(x)
             else:
                 self.add_strip = lambda x, s=self: None
-            self.conv = conv
             self.in_strip = None
             self.next_line()
 
@@ -793,8 +793,10 @@ def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, di
         dict = subst_dict(target, source)
     if conv is None:
         conv = _strconv[mode]
+    if gvars is None:
+        gvars = env.Dictionary()
 
-    ls = ListSubber(env, mode, target, source, conv)
+    ls = ListSubber(env, mode, target, source, conv, gvars)
     ls.substitute(strSubst, dict, 0)
 
     return ls.data
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 38e8c30..098c2d9 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -429,7 +429,7 @@ class UtilTestCase(unittest.TestCase):
         else:
             raise AssertionError, "did not catch expected UserError"
 
-        # Test we handle overriding the internal conversion routines.
+        # Test how we handle overriding the internal conversion routines.
         def s(obj):
             return obj
 
@@ -451,6 +451,13 @@ class UtilTestCase(unittest.TestCase):
         #func = scons_subst("$FUNCTION", env, mode=SUBST_SIG, call=None)
         #assert func is function_foo, func
 
+        # Test supplying an overriding gvars dictionary.
+        env = DummyEnv({'XXX' : 'xxx'})
+        result = scons_subst('$XXX', env)
+        assert result == 'xxx', result
+        result = scons_subst('$XXX', env, gvars={'XXX' : 'yyy'})
+        assert result == 'yyy', result
+
     def test_subst_list(self):
         """Testing the scons_subst_list() method..."""
         class MyNode(DummyNode):
@@ -797,6 +804,13 @@ class UtilTestCase(unittest.TestCase):
         node = scons_subst_list("$NODE", env, mode=SUBST_SIG, conv=s)
         assert node == [[n1]], node
 
+        # Test supplying an overriding gvars dictionary.
+        env = DummyEnv({'XXX' : 'xxx'})
+        result = scons_subst_list('$XXX', env)
+        assert result == [['xxx']], result
+        result = scons_subst_list('$XXX', env, gvars={'XXX' : 'yyy'})
+        assert result == [['yyy']], result
+
     def test_subst_once(self):
         """Testing the scons_subst_once() method"""
 
diff --git a/test/Command.py b/test/Command.py
index 2ee09db..f9111c5 100644
--- a/test/Command.py
+++ b/test/Command.py
@@ -69,8 +69,7 @@ env = Environment()
 env.Command(target = 'f1.out', source = 'f1.in',
             action = buildIt)
 env.Command(target = 'f2.out', source = 'f2.in',
-            action = r'%s' + " build.py temp2 $SOURCES\\n" + r'%s' + " build.py $TARGET temp2")
-
+            action = r"%s build.py temp2 $SOURCES" + '\\n' + r"%s build.py $TARGET temp2")
 env.Command(target = 'f3.out', source = 'f3.in',
             action = [ [ r'%s', 'build.py', 'temp3', '$SOURCES' ],
                        [ r'%s', 'build.py', '$TARGET', 'temp3'] ])
@@ -78,8 +77,12 @@ Command(target = 'f4.out', source = 'sub', action = sub)
 env.Command(target = 'f5.out', source = 'f5.in', action = buildIt,
             XYZZY="XYZZY is set")
 Command(target = 'f6.out', source = 'f6.in',
-        action = r'%s' + " build.py f6.out f6.in")
-""" % (python, python, python, python, python))
+        action = r"%s build.py f6.out f6.in")
+env.Command(target = 'f7.out', source = 'f7.in',
+            action = r"%s build.py $TARGET $SOURCE")
+Command(target = 'f8.out', source = 'f8.in',
+        action = r"%s build.py $TARGET $SOURCE")
+""" % (python, python, python, python, python, python, python))
 
 test.write('f1.in', "f1.in\n")
 test.write('f2.in', "f2.in\n")
@@ -89,6 +92,8 @@ test.write(['sub', 'f4b'], "sub/f4b\n")
 test.write(['sub', 'f4c'], "sub/f4c\n")
 test.write('f5.in', "f5.in\n")
 test.write('f6.in', "f6.in\n")
+test.write('f7.in', "f7.in\n")
+test.write('f8.in', "f8.in\n")
 
 test.run(arguments = '.')
 
@@ -98,5 +103,7 @@ test.must_match('f3.out', "f3.in\n")
 test.must_match('f4.out', "sub/f4a\nsub/f4b\nsub/f4c\n")
 test.must_match('f5.out', "XYZZY is set\nf5.in\n")
 test.must_match('f6.out', "f6.in\n")
+test.must_match('f7.out', "f7.in\n")
+test.must_match('f8.out', "f8.in\n")
 
 test.pass_test()