From e2b614ddb9c9b1b0e2241e6aff7432a488529e74 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 6 Mar 2023 16:03:22 -0700 Subject: Fix issue with CPPDEFINES and Clone environments Signed-off-by: Mats Wichmann --- CHANGES.txt | 32 ++---- RELEASE.txt | 6 +- SCons/EnvironmentTests.py | 250 +++++++++++++++++++++++++++------------------- SCons/Util/__init__.py | 4 +- 4 files changed, 158 insertions(+), 134 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index c158b49..6721d45 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,32 +7,12 @@ NOTE: The 4.0.0 Release of SCons dropped Python 2.7 Support NOTE: 4.3.0 now requires Python 3.6.0 and above. Python 3.5.x is no longer supported -RELEASE VERSION/DATE TO BE FILLED IN LATER +RELEASE 4.5.1 - From John Doe: - - - Whatever John Doe did. - - -RELEASE VERSION/DATE TO BE FILLED IN LATER - - From John Doe: - - - Whatever John Doe did. - - -RELEASE VERSION/DATE TO BE FILLED IN LATER - - From John Doe: - - - Whatever John Doe did. - - -RELEASE VERSION/DATE TO BE FILLED IN LATER - - From John Doe: - - - Whatever John Doe did. + From Mats Wichmann + - Fix a problem in 4.5.0 where in some circumstances a Clone environment + could share the CPPDEFINES data structure with the original, causing + leakage when they should be completely independent after the Clone. RELEASE 4.5.0 - Sun, 05 Mar 2023 14:08:29 -0700 @@ -160,7 +140,7 @@ RELEASE 4.5.0 - Sun, 05 Mar 2023 14:08:29 -0700 - Updated MSVC documentation - adds "version added" annotations on recently added construction variables and provides a version-mapping table. - Add Python 3.12 support, and indicate 3.11/3.12 support in package. - 3.12 is in alpha for this SCons release, the bytecode sequences + 3.12 is in alpha for this SCons release, the bytecode sequences embedded in SCons/ActionTests.py may need to change later, but based on what is known now, 3.12 itself should work with this release. - Add "append" keyword argument to Configure context's CheckLib and diff --git a/RELEASE.txt b/RELEASE.txt index 439c863..0726607 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -11,7 +11,7 @@ A new SCons release, 4.4.1, is now available on the SCons download page: https://scons.org/pages/download.html -Here is a summary of the changes since 4.4.0: +Here is a summary of the changes since 4.5.0: NEW FUNCTIONALITY ----------------- @@ -32,7 +32,9 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY FIXES ----- -- List fixes of outright bugs +- Fix a problem in 4.5.0 where in some circumstances a Clone environment + could share the CPPDEFINES data structure with the original, causing + leakage when they should be completely independent after the Clone. IMPROVEMENTS ------------ diff --git a/SCons/EnvironmentTests.py b/SCons/EnvironmentTests.py index a021794..0f429b4 100644 --- a/SCons/EnvironmentTests.py +++ b/SCons/EnvironmentTests.py @@ -28,7 +28,7 @@ import io import os import sys import unittest -from collections import UserDict as UD, UserList as UL +from collections import UserDict as UD, UserList as UL, deque import TestCmd @@ -1821,147 +1821,189 @@ def exists(env): updates and check that the original remains intact and the copy has the updated values. """ - env1 = self.TestEnvironment(XXX='x', YYY='y') - env2 = env1.Clone() - env1copy = env1.Clone() - assert env1copy == env1 - assert env2 == env1 - env2.Replace(YYY = 'yyy') - assert env1 != env2 - assert env1 == env1copy - - env3 = env1.Clone(XXX='x3', ZZZ='z3') - assert env3 != env1 - assert env3.Dictionary('XXX') == 'x3' - assert env1.Dictionary('XXX') == 'x' - assert env3.Dictionary('YYY') == 'y' - assert env3.Dictionary('ZZZ') == 'z3' - assert env1 == env1copy + with self.subTest(): + env1 = self.TestEnvironment(XXX='x', YYY='y') + env2 = env1.Clone() + env1copy = env1.Clone() + self.assertEqual(env1copy, env1) + self.assertEqual(env2, env1) + env2.Replace(YYY = 'yyy') + self.assertNotEqual(env1, env2) + self.assertEqual(env1, env1copy) + + env3 = env1.Clone(XXX='x3', ZZZ='z3') + self.assertNotEqual(env3, env1) + self.assertEqual(env3.Dictionary('XXX'), 'x3') + self.assertEqual(env1.Dictionary('XXX'), 'x') + self.assertEqual(env3.Dictionary('YYY'), 'y') + self.assertEqual(env3.Dictionary('ZZZ'), 'z3') + self.assertRaises(KeyError, env1.Dictionary, 'ZZZ') # leak test + self.assertEqual(env1, env1copy) # Ensure that lists and dictionaries are deep copied, but not instances - class TestA: - pass + with self.subTest(): + + class TestA: + pass - env1 = self.TestEnvironment(XXX=TestA(), YYY=[1, 2, 3], ZZZ={1: 2, 3: 4}) - env2 = env1.Clone() - env2.Dictionary('YYY').append(4) - env2.Dictionary('ZZZ')[5] = 6 - assert env1.Dictionary('XXX') is env2.Dictionary('XXX') - assert 4 in env2.Dictionary('YYY') - assert 4 not in env1.Dictionary('YYY') - assert 5 in env2.Dictionary('ZZZ') - assert 5 not in env1.Dictionary('ZZZ') - - # - env1 = self.TestEnvironment(BUILDERS={'b1': Builder()}) - assert hasattr(env1, 'b1'), "env1.b1 was not set" - assert env1.b1.object == env1, "b1.object doesn't point to env1" - env2 = env1.Clone(BUILDERS = {'b2' : Builder()}) - assert env2 != env1 - assert hasattr(env1, 'b1'), "b1 was mistakenly cleared from env1" - assert env1.b1.object == env1, "b1.object was changed" - assert not hasattr(env2, 'b1'), "b1 was not cleared from env2" - assert hasattr(env2, 'b2'), "env2.b2 was not set" - assert env2.b2.object == env2, "b2.object doesn't point to env2" + env1 = self.TestEnvironment( + XXX=TestA(), + YYY=[1, 2, 3], + ZZZ={1: 2, 3: 4} + ) + env2 = env1.Clone() + env2.Dictionary('YYY').append(4) + env2.Dictionary('ZZZ')[5] = 6 + self.assertIs(env1.Dictionary('XXX'), env2.Dictionary('XXX')) + self.assertIn(4, env2.Dictionary('YYY')) + self.assertNotIn(4, env1.Dictionary('YYY')) + self.assertIn(5, env2.Dictionary('ZZZ')) + self.assertNotIn(5, env1.Dictionary('ZZZ')) + + # We also need to look at the special cases in semi_deepcopy() + # used when cloning - these should not leak to the original either + with self.subTest(): + env1 = self.TestEnvironment( + XXX=deque([1, 2, 3]), + YYY=UL([1, 2, 3]), + ZZZ=UD({1: 2, 3: 4}), + ) + env2 = env1.Clone() + env2.Dictionary('XXX').append(4) + env2.Dictionary('YYY').append(4) + env2.Dictionary('ZZZ')[5] = 6 + self.assertIn(4, env2.Dictionary('XXX')) + self.assertNotIn(4, env1.Dictionary('XXX')) + self.assertIn(4, env2.Dictionary('YYY')) + self.assertNotIn(4, env1.Dictionary('YYY')) + self.assertIn(5, env2.Dictionary('ZZZ')) + self.assertNotIn(5, env1.Dictionary('ZZZ')) + + # BUILDERS is special... + with self.subTest(): + env1 = self.TestEnvironment(BUILDERS={'b1': Builder()}) + assert hasattr(env1, 'b1'), "env1.b1 was not set" + assert env1.b1.object == env1, "b1.object doesn't point to env1" + env2 = env1.Clone(BUILDERS = {'b2' : Builder()}) + assert env2 != env1 + assert hasattr(env1, 'b1'), "b1 was mistakenly cleared from env1" + assert env1.b1.object == env1, "b1.object was changed" + assert not hasattr(env2, 'b1'), "b1 was not cleared from env2" + assert hasattr(env2, 'b2'), "env2.b2 was not set" + assert env2.b2.object == env2, "b2.object doesn't point to env2" # Ensure that specifying new tools in a copied environment works. - def foo(env): env['FOO'] = 1 - def bar(env): env['BAR'] = 2 - def baz(env): env['BAZ'] = 3 - env1 = self.TestEnvironment(tools=[foo]) - env2 = env1.Clone() - env3 = env1.Clone(tools=[bar, baz]) - - assert env1.get('FOO') == 1 - assert env1.get('BAR') is None - assert env1.get('BAZ') is None - assert env2.get('FOO') == 1 - assert env2.get('BAR') is None - assert env2.get('BAZ') is None - assert env3.get('FOO') == 1 - assert env3.get('BAR') == 2 - assert env3.get('BAZ') == 3 + with self.subTest(): + + def foo(env): + env['FOO'] = 1 + + def bar(env): + env['BAR'] = 2 + + def baz(env): + env['BAZ'] = 3 + + env1 = self.TestEnvironment(tools=[foo]) + env2 = env1.Clone() + env3 = env1.Clone(tools=[bar, baz]) + + assert env1.get('FOO') == 1 + assert env1.get('BAR') is None + assert env1.get('BAZ') is None + assert env2.get('FOO') == 1 + assert env2.get('BAR') is None + assert env2.get('BAZ') is None + assert env3.get('FOO') == 1 + assert env3.get('BAR') == 2 + assert env3.get('BAZ') == 3 # Ensure that recursive variable substitution when copying # environments works properly. - env1 = self.TestEnvironment(CCFLAGS = '-DFOO', XYZ = '-DXYZ') - env2 = env1.Clone(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 + with self.subTest(): + env1 = self.TestEnvironment(CCFLAGS='-DFOO', XYZ='-DXYZ') + env2 = env1.Clone( + 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 # Ensure that special properties of a class don't get # lost on copying. - env1 = self.TestEnvironment(FLAGS = CLVar('flag1 flag2')) - x = env1.get('FLAGS') - assert x == ['flag1', 'flag2'], x - env2 = env1.Clone() - env2.Append(FLAGS = 'flag3 flag4') - x = env2.get('FLAGS') - assert x == ['flag1', 'flag2', 'flag3', 'flag4'], x - x = env1.get('FLAGS') - assert x == ['flag1', 'flag2'], x + with self.subTest(): + env1 = self.TestEnvironment(FLAGS=CLVar('flag1 flag2')) + x = env1.get('FLAGS') + assert x == ['flag1', 'flag2'], x + env2 = env1.Clone() + env2.Append(FLAGS='flag3 flag4') + x = env2.get('FLAGS') + assert x == ['flag1', 'flag2', 'flag3', 'flag4'], x + x = env1.get('FLAGS') + assert x == ['flag1', 'flag2'], x # Ensure that appending directly to a copied CLVar # doesn't modify the original. - env1 = self.TestEnvironment(FLAGS = CLVar('flag1 flag2')) - x = env1.get('FLAGS') - assert x == ['flag1', 'flag2'], x - env2 = env1.Clone() - env2['FLAGS'] += ['flag3', 'flag4'] - x = env2.get('FLAGS') - assert x == ['flag1', 'flag2', 'flag3', 'flag4'], x - x = env1.get('FLAGS') - assert x == ['flag1', 'flag2'], x + with self.subTest(): + env1 = self.TestEnvironment(FLAGS=CLVar('flag1 flag2')) + x = env1.get('FLAGS') + assert x == ['flag1', 'flag2'], x + env2 = env1.Clone() + env2['FLAGS'] += ['flag3', 'flag4'] + x = env2.get('FLAGS') + assert x == ['flag1', 'flag2', 'flag3', 'flag4'], x + x = env1.get('FLAGS') + assert x == ['flag1', 'flag2'], x # Test that the environment stores the toolpath and # re-uses it for copies. - test = TestCmd.TestCmd(workdir = '') + with self.subTest(): + test = TestCmd.TestCmd(workdir='') - test.write('xxx.py', """\ + test.write('xxx.py', """\ def exists(env): return True def generate(env): env['XXX'] = 'one' """) - test.write('yyy.py', """\ + test.write('yyy.py', """\ def exists(env): return True def generate(env): env['YYY'] = 'two' """) - env = self.TestEnvironment(tools=['xxx'], toolpath=[test.workpath('')]) - assert env['XXX'] == 'one', env['XXX'] - env = env.Clone(tools=['yyy']) - assert env['YYY'] == 'two', env['YYY'] - + env = self.TestEnvironment(tools=['xxx'], toolpath=[test.workpath('')]) + assert env['XXX'] == 'one', env['XXX'] + env = env.Clone(tools=['yyy']) + assert env['YYY'] == 'two', env['YYY'] # Test that - real_value = [4] + with self.subTest(): + real_value = [4] - def my_tool(env, rv=real_value): - assert env['KEY_THAT_I_WANT'] == rv[0] - env['KEY_THAT_I_WANT'] = rv[0] + 1 + def my_tool(env, rv=real_value): + assert env['KEY_THAT_I_WANT'] == rv[0] + env['KEY_THAT_I_WANT'] = rv[0] + 1 - env = self.TestEnvironment() + env = self.TestEnvironment() - real_value[0] = 5 - env = env.Clone(KEY_THAT_I_WANT=5, tools=[my_tool]) - assert env['KEY_THAT_I_WANT'] == real_value[0], env['KEY_THAT_I_WANT'] + real_value[0] = 5 + env = env.Clone(KEY_THAT_I_WANT=5, tools=[my_tool]) + assert env['KEY_THAT_I_WANT'] == real_value[0], env['KEY_THAT_I_WANT'] - real_value[0] = 6 - env = env.Clone(KEY_THAT_I_WANT=6, tools=[my_tool]) - assert env['KEY_THAT_I_WANT'] == real_value[0], env['KEY_THAT_I_WANT'] + real_value[0] = 6 + env = env.Clone(KEY_THAT_I_WANT=6, tools=[my_tool]) + assert env['KEY_THAT_I_WANT'] == real_value[0], env['KEY_THAT_I_WANT'] # test for pull request #150 - env = self.TestEnvironment() - env._dict.pop('BUILDERS') - assert ('BUILDERS' in env) is False - env2 = env.Clone() + with self.subTest(): + env = self.TestEnvironment() + env._dict.pop('BUILDERS') + assert ('BUILDERS' in env) is False + env2 = env.Clone() def test_Detect(self): """Test Detect()ing tools""" diff --git a/SCons/Util/__init__.py b/SCons/Util/__init__.py index 0281e1e..ffc6364 100644 --- a/SCons/Util/__init__.py +++ b/SCons/Util/__init__.py @@ -32,7 +32,7 @@ import os import re import sys import time -from collections import UserDict, UserList, OrderedDict +from collections import UserDict, UserList, OrderedDict, deque from contextlib import suppress from types import MethodType, FunctionType from typing import Optional, Union @@ -527,7 +527,7 @@ def semi_deepcopy(obj): if isinstance(obj, UserDict): return obj.__class__(semi_deepcopy_dict(obj)) - if isinstance(obj, UserList): + if isinstance(obj, (UserList, deque)): return obj.__class__(_semi_deepcopy_list(obj)) return obj -- cgit v0.12