Bug 460732 - Review Request: ocaml-reins - Library of OCaml persistent data structures
Summary: Review Request: ocaml-reins - Library of OCaml persistent data structures
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-08-30 14:50 UTC by Richard W.M. Jones
Modified: 2008-09-05 19:49 UTC (History)
3 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-09-05 19:49:47 UTC
Type: ---
Embargoed:
j: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Richard W.M. Jones 2008-08-30 14:50:14 UTC
Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-reins.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-reins-0.1-0.1.a.fc10.src.rpm
Description: Library of OCaml persistent data structures

This is a very simple package -- just a library containing a very
big collection of purely functional data structures.

rpmlint has this to say about the package:

  ocaml-reins.src:89: W: rpm-buildroot-usage %build export OCAMLFIND_DESTDIR=$RPM_BUILD_ROOT%{_libdir}/ocaml
  $RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it will
  break short circuiting.

I can't see a way to build this package without setting OCAMLFIND_DESTDIR
in the %build stage.  The package seems to stash this somewhere, and just
setting it later during %install doesn't work.  I don't know what
"short circuiting" is and whether it's a problem that we break it.
  
  ocaml-reins.i386: E: no-binary
  The package should be of the noarch architecture because it doesn't contain
  any binaries.

This is a well-known problem with OCaml packages and can be ignored
(for now).  RPM are promising to add a feature which will allow us
to build this as noarch, but it's not there yet.

Files in the main package:

/usr/lib/ocaml/reins
/usr/lib/ocaml/reins/META
/usr/lib/ocaml/reins/reins.cma
/usr/lib/ocaml/reins/reins.cmi
/usr/share/doc/ocaml-reins-0.1
/usr/share/doc/ocaml-reins-0.1/COPYING
/usr/share/doc/ocaml-reins-0.1/LGPL-2.1

Requires:

rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(VersionedDependencies) <= 3.0.3-1
ocaml(Big_int) = 992d682669507b99e689b5a2188c0b9a
ocaml(Buffer) = f6cef633ea14963b84b79c4095c63dc3
ocaml(Char) = e98bc9c9e918a84b3c1a5a122d42fac1
ocaml(Complex) = bb333e8e4cda78107ccf27048ca40492
ocaml(Hashtbl) = 083f2c94b44ff4e0b3220aaea6a783b4
ocaml(Int32) = 711321870c949bd3bbdd092d9bae92e4
ocaml(Int64) = f8f7e2e4c0667ead94596040b12e732d
ocaml(Lazy) = 8a4b5e7f0bdc6316df9264fd73cde981
ocaml(List) = da1ce9168f0408ff26158af757456948
ocaml(Map) = dedde7683d54ae7db1eb97cc868dd047
ocaml(Nat) = 0ea20dd1cc4533fd519b5542a89feb87
ocaml(Nativeint) = e79cdc4d3575c2ed044955cb7ef49aca
ocaml(Obj) = 5cfae708052c692ea39d23ed930fd64d
ocaml(Pervasives) = 8ba3d1faa24d659525c9025f41fd0c57
ocaml(Printexc) = 82717999a586ede6925c0aa18d6562ac
ocaml(Printf) = 5dbbf45a03b54e6dbfcf39178d0d6341
ocaml(Random) = 9936935480b36bcbc716ee513f37876c
ocaml(Ratio) = 7067125cce206dd2bbe93918ba7bdfe9
ocaml(String) = 2c162ab314b2f0a2cfd22d471b2e21ab
ocaml(Sys) = 0da495f5a80f31899139359805318f28
ocaml(Unix) = 9a46a8db115947409e54686ada118599
ocaml(runtime) = 3.10.2

Provides:

ocaml(Reins) = 80e0049b95f5867ce642993dddbe7e76
ocaml-reins = 0.1-0.1.a.fc10
ocaml-reins(x86-32) = 0.1-0.1.a.fc10

Comment 1 Jason Tibbitts 2008-09-03 03:36:15 UTC
Short-circuiting is simply calling rpmbuild with --short-circuit.  It lets you skip, say, directly to %build or %install without running %prep or %prep/%build, respectively, letting you tweak the package and test without having to sit through those potentially long steps.

I think the point of the rpmlint warning is to remind you not to write to the buildroot during %prep and %build.  This package doesn't even read from it.  A problem might arise if the buildroot isn't constant between rpmbuild calls, such as when mktemp is called as part of buildroot determination, but that's not the case here.  I can't think of a case with a constant buildroot which is neither being read nor written to which would cause a problem with short curcuiting.

BTW, the package stashes the destdir location in config.omake.omc, which is binary for some reason.  I hope this kind of thing doesn't catch on.

In any case, I don't think this is a significant issue, since I can't come up with any way that it hurts anything and outside of patching some binary file there seems to be no way around it.

Would you classify the version "0.1a" as an update from "0.1", or something which will later be released as "0.1"?  Because you've versioned things as the latter, but it seems to me that it should be the former.  I'd suggest
  Version: 0.1a
  Release: 1
unless you somehow don't trust upstream to keep a reasonable ordering.  See http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Post-Release_packages for more information.

Comment 2 Richard W.M. Jones 2008-09-03 07:46:01 UTC
I've asked upstream for a comment on versioning and future
releases.

Comment 3 Jason Tibbitts 2008-09-03 13:11:27 UTC
If you check the download area you'll see that 0.1 was released before 0.1a, strongly implying 0.1a is an update to 0.1.  Unless you really want to wait for upstream, I suggest just using the usual versioning scheme.

Comment 4 Richard W.M. Jones 2008-09-03 13:37:58 UTC
OK this is the package with normal versioning restored:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-reins.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-reins-0.1a-2.fc10.src.rpm

Comment 5 Jason Tibbitts 2008-09-03 23:27:12 UTC
* source files match upstream:
   3f3fa0ac27d35abc4000184733f2a8deabd2f87310078fc343951ecb37d15a76  
   ocaml-reins-0.1a.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints (as discussed above).
* final provides and requires are sane (as above).
* %check is not present, but a test suite runs automatically at build time:
  Ran: 642 tests in: 2.54 seconds.
  OK
  *** All tests passed ***

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* .cma, .cmi, .so, .so.owner, META files in the main package.
* .a, .cmxa, .cmx and .mli files are in the -devel subpackage.
* .cmo, .o and .ml files not included

APPROVED

Comment 6 Richard W.M. Jones 2008-09-04 08:50:13 UTC
New Package CVS Request
=======================
Package Name: ocaml-reins
Short Description: Library of OCaml persistent data structures
Owners: rjones
Branches: F-9
InitialCC: rjones

Comment 7 Kevin Fenzi 2008-09-05 17:00:00 UTC
cvs done.

Comment 8 Richard W.M. Jones 2008-09-05 19:08:45 UTC
Kevin, I know that you never sleep which must be the only
explanation for your extraordinary output, but I wonder if
the F-9 branch is missing from CVS?  (Or I might have just
made a mistake).

New Package CVS Request
=======================
Package Name: ocaml-reins
Short Description: Library of OCaml persistent data structures
Owners: rjones
Branches: F-9
InitialCC: rjones

Comment 9 Jason Tibbitts 2008-09-05 19:32:16 UTC
Should be fixed now.

Comment 10 Richard W.M. Jones 2008-09-05 19:40:33 UTC
Thank you.


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