Fix the Command() global function when the action is a command-line string.

5 files changed, 139 insertions, 31 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 538d0c0..648e910 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -34,6 +34,9 @@ RELEASE 0.96 - XXX
   - Make env.Append() and env.Prepend() act like the underlying Python
     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.
+
   From Gary Oberbrunner:
 
   - Add a --debug=presub option to print actions prior to substitution.
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index 62650d7..477149b 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -1179,3 +1179,33 @@ class Base:
 # class to SCons.Environment.Environment.
 
 Environment = Base
+
+# An entry point for returning a proxy subclass instance that overrides
+# the subst*() methods so they don't actually perform construction
+# variable substitution.  This is specifically intended to be the shim
+# layer in between global function calls (which don't want construction
+# variable substitution) and the DefaultEnvironment() (which would
+# substitute variables if left to its own devices)."""
+#
+# We have to wrap this in a function that allows us to delay definition of
+# the class until it's necessary, so that when it subclasses Environment
+# it will pick up whatever Environment subclass the wrapper interface
+# might have assigned to SCons.Environment.Environment.
+
+def NoSubstitutionProxy(subject):
+    class _NoSubstitutionProxy(Environment):
+        def __init__(self, subject):
+            self.__dict__['__subject'] = subject
+        def __getattr__(self, name):
+            return getattr(self.__dict__['__subject'], name)
+        def __setattr__(self, name, value):
+            return setattr(self.__dict__['__subject'], name, value)
+        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
+    return _NoSubstitutionProxy(subject)
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index 8d1c3f3..de313b4 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -2352,7 +2352,100 @@ class EnvironmentTestCase(unittest.TestCase):
         assert f == 'foo', f
 
 
+
+class NoSubstitutionProxyTestCase(unittest.TestCase):
+
+    def test___init__(self):
+        """Test NoSubstitutionProxy initialization"""
+        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']
+
+    def test_attributes(self):
+        """Test getting and setting NoSubstitutionProxy attributes"""
+        env = Environment()
+        setattr(env, 'env_attr', 'value1')
+
+        proxy = NoSubstitutionProxy(env)
+        setattr(proxy, 'proxy_attr', 'value2')
+
+        x = getattr(env, 'env_attr')
+        assert x == 'value1', x
+        x = getattr(proxy, 'env_attr')
+        assert x == 'value1', x
+
+        x = getattr(env, 'proxy_attr')
+        assert x == 'value2', x
+        x = getattr(proxy, 'proxy_attr')
+        assert x == 'value2', x
+
+    def test_subst(self):
+        """Test the NoSubstitutionProxy.subst() 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 = env.subst('$XXX')
+        assert x == 'x', x
+        x = proxy.subst('$XXX')
+        assert x == '$XXX', x
+
+        x = proxy.subst('$YYY', raw=7, target=None, source=None,
+                        dict=None, conv=None,
+                        extra_meaningless_keyword_argument=None)
+        assert x == '$YYY', x
+
+    def test_subst_kw(self):
+        """Test the NoSubstitutionProxy.subst_kw() 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 = env.subst_kw({'$XXX':'$YYY'})
+        assert x == {'x':'y'}, x
+        x = proxy.subst_kw({'$XXX':'$YYY'})
+        assert x == {'$XXX':'$YYY'}, x
+
+    def test_subst_list(self):
+        """Test the NoSubstitutionProxy.subst_list() 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 = env.subst_list('$XXX')
+        assert x == [['x']], x
+        x = proxy.subst_list('$XXX')
+        assert x == [['$XXX']], x
+
+        x = proxy.subst_list('$YYY', raw=7, target=None, source=None,
+                             dict=None, conv=None,
+                             extra_meaningless_keyword_argument=None)
+        assert x == [['$YYY']], x
+
+
+
 if __name__ == "__main__":
-    suite = unittest.makeSuite(EnvironmentTestCase, 'test_')
+    suite = unittest.TestSuite()
+    tclasses = [ EnvironmentTestCase,
+                 NoSubstitutionProxyTestCase ]
+    for tclass in tclasses:
+        names = unittest.getTestCaseNames(tclass, 'test_')
+        suite.addTests(map(tclass, names))
     if not unittest.TextTestRunner().run(suite).wasSuccessful():
-	sys.exit(1)
+        sys.exit(1)
diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py
index d6e40fd..417e1c0 100644
--- a/src/engine/SCons/Script/SConscript.py
+++ b/src/engine/SCons/Script/SConscript.py
@@ -550,30 +550,8 @@ _DefaultEnvironmentProxy = None
 def get_DefaultEnvironmentProxy():
     global _DefaultEnvironmentProxy
     if not _DefaultEnvironmentProxy:
-        class EnvironmentProxy(SCons.Environment.Environment):
-            """A proxy subclass for an environment instance that overrides
-            the subst() and subst_list() methods so they don't actually
-            actually perform construction variable substitution.  This is
-            specifically intended to be the shim layer in between global
-            function calls (which don't want want construction variable
-            substitution) and the DefaultEnvironment() (which would
-            substitute variables if left to its own devices)."""
-            def __init__(self, subject):
-                self.__dict__['__subject'] = subject
-            def __getattr__(self, name):
-                return getattr(self.__dict__['__subject'], name)
-            def __setattr__(self, name, value):
-                return setattr(self.__dict__['__subject'], name, value)
-            def subst(self, string, raw=0, target=None, source=None):
-                return string
-            def subst_kw(self, kw, raw=0, target=None, source=None):
-                return kw
-            def subst_list(self, string, raw=0, target=None, source=None):
-                if not SCons.Util.is_List(string):
-                    string = [[string]]
-                return string
         default_env = SCons.Defaults.DefaultEnvironment()
-        _DefaultEnvironmentProxy = EnvironmentProxy(default_env)
+        _DefaultEnvironmentProxy = SCons.Environment.NoSubstitutionProxy(default_env)
     return _DefaultEnvironmentProxy
 
 class DefaultEnvironmentCall:
diff --git a/test/Command.py b/test/Command.py
index 49a6ae3..2ee09db 100644
--- a/test/Command.py
+++ b/test/Command.py
@@ -77,7 +77,9 @@ env.Command(target = 'f3.out', source = 'f3.in',
 Command(target = 'f4.out', source = 'sub', action = sub)
 env.Command(target = 'f5.out', source = 'f5.in', action = buildIt,
             XYZZY="XYZZY is set")
-""" % (python, python, python, python))
+Command(target = 'f6.out', source = 'f6.in',
+        action = r'%s' + " build.py f6.out f6.in")
+""" % (python, python, python, python, python))
 
 test.write('f1.in', "f1.in\n")
 test.write('f2.in', "f2.in\n")
@@ -86,13 +88,15 @@ test.write(['sub', 'f4a'], "sub/f4a\n")
 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.run(arguments = '.')
 
-test.fail_test(test.read('f1.out') != "f1.in\n")
-test.fail_test(test.read('f2.out') != "f2.in\n")
-test.fail_test(test.read('f3.out') != "f3.in\n")
-test.fail_test(test.read('f4.out') != "sub/f4a\nsub/f4b\nsub/f4c\n")
-test.fail_test(test.read('f5.out') != "XYZZY is set\nf5.in\n")
+test.must_match('f1.out', "f1.in\n")
+test.must_match('f2.out', "f2.in\n")
+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.pass_test()