diff options
author | William Deegan <bill@baddogconsulting.com> | 2023-05-03 20:05:30 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-03 20:05:30 -0700 |
commit | 84859d565216af998f817e05d0696f3423bb7216 (patch) | |
tree | 203b499babac9dbed56afbd1e1eea548ac1b9932 | |
parent | 5717e06c245d99923f5c4a6e786a564799871c49 (diff) | |
parent | 239f437b463dc948690473830f1816e9a67b72c2 (diff) | |
download | scons-git-84859d565216af998f817e05d0696f3423bb7216.tar.gz |
Merge pull request #4336 from mwichmann/maint/validateOptions
Minor cleanup ValidateOptions doc/code/test
-rw-r--r-- | CHANGES.txt | 2 | ||||
-rw-r--r-- | SCons/Script/Main.py | 95 | ||||
-rw-r--r-- | SCons/Script/Main.xml | 127 | ||||
-rw-r--r-- | test/ValidateOptions.py | 64 | ||||
-rw-r--r-- | test/fixture/SConstruct-check-valid-options | 10 |
5 files changed, 186 insertions, 112 deletions
diff --git a/CHANGES.txt b/CHANGES.txt index e44c00fb5..ddb957ae1 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -15,6 +15,8 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER embedded in a sequence, or by itself. The conditional C scanner thus did not always properly apply the defines. The regular C scanner does not use these, so was not affected. [fixes #4193] + - Minor cleanup for ValidateOptions - docs and docstring tweaked, + add missed versionadded indicator. - Added some typing annotations generated by a tool, to eliminate manual work in future on things which are safe for the tool to produce. - Simplify some code due to pylint observation: "C2801: Unnecessarily diff --git a/SCons/Script/Main.py b/SCons/Script/Main.py index 05e070f61..c3f31ca3e 100644 --- a/SCons/Script/Main.py +++ b/SCons/Script/Main.py @@ -31,13 +31,8 @@ some other module. If it's specific to the "scons" script invocation, it goes here. """ -# these define the range of versions SCons supports -minimum_python_version = (3, 6, 0) -deprecated_python_version = (3, 6, 0) - import SCons.compat -import atexit import importlib.util import os import re @@ -46,6 +41,7 @@ import time import traceback import platform import threading +from typing import Optional, List import SCons.CacheDir import SCons.Debug @@ -66,6 +62,20 @@ import SCons.Script.Interactive from SCons import __version__ as SConsVersion +# these define the range of versions SCons supports +minimum_python_version = (3, 6, 0) +deprecated_python_version = (3, 6, 0) + +# ordered list of SConsctruct names to look for if there is no -f flag +KNOWN_SCONSTRUCT_NAMES = [ + 'SConstruct', + 'Sconstruct', + 'sconstruct', + 'SConstruct.py', + 'Sconstruct.py', + 'sconstruct.py', +] + # Global variables first_command_start = None last_command_end = None @@ -284,7 +294,7 @@ class BuildTask(SCons.Taskmaster.OutOfDateTask): node = buildError.node if not SCons.Util.is_List(node): - node = [ node ] + node = [node] nodename = ', '.join(map(str, node)) errfmt = "scons: *** [%s] %s\n" @@ -460,24 +470,29 @@ def python_version_deprecated(version=sys.version_info): class FakeOptionParser: - """ - A do-nothing option parser, used for the initial OptionsParser variable. + """A do-nothing option parser, used for the initial OptionsParser value. During normal SCons operation, the OptionsParser is created right - away by the main() function. Certain tests scripts however, can + away by the main() function. Certain test scripts however, can introspect on different Tool modules, the initialization of which can try to add a new, local option to an otherwise uninitialized OptionsParser object. This allows that introspection to happen without blowing up. - """ + class FakeOptionValues: def __getattr__(self, attr): return None + values = FakeOptionValues() + + # 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: pass + OptionsParser = FakeOptionParser() def AddOption(*args, **kw): @@ -492,22 +507,26 @@ def GetOption(name): def SetOption(name, value): return OptionsParser.values.set_option(name, value) - def ValidateOptions(throw_exception: bool=False) -> None: """Validate options passed to SCons on the command line. - If you call this after you set all your command line options with AddOption(), - it will verify that all command line options are valid. - So if you added an option --xyz and you call SCons with --xyy you can cause + Checks that all options given on the command line are known to this + instance of SCons. Call after all of the cli options have been set + up through :func:`AddOption` calls. For example, if you added an + option ``--xyz`` and you call SCons with ``--xyy`` you can cause SCons to issue an error message and exit by calling this function. - :param bool throw_exception: (Optional) Should this function raise an error if there's an invalid option on the command line, or issue a message and exit with error status. + Arguments: + throw_exception: if an invalid option is present on the command line, + raises an exception if this optional parameter evaluates true; + if false (the default), issue a message and exit with error status. - :raises SConsBadOptionError: If throw_exception is True and there are invalid options on command line. + Raises: + SConsBadOptionError: If *throw_exception* is true and there are invalid + options on the command line. .. versionadded:: 4.5.0 """ - OptionsParser.raise_exception_on_error = throw_exception OptionsParser.preserve_unknown_options = False OptionsParser.parse_args(OptionsParser.largs, OptionsParser.values) @@ -641,13 +660,24 @@ def _scons_internal_error() -> None: traceback.print_exc() sys.exit(2) -def _SConstruct_exists(dirname: str='', repositories=[], filelist=None): - """This function checks that an SConstruct file exists in a directory. - If so, it returns the path of the file. By default, it checks the - current directory. +def _SConstruct_exists( + dirname: str, repositories: List[str], filelist: List[str] +) -> Optional[str]: + """Check that an SConstruct file exists in a directory. + + Arguments: + dirname: the directory to search. If empty, look in cwd. + repositories: a list of repositories to search in addition to the + project directory tree. + filelist: names of SConstruct file(s) to search for. + If empty list, use the built-in list of names. + + Returns: + The path to the located SConstruct file, or ``None``. + """ if not filelist: - filelist = ['SConstruct', 'Sconstruct', 'sconstruct', 'SConstruct.py', 'Sconstruct.py', 'sconstruct.py'] + filelist = KNOWN_SCONSTRUCT_NAMES for file in filelist: sfile = os.path.join(dirname, file) if os.path.isfile(sfile): @@ -679,14 +709,14 @@ def _set_debug_values(options) -> None: SCons.Warnings.warn(SCons.Warnings.NoObjectCountWarning, msg) if "dtree" in debug_values: options.tree_printers.append(TreePrinter(derived=True)) - options.debug_explain = ("explain" in debug_values) + options.debug_explain = "explain" in debug_values if "findlibs" in debug_values: SCons.Scanner.Prog.print_find_libs = "findlibs" - options.debug_includes = ("includes" in debug_values) - print_memoizer = ("memoizer" in debug_values) + options.debug_includes = "includes" in debug_values + print_memoizer = "memoizer" in debug_values if "memory" in debug_values: memory_stats.enable(sys.stdout) - print_objects = ("objects" in debug_values) + print_objects = "objects" in debug_values if print_objects: SCons.Debug.track_instances = True if "presub" in debug_values: @@ -919,9 +949,9 @@ def _main(parser): target_top = None if options.climb_up: target_top = '.' # directory to prepend to targets - while script_dir and not _SConstruct_exists(script_dir, - options.repository, - options.file): + while script_dir and not _SConstruct_exists( + script_dir, options.repository, options.file + ): script_dir, last_part = os.path.split(script_dir) if last_part: target_top = os.path.join(last_part, target_top) @@ -951,8 +981,7 @@ def _main(parser): if options.file: scripts.extend(options.file) if not scripts: - sfile = _SConstruct_exists(repositories=options.repository, - filelist=options.file) + sfile = _SConstruct_exists("", options.repository, options.file) if sfile: scripts.append(sfile) @@ -1011,8 +1040,8 @@ def _main(parser): # Next, set up the variables that hold command-line arguments, # so the SConscript files that we read and execute have access to them. # TODO: for options defined via AddOption which take space-separated - # option-args, the option-args will collect into targets here, - # because we don't yet know to do any different. + # option-args, the option-args will collect into targets here, + # because we don't yet know to do any different. targets = [] xmit_args = [] for a in parser.largs: diff --git a/SCons/Script/Main.xml b/SCons/Script/Main.xml index 2070c36b0..379d5347e 100644 --- a/SCons/Script/Main.xml +++ b/SCons/Script/Main.xml @@ -749,7 +749,7 @@ Sets &scons; option variable <parameter>name</parameter> to <parameter>value</parameter>. These options are all also settable via command-line options but the variable name -may differ from the command-line option name - +may differ from the command-line option name - see the table for correspondences. A value set via command-line option will take precedence over one set with &f-SetOption;, which @@ -946,64 +946,79 @@ SetOption('max_drift', 0) </scons_function> - <scons_function name="ValidateOptions"> - <arguments signature="global"> - ([throw_exception=False]) - </arguments> - - <summary> - <para> - Check that all the options specified on the command line are either defined by SCons itself - or defined by calls to &f-link-AddOption;. - </para> - <para> - This function should only be called after the last &f-link-AddOption; call in your &SConscript; - logic. - </para> - <para> - Be aware that some tools call &f-link-AddOption;, if you are getting error messages for arguments - that they add, you will need to ensure that you load those tools before you call &f-ValidateOptions;. - </para> - <para> - If there are any command line options not defined, calling this function will cause SCons to issue an - error message and then exit with an error exit - status.</para> - <para>If the optional <parameter>throw_exception</parameter> is <literal>True</literal>, &f-ValidateOptions; will raise a - <exceptionname>SConsBadOptionError</exceptionname> - exception. This would allow the calling - &SConscript; logic can catch that exception and handle invalid options itself. - </para> - - <para> - Example: - </para> - - <example_commands> + <scons_function name="ValidateOptions"> + <arguments signature="global">([throw_exception=False])</arguments> + + <summary> + <para> + Check that all the options specified on the command line are either + &SCons; built-in options or defined via calls to &f-link-AddOption;. + &SCons; will eventually fail on unknown options anyway, but calling + this function allows the build to "fail fast" before executing + expensive logic later in the build. + </para> + + <para> + This function should only be called after the last &f-AddOption; + call in your &SConscript; logic. + Be aware that some tools call &f-AddOption;, if you are getting + error messages for arguments that they add, you will need to ensure + that those tools are loaded before calling &f-ValidateOptions;. + </para> + + <para> + If there are any unknown command line options, &f-ValidateOptions; + prints an error message and exits with an error exit status. + If the optional <parameter>throw_exception</parameter> argument is + <literal>True</literal> (default is <literal>False</literal>), + a <exceptionname>SConsBadOptionError</exceptionname> is raised, + giving an opportunity for the &SConscript; logic to catch that + exception and handle invalid options appropriately. Note that + this exception name needs to be imported (see the example below). + </para> + + <para> + A common build problem is typos (or thinkos) - a user enters an option + that is just a little off the expected value, or perhaps a different + word with a similar meaning. It may be useful to abort the build + before going too far down the wrong path. For example: + </para> + + <screen> +$ <userinput>scons --compilers=mingw</userinput> # the correct flag is --compiler + </screen> + + <para> + Here &SCons; could go off and run a bunch of configure steps with + the default value of <literal>--compiler</literal>, since the + incorrect command line did not actually supply a value to it, + costing developer time to track down why the configure logic + made the "wrong" choices. This example shows catching this: + </para> + + <programlisting language="python"> +from SCons.Script.SConsOptions import SConsBadOptionError + +AddOption( + '--compiler', + dest='compiler', + action='store', + default='gcc', + type='string', +) + +# ... other SConscript logic ... + try: ValidateOptions(throw_exception=True) except SConsBadOptionError as e: - print("Parser is SConsOptionParser:%s" % (isinstance(e.parser, SConsOptionParser))) - print("Message is :%s" % e.opt_str) + print(f"ValidateOptions detects a fail: ", e.opt_str) Exit(3) - </example_commands> - - <para> - This function is useful to force SCons to fail fast before you execute any expensive logic later in your - build logic. - For example if you specify build options via any flags, a simple typo could yield the incorrect build - option throughout your entire build. - </para> - <example_commands> -scons --compilers=mingw (the correct flag is --compiler) - </example_commands> - <para> - Could cause SCons to run configure steps with the incorrect compiler. Costing developer time trying to - track down why the configure logic failed with a compiler which should work. - </para> - <para> - <emphasis>New in version 4.5.0</emphasis> - </para> - </summary> - </scons_function> + </programlisting> + + <para><emphasis>New in version 4.5.0</emphasis></para> + + </summary> + </scons_function> </sconsdoc> diff --git a/test/ValidateOptions.py b/test/ValidateOptions.py index 9b53c0949..5dff386e2 100644 --- a/test/ValidateOptions.py +++ b/test/ValidateOptions.py @@ -28,38 +28,60 @@ Test ValidateOptions(). import TestSCons test = TestSCons.TestSCons() +# ref: test/fixture/SConstruct-check-valid-options test.file_fixture('fixture/SConstruct-check-valid-options', 'SConstruct') -# Should see "This is in SConstruct" because all options specified (none) are valid and -# so ValidatedOptions() won't exit before it's printed. +# Should see "This is in SConstruct" because all options specified (none) +# are valid and so ValidatedOptions() won't exit before it's printed. test.run() test.must_contain_single_instance_of(test.stdout(), ["This is in SConstruct"]) -# Should see "This is in SConstruct" because all options specified (--testing=abc) are valid and -# so ValidatedOptions() won't exit before it's printed. +# Should see "This is in SConstruct" because all options specified +# (--testing=abc) are valid and so ValidatedOptions() won't exit before +# it's printed. test.run(arguments="--testing=abc") test.must_contain_single_instance_of(test.stdout(), ["This is in SConstruct"]) -# Should not see "This is in SConstruct" because the option specified (--garbage=xyz) is invalid and -# so ValidatedOptions() will exit before it's printed. -test.run(arguments="--garbage=xyz", status=2, stderr=".*SCons Error: no such option: --garbage.*", - match=TestSCons.match_re_dotall) -test.fail_test(("This is in SConstruct" in test.stdout()), - message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed') +# Should not see "This is in SConstruct" because the option specified +# (--garbage=xyz) is invalid and so ValidatedOptions() will exit +# before it's printed. +test.run( + arguments="--garbage=xyz", + status=2, + stderr=".*SCons Error: no such option: --garbage.*", + match=TestSCons.match_re_dotall, +) +test.fail_test( + ("This is in SConstruct" in test.stdout()), + message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed', +) # Now we'll test having ValidateOptions raise a SConsBadOptionError exception -test.run(arguments="--garbage=xyz raise=1", status=2, - stderr=".*SConsBadOptionError: no such option: no such option: --garbage.*", - match=TestSCons.match_re_dotall) -test.fail_test(("This is in SConstruct" in test.stdout()), - message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed') +test.run( + arguments="--garbage=xyz raise=1", + status=2, + stderr=".*SConsBadOptionError: no such option: no such option: --garbage.*", + match=TestSCons.match_re_dotall, +) +test.fail_test( + ("This is in SConstruct" in test.stdout()), + message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed', +) -# Now we'll test having ValidateOptions raise a SConsBadOptionError exception and catching that exception -test.run(arguments="--garbage=xyz raise=2", status=3, - stdout=".*Parser is SConsOptionParser:True.*Message is .no such option. --garbage.*", - match=TestSCons.match_re_dotall) -test.fail_test(("This is in SConstruct" in test.stdout()), - message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed') +# Now we'll test having ValidateOptions raise a SConsBadOptionError +# exception and catching that exception +test.run( + arguments="--garbage=xyz raise=2", + status=3, + stdout=".*Parser is SConsOptionParser: True.*Message is. no such option. --garbage.*", + match=TestSCons.match_re_dotall, +) +test.fail_test( + ("This is in SConstruct" in test.stdout()), + message='"This is in SConstruct" should not be output. This means ValidateOptions() did not error out before this was printed', +) + +test.pass_test() # Local Variables: # tab-width:4 diff --git a/test/fixture/SConstruct-check-valid-options b/test/fixture/SConstruct-check-valid-options index 2c935a23c..53bdf89d3 100644 --- a/test/fixture/SConstruct-check-valid-options +++ b/test/fixture/SConstruct-check-valid-options @@ -1,3 +1,7 @@ +# SPDX-License-Identifier: MIT +# +# Copyright The SCons Foundation + import sys from SCons.Script.SConsOptions import SConsOptionParser, SConsBadOptionError @@ -12,8 +16,10 @@ elif ARGUMENTS.get('raise', 0) == '2': try: ValidateOptions(throw_exception=True) except SConsBadOptionError as e: - print("Parser is SConsOptionParser:%s" % (isinstance(e.parser, SConsOptionParser))) - print("Message is :%s" % e.opt_str) + print( + f"Parser is SConsOptionParser: {isinstance(e.parser, SConsOptionParser)}" + ) + print(f"Message is: {e.opt_str}") Exit(3) else: ValidateOptions() |