From 643800f298a4870d731a91820096893a2cfbf94b Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 23 Mar 2026 12:35:35 -0600 Subject: [PATCH 1/4] Fix AddOption handling of spaces Fix handling of AddOption for option-arguments with spaces (when omitting the = sign and/or when an option takes multiple option-arguments). Arguments unknown at the time the first pass splits the command line into arguments and targets would put such arguments into targets, and they remained there even after the AddOption calls were seen. Signed-off-by: Mats Wichmann --- CHANGES.txt | 10 + RELEASE.txt | 9 +- SCons/Script/SConsOptions.py | 24 +- test/AddOption/args-and-targets.py | 395 ++++++++++++++++++++++++++++- test/AddOption/multi-arg.py | 97 ++++--- 5 files changed, 486 insertions(+), 49 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index d8a7783890..293fe08187 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -45,6 +45,14 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER - Fix Appveyor scripting to install unavailable python versions when needed and use them for testing. + From Dillan Mills: + - Fix handling of AddOption for option-arguments with spaces + (when omitting the = sign or when an option takes multiple + option-arguments). Arguments unknown at the time the first pass + splits the command line into arguments and targets would put such + arguments into targets, and they remained there even after the + AddOption calls were seen. Closes #2748, #2805, #2977. + From Mats Wichmann: - Introduce some unit tests for the file locking utility routines - More clarifications in manpage Builder Methods section. @@ -81,6 +89,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER on a one-time uuid to make a path to the file. - Clarify VariantDir behavior when switching to not duplicate sources and tweak wording a bit. + - Complete the work from PR #3799 on AddOption handling (original work + credited to Dillan Mills) RELEASE 4.10.1 - Sun, 16 Nov 2025 10:51:57 -0700 diff --git a/RELEASE.txt b/RELEASE.txt index e09fc121a3..a84fad8c34 100644 --- a/RELEASE.txt +++ b/RELEASE.txt @@ -51,6 +51,13 @@ FIXES directory for each writer before doing the move, instead of depending on a one-time uuid to make a path to the file. +- Fix handling of AddOption for option-arguments with spaces (when omitting + the = sign or when an option takes multiple option-arguments). Arguments + unknown at the time the first pass splits the command line into arguments + and targets would put such arguments into targets, and they remained + there even after the AddOption calls were seen. + + IMPROVEMENTS ------------ @@ -71,7 +78,7 @@ IMPROVEMENTS - Simplified and sped up compilation database generation. No longer requires each entry to have a dedicated node that's always built; instead, the database *itself* is set to always build. - + - Switch remaining "original style" docstring parameter listings to Google style. - Additional small tweaks to Environment.py type hints, fold some overly diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index 52d7fca52e..a519679156 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -405,9 +405,18 @@ def _process_long_opt(self, rargs, values) -> None: % (opt, nargs)) elif nargs == 1: value = rargs.pop(0) + if not had_explicit_value: + SCons.Script._Remove_Target(value) + if '=' in value: + SCons.Script._Remove_Argument(value) else: value = tuple(rargs[0:nargs]) del rargs[0:nargs] + for i in range(len(value)): + if not had_explicit_value or i > 0: + SCons.Script._Remove_Target(value[i]) + if '=' in value[i]: + SCons.Script._Remove_Argument(value[i]) elif had_explicit_value: self.error(_("%s option does not take a value") % opt) @@ -447,11 +456,13 @@ def _process_short_opts(self, rargs, values) -> None: raise if option.takes_value(): + had_explicit_value = False # Any characters left in arg? Pretend they're the # next arg, and stop consuming characters of arg. if i < len(arg): rargs.insert(0, arg[i:]) stop = True + had_explicit_value = True nargs = option.nargs if len(rargs) < nargs: @@ -462,9 +473,19 @@ def _process_short_opts(self, rargs, values) -> None: % (opt, nargs)) elif nargs == 1: value = rargs.pop(0) + if not had_explicit_value: + SCons.Script._Remove_Target(value) + if '=' in value: + SCons.Script._Remove_Argument(value) else: value = tuple(rargs[0:nargs]) del rargs[0:nargs] + for i in range(len(value)): + if not had_explicit_value or i > 0: + SCons.Script._Remove_Target(value[i]) + if '=' in value[i]: + SCons.Script._Remove_Argument(value[i]) + else: # option doesn't take a value value = None @@ -475,6 +496,7 @@ def _process_short_opts(self, rargs, values) -> None: break + # TODO: this is now unused, remove? def reparse_local_options(self) -> None: """Re-parse the leftover command-line options. @@ -574,7 +596,7 @@ def add_local_option(self, *args, **kw) -> SConsOption: # right away. # TODO: what if dest is None? setattr(self.values.__defaults__, result.dest, result.default) - self.reparse_local_options() + self.parse_args(self.largs, self.values) if result.settable: SConsValues.settable.append(result.dest) diff --git a/test/AddOption/args-and-targets.py b/test/AddOption/args-and-targets.py index 900a42f89b..7517dc9ff6 100644 --- a/test/AddOption/args-and-targets.py +++ b/test/AddOption/args-and-targets.py @@ -30,14 +30,12 @@ import TestSCons -test = TestSCons.TestSCons() +test = TestSCons.TestSCons(match = TestSCons.match_re_dotall) -test.write( - 'SConstruct', - """\ +test.write('SConstruct', """\ DefaultEnvironment(tools=[]) -env = Environment(tools=[]) AddOption( + '-x', '--extra', nargs=1, dest='extra', @@ -53,8 +51,391 @@ ) # arg using = -test.run('-Q -q --extra=A TARG', status=1, stdout="A\n['TARG']\n") +test.run('-Q -q --extra=A TARG', status=1, stdout=r"A\n\['TARG'\]\n") # arg not using = -test.run('-Q -q --extra A TARG', status=1, stdout="A\n['TARG']\n") +test.run('-Q -q --extra A TARG', status=1, stdout=r"A\n\['TARG'\]\n") +# short arg with space +test.run('-Q -q -x A TARG', status=1, stdout=r"A\n\['TARG'\]\n") +# short arg with no space +test.run('-Q -q -xA TARG', status=1, stdout=r"A\n\['TARG'\]\n") + +test.write('SConstruct1', """\ +DefaultEnvironment(tools=[]) +AddOption( + '-x', + '--extra', + nargs=2, + dest='extra', + action='append', + type='string', + metavar='ARG1', + default=[], + help='An argument to the option', +) +print(str(GetOption('extra'))) +print(COMMAND_LINE_TARGETS) +""", +) + +# many args and opts +test.run( + '-f SConstruct1 -Q -q --extra=A B TARG1 -x C D TARG2 -xE F TARG3 --extra G H TARG4', + status=1, + stdout=r"\[\('A', 'B'\), \('C', 'D'\), \('E', 'F'\), \('G', 'H'\)\]\n\['TARG1', 'TARG2', 'TARG3', 'TARG4'\]\n", +) + +test.write('SConstruct2', """\ +DefaultEnvironment(tools=[]) +AddOption('-x', '--extra', + nargs=1, + dest='extra', + action='store', + type='string', + metavar='ARG1', + default=(), + help='An argument to the option') +print(str(GetOption('extra'))) +print(COMMAND_LINE_TARGETS) +print(ARGUMENTS.get('A', None)) +""", +) + +# opt value and target are same name +test.run( + arguments='-f SConstruct2 -Q -q --extra=TARG1 TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q --extra TARG1 TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -xTARG1 TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -x TARG1 TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) + +# target first +test.run( + arguments='-f SConstruct2 -Q -q TARG1 --extra=TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q TARG1 --extra TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q TARG1 -xTARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q TARG1 -x TARG1', + status=1, + stdout=r"TARG1\n\['TARG1'\]\nNone\n", +) + +# equals in opt value +test.run( + arguments='-f SConstruct2 -Q -q --extra=A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q --extra A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -xA=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nNone\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -x A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nNone\n", +) + +# equals in opt value and a different argument +test.run( + arguments='-f SConstruct2 -Q -q --extra=A=B A=C TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q --extra A=B A=C TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -xA=B A=C TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -x A=B A=C TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) + +# different argument first +test.run( + arguments='-f SConstruct2 -Q -q A=C --extra=A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=C --extra A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=C -xA=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=C -x A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nC\n", +) + +# equals in opt value and the same as an argument +test.run( + arguments='-f SConstruct2 -Q -q --extra=A=B A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q --extra A=B A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -xA=B A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q -x A=B A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) + +# same argument first +test.run( + arguments='-f SConstruct2 -Q -q A=B --extra=A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=B --extra A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=B -xA=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) +test.run( + arguments='-f SConstruct2 -Q -q A=B -x A=B TARG1', + status=1, + stdout=r"A=B\n\['TARG1'\]\nB\n", +) + +test.write('SConstruct3', """\ +DefaultEnvironment(tools=[]) +AddOption( + '-x', + '--extra', + nargs=1, + dest='extra', + action='store', + type='string', + metavar='ARG1', + default=(), + help='An argument to the option', +) +if 'A' in BUILD_TARGETS: + BUILD_TARGETS.append('B') +print(str(GetOption('extra'))) +print(BUILD_TARGETS) +""", +) + +# Nested target +test.run( + arguments='-f SConstruct3 -Q -q -x A TARG1', status=1, stdout=r"A\n\['TARG1'\]\n" +) +test.run( + arguments='-f SConstruct3 -Q -q -x A A TARG1', + status=1, + stdout=r"A\n\['A', 'TARG1', 'B'\]\n", +) +test.run( + arguments='-f SConstruct3 -Q -q A -x A TARG1', + status=1, + stdout=r"A\n\['A', 'TARG1', 'B'\]\n", +) + +test.write('SConstruct4', """\ +DefaultEnvironment(tools=[]) +AddOption( + '-x', + '--extra', + nargs=1, + dest='extra', + action='store', + type='string', + metavar='ARG1', + default=(), + help='An argument to the option', +) +if 'A' in BUILD_TARGETS: + AddOption( + '--foo', + nargs=1, + dest='foo', + action='store', + type='string', + metavar='FOO1', + default=(), + help='An argument to the option', + ) +print(str(GetOption('extra'))) +print(str(GetOption('foo'))) +print(COMMAND_LINE_TARGETS) +""", +) + +# nested option +expect = r"""AttributeError: 'Values' object has no attribute 'foo': + File ".+SConstruct4", line \d+: + print\(str\(GetOption\('foo'\)\)\) + File ".+SCons/Script/Main.py", line \d+: + return getattr\(OptionsParser.values, name\) + File ".+SCons/Script/SConsOptions.py", line \d+: + return getattr\(self.__dict__\['__defaults__'\], attr\) +""" +test.run( + arguments='-f SConstruct4 -Q -q -x A --foo=C TARG1', + status=2, + stdout=r"A\n", + stderr=expect, +) +test.run( + arguments='-f SConstruct4 -Q -q -x A A --foo=C TARG1', + status=1, + stdout=r"A\nC\n\['A', 'TARG1'\]\n", +) +test.run( + arguments='-f SConstruct4 -Q -q A -x A --foo=C TARG1', + status=1, + stdout=r"A\nC\n\['A', 'TARG1'\]\n", +) + +############################################ +################ Fail Case ################# +############################################ + +test.write('SConstruct5', """\ +DefaultEnvironment(tools=[]) +AddOption( + '-x', + '--extra', + nargs=1, + dest='extra', + action='store', + type='string', + metavar='ARG1', + default=(), + help='An argument to the option', +) +if 'A' in BUILD_TARGETS: + BUILD_TARGETS.append('B') +print(str(GetOption('extra'))) +print(BUILD_TARGETS) +""") + +# Nested target +test.run( + arguments='-f SConstruct5 -Q -q -x A TARG1', status=1, stdout=r"A\n\['TARG1'\]\n" +) +test.run( + arguments='-f SConstruct5 -Q -q -x A A TARG1', + status=1, + stdout=r"A\n\['A', 'TARG1', 'B'\]\n", +) +test.run( + arguments='-f SConstruct5 -Q -q A -x A TARG1', + status=1, + stdout=r"A\n\['A', 'TARG1', 'B'\]\n", +) + +test.write('SConstruct6', """\ +DefaultEnvironment(tools=[]) +AddOption( + '-x', + '--extra', + nargs=1, + dest='extra', + action='store', + type='string', + metavar='ARG1', + default=(), + help='An argument to the option', +) +if 'A' in BUILD_TARGETS: + AddOption( + '--foo', + nargs=1, + dest='foo', + action='store', + type='string', + metavar='FOO1', + default=(), + help='An argument to the option', + ) +print(str(GetOption('extra'))) +print(str(GetOption('foo'))) +print(COMMAND_LINE_TARGETS) +""") + +# nested option +expect=r"""AttributeError: 'Values' object has no attribute 'foo': + File ".+SConstruct6", line \d+: + print\(str\(GetOption\('foo'\)\)\) + File ".+SCons/Script/Main.py", line \d+: + return getattr\(OptionsParser.values, name\) + File ".+SCons/Script/SConsOptions.py", line \d+: + return getattr\(self.__dict__\['__defaults__'\], attr\) +""" +test.run( + arguments='-f SConstruct6 -Q -q -x A --foo=C TARG1', + status=2, + stdout=r"A\n", + stderr=expect, +) +test.run( + arguments='-f SConstruct6 -Q -q -x A A --foo=C TARG1', + status=1, + stdout=r"A\nC\n\['A', 'TARG1'\]\n", +) +test.run( + arguments='-f SConstruct6 -Q -q A -x A --foo=C TARG1', + status=1, + stdout=r"A\nC\n\['A', 'TARG1'\]\n", +) test.pass_test() diff --git a/test/AddOption/multi-arg.py b/test/AddOption/multi-arg.py index f625277a18..26d18fba2e 100644 --- a/test/AddOption/multi-arg.py +++ b/test/AddOption/multi-arg.py @@ -33,46 +33,50 @@ test = TestSCons.TestSCons() # First, test an option with nargs=2 and no others: -test.write( - 'SConstruct', - """\ +test.write('SConstruct', """\ DefaultEnvironment(tools=[]) -AddOption('--extras', - nargs=2, - dest='extras', - action='store', - type='string', - metavar='FILE1 FILE2', - default=(), - help='two extra files to install') -print(str(GetOption('extras'))) -""", +AddOption( + '-x', + '--extras', + nargs=2, + dest='extras', + action='store', + type='string', + metavar='FILE1 FILE2', + default=(), + help='two extra files to install', ) +print(str(GetOption('extras'))) +print(COMMAND_LINE_TARGETS) +""") # no args -test.run('-Q -q .', stdout="()\n") +test.run('-Q -q .', stdout="()\n['.']\n") # one arg, should fail -test.run( - '-Q -q . --extras A', - status=2, - stderr="""\ +test.run('-Q -q . --extras A', status=2, stderr="""\ usage: scons [OPTIONS] [VARIABLES] [TARGETS] SCons Error: --extras option requires 2 arguments -""", -) +""") +#one arg, short option +test.run('-Q -q . -x A', status=2, stderr="""\ +usage: scons [OPTIONS] [VARIABLES] [TARGETS] + +SCons Error: -x option requires 2 arguments +""") # two args -test.run('-Q -q . --extras A B', status=1, stdout="('A', 'B')\n") +test.run('-Q -q . --extras A B', stdout="('A', 'B')\n['.']\n") +# two args, short option +test.run('-Q -q . -x A B', stdout="('A', 'B')\n['.']\n") # -- means the rest are not processed as args -test.run('-Q -q . -- --extras A B', status=1, stdout="()\n") +test.run('-Q -q . -- --extras A B', status=1, stdout="()\n['.', '--extras', 'A', 'B']\n") # Now test what has been a bug: another option is # also defined, this impacts the collection of args for the nargs>1 opt -test.write( - 'SConstruct', - """\ +test.write('SConstruct', """\ DefaultEnvironment(tools=[]) AddOption( + '-P', '--prefix', nargs=1, dest='prefix', @@ -82,6 +86,7 @@ help='installation prefix', ) AddOption( + '-x', '--extras', nargs=2, dest='extras', @@ -93,26 +98,38 @@ ) print(str(GetOption('prefix'))) print(str(GetOption('extras'))) -""", -) - -# no options -test.run('-Q -q .', stdout="None\n()\n") -# one single-arg option -test.run('-Q -q . --prefix=/home/foo', stdout="/home/foo\n()\n") -# one two-arg option -test.run('-Q -q . --extras A B', status=2, stdout="None\n('A', 'B')\n") -# single-arg option followed by two-arg option +print(COMMAND_LINE_TARGETS) +""") +# no opts +test.run('-Q -q .', stdout="None\n()\n['.']\n") +# first opt long, one arg +test.run('-Q -q . --prefix=/home/foo', stdout="/home/foo\n()\n['.']\n") +test.run('-Q -q . --prefix /home/foo', stdout="/home/foo\n()\n['.']\n") +# first opt short, one arg +test.run('-Q -q . -P/home/foo', stdout="/home/foo\n()\n['.']\n") +test.run('-Q -q . -P /home/foo', stdout="/home/foo\n()\n['.']\n") +# second opt long, two args +test.run('-Q -q . --extras=A B', stdout="None\n('A', 'B')\n['.']\n") +test.run('-Q -q . --extras A B', stdout="None\n('A', 'B')\n['.']\n") +# second opt short, two args +test.run('-Q -q . -xA B', stdout="None\n('A', 'B')\n['.']\n") +test.run('-Q -q . -x A B', stdout="None\n('A', 'B')\n['.']\n") +# both opts long +test.run('-Q -q . --prefix=/home/foo --extras=A B', stdout="/home/foo\n('A', 'B')\n['.']\n") +test.run('-Q -q . --prefix /home/foo --extras A B', stdout="/home/foo\n('A', 'B')\n['.']\n") +# both opts short +test.run('-Q -q . -P/home/foo -xA B', stdout="/home/foo\n('A', 'B')\n['.']\n") +test.run('-Q -q . -P /home/foo -x A B', stdout="/home/foo\n('A', 'B')\n['.']\n") +# don't process test.run( - '-Q -q . --prefix=/home/foo --extras A B', + '-Q -q . -- --prefix=/home/foo --extras=A B', status=1, - stdout="/home/foo\n('A', 'B')\n", + stdout="None\n()\n['.', '--prefix=/home/foo', '--extras=A', 'B']\n", ) -# two-arg option followed by single-arg option test.run( - '-Q -q . --extras A B --prefix=/home/foo', + '-Q -q . -- --prefix /home/foo --extras A B', status=1, - stdout="/home/foo\n('A', 'B')\n", + stdout="None\n()\n['.', '--prefix', '/home/foo', '--extras', 'A', 'B']\n", ) test.pass_test() From 0a61b332ff41c47c29b56f80d709f0e2c170f0ce Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 23 Mar 2026 13:50:59 -0600 Subject: [PATCH 2/4] Enable the excluded tests for AddOption Testcases were added earlier (and recently expanded), but because they were previously illustarting a shortcoming, there were listed in the .exclude_tests file, and so not run unless explicitly listed. Removing that file should let the tests run. Signed-off-by: Mats Wichmann --- test/AddOption/.exclude_tests | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 test/AddOption/.exclude_tests diff --git a/test/AddOption/.exclude_tests b/test/AddOption/.exclude_tests deleted file mode 100644 index acc32f50ed..0000000000 --- a/test/AddOption/.exclude_tests +++ /dev/null @@ -1,4 +0,0 @@ -# for now, the tests showing problems with processing space-separated -# arguments are excluded, pending an implementation that doesn't fail. -args-and-targets.py -multi-arg.py From 359b2fbd043519b0609acdc2deffc609d31095b3 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 23 Mar 2026 16:08:32 -0600 Subject: [PATCH 3/4] AddOption tests: account for Windows paths The two expected exception message showed SCons paths with enough detail to include filesystem separators (SCons/Script/Main.py for example). Trimmed that back to include only the filename part so the slash-vs-backslash difference doesn't fail us. Signed-off-by: Mats Wichmann --- test/AddOption/args-and-targets.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/AddOption/args-and-targets.py b/test/AddOption/args-and-targets.py index 7517dc9ff6..6581989191 100644 --- a/test/AddOption/args-and-targets.py +++ b/test/AddOption/args-and-targets.py @@ -323,9 +323,9 @@ expect = r"""AttributeError: 'Values' object has no attribute 'foo': File ".+SConstruct4", line \d+: print\(str\(GetOption\('foo'\)\)\) - File ".+SCons/Script/Main.py", line \d+: + File "[^"]+Main.py", line \d+: return getattr\(OptionsParser.values, name\) - File ".+SCons/Script/SConsOptions.py", line \d+: + File "[^"]+SConsOptions.py", line \d+: return getattr\(self.__dict__\['__defaults__'\], attr\) """ test.run( @@ -416,9 +416,9 @@ expect=r"""AttributeError: 'Values' object has no attribute 'foo': File ".+SConstruct6", line \d+: print\(str\(GetOption\('foo'\)\)\) - File ".+SCons/Script/Main.py", line \d+: + File "[^"]+Main.py", line \d+: return getattr\(OptionsParser.values, name\) - File ".+SCons/Script/SConsOptions.py", line \d+: + File "[^"]+SConsOptions.py", line \d+: return getattr\(self.__dict__\['__defaults__'\], attr\) """ test.run( From d9ae6ad30a23db57ed458e0c99951f8d9ba7781a Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Wed, 15 Apr 2026 14:24:46 -0600 Subject: [PATCH 4/4] Docs changes for AddOption Change the manpage and user guide notes on options using spaces, now indicate you only need to worry about the issue if you need to run with old SCons versions. Also, per review, remove the now unused reparse_local_options method. Signed-off-by: Mats Wichmann --- SCons/Script/Main.xml | 27 ++++++-------- SCons/Script/SConsOptions.py | 62 -------------------------------- SCons/__init__.py | 10 +++--- doc/user/command-line.xml | 69 ++++++++++++++---------------------- 4 files changed, 42 insertions(+), 126 deletions(-) diff --git a/SCons/Script/Main.xml b/SCons/Script/Main.xml index 747b81c603..92bc2fbcee 100644 --- a/SCons/Script/Main.xml +++ b/SCons/Script/Main.xml @@ -172,22 +172,17 @@ see the &f-Help; documentation for details. -As an artifact of the internal implementation, -the behavior of options added by &AddOption; -which take option arguments is undefined -if whitespace -(rather than an = sign) is used as -the separator on the command line. -Users should avoid such usage; it is recommended -to add a note to this effect to project documentation -if the situation is likely to arise. -In addition, if the nargs -keyword is used to specify more than one following -option argument (that is, with a value of 2 -or greater), such arguments would necessarily -be whitespace separated, triggering the issue. -Developers should not use &AddOption; this way. -Future versions of &SCons; will likely forbid such usage. +Prior to version NEXT_RELEASE, +the behavior when options added by &AddOption; +are specified with whitespace was undefined and discouraged. +Such usage covers both the use of a space as the separator +(--opt arg vs +--opt=arg), +and where the number of option arguments +(nargs) is specified as greater than one. +While this issue has been corrected, +developers should be aware of it in case the +project is built with an older &SCons; version. diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py index a519679156..3ed89b51fd 100644 --- a/SCons/Script/SConsOptions.py +++ b/SCons/Script/SConsOptions.py @@ -496,68 +496,6 @@ def _process_short_opts(self, rargs, values) -> None: break - # TODO: this is now unused, remove? - def reparse_local_options(self) -> None: - """Re-parse the leftover command-line options. - - Leftover options are stored in ``self.largs``, so that any value - overridden on the command line is immediately available - if the user turns around and does a :func:`~SCons.Script.Main.GetOption` - right away. - - We mimic the processing of the single args - in the original OptionParser :func:`_process_args`, but here we - allow exact matches for long-opts only (no partial argument names!). - Otherwise there could be problems in :meth:`add_local_option` - below. When called from there, we try to reparse the - command-line arguments that haven't been processed so far - (``self.largs``), but are possibly not added to the options list yet. - - So, when we only have a value for ``--myargument`` so far, - a command-line argument of ``--myarg=test`` would set it, - per the behaviour of :func:`_match_long_opt`, - which allows for partial matches of the option name, - as long as the common prefix appears to be unique. - This would lead to further confusion, because we might want - to add another option ``--myarg`` later on (see issue #2929). - """ - rargs = [] - largs_restore = [] - # Loop over all remaining arguments - skip = False - for larg in self.largs: - if skip: - # Accept all remaining arguments as they are - largs_restore.append(larg) - else: - if len(larg) > 2 and larg[0:2] == "--": - # Check long option - lopt = [larg] - if "=" in larg: - # Split into option and value - lopt = larg.split("=", 1) - - if lopt[0] in self._long_opt: - # Argument is already known - rargs.append('='.join(lopt)) - else: - # Not known yet, so reject for now - largs_restore.append('='.join(lopt)) - else: - if larg in("--", "-"): - # Stop normal processing and don't - # process the rest of the command-line opts - largs_restore.append(larg) - skip = True - else: - rargs.append(larg) - - # Parse the filtered list - self.parse_args(rargs, self.values) - # Restore the list of leftover arguments for the - # next call of AddOption/add_local_option... - self.largs = self.largs + largs_restore - def add_local_option(self, *args, **kw) -> SConsOption: """Add a local option to the parser. diff --git a/SCons/__init__.py b/SCons/__init__.py index 9d0773be07..232476509d 100644 --- a/SCons/__init__.py +++ b/SCons/__init__.py @@ -1,9 +1,9 @@ __version__="4.10.2" __copyright__="Copyright (c) 2001 - 2026 The SCons Foundation" -__developer__="bdbaddog" -__date__="Sat, 03 Jan 2026 14:09:24 -0700" -__buildsys__="M1Dog2021" -__revision__="9573362eaf4dadc6368ad27a75bc2790a3e5e813" -__build__="9573362eaf4dadc6368ad27a75bc2790a3e5e813" +__developer__="mats" +__date__="Wed, 15 Apr 2026 13:01:10 -0600" +__buildsys__="boulder" +__revision__="359b2fbd043519b0609acdc2deffc609d31095b3" +__build__="359b2fbd043519b0609acdc2deffc609d31095b3" # make sure compatibility is always in place import SCons.compat # noqa \ No newline at end of file diff --git a/doc/user/command-line.xml b/doc/user/command-line.xml index 8e0e0a9de1..e61a8bdec2 100644 --- a/doc/user/command-line.xml +++ b/doc/user/command-line.xml @@ -741,51 +741,34 @@ foo.in + - The optparse parser which &SCons; uses - allows option-arguments to follow their options after either - an = or space separator, - however the latter form does not work well in &SCons; for - added options and should be avoided. - &SCons; does not place an ordering constraint on the - types of command-line arguments, - so while is unambiguous, - for - it is not possible to tell without instructions whether - ARG is an argument belonging to the - input option or a standalone word. - &SCons; considers words on the command line which do not - begin with hyphen as either command-line build variables - or command-line targets, - both of which are made available for use in an &SConscript; - (see the immediately following sections for details). - Thus, they must be collected before &SConscript; processing - takes place. &AddOption; calls do provide the - necessary instructions to resolve the ambiguity, - but as they appear in &SConscript; files, - &SCons; does not have the information early enough, - and unexpected things may happen, - such as option-arguments appearing in the list of targets, - and processing exceptions due to missing option-arguments. - - - As a result, - this usage style should be avoided when invoking &scons;. - For single-argument options, - tell your users to use the - form on the command line. - For multiple-argument options - (nargs value greater than one), - set nargs to one in the - &AddOption; call and either: combine the option-arguments into one word - with a separator, and parse the result in your own code - (see the built-in option, which - allows specifying multiple arguments as a single comma-separated - word, for an example of such usage); or allow the option to - be specified multiple times by setting - action='append'. Both methods can be - supported at the same time. + Prior to version NEXT_RELEASE, + the behavior when options added by &AddOption; + are specified on the command line with spaces was undefined, + and such usage was discouraged. + This applies to both using a space as the separator + (--opt arg vs + --opt=arg), + and when more than one option arguments is required. + In case your project may need to be built with an older + &SCons; version, you can take steps to avoid problems. + You cannot programmatically avoid the former case, + since it is allowed by the underlying &Python; module + that performs the command-line parsing, + but you can tell your users not to call that way. + For the latter case, + you can set the nargs to 1, + tell users to supply the arguments in comma-separated form + (--opt=arg1,arg2 instead of + --opt arg1 arg2), + and validate/parse that in the build script, + or alternatively, + allow the option to be specified multiple times by setting + action='append'. + Both methods can be supported at the same time. +