Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCons may convert another exception to BuildError incorrectly #4530

Closed
mwichmann opened this issue May 16, 2024 · 0 comments · Fixed by #4532
Closed

SCons may convert another exception to BuildError incorrectly #4530

mwichmann opened this issue May 16, 2024 · 0 comments · Fixed by #4532

Comments

@mwichmann
Copy link
Collaborator

mwichmann commented May 16, 2024

An internal SCons routine, SCons.Errors.convert_to_BuildError, attempts to synthesize a BuildError exception from whatever exception may have been raised when the Taskmaster executes an action. This can go wrong in certain circumstances. The docstring for the BuildError class contains these comments:

errstr: a description of the error message
status: the return code of the action that caused the build error. Must be set to a non-zero value even if the build error is not due to an action returning a non-zero returned code.

In the case reported in the linked thread, the Jinja templating engine (used in-process) raised a jinja2.exceptions.TemplateNotFound exception, which is initialized in a kind of odd way (separate topic), where the three important exception fields filename, strerror and errno are all set to None. The conversion routine then tries this (status contains the captured exception at this point):

    filename = getattr(status, 'filename', None) 
    strerror = getattr(status, 'strerror', str(status)) 
    errno = getattr(status, 'errno', 2) 

before stuffing those values into the BuildError instance. As written, strerror and errno will remain None if they were None in the original exception, which is not the intent of the routine, as described in the docstring (the getattr defaults only apply if the attributes are not present). The effect is that the exception appears to be "swallowed". SCons should be more tolerant of this, and get the defaults in if the fields were None.

Simple repro:

import jinja2

def small_builder(target, source, env):
    raise jinja2.exceptions.TemplateNotFound("You should see this message")

env=Environment()
env.Command(['dummy/TEST.COMPLETE'], [], small_builder, chdir=0)
mwichmann added a commit to mwichmann/scons that referenced this issue May 16, 2024
In some cases, the conversion of another exception to a BuildError
could yield undesired results: if the exception was a OSError/IOError,
and some fields were initially set to None, then the return code and error
string were set to None, rather than the SCons defaults (which are
return code 2, and the string set originally in the exception).

A unit test is added which attempts to build an OSError in the way
that the case "in the wild" does - confirmed failing to pick up
defaults without the change.

Fixes SCons#4530

Signed-off-by: Mats Wichmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant