-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Fix iteration over dict in loading.jl #48608
Conversation
@@ -1200,7 +1200,7 @@ function run_extension_callbacks(pkgid::PkgId) | |||
# take ownership of extids that depend on this pkgid | |||
extids = pop!(EXT_DORMITORY, pkgid, nothing) | |||
extids === nothing && return | |||
for extid in extids | |||
for extid in values(extids) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extids is an Array, so calling values
is unnecessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot that pop changes dicts into arrays.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't? The values of that dict are vectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah , sorry :(. In any case do we have a way to check if a pkg is an extension at this time? To get back the pretty printing?
for extid in values(EXT_DORMITORY) | ||
if extid.id == pkg | ||
print(extid.parentid.name, " → ") | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not return true for any element, so it could be simply replaced here by:
for extid in values(EXT_DORMITORY) | |
if extid.id == pkg | |
print(extid.parentid.name, " → ") | |
break | |
if false; if false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah :(. This broke the time_imports pretty printing for extensions.
Easier to fix this on top of #48586? |
Is that the PR that delays the loading of the extensions? Though in any case if we ever decide to do that this kind of printing won't work anymore because it relies on loading the extension immediately after the main package |
Should fix #48590, but I wasn't able to reproduce the original failure locally. So a working mwe for a test would be nice.
In the meanwhile, @anandijain could you check locally?