From 9ec0b9861951cc72de39c742c1162a84696ce91f Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sat, 21 Jul 2018 11:55:31 -0600 Subject: Add ability for SConscript to fail on missing script SConscript call now takes an optional must_exist flag, which defaults to False for compatiility with current behavior. If True, an exception is raised if the file is missing. To improve readability, the decision is moved off to a new function rather than being inline in _SConscript. A global setting to control the overall behavior is also added. A deprecation warning is added for the current behavior, which is printed only once. Signed-off-by: Mats Wichmann --- src/CHANGES.txt | 1 + src/engine/SCons/Script/SConscript.py | 24 ++++++++++++++++++++++-- src/engine/SCons/Script/SConscript.xml | 18 ++++++++++++++---- src/engine/SCons/Script/__init__.py | 10 ++++++++++ src/engine/SCons/Warnings.py | 7 +++++-- 5 files changed, 52 insertions(+), 8 deletions(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 79d25db..3b0e603 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -97,6 +97,7 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE This changes SCons to better comply with normal Python installation practices. From Mats Wichmann: + - Begin adding support for SConscript() failing on missing script - Updated manpage scons.xml to fix a nested list problem - Updated doc terminiology: use prepend instead of append as appropriate - xml validity fixes from SConstruct.py change diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py index db6552c..5968346 100644 --- a/src/engine/SCons/Script/SConscript.py +++ b/src/engine/SCons/Script/SConscript.py @@ -153,6 +153,27 @@ def Return(*vars, **kw): stack_bottom = '% Stack boTTom %' # hard to define a variable w/this name :) +def handle_missing_SConscript(f, must_exist): + """Take appropriate action on missing file in SConscript() call. + + The action may be to raise an exception, or print a warning. + On first warning, also print a deprecation warning. + """ + + if SCons.Script._no_missing_sconscript or must_exist: + msg = "Fatal: missing SConscript '%s'" % f.get_internal_path() + raise SCons.Errors.UserError(msg) + + if SCons.Script._warn_missing_sconscript_deprecated: + msg = "Calling missing SConscripts without error is deprecated.\n" + \ + "Transition by adding must_exist=0 to SConscript calls.\n" + \ + "Missing SConscript '%s'" % f.get_internal_path() + SCons.Warnings.warn(SCons.Warnings.DeprecatedMissingSConscriptWarning, msg) + SCons.Script._warn_missing_sconscript_deprecated = False + else: + msg = "Ignoring missing SConscript '%s'" % f.get_internal_path() + SCons.Warnings.warn(SCons.Warnings.MissingSConscriptWarning, msg) + def _SConscript(fs, *files, **kw): top = fs.Top sd = fs.SConstruct_dir.rdir() @@ -264,8 +285,7 @@ def _SConscript(fs, *files, **kw): if old_file is not None: call_stack[-1].globals.update({__file__:old_file}) else: - SCons.Warnings.warn(SCons.Warnings.MissingSConscriptWarning, - "Ignoring missing SConscript '%s'" % f.get_internal_path()) + handle_missing_SConscript(f, kw.get('must_exist', False)) finally: SCons.Script.sconscript_reading = SCons.Script.sconscript_reading - 1 diff --git a/src/engine/SCons/Script/SConscript.xml b/src/engine/SCons/Script/SConscript.xml index 8553fbe..29a89c2 100644 --- a/src/engine/SCons/Script/SConscript.xml +++ b/src/engine/SCons/Script/SConscript.xml @@ -357,12 +357,12 @@ Return('val1 val2') -(scripts, [exports, variant_dir, duplicate]) - +(scripts, [exports, variant_dir, duplicate, must_exist=flag]) + -(dirs=subdirs, [name=script, exports, variant_dir, duplicate]) - +(dirs=subdirs, [name=script, exports, variant_dir, duplicate, must_exist=flag]) + @@ -562,6 +562,16 @@ and what about this alternative? TODO??? SConscript('build/SConscript', src_dir='src') --> + +The optional +must_exist +argument, if true, causes an exception to be raised if a requested +&SConscript; file is not found. The default is false, +which only prints a warning, but this behavior is deprecated. +For scripts which truly intend to be optional, transition to +explicty supplying +must_exist=False to the call. + Here are some composite examples: diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py index 89fc061..90bc311 100644 --- a/src/engine/SCons/Script/__init__.py +++ b/src/engine/SCons/Script/__init__.py @@ -276,6 +276,16 @@ def HelpFunction(text, append=False): # Will be non-zero if we are reading an SConscript file. sconscript_reading = 0 +_no_missing_sconscript = False +_warn_missing_sconscript_deprecated = True + +def set_missing_sconscript_error(flag=1): + """Set behavior on missing file in SConscript() call. Returns previous value""" + global _no_missing_sconscript + old = _no_missing_sconscript + _no_missing_sconscript = flag + return old + # def Variables(files=[], args=ARGUMENTS): return SCons.Variables.Variables(files, args) diff --git a/src/engine/SCons/Warnings.py b/src/engine/SCons/Warnings.py index e8158a4..63acd22 100644 --- a/src/engine/SCons/Warnings.py +++ b/src/engine/SCons/Warnings.py @@ -147,6 +147,9 @@ class DeprecatedSigModuleWarning(MandatoryDeprecatedWarning): class DeprecatedBuilderKeywordsWarning(MandatoryDeprecatedWarning): pass +class DeprecatedMissingSConscriptWarning(DeprecatedWarning): + pass + # The below is a list of 2-tuples. The first element is a class object. # The second element is true if that class is enabled, false if it is disabled. @@ -179,8 +182,8 @@ def warn(clazz, *args): global _enabled, _warningAsException, _warningOut warning = clazz(args) - for clazz, flag in _enabled: - if isinstance(warning, clazz): + for cls, flag in _enabled: + if isinstance(warning, cls): if flag: if _warningAsException: raise warning -- cgit v0.12 From 14daa07c0d95e1cd806e92911a74bde3ccf95b9f Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sat, 28 Jul 2018 10:04:35 -0600 Subject: Add tests for SConscript(must_warn) option Testcases added to confirm the behavior of: first attempt to call a non-existent script gives a deprecation warning, additional ones give plain warning; True/False values for must_warn behave as expected; if scons default is changed to exception the call fails but if must_warn=False it still works. Tweaked the logic to actually get that last bit to work. Also minor doc update. Signed-off-by: Mats Wichmann --- src/engine/SCons/Script/SConscript.py | 7 +- src/engine/SCons/Script/SConscript.xml | 13 ++-- test/SConscript/must_exist.py | 119 +++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 9 deletions(-) create mode 100644 test/SConscript/must_exist.py diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py index 5968346..fb6ec0d 100644 --- a/src/engine/SCons/Script/SConscript.py +++ b/src/engine/SCons/Script/SConscript.py @@ -153,14 +153,14 @@ def Return(*vars, **kw): stack_bottom = '% Stack boTTom %' # hard to define a variable w/this name :) -def handle_missing_SConscript(f, must_exist): +def handle_missing_SConscript(f, must_exist=None): """Take appropriate action on missing file in SConscript() call. The action may be to raise an exception, or print a warning. On first warning, also print a deprecation warning. """ - if SCons.Script._no_missing_sconscript or must_exist: + if must_exist or (SCons.Script._no_missing_sconscript and must_exist is not False): msg = "Fatal: missing SConscript '%s'" % f.get_internal_path() raise SCons.Errors.UserError(msg) @@ -285,7 +285,7 @@ def _SConscript(fs, *files, **kw): if old_file is not None: call_stack[-1].globals.update({__file__:old_file}) else: - handle_missing_SConscript(f, kw.get('must_exist', False)) + handle_missing_SConscript(f, kw.get('must_exist', None)) finally: SCons.Script.sconscript_reading = SCons.Script.sconscript_reading - 1 @@ -568,6 +568,7 @@ class SConsEnvironment(SCons.Environment.Base): files, exports = self._get_SConscript_filenames(ls, subst_kw) subst_kw['exports'] = exports + return _SConscript(self.fs, *files, **subst_kw) def SConscriptChdir(self, flag): diff --git a/src/engine/SCons/Script/SConscript.xml b/src/engine/SCons/Script/SConscript.xml index 29a89c2..a6258c4 100644 --- a/src/engine/SCons/Script/SConscript.xml +++ b/src/engine/SCons/Script/SConscript.xml @@ -357,12 +357,12 @@ Return('val1 val2') -(scripts, [exports, variant_dir, duplicate, must_exist=flag]) - +(scripts, [exports, variant_dir, duplicate, must_exist]) + -(dirs=subdirs, [name=script, exports, variant_dir, duplicate, must_exist=flag]) - +(dirs=subdirs, [name=script, exports, variant_dir, duplicate, must_exist]) + @@ -562,12 +562,13 @@ and what about this alternative? TODO??? SConscript('build/SConscript', src_dir='src') --> + The optional must_exist argument, if true, causes an exception to be raised if a requested -&SConscript; file is not found. The default is false, -which only prints a warning, but this behavior is deprecated. +&SConscript; file is not found. The current default is false, +causing only a warning to be omitted, but this behavior is deprecated. For scripts which truly intend to be optional, transition to explicty supplying must_exist=False to the call. diff --git a/test/SConscript/must_exist.py b/test/SConscript/must_exist.py new file mode 100644 index 0000000..a4341fa --- /dev/null +++ b/test/SConscript/must_exist.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python +# +# __COPYRIGHT__ +# +# 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. +# + +__revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__" + +''' +Test handling of must_exist flag and global setting requiring the +file to exist in an SConscript call +''' + +import TestSCons + +test = TestSCons.TestSCons() + +# catch the exception if is raised, send it on as a warning +# this gives us traceability of the line responsible +SConstruct_path = test.workpath('SConstruct') +test.write(SConstruct_path, """\ +import SCons +from SCons.Warnings import _warningOut +import sys + +# 1. call should succeed with deprecation warning +try: + SConscript('missing/SConscript') +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +# 2. call should succeed with warning +try: + SConscript('missing/SConscript') +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +# 3. call should raise exception +try: + SConscript('missing/SConscript', must_exist=True) +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +# 4. call should succeed with warning +try: + SConscript('missing/SConscript', must_exist=False) +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +SCons.Script.set_missing_sconscript_error() +# 5. with system setting changed, should raise exception +try: + SConscript('missing/SConscript') +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +# 6. must_exist=False should override system setting +try: + SConscript('missing/SConscript', must_exist=False) +except SCons.Errors.UserError as e: + if _warningOut: + _warningOut(e) +""") + +# we should see two exceptions as "Fatal" and +# and see four warnings, the first having the depr message +warn1 = """ +scons: warning: Calling missing SConscripts without error is deprecated. +Transition by adding must_exist=0 to SConscript calls. +Missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 7) + +warn2 = """ +scons: warning: Ignoring missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 13) + +err1 = """ +scons: warning: Fatal: missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 22) + +warn3 = """ +scons: warning: Ignoring missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 25) + +err2 = """ +scons: warning: Fatal: missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 35) + +warn4 = """ +scons: warning: Ignoring missing SConscript 'missing/SConscript' +""" + test.python_file_line(SConstruct_path, 38) + +expect_stderr = warn1 + warn2 + err1 + warn3 + err2 + warn4 +test.run(arguments = ".", stderr = expect_stderr) +test.pass_test() + +# Local Variables: +# tab-width:4 +# indent-tabs-mode:nil +# End: +# vim: set expandtab tabstop=4 shiftwidth=4: -- cgit v0.12 From 6166782fe90fc96a866424958fd6a632c76318f6 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sun, 29 Jul 2018 08:12:42 -0600 Subject: Add a docstring for SConscript() Signed-off-by: Mats Wichmann --- src/engine/SCons/Script/SConscript.py | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py index fb6ec0d..3b73010 100644 --- a/src/engine/SCons/Script/SConscript.py +++ b/src/engine/SCons/Script/SConscript.py @@ -543,6 +543,31 @@ class SConsEnvironment(SCons.Environment.Base): raise SCons.Errors.UserError("Import of non-existent variable '%s'"%x) def SConscript(self, *ls, **kw): + """Execute SCons configuration files. + + Parameters: + *ls (str or list): configuration file(s) to execute. + + Keyword arguments: + dirs (list): execute SConscript in each listed directory. + name (str): execute script 'name' (used with 'dirs'). + exports (list or dict): locally export variables the script(s) + can import. + variant_dir (str): mirror sources needed for build to variant_dir + to allow building there. + duplicate (bool): pysically duplicate sources instead of just + adjusting paths of derived files (used only with 'variant_dir') + (default is True). + must_exist (bool): fail if a requested script is missing + (default is False, default is deprecated). + + Returns: + variables returned by the called script + + Raises: + UserError if a script is not found and such exceptions are enabled. + """ + if 'build_dir' in kw: msg = """The build_dir keyword has been deprecated; use the variant_dir keyword instead.""" SCons.Warnings.warn(SCons.Warnings.DeprecatedBuildDirWarning, msg) @@ -568,7 +593,6 @@ class SConsEnvironment(SCons.Environment.Base): files, exports = self._get_SConscript_filenames(ls, subst_kw) subst_kw['exports'] = exports - return _SConscript(self.fs, *files, **subst_kw) def SConscriptChdir(self, flag): -- cgit v0.12 From 44ed2ad161836d9d3f119fc6454c9e92a356c1e1 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Sun, 29 Jul 2018 20:24:18 -0600 Subject: Some further adjustments to missing-sconscript tests Signed-off-by: Mats Wichmann --- src/engine/SCons/Script/SConscript.py | 4 ++-- test/SConscript/must_exist.py | 4 ++-- test/option-f.py | 9 ++++++--- test/option/warn-missing-sconscript.py | 11 +++++++++-- 4 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py index 3b73010..1d83bb6 100644 --- a/src/engine/SCons/Script/SConscript.py +++ b/src/engine/SCons/Script/SConscript.py @@ -165,10 +165,10 @@ def handle_missing_SConscript(f, must_exist=None): raise SCons.Errors.UserError(msg) if SCons.Script._warn_missing_sconscript_deprecated: - msg = "Calling missing SConscripts without error is deprecated.\n" + \ + msg = "Calling missing SConscript without error is deprecated.\n" + \ "Transition by adding must_exist=0 to SConscript calls.\n" + \ "Missing SConscript '%s'" % f.get_internal_path() - SCons.Warnings.warn(SCons.Warnings.DeprecatedMissingSConscriptWarning, msg) + SCons.Warnings.warn(SCons.Warnings.MissingSConscriptWarning, msg) SCons.Script._warn_missing_sconscript_deprecated = False else: msg = "Ignoring missing SConscript '%s'" % f.get_internal_path() diff --git a/test/SConscript/must_exist.py b/test/SConscript/must_exist.py index a4341fa..ac90cd1 100644 --- a/test/SConscript/must_exist.py +++ b/test/SConscript/must_exist.py @@ -72,7 +72,7 @@ try: except SCons.Errors.UserError as e: if _warningOut: _warningOut(e) -# 6. must_exist=False should override system setting +# 6. must_exist=False overrides system setting, should emit warning try: SConscript('missing/SConscript', must_exist=False) except SCons.Errors.UserError as e: @@ -83,7 +83,7 @@ except SCons.Errors.UserError as e: # we should see two exceptions as "Fatal" and # and see four warnings, the first having the depr message warn1 = """ -scons: warning: Calling missing SConscripts without error is deprecated. +scons: warning: Calling missing SConscript without error is deprecated. Transition by adding must_exist=0 to SConscript calls. Missing SConscript 'missing/SConscript' """ + test.python_file_line(SConstruct_path, 7) diff --git a/test/option-f.py b/test/option-f.py index 21afacb..46e2686 100644 --- a/test/option-f.py +++ b/test/option-f.py @@ -97,9 +97,12 @@ test.run(arguments = '-f Build2 -f SConscript .', stdout=expect) test.run(arguments = '-f no_such_file .', stdout = test.wrap_stdout("scons: `.' is up to date.\n"), stderr = None) -test.fail_test(not test.match_re(test.stderr(), """ -scons: warning: Ignoring missing SConscript 'no_such_file' -""" + TestSCons.file_expr)) +expect = """ +scons: warning: Calling missing SConscript without error is deprecated. +Transition by adding must_exist=0 to SConscript calls. +Missing SConscript 'no_such_file'""" +stderr = test.stderr() +test.must_contain_all(test.stderr(), expect) test.pass_test() diff --git a/test/option/warn-missing-sconscript.py b/test/option/warn-missing-sconscript.py index 4f1f8bd..492131b 100644 --- a/test/option/warn-missing-sconscript.py +++ b/test/option/warn-missing-sconscript.py @@ -51,16 +51,23 @@ test.write("foo.c",""" """) expect = r""" -scons: warning: Ignoring missing SConscript 'no_such_file' +scons: warning: Calling missing SConscript without error is deprecated. +Transition by adding must_exist=0 to SConscript calls. +Missing SConscript 'no_such_file' """ + TestSCons.file_expr +# this is the old message: +#expect = r""" +#scons: warning: Ignoring missing SConscript 'no_such_file' +"" + TestSCons.file_expr + test.run(arguments = '--warn=missing-sconscript .', stderr = expect) test.run(arguments = '--warn=no-missing-sconscript .', stderr = "") test.run(arguments = 'WARN=missing-sconscript .', stderr = expect) -test.run(arguments = 'WARN=no-missing-sconscript .') +test.run(arguments = 'WARN=no-missing-sconscript .', stderr = "") test.pass_test() -- cgit v0.12 From b4d4d281f771c2bec37829188b96ec2b148a8198 Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 30 Jul 2018 07:33:30 -0600 Subject: Fix for #3162: tweak SConscript() docstrings a little more Also handle_missing_SConscript(), internal interface added by this patch series. Signed-off-by: Mats Wichmann --- src/engine/SCons/Script/SConscript.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py index 1d83bb6..560402c 100644 --- a/src/engine/SCons/Script/SConscript.py +++ b/src/engine/SCons/Script/SConscript.py @@ -156,8 +156,16 @@ stack_bottom = '% Stack boTTom %' # hard to define a variable w/this name :) def handle_missing_SConscript(f, must_exist=None): """Take appropriate action on missing file in SConscript() call. - The action may be to raise an exception, or print a warning. - On first warning, also print a deprecation warning. + Print a warning or raise an exception on missing file. + On first warning, print a deprecation message. + + Args: + f (str): path of missing configuration file + must_exist (bool): raise exception if file does not exist + + Raises: + UserError if 'must_exist' is True or if global + SCons.Script._no_missing_sconscript is True. """ if must_exist or (SCons.Script._no_missing_sconscript and must_exist is not False): @@ -550,22 +558,22 @@ class SConsEnvironment(SCons.Environment.Base): Keyword arguments: dirs (list): execute SConscript in each listed directory. - name (str): execute script 'name' (used with 'dirs'). - exports (list or dict): locally export variables the script(s) - can import. - variant_dir (str): mirror sources needed for build to variant_dir - to allow building there. - duplicate (bool): pysically duplicate sources instead of just + name (str): execute script 'name' (used only with 'dirs'). + exports (list or dict): locally export variables the + called script(s) can import. + variant_dir (str): mirror sources needed for the build in + a variant directory to allow building in it. + duplicate (bool): physically duplicate sources instead of just adjusting paths of derived files (used only with 'variant_dir') (default is True). must_exist (bool): fail if a requested script is missing (default is False, default is deprecated). Returns: - variables returned by the called script + list of variables returned by the called script Raises: - UserError if a script is not found and such exceptions are enabled. + UserError: a script is not found and such exceptions are enabled. """ if 'build_dir' in kw: -- cgit v0.12 From e5390b34767b3911108841caedd6c1c73f982fbd Mon Sep 17 00:00:00 2001 From: Mats Wichmann Date: Mon, 6 Aug 2018 18:01:23 -0600 Subject: Fix for #3162: be more descriptive in changelog --- src/CHANGES.txt | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/CHANGES.txt b/src/CHANGES.txt index 3b0e603..cf2a8b1 100644 --- a/src/CHANGES.txt +++ b/src/CHANGES.txt @@ -97,12 +97,23 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE This changes SCons to better comply with normal Python installation practices. From Mats Wichmann: - - Begin adding support for SConscript() failing on missing script - Updated manpage scons.xml to fix a nested list problem - Updated doc terminiology: use prepend instead of append as appropriate - xml validity fixes from SConstruct.py change - update wiki links to new github location - update bug links to new github location + - Make it easier for SConscript() call to fail on missing script. + It was possible to call SCons.Warnings.warningAsException + (not documented as a user API) to make all warnings fail. Now + SConscript can take an optional must_exist flag which if true fails + if the script does not exist. Not failing on missing script is + now considered deprecated, and the first instance will print a + deprecation message. It is now also possible to flip the scons + behavior (which still defaults to warn, not fail) by calling + SCons.Script.set_missing_sconscript_error, which is also not a + documented interface at the moment. + +Begin adding support for SConscript() failing on missing script From Hao Wu - typo in customized decider example in user guide -- cgit v0.12