-
Notifications
You must be signed in to change notification settings - Fork 61
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
Allow to opt-out of automatic msys2_mingw_dependency installation #332
Comments
Hi again, Yes, this is a tricky one. Another possibility would be to add some code to ruby-vips here: https://github.com/libvips/ruby-vips/blob/master/lib/vips.rb#L13-L34 It could check for an env var called def library_name(name, abi_number)
lib = ''
if ENV.has_key? 'VIPSHOME'
lib += "#{ENV['VIPSHOME']}/lib/"
end
if FFI::Platform.windows?
lib += "lib#{name}-#{abi_number}.dll"
elsif FFI::Platform.mac?
lib += "#{name}.#{abi_number}"
else
lib += "#{name}.so.#{abi_number}"
end
lib
end Though some platforms use |
Hi @jcupitt, thanks for the quick reply. That sounds even better! 👍🏻
If I get (or rather guess) it right on how ffi works, by explicitly loading the libraries from |
I'm honestly not sure :( We'd need to do some testing. Perhaps opening Thinking about it some more, really the best thing would be to fix the pacman package. Maybe open a bug report here? https://github.com/msys2/MINGW-packages/issues?q=is%3Aissue+is%3Aopen+libvips |
It seems that RubyInstaller honors the So, to avoid installing libvips from MSYS2, you can just do: $ gem install ruby-vips ffi --ignore-dependencies FWIW, I tried to debug that crash with GDB using the |
Hi @jcupitt , thanks for the link to the MINGW-packages repo, I didn't know where to report this 👍🏻 but yes, fixing the pacman package would help for #331 (the crash), but not for #332 (how to control libvips version) though... For sure, I can support with testing the @kleisauke yes, thanks for the hint regarding |
Yes, that's the only thing you'd need to change (I think!). But messing with paths and library resolution is extremely fragile and we're likely to trigger a lot of difficult to debug crashes. I was thinking about this again -- I now think your original idea was right and we should have an env var like Perhaps (untested): "msys2_mingw_dependencies" => ENV.key?("RUBYVIPS_EXTERNAL_LIBVIPS") ? "" : "libvips" So to use an external libvips you'd need to set Any thoughts? |
I came across this interesting article from a few years ago: https://bibwild.wordpress.com/2015/09/09/optional-gem-dependencies/ Which (I think) advocates removing optional dependencies and checking at runtime instead, perhaps with a helpful message. @jrochkind sorry to ping you, but I wonder if you have any insights on this issue? |
You can ping me anytime @jcupitt, I love vips and appreciate and am impressed by your work! (Although I am a rubyist and use vips, I don't currently use ruby-vips -- my code calls out to a shell calling command line vips currently!) There is no way to "opt out" of something in a ruby gemspec at runtime -- any conditional logic in the gemspec will be applied at gem build/release time, once when jcupitts builds and releases the gem, not at individual installation runtime. So definitely don't try that, which I think is unfortuantely what you were settling on -- either something is in a gemspec or is not, there's no way to make it conditional, with the conditions applied to the individual installation. Having something in the gemspec dependent on an ENV variable wont' work -- it'll be calculated at gem build/release time, and then fixed in place. I don't develop or run ruby on Windows though, so don't totally understand the windows-specific issues. Looking at the current ruby-vips gemspec... I see in gem metadata, the key/value With ordinary ruby gemspec dependencies (in gemspec as If you do have an "optional dependency" which is only enforced at runtime -- I think it's important for the runtime checking to be very robust and the error message very clear, including (when it comes to rubygems) expressing and reporting the range of versions of the optional dependency that would be acceptable. (Rails' own runtime optional checking is not as good as it could be here when it comes to reporting). That's my thinking on ordinary ruby gemspec dependencies. But this |
Hi! Thanks for the feedback. It sounds like the best thing is to remove Presumably a try / catch around the first ffi import, maybe here: https://github.com/libvips/ruby-vips/blob/master/lib/vips.rb#L45 Does anyone fancy taking a stab at a prototype PR? |
Oh, interesting. Do you use rails? Does ActiveRecord not fit your use case well? |
I do use rails. I think maybe you're asking about "ActiveStorage"? It's true I don't use ActiveStorage (I do use ActiveRecord, but I don't think it's relevant here), I do my own file handling and transformation with shrine. I could still use the image_processing gem that ActiveStorage uses (which is actually by janko, the same author as shrine) -- but I don't. Partially because some of this code pre-dates ActiveStorage/ImageProcessing, partially because file handling is central to my app and I just want complete control, including of things that may or may not be available via ImageProcessing. I definitely could still use ruby-vips but... I just don't? I just felt more comfortable not relying on anything but the command line, and not having to worry about any possible concurrency or ruby memory bugs. it's possible i'll switch to ruby-vips in the future but it's working fine. |
I now worry your proposal would make things "extremely difficult" for the common case, for the benefit of making something more convenient that's an edge case. But I don't use ruby on Windows, I don't fully understand the case here (as far as I know, on non-windows there isn't even a facility to formally express non-ruby dependencies like this, it's unique to windows!), so I'll stay out of any further discussion! |
Ah! I had an idea! How about removing the mingw dependency from ruby-vips, and making a tiny new gem called Now windows users who want the mingw binary can put I suppose the downside is that the |
That works to do what you say, and works for people who are including ruby-vips in an app. The problem is -- what if you are writing a gem, and want to declare ruby-vips as a dependency of that gem? You are writing a gem that works on any platform, so you aren't going to declare Someone using this gem that has ordinary ruby-vips as a intermediate dependency, on windows... would not get the special windows dependency. Which might mean things wouldn't work for them, unless they knew how to take care of it (say, by adding the windows version of the dependency to their app gemfile manually, after only getting the platform-independent one as an indirect dependency? Or just by manually installing windows libvips, that's what the special windows dependency is doing right?). Seems confusing to me. Whether it's worth the confusion I couldn't say, I still don't totally understand the problem case (or how these windows-only gem metadata dependencies work. I wonder if we can find anyone who is an expert in how these windows metadata dependencies work, to suggest alternate solutions?). |
Hmmm yes that's true. I was imagining downstream projects would continue to just have just ruby-vips as a dependency, and then at the final step anyone deploying on windows would add I'm now thinking this is unfixable. You can't safely mix the official libvips binaries with a mingw install (DLLs could crash into each other). If you want to use ruby-vips on windows with the libvips binaries, you really need to manage the whole binary stack yourself. Let's just fix the pacman libvips. It needs fixing anyway, and that'll resolve most of these immediate issues. |
I meant to add:
|
Interesting, thanks for the background. In other OS, with some gems you responsible for installing your own C or other binary components, usually with a package manager. The most you will get from the gem is a nice warning message telling you you need to install it if it isn't present (and more often a cryptic warning message). Or in some cases on other OSs with ruby, the gem will install the a binary library itself at install time (the message you see at gem install time when this happens is So rubygems on Windows, though, unique to rubygems on Windows, has a mechanism to trigger automatic install of binary dependencies from an external package manager. By listing something in the gemspec I'm not sure what aspect of Windows makes this more desirable such that they've rigged up this windows-ony feature -- maybe that it's harder to supply your own binary library with the gem on Windows, as is sometimes done on other OSs? Or it's just harder to install binary libraries yourself on Windows in general, in a linkable way? Some other gems use OS-provided binaries instead; if the latest standard package-manager-supplied binary library on another OS had problems with it (which seems to be the problem here), that would probably cause problems in that non-Windows situation too. They rely on the the package-manager-supplied binary library building -- or if it doesn't they have the option of supplying/compiling the binary library themselves at gem install time, which is an option not present or less feasible on Windows. |
Sorry for so many words, I'm just procesing, you don't have to read all this if it's not helpful! If your problem is that the pacman-managed libvips has problems with it, so you want to let people manually install a libvips... what if you just remove the The alternative (other than fixing the pacman one) is that naive users get a Windows ruby-vips install that initially appears to work, but has bugs/raises in some paths, but if they care/know, they can use a special install process to install their own libvips instead. That's the thing we're having trouble figuring out a good way to do, but... is that actually desirable? There are occasional ruby gems that on unix/MacOS just make you install the binary dependency yourself, although it does make users grumble. Then we're back to your second-gem idea.... as an alternative to just making the user install libvips themselves, they can optionally just drop in ruby-vips-windows to do it... this makes more sense to me now that I understand the situation. You could put a note in the |
... I made a PR to update the libvips version: msys2/MINGW-packages#11078 It seems to build OK, but I've not tested to see if it fixes the bug you were seeing, I wasn't sure how to make a ucrt package. |
Hey @jcupitt , @jrochkind , first of all, sorry for my slow response time this week, and dropping back in to the conversation only that late again... Re-reading the whole discussion I think there were many valuable aspects and new insights that have been brought up, thanks for that 👍🏻 @jcupitt special thanks for putting up the PR for pacman (I think I would have managed to increase the Regarding this issue: I still think the problem that is still unsolved is that there is currently no way to control which version of the underlying libvips library is getting installed. That's can also happen on Linux, e.g. when you install libvips via apt package manager and where the apt repositories may not retain older versions forever. That's why I especially liked the Considering the suggestion of publishing two variants of the Imho backwards-compatibility for the existing user base is super important, otherwise this will end up in confusion and additional support efforts. I think both approaches could be followed in a backwards-compatible way though. What do you think? |
I think my current thinking is that we can't safely mix binaries between the libvips win builds we make and the win builds made by the msys team. Many of the DLLs will clash and it's very likely to cause mysterious crashes. I'm often wrong though, so if I'm mixed up there, please do correct me! This means there's no safe way for people who use rubyinstaller to patch in a different libvips binary themselves. They must go via pacman and the msys toolchain. It's not actually that hard: you just make some edits to the PKGBUILD file and rerun it to make a new So I think that's probably the solution. You can stick with the libvips that ruby gives you automatically, or if you're determined, you can make your own package. Hopefully the updated libvips will fix the original crash too, though I've not been able to test it yet. |
I just want to emphasize that im pretty sure you can't put a conditional in
the gemspec on an env var that is different for different users. If you
want to try something like that please test it thoroughly to confirm.
|
Hi @jcupitt ,
as discussed in #331 (see comments), we have the need of more fine-grained control on the libvips library version to be used.
When you install the
ruby-vips
gem on Windows, it would automatically (and unconditionally) install the latest availablelibvips
dependency to your msys2 ruby devkit environment (see here in ruby-vips.gemspec).There is the
%RUBY_DLL_PATH%
environment variable which you can point to an external libvips installation (e.g. obtained from libvips/build-win64-mxe releases), but this only works when there is no libvips installation present in the msys2 ruby devkit environment.So in order to make use of
%RUBY_DLL_PATH%
to get control of the libvips version being installed, you need to make sure that there is no parallel libvips installation in the msys2 ruby devkit environment. But here's the catch: every time you re-install or update theruby-vips
gem, you will get an msys2 libvips installation as a side effect, breaking the use of the external libvips version in%RUBY_DLL_PATH%
.One solution to this could be to provide a possibility to opt-out of the installation of "msys2_mingw_dependencies" in the gemspec (e.g. via an env var like
SKIP_MSYS2_MINGW_DEPENDENCIES=1
or so).What do you think? Does that sound feasible / sensible? Do you have other suggestions on how to enable better control over the libvips library version?
Cheers,
Torben
The text was updated successfully, but these errors were encountered: