[Bug 1586199] New: Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

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: [e-mail]
          Reporter: [e-mail]
        QA Contact: [e-mail]
                CC: [e-mail]

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

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Pavel Valena  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |fedora-review?

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Jun Aruga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
       Docs Contact|                            |[e-mail]

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #1 from Jun Aruga  ---
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.
```

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #2 from Jun Aruga  ---
> <mock-chroot> sh-4.4# identify --help

Sorry I just made mistake. "identify -help" (single "-" is correct).

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #3 from Jun Aruga  ---
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.

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #4 from Pavel Valena  ---
(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!

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Jun Aruga  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #5 from Jun Aruga  ---
okay, I reviewed it again.
It looks good. I would ACCEPT it.

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Pavel Valena  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
       Docs Contact|[e-mail]           |
           Assignee|[e-mail]    |[e-mail]

--- Comment #6 from Pavel Valena  ---
Fixing assignee.

https://pagure.io/releng/fedora-scm-requests/issue/6946

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #7 from Jun Aruga  ---
Oh thanks for that.

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #8 from Gwyn Ciesla  ---
(fedrepo-req-admin):  The Pagure repository was created at
https://src.fedoraproject.org/rpms/rubygem-mini_magick

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #9 from Jun Aruga  ---
Hi Pavel,
After the package are prepared, can you close this ticket?
http://fedoraproject.org/wiki/Package_Review_Process

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Pavel Valena  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  ---
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.

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

--- Comment #11 from Jun Aruga  ---
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.

[Bug 1586199] Review Request: rubygem-mini_magick - Manipulate images with minimal use of memory via ImageMagick

https://bugzilla.redhat.com/show_bug.cgi?id=1586199

Pavel Valena  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |1267323
                 CC|                            |[e-mail]

--- Comment #12 from Pavel Valena  ---
*** 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