put Logging back in default sysimage #53660
Conversation
|
And now StyledStrings is in there as well? That feels like something people will really start relying on. @tecosaur, wasn't the plan to move the styled string rendering code into Base (which would allow Consologger to also be moved into Base. Alternatively the use of StyledStrings in logging could be reverted. |
|
Yes, one option is to move ConsoleLogger to base and merge it with SimpleLogger. Then it would be nice if it can avoid using StyledStrings, but I'm not sure that's possible without sacrificing functionality people want. |
|
Could we add a class of excised stdlibs that are pkgimages that are loaded immediately after the sysimage, before startup.jl, but after the active project is set, and there could be a flag to disable it. |
The (inconclusive) discussion on Triage partially recorded in Slack is this: https://julialang.slack.com/archives/C67TUQBG9/p1708007625965089 For posterity, I'll copy over the first (and most relevant) message here before it gets eaten by the slack-hole:
I don't really have a horse in this race, so long as we don't go down the "let's not use StyledStrings for stuff in Base/not-REPL stdlibs" path, since I have yet more hopes for using it to enhance other aspects like error messages (if it's somehow possible, I'd love to be able to use That said, I do personally somewhat like the sound of Ian's suggestion on the face of it 🙂. |
|
I'm fine with any of that, but I want to avoid doing significant development work / adding features in order to fix a regression. For 1.11, it looks to me like we need to put Logging back where it was and re-try moving it out when we have everything sorted to make that possible. |
|
Is this mergeable or does the StyledStrings consideration require changes? |
Backported PRs: - [x] #39071 <!-- Add a lazy `logrange` function and `LogRange` type --> - [x] #51802 <!-- Allow AnnotatedStrings in log messages --> - [x] #53369 <!-- Orthogonalize re-indexing for FastSubArrays --> - [x] #48050 <!-- improve `--heap-size-hint` arg handling --> - [x] #53482 <!-- add IR encoding for EnterNode --> - [x] #53499 <!-- Avoid compiler warning about redefining jl_globalref_t --> - [x] #53507 <!-- update staled `Core.Compiler.Effects` documentation --> - [x] #53408 <!-- task splitting: change additive accumulation to multiplicative --> - [x] #53523 <!-- add back an alias for `check_top_bit` --> - [x] #53377 <!-- add _readdirx for returning more object info gathered during dir scan --> - [x] #53525 <!-- fix InteractiveUtils call in Base.runtests on failure --> - [x] #53540 <!-- use more efficient `_readdirx` for tab completion --> - [x] #53545 <!-- use `_readdirx` for `walkdir` --> - [x] #53551 <!-- revert "Add @create_log_macro for making custom styled logging macros (#52196)" --> - [x] #53554 <!-- Always return a value in 1-d circshift! of abstractarray.jl --> - [x] #53424 <!-- yet more atomics & cache-line fixes on work-stealing queue --> - [x] #53571 <!-- Update Documenter to v1.3 for inventory writing --> - [x] #53403 <!-- Move parallel precompilation to Base --> - [x] #53589 <!-- add back `unsafe_convert` to pointer for arrays --> - [x] #53596 <!-- build: remove extra .a file --> - [x] #53606 <!-- fix error path in `precompilepkgs` --> - [x] #53004 <!-- Unexport with, at_with, and ScopedValue from Base --> - [x] #53629 <!-- typo fix in scoped values docs --> - [x] #53630 <!-- sroa: Fix incorrect scope counting --> - [x] #53598 <!-- Use Base parallel precompilation to build stdlibs --> - [x] #53649 <!-- precompilepkgs: package in boths deps and weakdeps are in fact only weak --> - [x] #53671 <!-- Fix bootstrap Base precompile in cross compile configuration --> - [x] #52125 <!-- Load Pkg if not already to reinstate missing package add prompt --> - [x] #53602 <!-- Handle zero on arrays of unions of number types and missings --> - [x] #53516 <!-- permit NamedTuple{<:Any, Union{}} to be created --> - [x] #53643 <!-- Bump CSL to 1.1.1 to fix libgomp bug --> - [x] #53679 <!-- move precompile workload back from Base --> - [x] #53663 <!-- add isassigned methods for reinterpretarray --> - [x] #53662 <!-- [REPL] fix incorrectly cleared line after completions accepted --> - [x] #53611 <!-- Linalg: matprod_dest for Diagonal and adjvec --> - [x] #53659 <!-- fix #52025, re-allow all implicit pointer casts in cconvert for Array --> - [x] #53631 <!-- LAPACK: validate input parameters to throw informative errors --> - [x] #53628 <!-- Make some improvements to the Scoped Values documentation. --> - [x] #53655 <!-- Change tbaa of ptr_phi to tbaa_value --> - [x] #53391 <!-- Default to the medium code model in x86 linux --> - [x] #53699 <!-- Move `isexecutable, isreadable, iswritable` to `filesystem.jl` --> - [x] #41232 <!-- Fix linear indexing for ReshapedArray if the parent has offset axes --> - [x] #53527 <!-- Enable analyzegc checks for try catch and fix found issues --> - [x] #52092 - [x] #53682 <!-- Increase build precompilation --> - [x] #53720 - [x] #53553 <!-- typeintersect: fix `UnionAll` unaliasing bug caused by innervars. --> Contains multiple commits, manual intervention needed: - [ ] #53305 <!-- Propagate inbounds in isassigned with CartesianIndex indices --> Non-merged PRs with backport label: - [ ] #53736 <!-- fix literal-pow to return the right type when the base is -1 --> - [ ] #53707 <!-- Make ScopedValue public --> - [ ] #53696 <!-- add invokelatest to on_done callback in bracketed paste --> - [ ] #53660 <!-- put Logging back in default sysimage --> - [ ] #53509 <!-- revert moving "creating packages" from Pkg.jl --> - [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} --> - [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` --> - [ ] #52694 <!-- Reinstate similar for AbstractQ for backward compatibility --> - [ ] #51928 <!-- Styled markdown, with a few tweaks --> - [ ] #51816 <!-- User-themable stacktraces --> - [ ] #51811 <!-- Make banner size depend on terminal size --> - [ ] #51479 <!-- prevent code loading from lookin in the versioned environment when building Julia -->
|
Adding to milestone to avoid logging UI regression. |
|
|
|
There's been talk for months now about how the rendering of StyledStrings has to be moved into Base if it wants to style things in Base and the same applies here since the default logging of Base should be "nice" but nothing happens. So I will just revert the StyledString usage in Logging, move ConsoleLogger.jl into Base so this is finally dealt with. |
Thanks to Kristoffer, this is now tracked as JuliaLang/StyledStrings.jl#58
This is an interpretation of events that differs from mine, as I'm still under the impression that there were ~3 options put forward to consider, and I don't have strong opinions on them and so have been waiting for one to be picked. I'm not too fussed, but if/when the new For a bit more context (as I recall, at least), Triage suggested that in this case that it could potentially have StyledStrings as a dep, and be loaded in interactive sessions and possibly by the compiler for nicer error messages. |
What I am saying is exactly point 2 there?
This would not be an stdlib. It would just be something that Base can do. And that would require the same as here, being able to render styled strings from Base. |
I may just be misinterpreting — I read your comment as saying that's the only way forward.
My "a potential new stdlib" comment is from when I popped into a recent triage meeting and basically asked "here's some functionality I think could be worth sorting on, what do you think and where should it go?" and the preliminary answer I got was "make it a new stdlib". |
Yes, the other ones don't make sense.
Well, that's just wrong... Error messages is a Base feature, it doesn't make sense for a "HelpfulErrors" stdlib. Either it would be a normal Base feature if it is stable and "good" enough, or it would be a separate package. |
|
ConsoleLogging got put into Base. |
|
I concur with traige's position that HelpfulErrorMessages should be a stdlib, not in Base. Exceptions and try/catch should be a part of Base, basic error messages should also imo be a part of base, but fancy helpful errors that try to provide insightful advice need not be. They can and imo should pirate base methods or hook into error hint API for this. HelpfulErrorMessages should be loaded by default in the REPL and when invoking julia from the command line. |
What?
This would be terrible for latency.
Yes, the existing error hint API sits there ready for contributions. |
|
Should we move this tangent into a dedicated issue/elsewhere? Anyway, my takeaway was that |
fixes #51493, fixes #52075