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

precompile: pipe error messages during autoprecompilation to Main.err for easier inspection #3536

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Jul 4, 2023

It seems the way auto-precompilation only summarizes which packages errored not the verbose error details is unpopular so this makes autoprecompilation show errors the way an explicit Pkg.precompile does.

Updated: Now autoprecompilation pipes any errors to Main.err

Precompiling project...
  ✗ PNGFiles
  0 dependencies successfully precompiled in 1 seconds. 38 already precompiled.
  To see a full report use `julia> err`

julia> err
The following 1 direct dependency failed to precompile:

PNGFiles [f57f5aa1-a3ce-4bc8-8ab9-96f992907883]

Failed to precompile PNGFiles [f57f5aa1-a3ce-4bc8-8ab9-96f992907883] to "/Users/ian/.julia/compiled/v1.11/PNGFiles/jl_X0edfU".
ERROR: LoadError: 
Stacktrace:
 [1] error()
   @ Base ./error.jl:44
 [2] top-level scope
   @ ~/Documents/GitHub/PNGFiles.jl/src/PNGFiles.jl:3
 [3] include
   @ Base ./Base.jl:493 [inlined]
 [4] include_package_for_output(pkg::Base.PkgId, input::String, depot_path::Vector{String}, dl_load_path::Vector{String}, load_path::Vector{String}, concrete_deps::Vector{Pair{Base.PkgId, UInt128}}, source::Nothing)
   @ Base ./loading.jl:2244
 [5] top-level scope
   @ stdin:3
in expression starting at /Users/ian/Documents/GitHub/PNGFiles.jl/src/PNGFiles.jl:1
in expression starting at stdin:3


julia> 

Also in noninteractive & CI modes it will always show errors now.

Closes #3444
Closes #3428

@IanButterworth IanButterworth changed the title precompile: always show errors precompile: show full error details during autoprecompilation Jul 4, 2023
@staticfloat
Copy link
Member

I have the following thoughts:

  1. I'm not against hiding errors by default (although I do generally want to see them), but the command to see the errors seemed unnecessarily verbose. I don't like that I have to run import Pkg; Pkg.precompile() to see them; I just ran ]precompile, why should there be a difference between the two?
  2. It would be nice if there were a way to interactively see the errors. Precompilation errors can be deceptively complex, and many times you end up chasing down 7 precompilation errors that are really one dependency deep in the stack that is failing and killing all dependent packages. My ideal interface would be to be able to view the errors that Pkg has already encountered through some kind of terminal menu, preferably topologically sorted to surface the deepest error first.
  3. I think hiding errors is only acceptable if the errors can be easily viewed afterward. So even if the dream scenario of (2) is not implemented, perhaps writing the precompilation errors out to a temporary directory that gets cleaned up on Julia process exit, (or a scratch space that gets cleaned out on next precompile) would allow for better error analysis.

@IanButterworth
Copy link
Member Author

It could be pushed to err, which would require making the error reports actual errors with show methods.

@IanButterworth
Copy link
Member Author

But in no interactive mode it should just show

@IanButterworth IanButterworth force-pushed the ib/precompile_always_show_errors branch 2 times, most recently from cc09eae to 9b31814 Compare August 8, 2023 14:54
@IanButterworth IanButterworth changed the title precompile: show full error details during autoprecompilation precompile: pipe error messages during autoprecompilation to Main.err for easier inspection Aug 8, 2023
@IanButterworth
Copy link
Member Author

Updated to use julia> err. See example up top

test/pkg.jl Outdated
@@ -692,7 +692,7 @@ end

@testset "PkgError printing" begin
err = PkgError("foobar")
@test occursin("PkgError(\"foobar\")", sprint(show, err))
@test occursin("PkgError: foobar", sprint(show, err))
Copy link
Member Author

@IanButterworth IanButterworth Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredrikekre is there a reason why chaning this show method is bad, given #1080 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to make a new error type with a show method, given I guess there was a reason why this was tested for

@IanButterworth IanButterworth force-pushed the ib/precompile_always_show_errors branch from fedad2a to c211673 Compare August 9, 2023 00:39
@IanButterworth
Copy link
Member Author

I'm going to merge and get this onto julia master so we can try it out before 1.10

@IanButterworth IanButterworth merged commit b044bf6 into JuliaLang:master Aug 9, 2023
12 of 13 checks passed
@IanButterworth IanButterworth deleted the ib/precompile_always_show_errors branch August 9, 2023 01:38
@IanButterworth
Copy link
Member Author

@KristofferC Maybe marking this for backport is too much? What do you think?

IanButterworth added a commit that referenced this pull request Aug 18, 2023
… for easier inspection (#3536)

- Sends errors during autoprecompilation to Main.err for inspection
- Or if noninteractive just shows autoprecompilation errors without throwing

(cherry picked from commit b044bf6)
IanButterworth added a commit that referenced this pull request Aug 18, 2023
… for easier inspection (#3536)

- Sends errors during autoprecompilation to Main.err for inspection
- Or if noninteractive just shows autoprecompilation errors without throwing

(cherry picked from commit b044bf6)
@IanButterworth IanButterworth mentioned this pull request Aug 18, 2023
14 tasks
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 this pull request may close these issues.

precompile now hides errors (unhelpful!) Add option to make autoprecompilation always show errors
2 participants