Bug 235805 - Review Request: ocaml-camlimages - OCaml image processing library
Review Request: ocaml-camlimages - OCaml image processing library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Package Reviews List
:
Depends On:
Blocks: 235815
  Show dependency treegraph
 
Reported: 2007-04-10 03:17 EDT by Nigel Jones
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-05-08 23:18:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
dennis: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nigel Jones 2007-04-10 03:17:15 EDT
Spec URL: http://dev.nigelj.com/SRPMS/camlimages.spec
SRPM URL: http://dev.nigelj.com/SRPMS/camlimages-2.2.0-1.src.rpm
Description: 
CamlImages is an image processing library for Objective CAML, which provides:
 basic functions for image processing and loading/saving
 various image file formats (hence providing a translation facility from
  format to format)
 an interface with the Caml graphics library allows to display
  images in the Graphics module screen and to mix them with Caml drawings

In addition, the library can handle huge images that cannot be (or can hardly
be) stored into the main memory (the library then automatically creates swap
files and escapes them to reduce the memory usage).

rpmlint:
camlimages-2.2.0-1.i386.rpm:
W: camlimages ocaml-naming-policy-not-applied /usr/lib/ocaml/camlimages/dllci_jpeg.so
(I don't know if an exception can be made here, I can't find much on the problem)
camlimages-devel-2.2.0-1.i386.rpm:
W: camlimages-devel no-documentation
W: camlimages-devel ocaml-naming-policy-not-applied /usr/lib/ocaml/camlimages/Makefile.config
(1: docs are in the main package, which is strictly depended on, 2: Ditto about exception)
camlimages-2.2.0-1.src.rpm:
Clean

Like with ocamlSDL (235804) I believe the files are in the right place etc etc, but I have one note:
One package which depends on this (which I'm putting in for Package Review later today), depends on this for building, but not for use, I'm worried that the libraries are somehow been configured statically, not sure if it's a problem with this package or the other package.
Comment 1 Nigel Jones 2007-04-10 18:44:16 EDT
Spec URL: http://dev.nigelj.com/SRPMS/camlimages.spec
SRPM URL: http://dev.nigelj.com/SRPMS/camlimages-2.2.0-2.src.rpm

Simple dependency update, now -2
Comment 2 Nigel Jones 2007-04-11 21:57:34 EDT
As per my comment on 235804...

I've done some more research into the issue with static linking etc etc, and
found out it is the case, I'd like to put this review on hold until I'm at least
sponsored (and can post on fedora-maintainers to get some more information on
the problem and possible solutions).
Comment 3 Hans de Goede 2007-05-02 14:59:23 EDT
Please update this to be inline with the proposed ocaml guidelines I've just
CC-ed you on. This should fix the ocaml naming policy warning.

Notice that I expect the main discussion on this proposal to happen on on
fedora-devel-list which you can join now. Also you can be added to
fedora-maintainers-list before being sponsored, just ask on the devel list.

Comment 4 Nigel Jones 2007-05-03 05:30:48 EDT
Hi Hans, as promised on the Freetennis bug, an update!

Spec URL: http://dev.nigelj.com/SRPMS/camlimages.spec
SRPM URL: http://dev.nigelj.com/SRPMS/camlimages-2.2.0-5.src.rpm

(They are uploading now)

Changelog:
* Thu May 03 2007 Nigel Jones <dev@nigelj.com> 2.2.0-5
- Revert -4 changes
- Remove excludedirs patch, replace with a sed
- Provide html documentation generated from running ocaml-ocamldoc

* Thu Apr 26 2007 Nigel Jones <dev@nigelj.com> 2.2.0-4
- Add Provides: camlimages-static, and LICENSE to -devel docs

* Thu Apr 12 2007 Nigel Jones <dev@nigelj.com> 2.2.0-3
- Remove .a & .o files

Some rationale:

- Users don't care about the library references unless they are a developer, so
I've placed them in -devel
- I don't think it's worth adding a seperate -doc package (it's only 90KB)
- rpmlint ocaml-naming-policy-not-applied should be ignored (it's actually
ignored on F7, but showing on FC6
- The excludelibs patch was a waste of time, the sed is much better
Comment 5 Nigel Jones 2007-05-03 05:48:37 EDT
Spec URL: http://dev.nigelj.com/SRPMS/camlimages.spec
SRPM URL: http://dev.nigelj.com/SRPMS/camlimages-2.2.0-6.src.rpm

Just including *.*a instead of deleting, per Debian's suggestion, uploading as I
type.
Comment 6 Hans de Goede 2007-05-03 08:52:30 EDT
MUST:
=====
0 rpmlint output is:
W: camlimages ocaml-naming-policy-not-applied
/usr/lib/ocaml/camlimages/dllci_freetype.so
W: camlimages-devel ocaml-naming-policy-not-applied
/usr/lib/ocaml/camlimages/Makefile.config
* Package and spec file named appropriately
* Packaged according to packaging guidelines
* License ok
* spec file is legible and in Am. English.
* Source matches upstream
* Compiles and builds on devel i386
* BR: ok
* No locales
0 No shared libraries, ldconfig not needed
* Not relocatable
* Package owns / or requires all dirs
* No duplicate files & Permissions ok
* %clean & macro usage OK
* Contains code only
* %doc does not affect runtime, and isn't large enough to warrent a sub package
* -devel package as needed
* no .desktop file needed

MUST FIX
========
* rename to ocaml-images or ocaml-camlimages (will fix rpmlint warning, and the
  latter is consistent with PLD)
* main package must require ocaml for /usr/lib/ocaml dir ownership
* remove the unnecessary ldconfig %post(un) scripts, this are not normally
  libs and since they are not installed in a path searched by ldconfig, calling
  ldconfig is useless.
* The .so files should be installed under /usr/lib/ocaml/stublibs


Should Fix
==========
* Please just list all the features one after the other seperated with ',', I 
  don't think all rpm (gui) tools will preserve your formatting, so better to
  not format at all.

* Stop the obfuscated double %setup usage, instead of the 2 lines you can just
write: "%setup -q -n camlimages-2.2 -a 1"

* Once the .so files are under %{_libdir}/ocaml/stublibs, the
  files for %devel can be written as just "%{_libdir}/ocaml/camlimages/"

* also please always make all your %files entries like this:
  %files
  %defattr(....)
  %doc ...
  <other files and dirs>

Comment 7 Nigel Jones 2007-05-03 18:29:15 EDT
(In reply to comment #6)
> MUST:
> =====
> 0 rpmlint output is:
> W: camlimages ocaml-naming-policy-not-applied
> /usr/lib/ocaml/camlimages/dllci_freetype.so
> W: camlimages-devel ocaml-naming-policy-not-applied
> /usr/lib/ocaml/camlimages/Makefile.config
Ignorable (it actually doesn't happen on a F7 box)
> MUST FIX
> ========
> * rename to ocaml-images or ocaml-camlimages (will fix rpmlint warning, and the
>   latter is consistent with PLD)
I tend to disagree, but I'm going to hold off uploading an updated spec, so I
can make a better judgement.
> * main package must require ocaml for /usr/lib/ocaml dir ownership
Done
> * remove the unnecessary ldconfig %post(un) scripts, this are not normally
>   libs and since they are not installed in a path searched by ldconfig, calling
>   ldconfig is useless.
Done
> * The .so files should be installed under /usr/lib/ocaml/stublibs
Done
> 
> 
> Should Fix
> ==========
> * Please just list all the features one after the other seperated with ',', I 
>   don't think all rpm (gui) tools will preserve your formatting, so better to
>   not format at all.
Done
> 
> * Stop the obfuscated double %setup usage, instead of the 2 lines you can just
> write: "%setup -q -n camlimages-2.2 -a 1"
Per ocaml-SDL review, I had trouble getting to work, but done
> 
> * Once the .so files are under %{_libdir}/ocaml/stublibs, the
>   files for %devel can be written as just "%{_libdir}/ocaml/camlimages/"
Done
> 
> * also please always make all your %files entries like this:
>   %files
>   %defattr(....)
>   %doc ...
>   <other files and dirs>
Done
Comment 8 Nigel Jones 2007-05-04 01:52:19 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > MUST:
> > =====
> > 0 rpmlint output is:
> > W: camlimages ocaml-naming-policy-not-applied
> > /usr/lib/ocaml/camlimages/dllci_freetype.so
> > W: camlimages-devel ocaml-naming-policy-not-applied
> > /usr/lib/ocaml/camlimages/Makefile.config
> Ignorable (it actually doesn't happen on a F7 box)
> > MUST FIX
> > ========
> > * rename to ocaml-images or ocaml-camlimages (will fix rpmlint warning, and the
> >   latter is consistent with PLD)
> I tend to disagree, but I'm going to hold off uploading an updated spec, so I
> can make a better judgement.
Okay, I've decided to change... ocaml-camlimages it is.

Spec URL: http://dev.nigelj.com/SRPMS/ocaml-camlimages.spec
SRPM URL: http://dev.nigelj.com/SRPMS/ocaml-camlimages-2.2.0-7.src.rpm

* Fri May 04 2007 Nigel Jones <dev@nigelj.com> 2.2.0-7
- Change to Makefile patch to move .so files to stublibs
- Rename to ocaml-camlimages
- Other changes per review
Comment 9 Hans de Goede 2007-05-04 03:17:44 EDT
All Must and Should fix items addressed, approved.
Comment 10 Nigel Jones 2007-05-04 03:36:21 EDT
(In reply to comment #9)
> All Must and Should fix items addressed, approved.

Thank you,

New Package CVS Request
=======================
Package Name:      ocaml-camlimages
Short Description: OCaml image processing library
Owners:            dev@nigelj.com
Branches:          FC-6 devel
InitialCC:         <empty>
Comment 11 Dennis Gilmore 2007-05-05 11:58:50 EDT
cvs done
Comment 12 Nigel Jones 2007-05-08 23:18:42 EDT
Excluding arch ppc64 so it will build in F7 (already built on FC6), for those
interested, the new bug is Bug #239518

Closing...

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