Spec URL: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01616639-rubygem-ruby-vips/rubygem-ruby-vips.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01616639-rubygem-ruby-vips/rubygem-ruby-vips-2.0.17-1.fc34.src.rpm Description: ruby-vips is a binding for the vips image processing library. It is fast and it can process large images without loading the whole image in memory. Fedora Account System Username: pvalena Builds & Tests run: https://gist.github.com/pvalena/75824094c5d97a347c036680bb2185ed
I'm taking this for a review.
* `BuildRequires: vips-devel` - What is the reason to include vips-devel instead of just vips? * Requires - I think there should be something aka `Requires: vips` - Possibly, there could be required all libraries loaded via `ffi_lib` call. BTW, I'd rather see requires such as 'libvips.so.42`, but they are not properly required [1]. * Wrong shebangs: ~~~ rubygem-ruby-vips-doc.noarch: E: env-script-interpreter /usr/share/gems/gems/ruby-vips-2.0.17/example/example1.rb /usr/bin/env ruby rubygem-ruby-vips-doc.noarch: E: env-script-interpreter /usr/share/gems/gems/ruby-vips-2.0.17/example/thumb.rb /usr/bin/env ruby ~~~ [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1870275
Thank you for your reviews! > - What is the reason to include vips-devel instead of just vips? You're right, I thought I needed headers, but I don't. > - I think there should be something aka `Requires: vips` Yes, you're right. I've tried using it in the test suite of image_processing gem (which succeeds). > * Wrong shebangs: Hmm. I've already fixed that (I've linked probably an earlier iteration of spec file by mistake). Refreshed SPEC and SRPM with the changes (the release bump is just for COPR). Spec: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01619049-rubygem-ruby-vips/rubygem-ruby-vips.spec SRPM: https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01619049-rubygem-ruby-vips/rubygem-ruby-vips-2.0.17-2.fc34.src.rpm Builds & Test log: https://gist.github.com/pvalena/7dc8d06603fc58c7f207cd854a1d4b8d
(In reply to Pavel Valena from comment #3) > > * Wrong shebangs: > > Hmm. I've already fixed that (I've linked probably an earlier iteration of > spec file by mistake). I admire your upstream convincing skills in this regard :) Otherwise LGTM => APPROVED
(In reply to Vít Ondruch from comment #2) > BTW, I'd rather see requires such as 'libvips.so.42`, but they are not > properly required [1]. And it turns out, there is way to do it according to RPM upstream [1]: ~~~ Requires: (libvips.so.42()(64bit) if libc.so.6()(64bit)) Requires: (libvips.so.42 if libc.so.6) ~~~ Because this is Ruby package, we could possible use libruby.so instead. [1]: https://github.com/rpm-software-management/rpm/issues/1344#issuecomment-681916527
(In reply to Vít Ondruch from comment #5) > Because this is Ruby package, we could possible use libruby.so instead. or libffi.so.6
(In reply to Vít Ondruch from comment #4) > (In reply to Pavel Valena from comment #3) > > > * Wrong shebangs: > > > > Hmm. I've already fixed that (I've linked probably an earlier iteration of > > spec file by mistake). > > I admire your upstream convincing skills in this regard :) > > Otherwise LGTM => APPROVED I think it was luck. (In reply to Vít Ondruch from comment #5) > (In reply to Vít Ondruch from comment #2) > > BTW, I'd rather see requires such as 'libvips.so.42`, but they are not > > properly required [1]. > > And it turns out, there is way to do it according to RPM upstream [1]: > > ~~~ > Requires: (libvips.so.42()(64bit) if libc.so.6()(64bit)) > Requires: (libvips.so.42 if libc.so.6) > ~~~ > > Because this is Ruby package, we could possible use libruby.so instead. > > > [1]: > https://github.com/rpm-software-management/rpm/issues/1344#issuecomment- > 681916527 Thanks of figuring this out! `libffi.so.6` seems like a good choice indeed.
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rubygem-ruby-vips