Allow nested $( $) sections

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 4a7576e0988a5fc4ef07526e16ef534ba25964a6..ad53d7da4209492d0f629992fff2f3b085de6986 100644 (file)
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -62,6 +62,8 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Fixed PCHPDBFLAGS causing a deprecation warning on MSVC v8 and later when
       using PCHs and PDBs together.
 
+  From Tom Tanner:
+    - Allow nested $( ... $) sections
 
 RELEASE 2.5.1 - Mon, 03 Nov 2016 13:37:42 -0400
 
@@ -146,7 +148,7 @@ RELEASE 2.4.1 - Mon, 07 Nov 2015 10:37:21 -0700
       and continuing.
 
   From Hiroaki Itoh :
-    - Add support `Microsoft Visual C++ Compiler for Python 2.7' 
+    - Add support `Microsoft Visual C++ Compiler for Python 2.7'
       Compiler can be obtained at: https://www.microsoft.com/en-us/download/details.aspx?id=44266
 
   From Florian Miedniak:
@@ -316,7 +318,6 @@ RELEASE 2.3.2
   From Paweł Tomulik:
     - Fix SConf tests that write output
 
->>>>>>> other
   From Gary Oberbrunner:
     - get default RPM architecture more robustly when building RPMs
 

diff --git a/src/engine/SCons/ActionTests.py b/src/engine/SCons/ActionTests.py
index b790ccc6c283e8d97ef8442b3d9d97a1009bd28e..f0c48ea9a1e436166c97ebac353a74790c4b2d91 100644 (file)
--- a/src/engine/SCons/ActionTests.py
+++ b/src/engine/SCons/ActionTests.py
@@ -1241,8 +1241,8 @@ class CommandActionTestCase(unittest.TestCase):
                    (env["foo"], env["bar"])
 
         # The number 1 is there to make sure all args get converted to strings.
-        a = SCons.Action.CommandAction(["|", "$(", "$foo", "|", "$bar",
-                                        "$)", "|", "$baz", 1])
+        a = SCons.Action.CommandAction(["|", "$(", "$foo", "|", "$(", "$bar",
+                                        "$)", "stuff",  "$)", "|", "$baz", 1])
         c = a.get_contents(target=[], source=[],
                            env=Environment(foo = 'FFF', bar = 'BBB',
                                            baz = CmdGen))
@@ -1257,7 +1257,7 @@ class CommandActionTestCase(unittest.TestCase):
         c = a.get_contents(target=DummyNode('ttt'), source = DummyNode('sss'),
                            env=SpecialEnvironment(foo = 'GGG', bar = 'CCC',
                                                   baz = 'ZZZ'))
-        assert c == b'subst_target_source: | $( $foo | $bar $) | $baz 1', c
+        assert c == b'subst_target_source: | $( $foo | $( $bar $) stuff $) | $baz 1', c
 
         # We've discussed using the real target and source names in a
         # CommandAction's signature contents.  This would have have the

diff --git a/src/engine/SCons/Subst.py b/src/engine/SCons/Subst.py
index 3c9b390d7faeaac9f7bfae33c22f7e771cf40476..b81d79c84828ce2d99180a41c071592f31485e49 100644 (file)
--- a/src/engine/SCons/Subst.py
+++ b/src/engine/SCons/Subst.py
@@ -338,24 +338,28 @@ SUBST_RAW = 1
 SUBST_SIG = 2
 
 _rm = re.compile(r'\$[()]')
-_remove = re.compile(r'\$\([^\$]*(\$[^\)][^\$]*)*\$\)')
+_rm_split = re.compile(r'(\$[()])')
 
 # Indexed by the SUBST_* constants above.
-_regex_remove = [ _rm, None, _remove ]
+_regex_remove = [ _rm, None, _rm_split ]
 
 def _rm_list(list):
     return [l for l in list if not l in ('$(', '$)')]
 
 def _remove_list(list):
     result = []
-    do_append = result.append
+    depth = 0
     for l in list:
         if l == '$(':
-            do_append = lambda x: None
+            depth += 1
         elif l == '$)':
-            do_append = result.append
-        else:
-            do_append(l)
+            depth -= 1
+            if depth < 0:
+                break
+        elif depth == 0:
+            result.append(l)
+    if depth != 0:
+        return None
     return result
 
 # Indexed by the SUBST_* constants above.
@@ -562,12 +566,19 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars={
     except KeyError:
         pass
 
+    res = result
     if is_String(result):
         # Remove $(-$) pairs and any stuff in between,
         # if that's appropriate.
         remove = _regex_remove[mode]
         if remove:
-            result = remove.sub('', result)
+            if mode == SUBST_SIG:
+                result = _list_remove[mode](remove.split(result))
+                if result is None:
+                    raise SCons.Errors.UserError("Unbalanced $(/$) in: " + res)
+                result = ' '.join(result)
+            else:
+                result = remove.sub('', result)
         if mode != SUBST_RAW:
             # Compress strings of white space characters into
             # a single space.
@@ -576,6 +587,8 @@ def scons_subst(strSubst, env, mode=SUBST_RAW, target=None, source=None, gvars={
         remove = _list_remove[mode]
         if remove:
             result = remove(result)
+            if result is None:
+                raise SCons.Errors.UserError("Unbalanced $(/$) in: " + str(res))
 
     return result
 

diff --git a/src/engine/SCons/SubstTests.py b/src/engine/SCons/SubstTests.py
index 8eda845b44a15db0009fa5b6b0de2c9b44985375..f2cbc3f95c2203b6e67abed3d94a41ffe59e582f 100644 (file)
--- a/src/engine/SCons/SubstTests.py
+++ b/src/engine/SCons/SubstTests.py
@@ -183,6 +183,9 @@ class SubstTestCase(unittest.TestCase):
         'HHH'       : 'III',
         'FFFIII'    : 'BADNEWS',
 
+        'THING1'    : "$(STUFF$)",
+        'THING2'    : "$THING1",
+
         'LITERAL'   : TestLiteral("$XXX"),
 
         # Test that we can expand to and return a function.
@@ -405,6 +408,11 @@ class scons_subst_TestCase(SubstTestCase):
             "test",
             "test",
 
+        "test $( $THING2 $)",
+            "test $( $(STUFF$) $)",
+            "test STUFF",
+            "test",
+
         "$AAA ${AAA}A $BBBB $BBB",
             "a aA  b",
             "a aA b",
@@ -544,6 +552,23 @@ class scons_subst_TestCase(SubstTestCase):
         else:
             raise AssertionError("did not catch expected UserError")
 
+    def test_subst_balance_errors(self):
+        """Test scons_subst():  handling syntax errors"""
+        env = DummyEnv(self.loc)
+        try:
+            scons_subst('$(', env, mode=SUBST_SIG)
+        except SCons.Errors.UserError as e:
+            assert str(e) == "Unbalanced $(/$) in: $(", str(e)
+        else:
+            raise AssertionError("did not catch expected UserError")
+
+        try:
+            scons_subst('$)', env, mode=SUBST_SIG)
+        except SCons.Errors.UserError as e:
+            assert str(e) == "Unbalanced $(/$) in: $)", str(e)
+        else:
+            raise AssertionError("did not catch expected UserError")
+
     def test_subst_type_errors(self):
         """Test scons_subst():  handling type errors"""
         env = DummyEnv(self.loc)