Bug 253571

Summary:	Review Request: ocaml-fileutils - OCaml library for common file and filename operations
Product:	[Fedora] Fedora	Reporter:	Richard W.M. Jones <rjones>
Component:	Package Review	Assignee:	Jason Tibbitts <j>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora-package-review, notting
Target Milestone:	---	Flags:	j: fedora-review+ kevin: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2008-02-22 18:43:08 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Richard W.M. Jones 2007-08-20 16:53:27 UTC

Spec URL: http://annexia.org/tmp/ocaml/ocaml-fileutils.spec
SRPM URL: http://annexia.org/tmp/ocaml/ocaml-fileutils-0.3.0-1.fc8.src.rpm
Description:
This library is intended to provide a basic interface to the most
common file and filename operation. It provides different filename
function : reduce, make_absolute, make_relative... It also enables to
manipulate real file : cp, mv, rm, touch...

It is separated in two modules : SysUtil and SysPath. The first one
manipulate files ( real one ), the second one is made for manipulating
abstract filename.

Comment 1 Jason Tibbitts 2008-01-27 06:11:46 UTC

This fails to build for me, again missing camlp4.  Did something change in
rawhide which breaks these packages?  I added the ocaml-camlp4-devel dependency
and things build OK.  rpmlint says:

  ocaml-fileutils.src:51: W: rpm-buildroot-usage %prep 
   --with-builddir=$RPM_BUILD_ROOT-tmp
Nasty.  I see the comment indicating why you needed to do this, but I think it
would be preferable for you to stay within the confines of the directory
rpmbuild has set up for you ($RPM_BUILD_ROOT) as you have no guarantee that you
can make another subdirectory of the parent directory.

  ocaml-fileutils.x86_64: E: no-binary
  ocaml-fileutils.x86_64: E: only-non-binary-in-usr-lib
I'm not really sure why rpmlint doesn't think a file full of stuff that
certainly isn't text is somehow not binary.  This can be ignored.

Comment 2 Richard W.M. Jones 2008-02-12 12:49:08 UTC

Yes I agree.  I've changed it so that it builds into a subdirectory
('tmp/') under the build directory.  Here is an updated SRPM:

Spec URL: http://www.annexia.org/tmp/ocaml-fileutils-0.3.0-2.fc8.src.rpm
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-fileutils.spec

This also adds the missing build dep on ocaml-camlp4-devel.

Comment 3 Richard W.M. Jones 2008-02-12 12:49:30 UTC

Right URLs this time ...

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-fileutils-0.3.0-2.fc8.src.rpm
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-fileutils.spec

Comment 4 Jason Tibbitts 2008-02-16 22:19:59 UTC

I figured this wouldn't build because of the ocaml update, but it turns out
that it builds fine.  rpmlint has only the two weird complaints above; someone
who understands what ocaml is doing really needs to get with the rpmlint
maintainer and figure out the source of these complaints.  Maybe it just
doesn't like the fact that the only thing directly in _libdir is a directory.

There's a test suite there; I tried running it but it seems to need the
package to be installed before it works, and I don't know if it's possible to
adjust the various search paths to pick up the files from the build directory.

There are several grammatical errors in the description which should be easy
to fix up.

I had hoped to find a better upstream URL than one pointing to a list of files
for download, but the wiki that upstream has doesn't seem to have any actual
content.

As far as I can tell, the License is LGPLv2+ plus some type of exception, so I
guess this is "LGPLv2+ with exceptions".  In addition, the COPYING file needs
to be added to the package.  You should probably add AUTHOR, CHANGELOG and
TODO as well.  I'm not sure about the stuff in the "website" directory.

I can't think of any better way to handle the broken build system of this
package, and you've adequately commented things, so that's OK.

The README file is duplicated between packages, but we already decided I
wouldn't block on this.

* source files match upstream:
   4b3929f758dac11bebdcbb84963ab44e2a5ebf06023665dd511d61b19e9654b8
   ocaml-fileutils-0.3.0.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* Follows ocaml packaging guidelines
* summary is OK.
X description could use some grammar fixes.
* dist tag is present.
* build root is OK.
X license field does not match the actual license.
* license is open source-compatible.
X license text should be included in the package.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* rpmlint seems to have only bogus complaints.
* final provides and requires are sane:
  ocaml-fileutils-0.3.0-2.fc9.x86_64.rpm
   ocaml(CygwinPath) = 55504b4f452dc9b72bcd7564a4c5a991
   ocaml(CygwinPath_lexer) = 29edb3b5196b78e256fdcbafa566486b
   ocaml(CygwinPath_parser) = d7c4325a803605bd4737615702dcb060
   ocaml(FilePath) = 80389f0c61bde05b590de9a464aa1831
   ocaml(FilePath_type) = b20c6b0e73bf51cf8d25a2f256443dfc
   ocaml(FileUtil) = bd8589f4d97317061dd24d80db6eeacc
   ocaml(GenericPath_lexer) = d564536f943cee33d33f51880f85406d
   ocaml(GenericPath_parser) = 177c3bf79cd035b170e1d2efce3a36ff
   ocaml(MacOSPath) = 6eecbacbd1f32d7cbddb7f1e269358b7
   ocaml(MacOSPath_lexer) = 473b7b6c310c611393a88e6922dfc897
   ocaml(MacOSPath_parser) = 323e664f9989fd08de7097f80bc34995
   ocaml(Str) = 56bb7ee61b2da83d42394686e3558fe4
   ocaml(Unix) = 9a46a8db115947409e54686ada118599
   ocaml(UnixLabels) = daaededf8793acd8b374c4e7c5d1fd16
   ocaml(UnixPath) = 1538fbea534e0ac1e7f3a48c81330a6c
   ocaml(UnixPath_lexer) = 52138249abfd7fd19243bae955fec96a
   ocaml(UnixPath_parser) = 34b22f7becddd052a253243d516b2685
   ocaml(Win32Path) = ef3947425c94dcbbbb4e80a851401ab0
   ocaml(Win32Path_lexer) = 934bf4ebb71efb7a397e9fd8ce16a1b2
   ocaml(Win32Path_parser) = b55b3f0165af59f53b11c55fa4f2495d
   ocaml-fileutils = 0.3.0-2.fc9
  =
   ocaml(Array) = aa8e3cd5824f9bb40b93fcd38d0c95b5
   ocaml(Buffer) = f6cef633ea14963b84b79c4095c63dc3
   ocaml(Callback) = e5ca1fb5990fac2b7b17cbb1712cffe2
   ocaml(Char) = e98bc9c9e918a84b3c1a5a122d42fac1
   ocaml(Hashtbl) = 083f2c94b44ff4e0b3220aaea6a783b4
   ocaml(Lexing) = b1793496643444d3762dd42bebe2cfe3
   ocaml(List) = da1ce9168f0408ff26158af757456948
   ocaml(Map) = dedde7683d54ae7db1eb97cc868dd047
   ocaml(Obj) = 5cfae708052c692ea39d23ed930fd64d
   ocaml(Parsing) = 62cca107e4e88af303516459a87c3e9a
   ocaml(Pervasives) = 8ba3d1faa24d659525c9025f41fd0c57
   ocaml(Printf) = 5dbbf45a03b54e6dbfcf39178d0d6341
   ocaml(Set) = 7da14e671a035f12386ace3890018ef3
   ocaml(Stream) = 21a833e12efd34ea0c87d8d9da959809
   ocaml(String) = 2c162ab314b2f0a2cfd22d471b2e21ab
   ocaml(Sys) = 0da495f5a80f31899139359805318f28
   ocaml(runtime) = 3.10.1

  ocaml-fileutils-devel-0.3.0-2.fc9.x86_64.rpm
   ocaml-fileutils-devel = 0.3.0-2.fc9
  =
   ocaml-fileutils = 0.3.0-2.fc9

* %check is not present; upstream test suite cannot easily be run.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
? README file is duplicated.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Comment 5 Richard W.M. Jones 2008-02-21 12:52:45 UTC

Here's an updated version:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-fileutils.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-fileutils-0.3.0-3.fc9.src.rpm

* Thu Feb 21 2008 Richard W.M. Jones <rjones> - 0.3.0-3
- Fixed grammar in the description section.
- License is LGPLv2 with exceptions
- Include license file with both RPMs.
- Include other documentation only in the -devel RPM.

BTW, my reading of the license is that it's LGPLv2 only (not "or later ...").
Am I mistaken?

Comment 6 Richard W.M. Jones 2008-02-21 13:55:10 UTC

About rpmlint & warnings, see bug 433783.

Comment 7 Jason Tibbitts 2008-02-21 21:33:46 UTC

Well, the COPYING file says:

The Library is distributed under the terms of the GNU Library General
Public License version 2 (found in /usr/share/common-licenses/LGPL-2
on debian systems).

but all of the files in the libfileutils-ocaml directory say:

(*   This program is free software; you can redistribute it and/or        *)
(*   modify it under the terms of the GNU Library General Public          *)
(*   License as published by the Free Software Foundation; either         *)
(*   version 2 of the License, or any later version ; with the OCaml      *)
(*   static compilation exception.                                        *)

and the statements on the code always win unless, of course, there is an issue
with combining them which would somehow limit the license.

Of course, someone should introduce a clue to upstream about this, but in the
meantime it really does look like LGPLv2+ to me.

Honestly the issue of a plus or not isn't something worth holding this package
up over; you can fix it when you check in.

APPROVED

Comment 8 Richard W.M. Jones 2008-02-21 22:31:00 UTC

New Package CVS Request
=======================
Package Name: ocaml-fileutils
Short Description: OCaml library for common file and filename operations
Owners: rjones
Branches: F-8
InitialCC: rjones
Cvsextras Commits: yes

Comment 9 Kevin Fenzi 2008-02-22 17:58:53 UTC

cvs done.

Comment 10 Richard W.M. Jones 2008-02-22 18:43:08 UTC

I discussed license with upstream and he says he's going to resolve
it in the tarball soon.

In the meantime, I'm building it in Fedora 8 and Rawhide.

Comment 11 Jason Tibbitts 2008-02-22 21:40:12 UTC

What did upstream say the actual license is supposed to be?  If a new upstream
release isn't coming soon then it might be nice to include the email with the
license clarification in the package as %doc.

Comment 12 Richard W.M. Jones 2008-02-23 14:14:32 UTC

He didn't, it's still existing in a quantum state of LGPLv2 or LGPLv2+ :-)
However Sylvain did put it on his buglist to clarify it ...

http://le-gall.net/sylvain+violaine/bts/?do=details&id=55