From 9c229f6b463a109c4bdc3e34bf33ef1c2ec814db Mon Sep 17 00:00:00 2001 From: Ian Butterworth Date: Thu, 19 Sep 2024 15:43:49 -0400 Subject: [PATCH 1/3] hide loading internals in LoadError / precompilation errors --- base/errorshow.jl | 6 +++++- base/loading.jl | 13 ++++++++++++- base/precompilation.jl | 4 ++-- base/stacktraces.jl | 19 +++++++++++++++---- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index d805cb64fb81e..3d49f5cd90feb 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -111,8 +111,12 @@ end function showerror(io::IO, ex::LoadError, bt; backtrace=true) !isa(ex.error, LoadError) && print(io, "LoadError: ") + # Remove the internals of how precompilation is set up + StackTraces.remove_frames!(bt, :include_package_for_output; reverse=true, n=1) showerror(io, ex.error, bt, backtrace=backtrace) - print(io, "\nin expression starting at $(ex.file):$(ex.line)") + if ex.file !== "stdin" + print(io, "\nin expression starting at $(ex.file):$(ex.line)") + end end showerror(io::IO, ex::LoadError) = showerror(io, ex, []) diff --git a/base/loading.jl b/base/loading.jl index 8d180845f942f..b547796002e5c 100644 --- a/base/loading.jl +++ b/base/loading.jl @@ -2992,6 +2992,17 @@ end const MAX_NUM_PRECOMPILE_FILES = Ref(10) +struct PackagePrecompileError <: Exception + pkg::PkgId + path::String +end +function showerror(io::IO, ex::PackagePrecompileError, bt; backtrace) + showerror(io, ex) +end +function showerror(io::IO, ex::PackagePrecompileError) + print(io, "Failed to precompile $(repr("text/plain", ex.pkg)) to $(repr(ex.path)).") +end + function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, internal_stdout::IO = stdout, keep_loaded_modules::Bool = true; flags::Cmd=``, cacheflags::CacheFlags=CacheFlags(), reasons::Union{Dict{String,Int},Nothing}=Dict{String,Int}()) @@ -3114,7 +3125,7 @@ function compilecache(pkg::PkgId, path::String, internal_stderr::IO = stderr, in if p.exitcode == 125 return PrecompilableError() else - error("Failed to precompile $(repr("text/plain", pkg)) to $(repr(tmppath)).") + throw(PackagePrecompileError(pkg, tmppath)) end end diff --git a/base/precompilation.jl b/base/precompilation.jl index d3f076633f386..617e8ac83e78d 100644 --- a/base/precompilation.jl +++ b/base/precompilation.jl @@ -2,7 +2,7 @@ module Precompilation using Base: PkgId, UUID, SHA1, parsed_toml, project_file_name_uuid, project_names, project_file_manifest_path, get_deps, preferences_names, isaccessibledir, isfile_casesensitive, - base_project + base_project, PackagePrecompileError # This is currently only used for pkgprecompile but the plan is to use this in code loading in the future # see the `kc/codeloading2.0` branch @@ -871,7 +871,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; # @show err close(std_pipe.in) # close pipe to end the std output monitor wait(t_monitor) - if err isa ErrorException || (err isa ArgumentError && startswith(err.msg, "Invalid header in cache file")) + if err isa PackagePrecompileError || (err isa ArgumentError && startswith(err.msg, "Invalid header in cache file")) failed_deps[pkg_config] = (strict || is_direct_dep) ? string(sprint(showerror, err), "\n", strip(get(std_outputs, pkg_config, ""))) : "" delete!(std_outputs, pkg_config) # so it's not shown as warnings, given error report !fancyprint && lock(print_lock) do diff --git a/base/stacktraces.jl b/base/stacktraces.jl index 102e415a22de2..a3d04fd8e3ac9 100644 --- a/base/stacktraces.jl +++ b/base/stacktraces.jl @@ -198,15 +198,26 @@ Base.@constprop :none function stacktrace(c_funcs::Bool=false) end """ - remove_frames!(stack::StackTrace, name::Symbol) + remove_frames!(stack::StackTrace, name::Symbol; reverse::Bool=false) Takes a `StackTrace` (a vector of `StackFrames`) and a function name (a `Symbol`) and removes the `StackFrame` specified by the function name from the `StackTrace` (also removing all frames above the specified function). Primarily used to remove `StackTraces` functions -from the `StackTrace` prior to returning it. +from the `StackTrace` prior to returning it (or after if `reverse` is `true`). """ -function remove_frames!(stack::StackTrace, name::Symbol) - deleteat!(stack, 1:something(findlast(frame -> frame.func == name, stack), 0)) +function remove_frames!(stack::StackTrace, name::Symbol; reverse::Bool=false, n::Int=0) + if reverse + f = findfirst(frame -> frame.func == name, stack) + if f !== nothing + stack_length = length(stack) + deleteat!(stack, (f - n):stack_length) + end + else + f = findlast(frame -> frame.func == name, stack) + if f !== nothing + deleteat!(stack, 1:(f + n)) + end + end return stack end From de6a472e8304d2854fbedb3f6fc8375c42e0a5e4 Mon Sep 17 00:00:00 2001 From: Ian Butterworth Date: Thu, 19 Sep 2024 17:18:51 -0400 Subject: [PATCH 2/3] rename PkgPrecompileError -> PrecompilePkgsError --- base/precompilation.jl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/base/precompilation.jl b/base/precompilation.jl index 617e8ac83e78d..7241b33c71fab 100644 --- a/base/precompilation.jl +++ b/base/precompilation.jl @@ -332,14 +332,14 @@ end ############ -struct PkgPrecompileError <: Exception +struct PrecompilePkgsError <: Exception msg::String end -Base.showerror(io::IO, err::PkgPrecompileError) = print(io, err.msg) -Base.showerror(io::IO, err::PkgPrecompileError, bt; kw...) = Base.showerror(io, err) # hide stacktrace +Base.showerror(io::IO, err::PrecompilePkgsError) = print(io, err.msg) +Base.showerror(io::IO, err::PrecompilePkgsError, bt; kw...) = Base.showerror(io, err) # hide stacktrace # This needs a show method to make `julia> err` show nicely -Base.show(io::IO, err::PkgPrecompileError) = print(io, "PkgPrecompileError: ", err.msg) +Base.show(io::IO, err::PrecompilePkgsError) = print(io, "PrecompilePkgsError: ", err.msg) import Base: StaleCacheKey @@ -993,7 +993,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; plural1 = length(failed_deps) == 1 ? "y" : "ies" println(io, " ", color_string("$(length(failed_deps))", Base.error_color()), " dependenc$(plural1) errored.") println(io, " For a report of the errors see `julia> err`. To retry use `pkg> precompile`") - setglobal!(Base.MainInclude, :err, PkgPrecompileError(err_msg)) + setglobal!(Base.MainInclude, :err, PrecompilePkgsError(err_msg)) else # auto-precompilation shouldn't throw but if the user can't easily access the # error messages, just show them @@ -1001,7 +1001,7 @@ function precompilepkgs(pkgs::Vector{String}=String[]; end else println(io) - throw(PkgPrecompileError(err_msg)) + throw(PrecompilePkgsError(err_msg)) end end end From 17633cad0c53522a97c3101fd0f19f6be3418632 Mon Sep 17 00:00:00 2001 From: Ian Butterworth Date: Fri, 20 Sep 2024 07:18:30 -0400 Subject: [PATCH 3/3] fixes --- base/errorshow.jl | 6 ++++-- test/gcext/gcext-test.jl | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/base/errorshow.jl b/base/errorshow.jl index 3d49f5cd90feb..935aab29ec392 100644 --- a/base/errorshow.jl +++ b/base/errorshow.jl @@ -111,8 +111,10 @@ end function showerror(io::IO, ex::LoadError, bt; backtrace=true) !isa(ex.error, LoadError) && print(io, "LoadError: ") - # Remove the internals of how precompilation is set up - StackTraces.remove_frames!(bt, :include_package_for_output; reverse=true, n=1) + if bt isa StackTraces.StackTrace + # Remove the internals of how precompilation is set up + StackTraces.remove_frames!(bt, :include_package_for_output; reverse=true, n=1) + end showerror(io, ex.error, bt, backtrace=backtrace) if ex.file !== "stdin" print(io, "\nin expression starting at $(ex.file):$(ex.line)") diff --git a/test/gcext/gcext-test.jl b/test/gcext/gcext-test.jl index 81637392e3c5d..cdc5f043c2f50 100644 --- a/test/gcext/gcext-test.jl +++ b/test/gcext/gcext-test.jl @@ -55,7 +55,7 @@ end Base.compilecache(Base.identify_package("DependsOnForeign")) push!(LOAD_PATH, joinpath(@__DIR__, "ForeignObjSerialization")) - @test_throws ErrorException Base.compilecache(Base.identify_package("ForeignObjSerialization"), Base.DevNull()) + @test_throws Base.PackagePrecompileError Base.compilecache(Base.identify_package("ForeignObjSerialization"), Base.DevNull()) pop!(LOAD_PATH) (@eval (using Foreign))