Skip to content

Commit

Permalink
Change the default to error on missing SConscript
Browse files Browse the repository at this point in the history
This completes a change begun in 3.0.2, when the behavior changed
from "skip silently" to "skip but issue a warning"; that behavior
was marked deprecated in 3.1.  Use must_exist=False to get the old
"skip silently" behavior.

Fixes #3958

Signed-off-by: Mats Wichmann <mats@linux.com>
  • Loading branch information
mwichmann committed Aug 6, 2023
1 parent e141cd3 commit fd6676a
Show file tree
Hide file tree
Showing 12 changed files with 154 additions and 136 deletions.
3 changes: 3 additions & 0 deletions CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ RELEASE VERSION/DATE TO BE FILLED IN LATER
- More tweaking of test framework overview (which is duplicated onto
the website, but not in the regular documentation section).
- Extend range of recognized Java versions to 20.
- Finish the change to make calling SConscript() with a nonexistent
file an error. It has issued a warning since 3.0, with "warn instead
of fail" deprecated since 3.1. Fixes #3958.

From Jonathon Reinhart:
- Fix another instance of `int main()` in CheckLib() causing failures
Expand Down
2 changes: 1 addition & 1 deletion README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ messages during installation like this::
Consider adding this directory to PATH or, if you prefer to suppress this warning,
use --no-warn-script-location.

If you are running on a system which uses a package manager
If you are running on a system which uses a package manager
(for example most Linux distributions), you may, at your option,
use the package manager (e.g. ``apt``, ``dnf``, ``yum``,
``zypper``, ``brew``, ``pacman`` etc.) to install a version
Expand Down
6 changes: 6 additions & 0 deletions RELEASE.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ CHANGED/ENHANCED EXISTING FUNCTIONALITY
architecture combination. For example, when using VS2022 on arm64, the arm64 native
tools are only installed for the 14.3x toolsets.
- Extend range of recognized Java versions to 20.
- Calling SConscript() with a nonexistent file is now an error.
Previously this succeeded - prior to SCons 3.0, silently; since 3.0, with
a warning. Developers can still instruct such an SConscript() call not
to fail by being explicit: pass keyword argument "must_exist=False".
The "--warn=missing-sconscript" commandline option is no longer available
as the warning was part of the transitional phase.

FIXES
-----
Expand Down
42 changes: 17 additions & 25 deletions SCons/Script/SConscript.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,40 +145,32 @@ def Return(*vars, **kw):

stack_bottom = '% Stack boTTom %' # hard to define a variable w/this name :)

def handle_missing_SConscript(f, must_exist=None):
def handle_missing_SConscript(f: str, must_exist: bool = True) -> None:
"""Take appropriate action on missing file in SConscript() call.
Print a warning or raise an exception on missing file, unless
missing is explicitly allowed by the *must_exist* value.
On first warning, print a deprecation message.
missing is explicitly allowed by the *must_exist* parameter or by
a global flag.
Args:
f (str): path of missing configuration file
must_exist (bool): if true, fail. If false, but not ``None``,
allow the file to be missing. The default is ``None``,
which means issue the warning. The default is deprecated.
f: path to missing configuration file
must_exist: if true (the default), fail. If false
do nothing, allowing a build to declare it's okay to be missing.
Raises:
UserError: if *must_exist* is true or if global
UserError: if *must_exist* is true or if global
:data:`SCons.Script._no_missing_sconscript` is true.
.. versionchanged: 4.6.0
Changed default from False.
"""
if not must_exist: # explicitly set False: ok
return
if not SCons.Script._no_missing_sconscript: # system default changed: ok
return
msg = f"Fatal: missing SConscript '{f.get_internal_path()}'"
raise SCons.Errors.UserError(msg)

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 must_exist is None:
if SCons.Script._warn_missing_sconscript_deprecated:
msg = (
"Calling missing SConscript without error is deprecated.\n"
"Transition by adding must_exist=False 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
Expand Down Expand Up @@ -294,7 +286,7 @@ def _SConscript(fs, *files, **kw):
call_stack[-1].globals.update({__file__:old_file})

else:
handle_missing_SConscript(f, kw.get('must_exist', None))
handle_missing_SConscript(f, kw.get('must_exist', True))

finally:
SCons.Script.sconscript_reading = SCons.Script.sconscript_reading - 1
Expand Down
17 changes: 10 additions & 7 deletions SCons/Script/SConscript.xml
Original file line number Diff line number Diff line change
Expand Up @@ -538,17 +538,20 @@ TODO??? SConscript('build/SConscript', src_dir='src')
<para>
If the optional
<varname>must_exist</varname>
is <constant>True</constant>,
is <constant>True</constant> (the default),
causes an exception to be raised if a requested
SConscript file is not found. The current default is
<constant>False</constant>,
causing only a warning to be emitted, but this default is deprecated
(<emphasis>since 3.1</emphasis>).
For scripts which truly intend to be optional, transition to
explicitly supplying
SConscript file is not found.
To allow missing scripts to be silently ignored
(the default behavior prior to &SCons; version 3.1),
pass
<literal>must_exist=False</literal> to the &f-SConscript; call.
</para>

<para>
<emphasis>Changed in 4.6.0</emphasis>: <parameter>must_exist</parameter>
now defaults to <constant>True</constant>.
</para>

<para>
Here are some composite examples:
</para>
Expand Down
6 changes: 3 additions & 3 deletions SCons/Script/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,10 @@ def HelpFunction(text, append: bool=False) -> None:
# Will be non-zero if we are reading an SConscript file.
sconscript_reading = 0

_no_missing_sconscript = False
_warn_missing_sconscript_deprecated = True
_no_missing_sconscript = True
_warn_missing_sconscript_deprecated = False # TODO: now unused

def set_missing_sconscript_error(flag: int=1):
def set_missing_sconscript_error(flag: bool = True) -> bool:
"""Set behavior on missing file in SConscript() call.
Returns:
Expand Down
1 change: 1 addition & 0 deletions SCons/Warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ class LinkWarning(WarningOnByDefault):
class MisleadingKeywordsWarning(WarningOnByDefault):
pass

# TODO: no longer needed, now an error instead of warning. Leave for a bit.
class MissingSConscriptWarning(WarningOnByDefault):
pass

Expand Down
4 changes: 2 additions & 2 deletions doc/man/scons.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2272,13 +2272,13 @@ of this tool module.
</listitem>
</varlistentry>

<varlistentry>
<!--varlistentry> Removed in 4.6.0
<term><emphasis role="bold">missing-sconscript</emphasis></term>
<listitem>
<para>Warnings about missing &SConscript; files.
These warnings are enabled by default.</para>
</listitem>
</varlistentry>
</varlistentry-->

<varlistentry>
<term><emphasis role="bold">no-object-count</emphasis></term>
Expand Down
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,10 @@ def build(target, source, env):
#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 .', stderr = "")

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 .', stderr="")

test.pass_test()

Expand Down
135 changes: 76 additions & 59 deletions test/SConscript/must_exist.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,76 +37,93 @@
# 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
import traceback
import SCons.Script
from SCons.Errors import UserError
from SCons.Script.Main import find_deepest_user_frame
DefaultEnvironment(tools=[])
# 1. 1st default call should succeed with deprecation warning
try:
SConscript('missing/SConscript')
except SCons.Errors.UserError as e:
if _warningOut:
_warningOut(e)
# 2. 2nd default call should succeed with warning (no depr)
def user_error(e):
"Synthesize msg from UserError."
# Borrowed from SCons.Script._scons_user_error which we don't use
# because it exits - we only want the message.
etype, value, tb = sys.exc_info()
filename, lineno, routine, _ = find_deepest_user_frame(traceback.extract_tb(tb))
sys.stderr.write(f"\\nscons: *** {value}\\n")
sys.stderr.write(f'File "{filename}", line {lineno}, in {routine}\\n')
# 1. Call with defaults raises exception
try:
SConscript('missing/SConscript')
except SCons.Errors.UserError as e:
if _warningOut:
_warningOut(e)
# 3. must_exist True call should raise exception
SConscript("missing/SConscript")
except UserError as e:
user_error(e)
# 2. Call with must_exist=True raises exception
try:
SConscript('missing/SConscript', must_exist=True)
except SCons.Errors.UserError as e:
if _warningOut:
_warningOut(e)
# 4. must_exist False call should succeed silently
SConscript("missing/SConscript", must_exist=True)
except UserError as e:
user_error(e)
# 3. Call with must_exist=False call should succeed silently
try:
SConscript('missing/SConscript', must_exist=False)
except SCons.Errors.UserError as e:
if _warningOut:
_warningOut(e)
# 5. with system setting changed, should raise exception
SCons.Script.set_missing_sconscript_error()
SConscript("missing/SConscript", must_exist=False)
except UserError as e:
user_error(e)
# 4. with system setting changed, should succeed silently
SCons.Script.set_missing_sconscript_error(flag=False)
try:
SConscript('missing/SConscript')
except SCons.Errors.UserError as e:
if _warningOut:
_warningOut(e)
# 6. must_exist=False overrides system setting, should emit warning
SConscript("missing/SConscript")
except UserError as e:
user_error(e)
# 5. must_exist=True "wins" over 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
# need to build the path in the expected msg in an OS-agnostic way
missing = os.path.normpath('missing/SConscript')
warn1 = """
scons: warning: Calling missing SConscript without error is deprecated.
Transition by adding must_exist=False to SConscript calls.
Missing SConscript '{}'
""".format(missing) + test.python_file_line(SConstruct_path, 8)

warn2 = """
scons: warning: Ignoring missing SConscript '{}'
""".format(missing) + test.python_file_line(SConstruct_path, 14)

err1 = """
scons: warning: Fatal: missing SConscript '{}'
""".format(missing) + test.python_file_line(SConstruct_path, 23)

err2 = """
scons: warning: Fatal: missing SConscript '{}'
""".format(missing) + test.python_file_line(SConstruct_path, 36)
SConscript("missing/SConscript", must_exist=True)
except UserError as e:
user_error(e)
""",
)

missing = "missing/SConscript"
err1 = f"""
scons: *** Fatal: missing SConscript {missing!r}
""" + test.python_file_line(
SConstruct_path, 21
)

err2 = f"""
scons: *** Fatal: missing SConscript {missing!r}
""" + test.python_file_line(
SConstruct_path, 27
)

err3 = f"""
scons: *** Fatal: missing SConscript {missing!r}
""" + test.python_file_line(
SConstruct_path, 33
)

err4 = f"""
scons: *** Fatal: missing SConscript {missing!r}
""" + test.python_file_line(
SConstruct_path, 40
)

err5 = f"""
scons: *** Fatal: missing SConscript {missing!r}
""" + test.python_file_line(
SConstruct_path, 46
)

nowarn = ""

expect_stderr = warn1 + warn2 + err1 + nowarn + err2 + nowarn
test.run(arguments = ".", stderr = expect_stderr)
# of the five tests, we actually expect fails from 1 and 2
expect_stderr = err1 + err2 + nowarn + nowarn + nowarn
test.run(arguments=".", stderr=expect_stderr)
test.pass_test()

# Local Variables:
Expand Down
Loading

0 comments on commit fd6676a

Please sign in to comment.