Add internal routines to remove args and targets

Adds SCons.Script._Remove_Target and SCons.Script._Remove_Argument to
allow taking away values placed the public attributes BUILD_TARGETS,
COMMAND_LINE_TARGETS, ARGUMENTS and ARGLIST. Part two of three harvesting
from old PR SCons#3799 (the short-option piece was merged as part of SCons#4598).
Intended customer will be the Options logic,

Unit tests created, also for existing SCons.Script._Add_Targets and
SCons.Script._Add_Arguments.

Signed-off-by: Mats Wichmann <[email protected]>

Author: mwichmann

CHANGES.txt

@@ -40,6 +40,12 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       one from PEP 308 introduced in Python 2.5 (2006). The idiom being
       replaced (using and/or) is regarded as error prone.
     - Improve the description of PackageVariable.
+    - Add internal routines _Remove_Targets and _Remove_Arguments to
+      allow taking away values placed the public attributes BUILD_TARGETS,
+      COMMAND_LINE_TARGETS, ARGUMENTS and ARGLIST. This is a step towards
+      fixing the handling of option-arguments specified with a space
+      separator (multiple issues, harvested from #3799).  These interfaces
+      are not part of the public API.
 
 
 RELEASE 4.9.1 -  Thu, 27 Mar 2025 11:40:20 -0700

RELEASE.txt

@@ -48,6 +48,9 @@ IMPROVEMENTS
   documentation:  performance improvements (describe the circumstances
   under which they would be observed), or major code cleanups
 
+- Add internal routines to maniplutate publicly visible argument and
+  target lists. These interfaces are not part of the public API.
+
 PACKAGING
 ---------
 

SCons/Script/ScriptTests.py

@@ -0,0 +1,181 @@
+# MIT License
+#
+# Copyright The SCons Foundation
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+
+# The above copyright notice and this permission notice shall be included
+# in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
+# KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE
+# WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+# Unit tests of functionality from SCons.Script._init__.py.
+#
+# Most of the tests of this functionality are actually end-to-end scripts
+# in the test/ hierarchy.
+#
+# This module is for specific bits of functionality that seem worth
+# testing here - particularly if there's private data involved.
+
+import unittest
+
+from SCons.Script import (
+    _Add_Arguments,
+    _Add_Targets,
+    _Remove_Argument,
+    _Remove_Target,
+    ARGLIST,
+    ARGUMENTS,
+    BUILD_TARGETS,
+    COMMAND_LINE_TARGETS,
+    _build_plus_default,
+)
+
+
+class TestScriptFunctions(unittest.TestCase):
+    def setUp(self):
+        # Clear global state before each test
+        ARGUMENTS.clear()
+        ARGLIST.clear()
+        del COMMAND_LINE_TARGETS[:]
+        del BUILD_TARGETS[:]
+        del _build_plus_default[:]
+
+    def test_Add_Arguments(self):
+        test_args = ['foo=bar', 'spam=eggs']
+
+        _Add_Arguments(test_args)
+        self.assertEqual(ARGUMENTS, {'foo': 'bar', 'spam': 'eggs'})
+        self.assertEqual(ARGLIST, [('foo', 'bar'), ('spam', 'eggs')])
+
+    def test_Add_Arguments_empty(self):
+        # Adding am empty argument is a no-op, with no error
+        _Add_Arguments([])
+        self.assertEqual(ARGUMENTS, {})
+        self.assertEqual(ARGLIST, [])
+
+    def test_Add_Targets(self):
+        test_targets = ['target1', 'target2']
+        _Add_Targets(test_targets)
+
+        self.assertEqual(COMMAND_LINE_TARGETS, ['target1', 'target2'])
+        self.assertEqual(BUILD_TARGETS, ['target1', 'target2'])
+        self.assertEqual(_build_plus_default, ['target1', 'target2'])
+
+        # Test that methods were replaced
+        self.assertEqual(BUILD_TARGETS._add_Default, BUILD_TARGETS._do_nothing)
+        self.assertEqual(BUILD_TARGETS._clear, BUILD_TARGETS._do_nothing)
+        self.assertEqual(
+            _build_plus_default._add_Default, _build_plus_default._do_nothing
+        )
+        self.assertEqual(
+            _build_plus_default._clear, _build_plus_default._do_nothing
+        )
+
+    def test_Add_Targets_empty(self):
+        # Adding am empty argument is a no-op, with no error
+        _Add_Targets([])
+        self.assertEqual(COMMAND_LINE_TARGETS, [])
+        self.assertEqual(BUILD_TARGETS, [])
+        self.assertEqual(_build_plus_default, [])
+
+    def test_Remove_Argument(self):
+        ARGLIST.extend([
+            ('key1', 'value1'),
+            ('key2', 'value2')
+        ])
+        ARGUMENTS.update({'key1': 'value1', 'key2': 'value2'})
+
+        _Remove_Argument('key1=value1')
+        self.assertEqual(ARGUMENTS, {'key2': 'value2'})
+        self.assertEqual(ARGLIST, [('key2', 'value2')])
+
+    def test_Remove_Argument_key_with_multiple_values(self):
+        ARGLIST.extend([
+            ('key1', 'value1'),
+            ('key1', 'value2')
+        ])
+        ARGUMENTS['key1'] = 'value2'  # ARGUMENTS only keeps last, emulate
+
+        _Remove_Argument('key1=value1')
+        self.assertEqual(ARGLIST, [('key1', 'value2')])
+        # ARGUMENTS must be reconstructed
+        self.assertEqual(ARGUMENTS, {'key1': 'value2'})
+
+    def test_Remove_Argument_nonexistent(self):
+        # Removing a nonexistent argument is a no-op with no error
+        ARGUMENTS['key1'] = 'value1'
+        ARGLIST.append(('key1', 'value1'))
+
+        _Remove_Argument('nonexistent=value')
+        self.assertEqual(ARGUMENTS, {'key1': 'value1'})
+        self.assertEqual(ARGLIST, [('key1', 'value1')])
+
+    def test_Remove_Argument_empty(self):
+        # Removing an empty argument is also a no-op with no error
+        ARGUMENTS['key1'] = 'value1'
+        ARGLIST.append(('key1', 'value1'))
+
+        _Remove_Argument('')
+        self.assertEqual(ARGUMENTS, {'key1': 'value1'})
+        self.assertEqual(ARGLIST, [('key1', 'value1')])
+
+    # XXX where does TARGETS come in?
+    def test_Remove_Target(self):
+        BUILD_TARGETS.extend(['target1', 'target2', 'target3'])
+        COMMAND_LINE_TARGETS.extend(['target1', 'target2', 'target3'])
+
+        _Remove_Target('target2')
+        self.assertEqual(BUILD_TARGETS, ['target1', 'target3'])
+        self.assertEqual(COMMAND_LINE_TARGETS, ['target1', 'target3'])
+
+    def test_Remove_Target_duplicated(self):
+        # Targets can be duplicated, only one should be removed
+        # There is not a good way to determine which instance was added
+        # "in error" so all we can do is check *something* was removed.
+        BUILD_TARGETS.extend(['target1', 'target1'])
+        COMMAND_LINE_TARGETS.extend(['target1', 'target1'])
+
+        _Remove_Target('target1')
+        self.assertEqual(BUILD_TARGETS, ['target1'])
+        self.assertEqual(COMMAND_LINE_TARGETS, ['target1'])
+
+    def test_Remove_Target_nonexistent(self):
+        # Asking to remove a nonexistent argument is a no-op with no error
+        BUILD_TARGETS.append('target1')
+        COMMAND_LINE_TARGETS.append('target1')
+
+        _Remove_Target('nonexistent')
+        self.assertEqual(BUILD_TARGETS, ['target1'])
+        self.assertEqual(COMMAND_LINE_TARGETS, ['target1'])
+
+    def test_Remove_Target_empty(self):
+        # Asking to remove an empty argument is also a no-op with no error
+        BUILD_TARGETS.append('target1')
+        COMMAND_LINE_TARGETS.append('target1')
+
+        _Remove_Target('')
+        self.assertEqual(BUILD_TARGETS, ['target1'])
+        self.assertEqual(COMMAND_LINE_TARGETS, ['target1'])
+
+
+if __name__ == '__main__':
+    unittest.main()
+
+# Local Variables:
+# tab-width:4
+# indent-tabs-mode:nil
+# End:
+# vim: set expandtab tabstop=4 shiftwidth=4:

SCons/Script/__init__.py

@@ -30,16 +30,16 @@
 some other module.  If it's specific to the "scons" script invocation,
 it goes here.
 """
-
-import time
-start_time = time.time()
+from __future__ import annotations
 
 import collections
 import itertools
 import os
+import sys
+import time
 from io import StringIO
 
-import sys
+start_time = time.time()
 
 # Special chicken-and-egg handling of the "--debug=memoizer" flag:
 #
@@ -135,7 +135,7 @@
 #profiling               = Main.profiling
 #repositories            = Main.repositories
 
-from . import SConscript as _SConscript
+from . import SConscript as _SConscript  # pylint: disable=import-outside-toplevel
 
 call_stack              = _SConscript.call_stack
 
@@ -208,13 +208,15 @@ def _clear(self) -> None:
 # own targets to BUILD_TARGETS.
 _build_plus_default = TargetList()
 
-def _Add_Arguments(alist) -> None:
+def _Add_Arguments(alist: list[str]) -> None:
+    """Add value(s) to ``ARGLIST`` and ``ARGUMENTS``."""
     for arg in alist:
         a, b = arg.split('=', 1)
         ARGUMENTS[a] = b
         ARGLIST.append((a, b))
 
-def _Add_Targets(tlist) -> None:
+def _Add_Targets(tlist: list[str]) -> None:
+    """Add value(s) to ``COMMAND_LINE_TARGETS`` and ``BUILD_TARGETS``."""
     if tlist:
         COMMAND_LINE_TARGETS.extend(tlist)
         BUILD_TARGETS.extend(tlist)
@@ -224,6 +226,54 @@ def _Add_Targets(tlist) -> None:
         _build_plus_default._add_Default = _build_plus_default._do_nothing
         _build_plus_default._clear = _build_plus_default._do_nothing
 
+def _Remove_Argument(aarg: str) -> None:
+    """Remove *aarg* from ``ARGLIST`` and ``ARGUMENTS``.
+
+    Used to remove a variables-style argument that is no longer valid.
+    This can happpen because the command line is processed once early,
+    before we see any :func:`SCons.Script.Main.AddOption` calls, so we
+    could not recognize it belongs to an option and is not a standalone
+    variable=value argument.
+
+    .. versionaddedd:: NEXT_RELEASE
+    """
+    if aarg:
+        a, b = aarg.split('=', 1)
+
+        # remove from ARGLIST first which would contain duplicates if
+        # -x A=B A=B was specified on the CL
+        if (a, b) in ARGLIST:
+            ARGLIST.remove((a, b))
+
+            # Remove first in case no matching values left in ARGLIST
+            ARGUMENTS.pop(a, None)
+            # Set ARGUMENTS[A] back to latest value in ARGLIST
+            # (assuming order matches CL order)
+            for item in ARGLIST:
+                if item[0] == a:
+                    ARGUMENTS[a] = item[1]
+
+def _Remove_Target(targ: str) -> None:
+    """Remove *targ* from ``BUILD_TARGETS`` and ``COMMAND_LINE_TARGETS``.
+
+    Used to remove a target that is no longer valid. This can happpen
+    because the command line is processed once early, before we see any
+    :func:`SCons.Script.Main.AddOption` calls, so we could not recognize
+    it belongs to an option standalone target argument.
+
+    Since we are "correcting an error", we also have to fix up the internal
+    :data:`_build_plus_default` list.
+
+    .. versionaddedd:: NEXT_RELEASE
+    """
+    if targ:
+        if targ in COMMAND_LINE_TARGETS:
+            COMMAND_LINE_TARGETS.remove(targ)
+        if targ in BUILD_TARGETS:
+            BUILD_TARGETS.remove(targ)
+        if targ in _build_plus_default:
+            _build_plus_default.remove(targ)
+
 def _Set_Default_Targets_Has_Been_Called(d, fs):
     return DEFAULT_TARGETS