Merge pull request #3154 from mwichmann/wip-missing-scons

Add ability for SConscript to fail on missing script

src/CHANGES.txt

@@ -103,6 +103,16 @@ RELEASE 3.1.0.alpha.yyyymmdd - NEW DATE WILL BE INSERTED HERE
     - 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.
     - convert TestCmd.read to use with statement on open (quiets 17 py3 warnings)
     - quiet warning in UtilTests.py
     - fix tests specifying octal constants for Py3

src/engine/SCons/Script/SConscript.py

@@ -153,6 +153,35 @@ def Return(*vars, **kw):
 
 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.
+
+    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):
+        msg = "Fatal: missing SConscript '%s'" % f.get_internal_path()
+        raise SCons.Errors.UserError(msg)
+
+    if SCons.Script._warn_missing_sconscript_deprecated:
+        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.MissingSConscriptWarning, 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 +293,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', None))
 
         finally:
             SCons.Script.sconscript_reading = SCons.Script.sconscript_reading - 1
@@ -523,6 +551,31 @@ def Import(self, *vars):
             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 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:
+            list of variables returned by the called script
+
+        Raises:
+            UserError: 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)

src/engine/SCons/Script/SConscript.xml

@@ -357,12 +357,12 @@ Return('val1 val2')
 
 <scons_function name="SConscript">
 <arguments>
-(scripts, [exports, variant_dir, duplicate])
-<!-- (scripts, [exports, variant_dir, src_dir, duplicate]) -->
+(scripts, [exports, variant_dir, duplicate, must_exist])
+<!-- (scripts, [exports, variant_dir, src_dir, duplicate, must_exist]) -->
 </arguments>
 <arguments>
-(dirs=subdirs, [name=script, exports, variant_dir, duplicate])
-<!-- (dirs=subdirs, [name=script, exports, variant_dir, src_dir, duplicate]) -->
+(dirs=subdirs, [name=script, exports, variant_dir, duplicate, must_exist])
+<!-- (dirs=subdirs, [name=script, exports, variant_dir, src_dir, duplicate, must_exist]) -->
 </arguments>
 <summary>
 <para>
@@ -563,6 +563,17 @@ TODO??? SConscript('build/SConscript', src_dir='src')
 -->
 </para>
 
+<para>
+The optional
+<varname>must_exist</varname>
+argument, if true, causes an exception to be raised if a requested
+&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
+<literal>must_exist=False</literal> to the call.
+</para>
+
 <para>
 Here are some composite examples:
 </para>

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)

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

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 overrides system setting, should emit warning
+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 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)
+
+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:

test/option-f.py

@@ -97,9 +97,12 @@
 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()
 

test/option/warn-missing-sconscript.py

@@ -51,16 +51,23 @@ def build(target, source, env):
 """)
 
 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()