Improve package source code context display on error (#37655)
Spack displays package code context when it shouldn't (e.g., on `FetchError`s)
and doesn't display it when it should (e.g., when errors occur in builder classes.
The line attribution can sometimes be off by one, as well.
- [x] Display package context when errors occur in a subclass of `PackageBase`
- [x] Display package context when errors occur in a subclass of `BaseBuilder`
- [x] Do not display package context when errors occur in `PackageBase`,
      `BaseBuilder` or other core code that is not in a `package.py` file.
- [x] Fix off-by-one error for core code (don't subtract one from the line number *unless*
      it's in an actual `package.py` file.
---------
Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>
			
			
This commit is contained in:
		@@ -1216,6 +1216,9 @@ def child_fun():
 | 
			
		||||
    return child_result
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
CONTEXT_BASES = (spack.package_base.PackageBase, spack.build_systems._checks.BaseBuilder)
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
def get_package_context(traceback, context=3):
 | 
			
		||||
    """Return some context for an error message when the build fails.
 | 
			
		||||
 | 
			
		||||
@@ -1244,32 +1247,38 @@ def make_stack(tb, stack=None):
 | 
			
		||||
 | 
			
		||||
    stack = make_stack(traceback)
 | 
			
		||||
 | 
			
		||||
    basenames = tuple(base.__name__ for base in CONTEXT_BASES)
 | 
			
		||||
    for tb in stack:
 | 
			
		||||
        frame = tb.tb_frame
 | 
			
		||||
        if "self" in frame.f_locals:
 | 
			
		||||
            # Find the first proper subclass of PackageBase.
 | 
			
		||||
            # Find the first proper subclass of the PackageBase or BaseBuilder, but
 | 
			
		||||
            # don't provide context if the code is actually in the base classes.
 | 
			
		||||
            obj = frame.f_locals["self"]
 | 
			
		||||
            if isinstance(obj, spack.package_base.PackageBase):
 | 
			
		||||
            func = getattr(obj, tb.tb_frame.f_code.co_name, "")
 | 
			
		||||
            if func:
 | 
			
		||||
                typename, *_ = func.__qualname__.partition(".")
 | 
			
		||||
 | 
			
		||||
            if isinstance(obj, CONTEXT_BASES) and typename not in basenames:
 | 
			
		||||
                break
 | 
			
		||||
    else:
 | 
			
		||||
        return None
 | 
			
		||||
 | 
			
		||||
    # We found obj, the Package implementation we care about.
 | 
			
		||||
    # Point out the location in the install method where we failed.
 | 
			
		||||
    lines = [
 | 
			
		||||
        "{0}:{1:d}, in {2}:".format(
 | 
			
		||||
            inspect.getfile(frame.f_code),
 | 
			
		||||
            frame.f_lineno - 1,  # subtract 1 because f_lineno is 0-indexed
 | 
			
		||||
            frame.f_code.co_name,
 | 
			
		||||
        )
 | 
			
		||||
    ]
 | 
			
		||||
    filename = inspect.getfile(frame.f_code)
 | 
			
		||||
    lineno = frame.f_lineno
 | 
			
		||||
    if os.path.basename(filename) == "package.py":
 | 
			
		||||
        # subtract 1 because we inject a magic import at the top of package files.
 | 
			
		||||
        # TODO: get rid of the magic import.
 | 
			
		||||
        lineno -= 1
 | 
			
		||||
 | 
			
		||||
    lines = ["{0}:{1:d}, in {2}:".format(filename, lineno, frame.f_code.co_name)]
 | 
			
		||||
 | 
			
		||||
    # Build a message showing context in the install method.
 | 
			
		||||
    sourcelines, start = inspect.getsourcelines(frame)
 | 
			
		||||
 | 
			
		||||
    # Calculate lineno of the error relative to the start of the function.
 | 
			
		||||
    # Subtract 1 because f_lineno is 0-indexed.
 | 
			
		||||
    fun_lineno = frame.f_lineno - start - 1
 | 
			
		||||
    fun_lineno = lineno - start
 | 
			
		||||
    start_ctx = max(0, fun_lineno - context)
 | 
			
		||||
    sourcelines = sourcelines[start_ctx : fun_lineno + context + 1]
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -2017,7 +2017,8 @@ def test_title(purpose, test_name):
 | 
			
		||||
                    # stack instead of from traceback.
 | 
			
		||||
                    # The traceback is truncated here, so we can't use it to
 | 
			
		||||
                    # traverse the stack.
 | 
			
		||||
                    m = "\n".join(spack.build_environment.get_package_context(tb))
 | 
			
		||||
                    context = spack.build_environment.get_package_context(tb)
 | 
			
		||||
                    m = "\n".join(context) if context else ""
 | 
			
		||||
 | 
			
		||||
                exc = e  # e is deleted after this block
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user