Fix problem when MergeFlags adds to existing CPPDEFINES

MergeFlags has a post-processing step if the *unique* flag evaluates True
which loops through and removes the duplicates. This step uses slicing
(for v in orig[::-1]), which fails if the item being cleaned is a deque -
which CPPDEFINES can now be. It would also cause the deque to be
replaced with a list.  Detect this case and handle separately.

Note the same post-processing step assures each modified object will
be replaced - Override(parse_flags=xxx) silently counted on this so
it does not end up sharing variables with the overridden env. This
situation remains, and is accounted for by the patch.

Unit test and e2e tests are extended to check that MergeFlags can now
add correctly, and that Override leaves the variables independent,
not shared.

Fixes #4231

Signed-off-by: Mats Wichmann <mats@linux.com>

1 files changed, 14 insertions, 9 deletions

diff --git a/SCons/EnvironmentTests.py b/SCons/EnvironmentTests.py
index fb089b1a5..81143d5c0 100644
--- a/SCons/EnvironmentTests.py
+++ b/SCons/EnvironmentTests.py
@@ -902,6 +902,11 @@ sys.exit(0)
         assert env['A'] == ['aaa'], env['A']
         assert env['B'] == ['b', 'bb', 'bbb'], env['B']
 
+        # issue #4231: CPPDEFINES can be a deque, tripped up merge logic
+        env = Environment(CPPDEFINES=deque(['aaa', 'bbb']))
+        env.MergeFlags({'CPPDEFINES': 'ccc'})
+        self.assertEqual(env['CPPDEFINES'], deque(['aaa', 'bbb', 'ccc']))
+
         # issue #3665: if merging dict which is a compound object
         # (i.e. value can be lists, etc.), the value object should not
         # be modified. per the issue, this happened if key not in env.
@@ -2167,7 +2172,7 @@ def generate(env):
                                       ('-isystem', '/usr/include/foo2'),
                                       ('-idirafter', '/usr/include/foo3'),
                                       '+DD64'], env['CCFLAGS']
-            assert env['CPPDEFINES'] == ['FOO', ['BAR', 'value']], env['CPPDEFINES']
+            self.assertEqual(list(env['CPPDEFINES']), ['FOO', ['BAR', 'value']])
             assert env['CPPFLAGS'] == ['', '-Wp,-cpp'], env['CPPFLAGS']
             assert env['CPPPATH'] == ['string', '/usr/include/fum', 'bar'], env['CPPPATH']
             assert env['FRAMEWORKPATH'] == ['fwd1', 'fwd2', 'fwd3'], env['FRAMEWORKPATH']
@@ -3662,10 +3667,10 @@ def generate(env):
         env = Environment(tools=[], CCFLAGS=None, parse_flags = '-Y')
         assert env['CCFLAGS'] == ['-Y'], env['CCFLAGS']
 
-        env = Environment(tools=[], CPPDEFINES = 'FOO', parse_flags = '-std=c99 -X -DBAR')
+        env = Environment(tools=[], CPPDEFINES='FOO', parse_flags='-std=c99 -X -DBAR')
         assert env['CFLAGS']  == ['-std=c99'], env['CFLAGS']
         assert env['CCFLAGS'] == ['-X'], env['CCFLAGS']
-        assert env['CPPDEFINES'] == ['FOO', 'BAR'], env['CPPDEFINES']
+        self.assertEqual(list(env['CPPDEFINES']), ['FOO', 'BAR'])
 
     def test_clone_parse_flags(self):
         """Test the env.Clone() parse_flags argument"""
@@ -3687,8 +3692,7 @@ def generate(env):
         assert 'CCFLAGS' not in env
         assert env2['CCFLAGS'] == ['-X'], env2['CCFLAGS']
         assert env['CPPDEFINES'] == 'FOO', env['CPPDEFINES']
-        assert env2['CPPDEFINES'] == ['FOO','BAR'], env2['CPPDEFINES']
-
+        self.assertEqual(list(env2['CPPDEFINES']), ['FOO','BAR'])
 
 
 class OverrideEnvironmentTestCase(unittest.TestCase,TestEnvironmentFixture):
@@ -3978,15 +3982,16 @@ class OverrideEnvironmentTestCase(unittest.TestCase,TestEnvironmentFixture):
         assert env['CCFLAGS'] is None, env['CCFLAGS']
         assert env2['CCFLAGS'] == ['-Y'], env2['CCFLAGS']
 
-        env = SubstitutionEnvironment(CPPDEFINES = 'FOO')
-        env2 = env.Override({'parse_flags' : '-std=c99 -X -DBAR'})
+        env = SubstitutionEnvironment(CPPDEFINES='FOO')
+        env2 = env.Override({'parse_flags': '-std=c99 -X -DBAR'})
         assert 'CFLAGS' not in env
         assert env2['CFLAGS']  == ['-std=c99'], env2['CFLAGS']
         assert 'CCFLAGS' not in env
         assert env2['CCFLAGS'] == ['-X'], env2['CCFLAGS']
+        # make sure they are independent
+        self.assertIsNot(env['CPPDEFINES'], env2['CPPDEFINES'])
         assert env['CPPDEFINES'] == 'FOO', env['CPPDEFINES']
-        assert env2['CPPDEFINES'] == ['FOO','BAR'], env2['CPPDEFINES']
-
+        self.assertEqual(list(env2['CPPDEFINES']), ['FOO','BAR'])
 
 
 class NoSubstitutionProxyTestCase(unittest.TestCase,TestEnvironmentFixture):