-
Notifications
You must be signed in to change notification settings - Fork 44
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
replace Julia's gcc dlls with WinRPM versions during deps/build.jl #34
Conversation
handle failure somewhat gracefully, provide informative messages
5ddab65
to
f2b01b5
Compare
replace Julia's gcc dlls with WinRPM versions during deps/build.jl
yipes. well i guess this is the least bad option, thanks. i'm a little bit surprised that this works for the gcc libs, which presumably are open when julia is running. |
info("Updated Julia's gcc dlls, you may need to restart Julia for some WinRPM packages to work.") | ||
catch | ||
warn("Could not update Julia's gcc dlls, some WinRPM packages may not work.\n" * | ||
"Try running Julia as administrator and calling `Pkg.build(\"WinRPM\")`") |
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.
would be helpful to still include the error message here
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.
would that just be catch err
, and add a line "Error was: $err\n" *
in the warning?
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.
Or Base.show_error()
@ihnorton I'm a little surprised too, I was thinking we'd have to do something trickier. The old dll's are still loaded in memory by Julia, so you do need to restart Julia to actually fix the issue. I'm going to try to write a build-time mini-WinRPM to get these dll's from opensuse in the first place for the binaries. @vtjnash I'll commit the following if it LGTY
|
lgtm |
handle failure somewhat gracefully, provide informative messages
@vtjnash @ihnorton if no objections I'll want to merge and tag this pretty soon, works on my machine for both 32 and 64 bit, and I think I've handled failure cases here