-
-
Notifications
You must be signed in to change notification settings - Fork 269
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 JLL version fix fix #4130
Fix JLL version fix fix #4130
Conversation
I don't understand why currently but this bug only happens when the JLLs are in the sysimage |
src/Operations.jl
Outdated
vers_fix[uuid] = old_v | ||
new_v != old_v && delete!(compat_map[uuid], new_v) |
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.
Maybe this shouldn't be deleted, for cases where the package version in Project.toml and the manifest don't match, and the package is in the sysimage. i.e. #4131
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.
@KristofferC do you think this delete!
should be here? I'm leaning towards no to be safer in case I haven't thought through all situations, but not sure
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.
I've removed it with a comment
Tested and appears to work. Master
This PR
|
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.
Makes sense to me. I wonder how hard it would be to make a test.
If there was a stdlib JLL that's marked as upgradable, I think we could do a test relatively easily. That's not the case, I believe? But without that it'd require PackageCompiler which would take time and need manual installation over on julia CI because test deps aren't automatically installed there. |
(cherry picked from commit bc9fb21)
(cherry picked from commit bc9fb21)
Reported here JuliaRegistries/General#122129 (comment)