-
Notifications
You must be signed in to change notification settings - Fork 5
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 compilation failure on Julia 1.10 #6
Conversation
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 am okay with that. @joshday, what is your opinion?
I don't like that you have to separately call |
Thanks, please merge when ready. Indeed it is a hard requirement of Julia 1.10, otherwise precompilation of downstream packages will hang [source]:
|
An alternative is perhaps to only run it when precompilation is not happening? |
What's tricky is that it doesn't affect precompilation of PlotlyKaleido, because during precompilation we don't call But thinking about it again, I guess you're proposing that the |
How could we do that? |
Magic incantation: |
PyPlot.jl uses this trick but somehow it still runs into the I/O issue seen here: https://github.com/JuliaPy/PyPlot.jl/blob/343430ac4f822a84a5e19bf5dabf7a846b171c9b/src/init.jl#L171-L209 |
I mean, either the code that starts the process runs or not. So are you saying it still runs even with that guard? |
I haven’t debugged the details so I can’t answer. But PyPlot.jl does have the I/O issue and I couldn’t find any other I/O resources other than what’s in that |
I'll tackle this (but not today), I want to write up a little guide on how you debug these things as I go. |
Julia 1.10 requires that no I/O resources are left open during module initialization. Thus, the code:
needs to be removed, and the user must call
manually upon initialization.
I have implemented these changes. I have also updated the package version to 2.0 so that this does not introduce breaking changes in any existing dependent projects (though they will soon break anyways).
I also fixed a bug where
kill()
is defined incorrectly, as it conflicts withBase.kill()
. It is nowkill_kaleido
.Fixes #5
@BeastyBlacksmith would you be willing to merge this relatively quickly? This is currently breaking my package AirspeedVelocity.jl on Julia 1.10.