precompile: fail in (closer to) linear time and linear error messages #59765
Conversation
Do not allow serial compile fallbacks, since this causes performance and output degredation when serial precompile fails. The precompilepkg driver already ensured DAG ordering, so this work is not necessary or useful to repeat. A bit of code cleanup and asking for Claude to describe the argument list as well, plus a test to start to cover any of this code functionality in CI.
vtjnash
changed the title
vtjnash
force-pushed
the
jn/precompile-linear-fail
branch
from
875e4f5 to
3a80bab
Compare
I counted at least 4 different places where an (overlapping) subset of package IO might be printed. Now try to be slightly more careful about that: progress goes to logio (either io or devnull) while output goes to io always. Additionally, we don't return an extra copy of some output in the error message (since we already printed that). Additionally, we print all output if the requested packages failed, but we print only output from successful package if all requested packages succeeded. If a package was successful, then its output is required to be printed since it will not be regenerated in the future (we don't reprint precompile output on loading since that might be obnoxious for us to implement). If a required package is successful however, none of the failures mattered and they will be regenerated in the future when they are required, so those should be filtered out.
vtjnash
force-pushed
the
jn/precompile-linear-fail
branch
from
3a80bab to
1b89cc2
Compare
|
This PR was reverted because it caused widespread breakage in the ecosystem: #59956. A reland PR should really run pkgeval. |
|
This ends up checking the correctness of the |
DilumAluthge
added
the
needs pkgeval
|
I'm not saying all the issues shouldn't be fixed (they should!), but this PR as is was creating real problems to loads of packages and three days after reporting the issue there was no action, so the only short-term option became to revert this PR. |
|
@vtjnash I think the issue here is not tracking packages in the sysimage |
|
I also saw stuff like 
where the |
| # for extensions, any extension in our direct dependencies is one we have a right to load | ||
| # for packages, we may load any extension (all possible triggers are accounted for above) | ||
| loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), direct_deps[pkg]) : nothing | ||
| loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), deps) : nothing |
There was a problem hiding this comment.
Based on a bit of Claude.. I think this sounds reasonable:
| loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), deps) : nothing | |
| loadable_exts = haskey(ext_to_parent, pkg) ? filter((dep)->haskey(ext_to_parent, dep), triggers[pkg]) : nothing |
For an extension ext, triggers[ext] contains all triggers (including parent + trigger packages), some of which may be in the sysimage. direct_deps[ext] is filter(!Base.in_sysimage, triggers[ext]) - so it excludes sysimage packages
Do not allow serial compile fallbacks, since this causes performance
and output degredation when serial precompile fails. The precompilepkg
driver already ensured DAG ordering, so this work is not necessary or
useful to repeat. A bit of code cleanup and asking for Claude to
describe the argument list as well, plus a test to start to cover any
of this file's code functionality in CI unit tests.
This then lets us fix a lot of the error printing issues too, since we
know each package will only get one chance to load, not once for each
appearance in the dependency tree. Now it tries to be slightly more
careful about when and where output should be printed: progress goes to
logio (either io or devnull) while output goes to io always and errors
get returned without duplicating both the error text and output
messages of them also.
The main logic here is that we need to print all output if any
requested packages failed, but we print only output from successful
packages if all requested packages succeeded. If a package was
successful, then its output is required to be printed since it will not
be regenerated in the future (we don't reprint precompile output on
loading since that might be obnoxious for us to implement). If a
required package is successful however, none of the failures mattered
and they will be regenerated in the future when they are required, so
those should be completely filtered out.