Handle recursive substitution in overrides.

8 files changed, 195 insertions, 15 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 42c5de1..be8dfa0 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -125,7 +125,8 @@ RELEASE 0.95 - XXX
 
   - Refactor construction variable expansion to support recursive
     expansion of variables (e.g. CCFLAGS = "$CCFLAGS -g") without going
-    into an infinite loop.
+    into an infinite loop.  Support this in all construction variable
+    overrides, as well as when copying Environments.
 
   - Fix calling Configure() from more than one subsidiary SConscript file.
 
diff --git a/src/engine/SCons/Environment.py b/src/engine/SCons/Environment.py
index a626204..c6bc2a9 100644
--- a/src/engine/SCons/Environment.py
+++ b/src/engine/SCons/Environment.py
@@ -505,7 +505,10 @@ class Base:
         apply_tools(clone, tools, toolpath)
 
         # Apply passed-in variables after the new tools.
-        apply(clone.Replace, (), kw)
+        new = {}
+        for key, value in kw.items():
+            new[key] = SCons.Util.scons_subst_once(value, self, key)
+        apply(clone.Replace, (), new)
         return clone
 
     def Detect(self, progs):
@@ -561,7 +564,10 @@ class Base:
             env = copy.copy(self)
             env._dict = copy.copy(self._dict)
             env['__env__'] = env
-            env._dict.update(overrides)
+            new = {}
+            for key, value in overrides.items():
+                new[key] = SCons.Util.scons_subst_once(value, self, key)
+            env._dict.update(new)
             return env
         else:
             return self
diff --git a/src/engine/SCons/EnvironmentTests.py b/src/engine/SCons/EnvironmentTests.py
index 330bb7c..a5cf171 100644
--- a/src/engine/SCons/EnvironmentTests.py
+++ b/src/engine/SCons/EnvironmentTests.py
@@ -1019,6 +1019,16 @@ class EnvironmentTestCase(unittest.TestCase):
         assert env3.get('BAR') is 2
         assert env3.get('BAZ') is 3
 
+        # Ensure that recursive variable substitution when copying
+        # environments works properly.
+        env1 = Environment(CCFLAGS = '-DFOO', XYZ = '-DXYZ')
+        env2 = env1.Copy(CCFLAGS = '$CCFLAGS -DBAR',
+                         XYZ = ['-DABC', 'x $XYZ y', '-DDEF'])
+        x = env2.get('CCFLAGS')
+        assert x == '-DFOO -DBAR', x
+        x = env2.get('XYZ')
+        assert x == ['-DABC', 'x -DXYZ y', '-DDEF'], x
+
     def test_Detect(self):
         """Test Detect()ing tools"""
         test = TestCmd.TestCmd(workdir = '')
@@ -1122,16 +1132,28 @@ class EnvironmentTestCase(unittest.TestCase):
 
     def test_Override(self):
         "Test overriding construction variables"
-        env = Environment(ONE=1, TWO=2)
-        assert env['ONE'] == 1
-        assert env['TWO'] == 2
-        env2 = env.Override({'TWO':'10'})
-        assert env2['ONE'] == 1
-        assert env2['TWO'] == '10'
-        assert env['TWO'] == 2
+        env = Environment(ONE=1, TWO=2, THREE=3, FOUR=4)
+        assert env['ONE'] == 1, env['ONE']
+        assert env['TWO'] == 2, env['TWO']
+        assert env['THREE'] == 3, env['THREE']
+        assert env['FOUR'] == 4, env['FOUR']
+
+        env2 = env.Override({'TWO'   : '10',
+                             'THREE' :'x $THREE y',
+                             'FOUR'  : ['x', '$FOUR', 'y']})
+        assert env2['ONE'] == 1, env2['ONE']
+        assert env2['TWO'] == '10', env2['TWO']
+        assert env2['THREE'] == 'x 3 y', env2['THREE']
+        assert env2['FOUR'] == ['x', 4, 'y'], env2['FOUR']
+
+        assert env['ONE'] == 1, env['ONE']
+        assert env['TWO'] == 2, env['TWO']
+        assert env['THREE'] == 3, env['THREE']
+        assert env['FOUR'] == 4, env['FOUR']
+
         env2.Replace(ONE = "won")
-        assert env2['ONE'] == "won"
-        assert env['ONE'] == 1
+        assert env2['ONE'] == "won", env2['ONE']
+        assert env['ONE'] == 1, env['ONE']
 
         assert env['__env__'] is env, env['__env__']
         assert env2['__env__'] is env2, env2['__env__']
diff --git a/src/engine/SCons/Executor.py b/src/engine/SCons/Executor.py
index 8151f87..b636f60 100644
--- a/src/engine/SCons/Executor.py
+++ b/src/engine/SCons/Executor.py
@@ -69,6 +69,12 @@ class Executor:
                 # The normal case:  use the Environment that was
                 # used to specify how these targets will be built.
                 env = self.env
+
+            # Create the build environment instance with appropriate
+            # overrides.  These get evaluated against the current
+            # environment's construction variables so that users can
+            # add to existing values by referencing the variable in
+            # the expansion.
             overrides = {}
             overrides.update(self.builder.overrides)
             overrides.update(self.overrides)
@@ -78,8 +84,13 @@ class Executor:
                 pass
             else:
                 overrides.update(generate_build_dict())
-            overrides.update(SCons.Util.subst_dict(self.targets, self.sources))
             self.build_env = env.Override(overrides)
+
+            # Now update the build environment with the things that we
+            # don't want expanded against the current construction
+            # variables.
+            self.build_env._update(SCons.Util.subst_dict(self.targets,
+                                                         self.sources))
             return self.build_env
 
     def get_action_list(self, target):
diff --git a/src/engine/SCons/ExecutorTests.py b/src/engine/SCons/ExecutorTests.py
index d43f7e2..9c64012 100644
--- a/src/engine/SCons/ExecutorTests.py
+++ b/src/engine/SCons/ExecutorTests.py
@@ -34,10 +34,12 @@ class MyEnvironment:
     def __init__(self, **kw):
         self._dict = {}
         self._dict.update(kw)
+    def __getitem__(self, key):
+        return self._dict[key]
     def Override(self, overrides):
         d = self._dict.copy()
         d.update(overrides)
-        return d
+        return apply(MyEnvironment, (), d)
     def _update(self, dict):
         self._dict.update(dict)
 
diff --git a/src/engine/SCons/Node/NodeTests.py b/src/engine/SCons/Node/NodeTests.py
index 2e053be..63b945f 100644
--- a/src/engine/SCons/Node/NodeTests.py
+++ b/src/engine/SCons/Node/NodeTests.py
@@ -78,10 +78,19 @@ class MyNonGlobalAction:
         return [self]
 
 class Environment:
+    def __init__(self, **kw):
+        self._dict = {}
+        self._dict.update(kw)
+    def __getitem__(self, key):
+        return self._dict[key]
     def Dictionary(self, *args):
         return {}
     def Override(self, overrides):
-        return overrides
+        d = self._dict.copy()
+        d.update(overrides)
+        return apply(Environment, (), d)
+    def _update(self, dict):
+        self._dict.update(dict)
 
 class Builder:
     def __init__(self):
diff --git a/src/engine/SCons/Util.py b/src/engine/SCons/Util.py
index 6643393..4ca25b2 100644
--- a/src/engine/SCons/Util.py
+++ b/src/engine/SCons/Util.py
@@ -788,6 +788,55 @@ def scons_subst_list(strSubst, env, mode=SUBST_RAW, target=None, source=None, di
 
     return ls.data
 
+def scons_subst_once(strSubst, env, key):
+    """Perform single (non-recursive) substitution of a single
+    construction variable keyword.
+
+    This is used when setting a variable when copying or overriding values
+    in an Environment.  We want to capture (expand) the old value before
+    we override it, so people can do things like:
+
+        env2 = env.Copy(CCFLAGS = '$CCFLAGS -g')
+
+    We do this with some straightforward, brute-force code here...
+    """
+    matchlist = ['$' + key, '${' + key + '}']
+    if is_List(strSubst):
+        result = []
+        for arg in strSubst:
+            if is_String(arg):
+                if arg in matchlist:
+                    arg = env[key]
+                    if is_List(arg):
+                        result.extend(arg)
+                    else:
+                        result.append(arg)
+                else:
+                    r = []
+                    for a in _separate_args.findall(arg):
+                        if a in matchlist:
+                            a = env[key]
+                        if is_List(a):
+                            r.extend(string.join(map(str, a)))
+                        else:
+                            r.append(str(a))
+                    result.append(string.join(r, ''))
+            else:
+                result.append(arg)
+        return result
+    elif is_String(strSubst):
+        result = []
+        for a in _separate_args.findall(strSubst):
+            if a in matchlist:
+                a = env[key]
+            if is_List(a):
+                result.extend(string.join(map(str, a)))
+            else:
+                result.append(str(a))
+        return string.join(result, '')
+    else:
+        return strSubst
+
 def render_tree(root, child_func, prune=0, margin=[0], visited={}):
     """
     Render a tree of nodes into an ASCII tree view.
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index ce82013..d981abc 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -63,6 +63,9 @@ class DummyEnv:
             return self.dict
         return self.dict[key]
 
+    def __getitem__(self, key):
+        return self.dict[key]
+
     def sig_dict(self):
         dict = self.dict.copy()
         dict["TARGETS"] = 'tsig'
@@ -289,6 +292,9 @@ class UtilTestCase(unittest.TestCase):
             '$S',                   'x y',
             '$LS',                  'x y',
             '$L',                   'x y',
+            '$S z',                 'x y z',
+            '$LS z',                'x y z',
+            '$L z',                 'x y z',
             #cs,                     'cs',
             #cl,                     'cl',
             '$CS',                  'cs',
@@ -590,11 +596,20 @@ class UtilTestCase(unittest.TestCase):
             #'$R',                   [[]],
             #['$R'],                 [[]],
             '$S',                   [['x', 'y']],
+            '$S z',                 [['x', 'y', 'z']],
             ['$S'],                 [['x', 'y']],
+            ['$S z'],               [['x', 'y z']],     # XXX - IS THIS BEST?
+            ['$S', 'z'],            [['x', 'y', 'z']],
             '$LS',                  [['x y']],
+            '$LS z',                [['x y', 'z']],
             ['$LS'],                [['x y']],
+            ['$LS z'],              [['x y z']],
+            ['$LS', 'z'],           [['x y', 'z']],
             '$L',                   [['x', 'y']],
+            '$L z',                 [['x', 'y', 'z']],
             ['$L'],                 [['x', 'y']],
+            ['$L z'],               [['x', 'y z']],     # XXX - IS THIS BEST?
+            ['$L', 'z'],            [['x', 'y', 'z']],
             cs,                     [['cs']],
             [cs],                   [['cs']],
             cl,                     [['cl']],
@@ -756,6 +771,71 @@ class UtilTestCase(unittest.TestCase):
         else:
             raise AssertionError, "did not catch expected SyntaxError"
 
+    def test_subst_once(self):
+        """Testing the scons_subst_once() method"""
+
+        loc = {
+            'CCFLAGS'           : '-DFOO',
+            'ONE'               : 1,
+            'RECURSE'           : 'r $RECURSE r',
+            'LIST'              : ['a', 'b', 'c'],
+        }
+
+        env = DummyEnv(loc)
+
+        cases = [
+            '$CCFLAGS -DBAR',
+            'OTHER_KEY',
+            '$CCFLAGS -DBAR',
+
+            '$CCFLAGS -DBAR',
+            'CCFLAGS',
+            '-DFOO -DBAR',
+
+            'x $ONE y',
+            'ONE',
+            'x 1 y',
+
+            'x $RECURSE y',
+            'RECURSE',
+            'x r $RECURSE r y',
+
+            '$LIST',
+            'LIST',
+            'a b c',
+
+            ['$LIST'],
+            'LIST',
+            ['a', 'b', 'c'],
+
+            ['x', '$LIST', 'y'],
+            'LIST',
+            ['x', 'a', 'b', 'c', 'y'],
+
+            ['x', 'x $LIST y', 'y'],
+            'LIST',
+            ['x', 'x a b c y', 'y'],
+
+            ['x', 'x $CCFLAGS y', 'y'],
+            'LIST',
+            ['x', 'x $CCFLAGS y', 'y'],
+
+            ['x', 'x $RECURSE y', 'y'],
+            'LIST',
+            ['x', 'x $RECURSE y', 'y'],
+        ]
+
+        failed = 0
+        while cases:
+            input, key, expect = cases[:3]
+            result = scons_subst_once(input, env, key)
+            if result != expect:
+                if failed == 0: print
+                print "    input %s (%s) => %s did not match %s" % (repr(input), repr(key), repr(result), repr(expect))
+                failed = failed + 1
+            del cases[:3]
+        assert failed == 0, "%d subst() cases failed" % failed
+
     def test_splitext(self):
         assert splitext('foo') == ('foo','')
         assert splitext('foo.bar') == ('foo','.bar')