AddOption now recognizes "settable" option

AddOption and the internal add_local_option which AddOption calls now
recognize a "settable" keyword argument to indicate a project-added
option can also be modified using SetOption.

There was a TODO in SCons/Script/SConsOptions.py about removing
three hardcoded ninja options in the "settable" list once this
change was made. When that was done it turned out one test failed,
because it called the SetOption before the ninja tool was initialized.
Logic reordered in the test, but this is a thing to watch out for.

Closes #3983.

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

11 files changed, 212 insertions, 135 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index d0eb245..8005b68 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -99,6 +99,9 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
       SCons.Environment to SCons.Util to avoid the chance of import loops. Variables
       and Environment both use the routine and Environment() uses a Variables()
       object so better to move to a safer location.
+    - AddOption and the internal add_local_option which AddOption calls now
+      recognize a "settable" keyword argument to indicate a project-added
+      option can also be modified using SetOption. Fixes #3983.
 
 
 RELEASE 4.7.0 -  Sun, 17 Mar 2024 17:22:20 -0700
diff --git a/RELEASE.txt b/RELEASE.txt
index 8955e46..4d61af3 100644
--- a/RELEASE.txt
+++ b/RELEASE.txt
@@ -53,6 +53,9 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
 - The Variables object Add method now accepts a subst keyword argument
   (defaults to True) which can be set to inhibit substitution prior to
   calling the variable's converter and validator.
+- AddOption and the internal add_local_option which AddOption calls now
+  recognize a "settable" keyword argument to indicate a project-added
+  option can also be modified using SetOption.
 
 FIXES
 -----
diff --git a/SCons/Script/Main.py b/SCons/Script/Main.py
index d00da42..a5f8f42 100644
--- a/SCons/Script/Main.py
+++ b/SCons/Script/Main.py
@@ -41,7 +41,7 @@ import time
 import traceback
 import platform
 import threading
-from typing import Optional, List
+from typing import Optional, List, TYPE_CHECKING
 
 import SCons.CacheDir
 import SCons.Debug
@@ -59,6 +59,8 @@ import SCons.Taskmaster
 import SCons.Util
 import SCons.Warnings
 import SCons.Script.Interactive
+if TYPE_CHECKING:
+    from SCons.Script import SConsOption
 from SCons.Util.stats import count_stats, memory_stats, time_stats, ENABLE_JSON, write_scons_stats_file, JSON_OUTPUT_FILE
 
 from SCons import __version__ as SConsVersion
@@ -174,6 +176,7 @@ class Progressor:
 ProgressObject = SCons.Util.Null()
 
 def Progress(*args, **kw) -> None:
+    """Show progress during building - Public API."""
     global ProgressObject
     ProgressObject = Progressor(*args, **kw)
 
@@ -501,29 +504,47 @@ class FakeOptionParser:
     # TODO: to quiet checkers, FakeOptionParser should also define
     #   raise_exception_on_error, preserve_unknown_options, largs and parse_args
 
-    def add_local_option(self, *args, **kw) -> None:
+    def add_local_option(self, *args, **kw) -> "SConsOption":
         pass
 
 
 OptionsParser = FakeOptionParser()
 
-def AddOption(*args, **kw):
+def AddOption(*args, settable: bool = False, **kw) -> "SConsOption":
+    """Add a local option to the option parser - Public API.
+
+    If the *settable* parameter is true, the option will be included in the
+    list of settable options; all other keyword arguments are passed on to
+    :meth:`~SCons.Script.SConsOptions.SConsOptionParser.add_local_option`.
+
+    .. versionchanged:: 4.8.0
+       The *settable* parameter added to allow including the new option
+       to the table of options eligible to use :func:`SetOption`.
+
+    """
     if 'default' not in kw:
         kw['default'] = None
+    kw['settable'] = settable
     result = OptionsParser.add_local_option(*args, **kw)
     return result
 
-def GetOption(name):
+def GetOption(name: str):
+    """Get the value from an option - Public API."""
     return getattr(OptionsParser.values, name)
 
-def SetOption(name, value):
+def SetOption(name: str, value):
+    """Set the value of an option - Public API."""
     return OptionsParser.values.set_option(name, value)
 
-def DebugOptions(json=None):
-    """
-    API to allow specifying options to SCons debug logic
-    Currently only json is supported which changes the
-    json file written by --debug=json from the default
+def DebugOptions(json: Optional[str] = None) -> None:
+    """Specify options to SCons debug logic - Public API.
+
+    Currently only *json* is supported, which changes the JSON file
+    written to if the ``--debug=json`` command-line option is specified
+    to the value supplied.
+
+    .. versionadded:: 4.6.0
+
     """
     if json is not None:
         json_node = SCons.Defaults.DefaultEnvironment().arg2nodes(json)
@@ -540,7 +561,7 @@ def DebugOptions(json=None):
             raise SCons.Errors.UserError(f"Unable to create directory for JSON debug output file: {SCons.Util.stats.JSON_OUTPUT_FILE}")
 
 
-def ValidateOptions(throw_exception: bool=False) -> None:
+def ValidateOptions(throw_exception: bool = False) -> None:
     """Validate options passed to SCons on the command line.
 
     Checks that all options given on the command line are known to this
diff --git a/SCons/Script/Main.xml b/SCons/Script/Main.xml
index 36e7d30..6b63e91 100644
--- a/SCons/Script/Main.xml
+++ b/SCons/Script/Main.xml
@@ -93,21 +93,25 @@ the option value may be accessed using
 &f-link-GetOption;
 or
 &f-link-env-GetOption;.
-&f-link-SetOption; is not currently supported for
-options added with &f-AddOption;.
-<!-- was:
-The value may also be set using
-&f-SetOption;
+If the <parameter>settable=True</parameter> argument
+was supplied in the &AddOption; call,
+the value may also be set later using
+&f-link-SetOption;
 or
-&f-env.SetOption;,
-if conditions in a
-&SConscript;
+&f-link-env-SetOption;,
+if conditions in an
+&SConscript; file
 require overriding any default value.
 Note however that a
 value specified on the command line will
 <emphasis>always</emphasis>
-override a value set by any SConscript file.
--->
+override a value set in an SConscript file.
+</para>
+
+<para>
+<emphasis>Changed in 4.8.0</emphasis>: added the
+<parameter>settable</parameter> keyword argument
+to enable an added option to be settable via &SetOption;.
 </para>
 
 <para>
@@ -196,6 +200,7 @@ Allows setting options for SCons debug options. Currently the only supported val
   <example_commands>
 DebugOptions(json='#/build/output/scons_stats.json')
 </example_commands>
+<para><emphasis>New in version 4.6.0.</emphasis></para>
 </summary>
 </scons_function>
 
@@ -334,9 +339,10 @@ atexit.register(print_build_failures)
 <summary>
 <para>
 Query the value of settable options which may have been set
-on the command line, or by using the &f-link-SetOption; function.
+on the command line, via option defaults,
+or by using the &f-link-SetOption; function.
 The value of the option is returned in a type matching how the
-option was declared - see the documentation for the
+option was declared - see the documentation of the
 corresponding command line option for information about each specific
 option.
 </para>
@@ -802,6 +808,16 @@ be read in order to find the &f-SetOption; call in the first place.
 </para>
 
 <para>
+For project-specific options (sometimes called
+<firstterm>local options</firstterm>)
+added via an &f-link-AddOption; call,
+&f-SetOption; is available only after the
+&f-AddOption; call has completed successfully,
+and only if that call included the
+<parameter>settable=True</parameter> argument.
+</para>
+
+<para>
 The settable variables with their associated command-line options are:
 </para>
 
diff --git a/SCons/Script/SConsOptions.py b/SCons/Script/SConsOptions.py
index e8e5cbf..780c83b 100644
--- a/SCons/Script/SConsOptions.py
+++ b/SCons/Script/SConsOptions.py
@@ -66,17 +66,18 @@ def diskcheck_convert(value):
 
 
 class SConsValues(optparse.Values):
-    """
-    Holder class for uniform access to SCons options, regardless
-    of whether they can be set on the command line or in the
-    SConscript files (using the SetOption() function).
+    """Holder class for uniform access to SCons options.
+
+    Usable whether or not of whether they can be set on the command line
+    or in the SConscript files (using the :func:`~SComs.Main.SetOption`
+    function).
 
     A SCons option value can originate three different ways:
 
-        1)  set on the command line;
-        2)  set in an SConscript file;
-        3)  the default setting (from the the op.add_option()
-            calls in the Parser() function, below).
+    1) set on the command line;
+    2) set in an SConscript file;
+    3) the default setting (from the the ``op.add_option``
+       calls in the :func:`Parser` function, below).
 
     The command line always overrides a value set in a SConscript file,
     which in turn always overrides default settings.  Because we want
@@ -87,15 +88,16 @@ class SConsValues(optparse.Values):
 
     The solution implemented in this class is to keep these different sets
     of settings separate (command line, SConscript file, and default)
-    and to override the __getattr__() method to check them in turn.
+    and to override the :meth:`__getattr__` method to check them in turn
+    (a little similar in concept to a ChainMap).
     This should allow the rest of the code to just fetch values as
     attributes of an instance of this class, without having to worry
     about where they came from.
 
     Note that not all command line options are settable from SConscript
     files, and the ones that are must be explicitly added to the
-    "settable" list in this class, and optionally validated and coerced
-    in the set_option() method.
+    :attr:`settable` list in this class, and optionally validated and coerced
+    in the :meth:`set_option` method.
     """
 
     def __init__(self, defaults) -> None:
@@ -103,10 +105,11 @@ class SConsValues(optparse.Values):
         self.__SConscript_settings__ = {}
 
     def __getattr__(self, attr):
-        """
-        Fetches an options value, checking first for explicit settings
-        from the command line (which are direct attributes), then the
-        SConscript file settings, then the default values.
+        """Fetch an options value, respecting priority rules.
+
+        Check first for explicit settings from the command line
+        (which are direct attributes), then the SConscript file settings,
+        then the default values.
         """
         try:
             return self.__dict__[attr]
@@ -148,26 +151,24 @@ class SConsValues(optparse.Values):
         'silent',
         'stack_size',
         'warn',
-
-        # TODO: Remove these once we update the AddOption() API to allow setting
-        #       added flag as settable.
-        # Requested settable flag in : https://github.com/SCons/scons/issues/3983
-        # From experimental ninja
-        'disable_execute_ninja',
-        'disable_ninja',
-        'skip_ninja_regen'
     ]
 
     def set_option(self, name, value):
-        """Sets an option from an SConscript file.
+        """Sets an option *name* from an SConscript file.
+
+        Any necessary validation steps are in-line here. Validation should
+        be along the same lines as for options processed from the command
+        line - it's kind of a pain to have to duplicate. On the other
+        hand, we can hope that the build maintainer will be more careful
+        about correct SetOption values and it's not as big a deal to
+        have validation for everything.
 
         Raises:
-            UserError: invalid or malformed option ("error in your script")
+            UserError: the option is not settable.
         """
-
         if name not in self.settable:
             raise SCons.Errors.UserError(
-                "This option is not settable from a SConscript file: %s" % name
+                f"This option is not settable from an SConscript file: {name!r}"
             )
 
         # the following are for options that need some extra processing
@@ -247,7 +248,6 @@ class SConsOption(optparse.Option):
             return tuple([self.check_value(opt, v) for v in value])
 
     def process(self, opt, value, values, parser):
-
         # First, convert the value(s) to the right type.  Howl if any
         # value(s) are bogus.
         value = self.convert_value(opt, value)
@@ -397,46 +397,46 @@ class SConsOptionParser(optparse.OptionParser):
         option.process(opt, value, values, self)
 
     def reparse_local_options(self) -> None:
-        """ Re-parse the leftover command-line options.
+        """Re-parse the leftover command-line options.
 
-        Parse options stored in `self.largs`, so that any value
+        Parse 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:`GetOption` right away.
+        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 :func:`add_local_option`
+        Otherwise there could be  problems in :meth:`add_local_option`
         below. When called from there, we try to reparse the
         command-line arguments that
 
         1. haven't been processed so far (`self.largs`), but
         2. are possibly not added to the list of options yet.
 
-        So, when we only have a value for "--myargument" so far,
-        a command-line argument of "--myarg=test" would set it,
+        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).
-
+        to add another option ``--myarg`` later on (see issue #2929).
         """
         rargs = []
         largs_restore = []
         # Loop over all remaining arguments
         skip = False
-        for l in self.largs:
+        for larg in self.largs:
             if skip:
                 # Accept all remaining arguments as they are
-                largs_restore.append(l)
+                largs_restore.append(larg)
             else:
-                if len(l) > 2 and l[0:2] == "--":
+                if len(larg) > 2 and larg[0:2] == "--":
                     # Check long option
-                    lopt = (l,)
-                    if "=" in l:
+                    lopt = [larg]
+                    if "=" in larg:
                         # Split into option and value
-                        lopt = l.split("=", 1)
+                        lopt = larg.split("=", 1)
 
                     if lopt[0] in self._long_opt:
                         # Argument is already known
@@ -445,26 +445,35 @@ class SConsOptionParser(optparse.OptionParser):
                         # Not known yet, so reject for now
                         largs_restore.append('='.join(lopt))
                 else:
-                    if l == "--" or l == "-":
+                    if larg in("--", "-"):
                         # Stop normal processing and don't
                         # process the rest of the command-line opts
-                        largs_restore.append(l)
+                        largs_restore.append(larg)
                         skip = True
                     else:
-                        rargs.append(l)
+                        rargs.append(larg)
 
         # Parse the filtered list
         self.parse_args(rargs, self.values)
-        # Restore the list of remaining arguments for the
+        # 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):
+    def add_local_option(self, *args, **kw) -> SConsOption:
         """ Adds a local option to the parser.
 
-        This is initiated by an :func:`AddOption` call to add a user-defined
-        command-line option.  We add the option to a separate option
-        group for the local options, creating the group if necessary.
+        This is initiated by an :func:`~SCons.Script.Main.AddOption` call to
+        add a user-defined command-line option. Add the option to a separate
+        option group for the local options, creating the group if necessary.
+
+        The keyword argument *settable* is recognized specially (and
+        removed from *kw*). If true, the option is marked as modifiable;
+        by default "local" (project-added) options are not eligible for
+        for :func:`~SCons.Script.Main.SetOption` calls.
+
+        .. versionchanged:: 4.8.0
+           Added special handling of *settable*.
+
         """
         try:
             group = self.local_option_group
@@ -473,6 +482,7 @@ class SConsOptionParser(optparse.OptionParser):
             group = self.add_option_group(group)
             self.local_option_group = group
 
+        settable = kw.pop('settable')
         result = group.add_option(*args, **kw)
         if result:
             # The option was added successfully.  We now have to add the
@@ -485,6 +495,8 @@ class SConsOptionParser(optparse.OptionParser):
             # right away.
             setattr(self.values.__defaults__, result.dest, result.default)
             self.reparse_local_options()
+            if settable:
+                SConsValues.settable.append(result.dest)
 
         return result
 
@@ -614,7 +626,8 @@ class SConsIndentedHelpFormatter(optparse.IndentedHelpFormatter):
         """Local-only version of store_option_strings.
 
         We need to replicate this so the formatter will be set up
-        properly if we didn't go through the "normal" store_option_strings
+        properly if we didn't go through the "normal"
+        :math:`~optparse.HelpFormatter.store_option_strings`.
 
         .. versionadded:: 4.6.0
         """
@@ -633,15 +646,18 @@ def Parser(version):
     """Returns a parser object initialized with the standard SCons options.
 
     Add options in the order we want them to show up in the ``-H`` help
-    text, basically alphabetical.  Each ``op.add_option()`` call
-    should have a consistent format::
-
-      op.add_option("-L", "--long-option-name",
-                    nargs=1, type="string",
-                    dest="long_option_name", default='foo',
-                    action="callback", callback=opt_long_option,
-                    help="help text goes here",
-                    metavar="VAR")
+    text, basically alphabetical.  For readability, Each
+    :meth:`~optparse.OptionContainer.add_option` call should have a
+    consistent format::
+
+      op.add_option(
+          "-L", "--long-option-name",
+          nargs=1, type="string",
+          dest="long_option_name", default='foo',
+          action="callback", callback=opt_long_option,
+          help="help text goes here",
+          metavar="VAR"
+      )
 
     Even though the :mod:`optparse` module constructs reasonable default
     destination names from the long option names, we're going to be
diff --git a/SCons/Tool/ninja/Utils.py b/SCons/Tool/ninja/Utils.py
index 126d5de..cdce928 100644
--- a/SCons/Tool/ninja/Utils.py
+++ b/SCons/Tool/ninja/Utils.py
@@ -45,6 +45,7 @@ def ninja_add_command_line_options() -> None:
               metavar='BOOL',
               action="store_true",
               default=False,
+              settable=True,
               help='Disable automatically running ninja after scons')
 
     AddOption('--disable-ninja',
@@ -52,6 +53,7 @@ def ninja_add_command_line_options() -> None:
               metavar='BOOL',
               action="store_true",
               default=False,
+              settable=True,
               help='Disable ninja generation and build with scons even if tool is loaded. '+
                    'Also used by ninja to build targets which only scons can build.')
 
@@ -60,6 +62,7 @@ def ninja_add_command_line_options() -> None:
               metavar='BOOL',
               action="store_true",
               default=False,
+              settable=True,
               help='Allow scons to skip regeneration of the ninja file and restarting of the daemon. ' +
                     'Care should be taken in cases where Glob is in use or SCons generated files are used in ' +
                     'command lines.')
diff --git a/doc/user/command-line.xml b/doc/user/command-line.xml
index fea1af1..d4bb83d 100644
--- a/doc/user/command-line.xml
+++ b/doc/user/command-line.xml
@@ -417,8 +417,10 @@ foo.in
 
       <para>
 
-      &SetOption; is not currently supported for
-      options added with &AddOption;.
+      &SetOption; works for options added with &AddOption;,
+      but only if they were created with
+      <parameter>settable=True</parameter> in the call to &AddOption;
+      (only available in SCons 4.8.0 and later).
 
       </para>
 
@@ -660,14 +662,12 @@ foo.in
       </para>
 
       <para>
-      &SetOption; is not currently supported for
-      options added with &AddOption;.
-      <!--
-      (The value can also be set using &SetOption;,
-      although that's not very useful in practice
-      because a default value can be specified in
-      directly in the &AddOption; call.)
-      -->
+
+      &f-link-SetOption; works for options added with &AddOption;,
+      but only if they were created with
+      <parameter>settable=True</parameter> in the call to &AddOption;
+      (only available in SCons 4.8.0 and later).
+
       </para>
 
       <para>
diff --git a/test/AddOption/basic.py b/test/AddOption/basic.py
index 64fb7ba..9fc3c4d 100644
--- a/test/AddOption/basic.py
+++ b/test/AddOption/basic.py
@@ -24,8 +24,8 @@
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 """
-Verify the help text when the AddOption() function is used (and when
-it's not).
+Verify added options give the expected default/command line values
+when fetched with GetOption.
 """
 
 import TestSCons
@@ -35,35 +35,44 @@ test = TestSCons.TestSCons()
 test.write('SConstruct', """\
 DefaultEnvironment(tools=[])
 env = Environment(tools=[])
-AddOption('--force',
-          action="store_true",
-          help='force installation (overwrite any existing files)')
-AddOption('--prefix',
-          nargs=1,
-          dest='prefix',
-          action='store',
-          type='string',
-          metavar='DIR',
-          help='installation prefix')
+AddOption(
+    '--force',
+    action="store_true",
+    help='force installation (overwrite any existing files)',
+)
+AddOption(
+    '--prefix',
+    nargs=1,
+    dest='prefix',
+    action='store',
+    type='string',
+    metavar='DIR',
+    settable=True,
+    help='installation prefix',
+)
+AddOption(
+    '--set',
+    action="store_true",
+    help="try SetOption of 'prefix' to '/opt/share'"
+)
 f = GetOption('force')
 if f:
     f = "True"
 print(f)
 print(GetOption('prefix'))
+if GetOption('set'):
+    SetOption('prefix', '/opt/share')
+    print(GetOption('prefix'))
 """)
 
-test.run('-Q -q .',
-         stdout="None\nNone\n")
-
-test.run('-Q -q . --force',
-         stdout="True\nNone\n")
-
-test.run('-Q -q . --prefix=/home/foo',
-         stdout="None\n/home/foo\n")
-
-test.run('-Q -q . -- --prefix=/home/foo --force',
-         status=1,
-         stdout="None\nNone\n")
+test.run('-Q -q .', stdout="None\nNone\n")
+test.run('-Q -q . --force', stdout="True\nNone\n")
+test.run('-Q -q . --prefix=/home/foo', stdout="None\n/home/foo\n")
+test.run('-Q -q . -- --prefix=/home/foo --force', status=1, stdout="None\nNone\n")
+# check that SetOption works on prefix...
+test.run('-Q -q . --set', stdout="None\nNone\n/opt/share\n")
+# but the "command line wins" rule is not violated
+test.run('-Q -q . --set --prefix=/home/foo', stdout="None\n/home/foo\n/home/foo\n")
 
 test.pass_test()
 
diff --git a/test/AddOption/help.py b/test/AddOption/help.py
index 6d855bd..772a381 100644
--- a/test/AddOption/help.py
+++ b/test/AddOption/help.py
@@ -23,6 +23,11 @@
 # OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
 # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
+"""
+Verify the help text when the AddOption() function is used (and when
+it's not).
+"""
+
 import TestSCons
 
 test = TestSCons.TestSCons()
@@ -30,16 +35,20 @@ test = TestSCons.TestSCons()
 test.write('SConstruct', """\
 DefaultEnvironment(tools=[])
 env = Environment(tools=[])
-AddOption('--force',
-          action="store_true",
-          help='force installation (overwrite existing files)')
-AddOption('--prefix',
-          nargs=1,
-          dest='prefix',
-          action='store',
-          type='string',
-          metavar='DIR',
-          help='installation prefix')
+AddOption(
+    '--force',
+    action="store_true",
+    help='force installation (overwrite existing files)',
+)
+AddOption(
+    '--prefix',
+    nargs=1,
+    dest='prefix',
+    action='store',
+    type='string',
+    metavar='DIR',
+    help='installation prefix',
+)
 """)
 
 expected_lines = [
diff --git a/test/GetOption/BadSetOption.py b/test/GetOption/BadSetOption.py
index 7b0a33d..f408e39 100644
--- a/test/GetOption/BadSetOption.py
+++ b/test/GetOption/BadSetOption.py
@@ -32,7 +32,7 @@ import TestSCons
 test = TestSCons.TestSCons()
 
 badopts = (
-    ("no_such_var", True, "This option is not settable from a SConscript file: no_such_var"),
+    ("no_such_var", True, "This option is not settable from an SConscript file: 'no_such_var'"),
     ("num_jobs", -22, "A positive integer is required: -22"),
     ("max_drift", "'Foo'", "An integer is required: 'Foo'"),
     ("duplicate", "'cookie'", "Not a valid duplication style: cookie"),
@@ -45,11 +45,10 @@ for opt, value, expect in badopts:
     SConstruct = "SC-" + opt
     test.write(
         SConstruct,
-        """\
+        f"""\
 DefaultEnvironment(tools=[])
-SetOption("%(opt)s", %(value)s)
-"""
-        % locals(),
+SetOption("{opt}", {value})
+""",
     )
     expect = r"scons: *** %s" % expect
     test.run(arguments='-Q -f %s .' % SConstruct, stderr=None, status=2)
diff --git a/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism
index a7982d4..6d20636 100644
--- a/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism
+++ b/test/ninja/ninja_test_sconscripts/sconstruct_ninja_determinism
@@ -5,12 +5,10 @@
 import random
 
 SetOption('experimental', 'ninja')
-SetOption('skip_ninja_regen', True)
 DefaultEnvironment(tools=[])
-
 env = Environment(tools=[])
-
 env.Tool('ninja')
+SetOption('skip_ninja_regen', True)  # must wait until tool creates the option
 
 # make the dependency list vary in order. Ninja tool should sort them to be deterministic.
 for i in range(1, 10):