https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Bug ID: 1586199 Summary: Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: pvalena@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://pvalena.fedorapeople.org/gems/rubygem-mini_magick.spec SRPM URL: https://pvalena.fedorapeople.org/gems/rubygem-mini_magick-4.8.0-1.fc29.src.r... Description: Fedora Account System Username: pvalena Koji build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27441109
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Jun Aruga jaruga@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |jaruga@redhat.com Docs Contact| |jaruga@redhat.com
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #1 from Jun Aruga jaruga@redhat.com --- At first 2 things that I want to ask.
1.
#-devel # BuildRequires: rubygem(posix-spawn)
Why does the only gem: posix-spawn for development is kept as commented line? While the other gems guard are removed.
2.
# Test failing with: # expected: "mogrify", got: nil sed -i '/^ it "assigns :mogrify by default" do$/,/ end/ s/^/#/g' \ spec/lib/mini_magick/configuration_spec.rb
Seeing the source code, this failure happens when mogrify command (ImageMagick package) is not installed. As you are setting ImageMagick as a build dependency, you can remove this sed command line, right? But after removing the line, you will face a different test failure of ImageMagick unique tests.
``` Failures:
1) With ImageMagick MiniMagick::Image#details returns a hash of verbose information Failure/Error: expect(subject.details["Channel depth"]["Red"]).to eq "8-bit"
expected: "8-bit" got: nil
(compared using ==) # ./spec/lib/mini_magick/image_spec.rb:423:in `block (5 levels) in <top (required)>' ```
Seeing the source code, the MiniMagick::Image#details is an output of "identify -verbose" command. But maybe the result is invalid. Maybe ImageMagik on Fedora is something wrong. This happens on your environment? Can you dig this or report to the project?
``` <mock-chroot> sh-4.4# rpm -qf /usr/bin/identify ImageMagick-6.9.9.38-1.fc29.x86_64
<mock-chroot> sh-4.4# identify -verbose => The result is empty
<mock-chroot> sh-4.4# identify --help identify: unable to open image `--help': No such file or directory @ error/blob.c/OpenBlob/2761. identify: no decode delegate for this image format `' @ error/constitute.c/ReadImage/504. ```
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #2 from Jun Aruga jaruga@redhat.com ---
<mock-chroot> sh-4.4# identify --help
Sorry I just made mistake. "identify -help" (single "-" is correct).
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #3 from Jun Aruga jaruga@redhat.com --- Below is a result of fedora-review command.
[!]: Patches link to upstream bugs/comments/lists or are otherwise justified. The sed command line for "Test failing with" needs a link to upstream, if the sed command is really needed.
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #4 from Pavel Valena pvalena@redhat.com --- (In reply to Jun Aruga from comment #1)
#-devel # BuildRequires: rubygem(posix-spawn)
Why does the only gem: posix-spawn for development is kept as commented line? While the other gems guard are removed.
Sorry, I forgot to remove those as well.
# Test failing with: # expected: "mogrify", got: nil sed -i '/^ it "assigns :mogrify by default" do$/,/ end/ s/^/#/g' \ spec/lib/mini_magick/configuration_spec.rb
Seeing the source code, this failure happens when mogrify command (ImageMagick package) is not installed. As you are setting ImageMagick as a build dependency, you can remove this sed command line, right?
You're right. I don't know why I did add the `sed`. Removed.
But after removing the line, you will face a different test failure of ImageMagick unique tests.
No, that failure is unrelated, the tests suceed, see bellow.
Failures: 1) With ImageMagick MiniMagick::Image#details returns a hash of verbose information Failure/Error: expect(subject.details["Channel depth"]["Red"]).to eq "8-bit" expected: "8-bit" got: nil (compared using ==) # ./spec/lib/mini_magick/image_spec.rb:423:in `block (5 levels) in <top (required)>'
This is actually fixed by Patch0. https://github.com/minimagick/minimagick/pull/454/
--
I've updated the srpm and .spec file in Description. New Scratch-build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27475124
Thanks for finding those issues!
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Jun Aruga jaruga@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #5 from Jun Aruga jaruga@redhat.com --- okay, I reviewed it again. It looks good. I would ACCEPT it.
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Docs Contact|jaruga@redhat.com | Assignee|nobody@fedoraproject.org |jaruga@redhat.com
--- Comment #6 from Pavel Valena pvalena@redhat.com --- Fixing assignee.
https://pagure.io/releng/fedora-scm-requests/issue/6946
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #7 from Jun Aruga jaruga@redhat.com --- Oh thanks for that.
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #8 from Gwyn Ciesla limburgher@gmail.com --- (fedrepo-req-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/rubygem-mini_magick
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #9 from Jun Aruga jaruga@redhat.com --- Hi Pavel, After the package are prepared, can you close this ticket? http://fedoraproject.org/wiki/Package_Review_Process
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Fixed In Version| |rubygem-mini_magick-4.8.0-1 | |.fc29 Resolution|--- |RAWHIDE Last Closed| |2018-06-12 06:46:41
--- Comment #10 from Pavel Valena pvalena@redhat.com --- Jun,
I am familiar with the review process, no need to remind me all the time. Unless, off course, there's some specific detail you want you point out. Which you didn't.
Yes, the package was built 4 days ago, I just didn't get around to close this ticket. It was in my next cycle.
Thanks for the review.
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
--- Comment #11 from Jun Aruga jaruga@redhat.com --- Pavel, I believed you are familiear with the review process. As I saw my assigned BZ is kept not closed for a time, I wanted to remind you.
https://bugzilla.redhat.com/show_bug.cgi?id=1586199
Pavel Valena pvalena@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |1267323 CC| |ilya.gradina@gmail.com
--- Comment #12 from Pavel Valena pvalena@redhat.com --- *** Bug 1264660 has been marked as a duplicate of this bug. ***
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1267323 [Bug 1267323] Review Request: rubygem-carrierwave - Ruby file upload library
package-review@lists.fedoraproject.org