Rework broken-precompilation warning #35410
Conversation
|
We strongly don't want this to go through the normal machinery—this is explicitly trying to print a fatal error message and should not be easily diverted. I'd be well okay with turning this into a call to |
|
I'd be ok with making it throw because the stacktrace is actually the most important thing for users to diagnose problems here (it's the module calling Jeff asked why this was even necessary given that |
|
Ok, so here's the story:
It turns out that |
|
Ok, how about this? I've added a special case to consider Changes to fix the bug in |
|
Ah, ok, great. I like making it an error --- good to be decisive about what is and isn't allowed! Will be good to close the |
c42f
changed the title
|
I thought I should just go ahead and close the |
|
Tests added. Hopefully this is good to go now provided CI passes. |
|
Test failures look spurious. Win32 is a code signing problem, aarch64 presumably whatever existing problem has been causing it to segfault in several recent PRs. |
|
A rebase should fix the aarch problem. |
* Make incremental compilation failures an error. * Remove loophole which allowed Base.include to skip the incremental compilation warning * Add special case so that `Base.__toplevel__` is never considered closed, allowing normal use of include()
c42f
force-pushed
the
cjf/eval-precomp-warning
branch
from
a8e93f1
to
2b3fde4
Compare
Done + squashed. |
* Make incremental compilation failures an error. * Remove loophole which allowed Base.include to skip the incremental compilation warning * Add special case so that `Base.__toplevel__` is never considered closed, allowing normal use of include()
* Make incremental compilation failures an error. * Remove loophole which allowed Base.include to skip the incremental compilation warning * Add special case so that `Base.__toplevel__` is never considered closed, allowing normal use of include()
eval Base-based monkey-patching stopped working as of Julia 1.5 presumably because eval-ing to another module during precompilation now throws an error: JuliaLang/julia#35410 This PR workarounds this by creating the system image in two stages: 1. The first stage is monkey-patching as done before but without PyCall. This way, we don't get the error because julia does not try to precompile any packages. 2. The second stage is the inclusion of PyCall. At this point, we are in a monkey-patched system image. So, it's possible to precompile PyCall now. Note that even importing PackageCompiler.jl itself inside julia-py is problematic because (normally) it has to be precompiled. We avoid this problem by running PackageCompiler.jl under --compiled-modules=no. This is safe to do thanks to PackageCompiler.do_ensurecompiled (https://github.com/JuliaLang/PackageCompiler.jl/blob/3f0f4d882c560c4e4ccc6ab9a8b51ced380bb0d5/src/PackageCompiler.jl#L181-L188) using a custom function PackageCompiler.get_julia_cmd (https://github.com/JuliaLang/PackageCompiler.jl/blob/3f0f4d882c560c4e4ccc6ab9a8b51ced380bb0d5/src/PackageCompiler.jl#L113-L116) (instead of Base.julia_cmd) and ignores --compiled-modules=no of the current process.
This allows me to use
Core.eval()for replacing parts ofjl_parse_eval_allwith Julia code (part of #35243) by splitting out the warning separately. It also allows us to use the normal logging system and Base infrastructure to format the warning more nicely, including the stacktrace and pretty printing of the expression.For bootstrap I've copied @vtjnash's bootstrap trick where
includemethods are deleted and redefined. Does this seem ok for use witheval? I'm unsure whether it can leave unwanted traces of the old methods in the sysimg, as I don't fully understand the world age and code caching infrastructure.