From 33752eabb876682d9a55589903df4e38f226b710 Mon Sep 17 00:00:00 2001 From: Carson Woods Date: Mon, 15 May 2023 16:38:11 -0400 Subject: 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 --- lib/spack/spack/build_environment.py | 31 ++++++++++++++++++++----------- lib/spack/spack/package_base.py | 3 ++- 2 files changed, 22 insertions(+), 12 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 870ac86fc9..c1bafe701b 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -1216,6 +1216,9 @@ def start_build_process(pkg, function, kwargs): 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 get_package_context(traceback, context=3): 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] diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index e1741e9fc0..3488b2624e 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -2017,7 +2017,8 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta): # 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 -- cgit v1.2.3-60-g2f50