Bug 962160 - Review Request: iipsrv - Light-weight streaming for viewing and zooming of ultra high-resolution images
Review Request: iipsrv - Light-weight streaming for viewing and zooming of ul...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Remi Collet
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-12 02:36 EDT by Johan Cwiklinski
Modified: 2013-06-13 18:30 EDT (History)
4 users (show)

See Also:
Fixed In Version: iipsrv-1.0.0-0.7.git0b63de7.el5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-06-07 00:31:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
fedora: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
review.txt (9.76 KB, text/plain)
2013-05-24 09:03 EDT, Remi Collet
rcollet: review?
Details

  None (edit)
Description Johan Cwiklinski 2013-05-12 02:36:46 EDT
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.1.git0b63de7.fc18.src.rpm
Description: Light-weight streaming for viewing and zooming of ultra high-resolution images
Fedora Account System Username: trasher
Comment 1 Johan Cwiklinski 2013-05-13 14:23:10 EDT
I've noticed that I used "%define debug_package %{nil}" instead of "%global debug_package %{nil}"; I'll fix that in the next release.
Comment 2 Johan Cwiklinski 2013-05-18 11:54:29 EDT
I've fixed the %define, and made a bit of cleanup.

The new version:
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.2.git0b63de7.fc18.src.rpm
Comment 3 Remi Collet 2013-05-19 02:22:51 EDT
Quick notes:

- why no debuginfo package ? (simply drop the "strip" command)

- must use system fcgi library instead of bundled copy (and so, no need for -devel subpackage which is not usable and conflicts with fcgi-devel)

- I don't like the idea of package installing stuff in /var/www.
I Will prefer an alias to the binary + SetHandler

- should consider having httpd conf in a subpackage to allow use with other webservers

- why a git snapshot ? stable realease (0.9.9) is not ok ?

- From http://iipimage.sourceforge.net/documentation/server/#commandline
seems it could be used as a service, and then used with a simple ProxyPass from apache 2.4 (note: default value of 9000 is already used by php-fpm).
Comment 4 Johan Cwiklinski 2013-05-19 02:48:53 EDT
(In reply to comment #3)
> - why no debuginfo package ? (simply drop the "strip" command)

I'll try that, thanks.

> - must use system fcgi library instead of bundled copy (and so, no need for
> -devel subpackage which is not usable and conflicts with fcgi-devel)

OK, I'll change that.

> - I don't like the idea of package installing stuff in /var/www.
> I Will prefer an alias to the binary + SetHandler

OK, I'll change that too.

> - should consider having httpd conf in a subpackage to allow use with other
> webservers

Indeed, as IIP can run with other webservers as well.

> - why a git snapshot ? stable realease (0.9.9) is not ok ?

I recently have issues with some images with 0.9.9, that were fixed with the git snapshtop.

> - From http://iipimage.sourceforge.net/documentation/server/#commandline
> seems it could be used as a service, and then used with a simple ProxyPass
> from apache 2.4 (note: default value of 9000 is already used by php-fpm).

I'll take care of that, and change the default port; thank you.
Comment 5 Johan Cwiklinski 2013-05-19 05:23:31 EDT
Thanks to your various notes, I've made a new version.

I've fixed debuginfo package, changed /var/www for /usr/libexec and I created a httpd subpackage.

Here the new version:
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.3.git0b63de7.fc18.src.rpm

I took a look at the service part, but that is too much work for today... I have to deal with both systemd and sysv, to take care of IP and port, and I should also take care of various options that can be exported according to the documentation). Will be in a future version :)
Comment 6 Johan Cwiklinski 2013-05-19 06:58:10 EDT
Hum... I forgot to remove Requires: mod_fcgi on the main package... Will do.
Comment 7 Johan Cwiklinski 2013-05-19 15:39:47 EDT
Well, I've added Systemd files for F >= 18 (tested on F-18).

I've tried to add a SysV script too, but I've not been able to have something working (executable never returns, that seem to cause an issue). I'm unsure I'll provide a Sysv script.

I've also removed the bad require on mod_fcgi. Here the new version:
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.4.git0b63de7.fc18.src.rpm
Comment 8 Johan Cwiklinski 2013-05-20 15:40:29 EDT
And a new version, with initd script for EL :)

Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.5.git0b63de7.fc18.src.rpm
Comment 9 Johan Cwiklinski 2013-05-22 14:33:21 EDT
I've removed config file in /etc/iipsrv, and placed its content in unit file.

New version:
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.6.git0b63de7.fc18.src.rpm
Comment 10 Remi Collet 2013-05-24 09:03:27 EDT
Created attachment 752589 [details]
review.txt

Generated by fedora-review 0.4.1 (b2e211f) last change: 2013-04-29
Buildroot used: fedora-rawhide-x86_64
Command line :/usr/bin/fedora-review -b 962160
Comment 11 Remi Collet 2013-05-24 09:04:18 EDT
Please check attachment for comment.
Comment 12 Johan Cwiklinski 2013-05-24 19:58:05 EDT
Thank you for taking the review :)

I've fixed the issues you've pointed out in the review.txt file. I've also changed mod_fcgid directives names, except for EL-5 since the shipped version of mod_fcgid is still the pre ASF one (see http://httpd.apache.org/mod_fcgid/mod/mod_fcgid.html#upgrade).

Here is the new version:
Spec URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv.spec
SRPM URL: http://odysseus.x-tnd.be/fedora/iipsrv/iipsrv-1.0.0-0.7.git0b63de7.fc18.src.rpm
Comment 13 Remi Collet 2013-05-27 04:35:00 EDT
[x]: Fully versioned dependency in subpackages, if present.
[x]: License field in the package spec file matches the actual license.
[x]: All build dependencies are listed in BuildRequires, except for any that
[x]: Scriptlets must be sane, if used.
[x]: Requires correct, justified where necessary.
  missing Requires(pre): /usr/sbin/useradd (not a blocker)
[x]: could probably add comment in top of the unit files

Koji scratch build EL-5 
http://koji.fedoraproject.org/koji/taskinfo?taskID=5428980

Koji scratch build rawhide (OK)
http://koji.fedoraproject.org/koji/taskinfo?taskID=5428967


All blockers fixed.

=== APPROVED ===
Comment 14 Johan Cwiklinski 2013-05-27 11:54:50 EDT
Thank you very much Remi for your help and for the review :)
Comment 15 Johan Cwiklinski 2013-05-27 11:55:52 EDT
New Package SCM Request
=======================
Package Name: iipsrv
Short Description: Light-weight streaming for viewing and zooming of ultra high-resolution images
Owners: trasher
Branches: f18 f19 el5 el6
InitialCC:
Comment 16 Gwyn Ciesla 2013-05-28 11:36:07 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2013-05-28 15:49:10 EDT
iipsrv-1.0.0-0.7.git0b63de7.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/iipsrv-1.0.0-0.7.git0b63de7.el6
Comment 18 Fedora Update System 2013-05-28 15:49:20 EDT
iipsrv-1.0.0-0.7.git0b63de7.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/iipsrv-1.0.0-0.7.git0b63de7.fc18
Comment 19 Fedora Update System 2013-05-28 15:49:30 EDT
iipsrv-1.0.0-0.7.git0b63de7.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/iipsrv-1.0.0-0.7.git0b63de7.fc19
Comment 20 Fedora Update System 2013-05-28 15:49:40 EDT
iipsrv-1.0.0-0.7.git0b63de7.el5 has been submitted as an update for Fedora EPEL 5.
https://admin.fedoraproject.org/updates/iipsrv-1.0.0-0.7.git0b63de7.el5
Comment 21 Fedora Update System 2013-05-29 13:34:20 EDT
iipsrv-1.0.0-0.7.git0b63de7.el6 has been pushed to the Fedora EPEL 6 testing repository.
Comment 22 Fedora Update System 2013-06-07 00:31:58 EDT
iipsrv-1.0.0-0.7.git0b63de7.fc19 has been pushed to the Fedora 19 stable repository.
Comment 23 Fedora Update System 2013-06-07 19:51:14 EDT
iipsrv-1.0.0-0.7.git0b63de7.fc18 has been pushed to the Fedora 18 stable repository.
Comment 24 Fedora Update System 2013-06-13 18:28:41 EDT
iipsrv-1.0.0-0.7.git0b63de7.el6 has been pushed to the Fedora EPEL 6 stable repository.
Comment 25 Fedora Update System 2013-06-13 18:30:33 EDT
iipsrv-1.0.0-0.7.git0b63de7.el5 has been pushed to the Fedora EPEL 5 stable repository.

Note You need to log in before you can comment on or make changes to this bug.