Bug 445068 (ocaml-bin-prot)

Summary:	Review Request: ocaml-bin-prot - Read and write OCaml values in a type-safe binary protocol
Product:	[Fedora] Fedora	Reporter:	Richard W.M. Jones <rjones>
Component:	Package Review	Assignee:	Jason Tibbitts <j>
Status:	CLOSED ERRATA	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
URL:	http://www.ocaml.info/home/ocaml_sources.html#bin-prot
Whiteboard:
Fixed In Version:	1.2.21-1.fc12.1	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2010-01-12 23:40:32 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:
Bug Depends On:	445067
Bug Blocks:	445074, 528454

Description Richard W.M. Jones 2008-05-03 06:55:03 UTC

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot-1.0.5-1.fc9.src.rpm
Description: Read and write OCaml values in a type-safe binary protocol

rpmlint says:

ocaml-bin-prot.i386: W: devel-file-in-non-devel-package /usr/lib/ocaml/bin_prot/map_to_safe.ml

This is OK because of the exception for *.ml files detailed here:
https://fedoraproject.org/wiki/Packaging/OCaml#head-f1fbee40338b95c780f241fa3a86f6a82e5830dd
(ie. there is no corresponding map_to_safe.mli file).

Note that this package is only available on little-endian
architectures, and is only tested upstream on x86, hence the
ExclusiveArch.

Comment 1 Richard W.M. Jones 2008-05-12 20:01:50 UTC

My phony explanation for 'W: devel-file-in-non-devel-package' in the
previous comment is completely wrong!  This file _is_ allowed, but is in
the wrong subpackage.

The one is rpmlint clean.

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot-1.0.5-2.fc9.src.rpm

* Mon May 12 2008 Richard W.M. Jones <rjones> - 1.0.5-2
- Remove ExclusiveArch, but add a Fedora README file warning about
  shortcomings on non-x86 architectures.
- Added missing dependency ocaml-type-conv.
- Move *.ml file to devel package.

Comment 2 Richard W.M. Jones 2008-05-13 12:56:25 UTC

Hmm, this is failing to build on PPC & PPC64.  Changed to NEEDINFO of
me to look into this.

Comment 3 Richard W.M. Jones 2008-05-16 10:14:33 UTC

Got a little bit further, now it fails the unit tests on ppc:

http://koji.fedoraproject.org/koji/getfile?taskID=612162&name=build.log

Since I don't have access to a working ppc machine, raised with
upstream to see if they know anything.

Comment 4 Jason Tibbitts 2008-06-27 18:58:50 UTC

Any progress here?

Comment 5 Richard W.M. Jones 2008-06-28 14:55:49 UTC

I'm very busy at the moment because my house is having some serious
building work done.  It'll be a few weeks before I get back on top of
this and other reviews.

Comment 6 Jason Tibbitts 2008-06-29 20:17:50 UTC

No problem; just clear the whiteboard when you're ready for a review.

Comment 7 Richard W.M. Jones 2009-09-05 18:18:57 UTC

New upstream version has appeared.  This one is also rpmlint clean:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot-1.2.18-1.fc11.src.rpm

Comment 8 Richard W.M. Jones 2009-10-12 10:26:10 UTC

New upstream version:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot-1.2.20-1.fc11.src.rpm

Comment 9 Richard W.M. Jones 2009-10-12 11:08:43 UTC

Koji scratch-build of the new version:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1741769

Comment 10 Jason Tibbitts 2009-11-07 15:10:17 UTC

OK, I'm going to try for a third time to review this.  The first time, firefox crashed and I lost the review; the second time, the USB controller on my motherboard fried and I had no keyboard with which to complete the review.  Not a good couple of days for me.

Comment 11 Jason Tibbitts 2009-11-07 17:15:37 UTC

OK, this builds fine and rpmlint is silent

In multiple licensing scenarios, you'll need to add some indication of which files are under which license.  The easiest way is to just add a comment to the spec.  http://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios

You've duplicated the license files between -devel and the main package.  This is clearly against the guidelines
  http://fedoraproject.org/wiki/Packaging:Guidelines#Duplicate_Files
but I know we've gone back and forth on this so I've requested that we get an opinion from legal and get the guidelines modified if necessary.

The -devel package includes a .ml file; the OCaml guidelines say that these shouldn't be  included except in specific circumstances.  I can't determine from the information I have whether or not those circumstances apply here, and there's no comment in the spec about it.

* source files match upstream.  sha256sum:
   1a8569a75195b35ec17348046b3aa8823f9f27e451214717692a86075cfbdb9b
   release-1.2.20.tar.bz2
* 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 is silent.
* final provides and requires are sane:
  ocaml-bin-prot-1.2.20-1.fc12.x86_64.rpm
   dllbin_prot_stubs.so()(64bit)            
   ocaml(Bin_prot) = 68b4c083005ed9d5cde0767b0e6a9782
   ocaml(Pa_bin_prot) = d894b67ca8604bc43a9721e774c08303
   ocaml-bin-prot = 1.2.20-1.fc12                       
   ocaml-bin-prot(x86-64) = 1.2.20-1.fc12               
  =                                                  
   ocaml(Arg) = b6513be035dc9c8a458c189cd8841700        
   ocaml(Array) = 9c9fa5f11e2d6992c427dde4d1168489      
   ocaml(Bigarray) = fc2b6c88ffd318b9f111abe46ba99902   
   ocaml(Buffer) = 23af67395823b652b807c4ae0b581211     
   ocaml(Callback) = 71e1f9b7f211661f1dfeedab5ffae0cc   
   ocaml(CamlinternalLazy) = ed280fb9736e9200aa47db73c5ff077f
   ocaml(Camlp4) = bb930f7c2bed5d057c794fe07dc8596a          
   ocaml(Camlp4_config) = 80b5d58834366711574a5ec4dfb123fd   
   ocaml(Camlp4_import) = 4d17b58763ba1f0aac92fd5dbb558b59   
   ocaml(Char) = 3da72249626c7db769beafc97036cb4f
   ocaml(Complex) = 73899d718b62e5534e8737bb363dbf71
   ocaml(Filename) = 7cd172f02b7ee9b8d7bda3bb92144951
   ocaml(Format) = b7ba3152a5eec5609d6ab86e6c51eebb
   ocaml(Hashtbl) = ee2a3220e38a4350c5bc131ce9f3f6ce
   ocaml(Int32) = b2545c419b6b6a173cac4c0a3e7e0277
   ocaml(Int64) = d501d6e89fdce41c79f274fb464995d5
   ocaml(Lazy) = 4c7ed568fa7b5f73a2aa02eeb0e5e12b
   ocaml(Lexing) = 4d17267334f1a6c75730dc3fae21fb9b
   ocaml(List) = a0e2e49d266ff302f8667651a43f71ba
   ocaml(Nativeint) = 7233ce5207a538fea4f0c61ed411ea2c
   ocaml(Obj) = c827f726ce05da709cf7de58fc15e324
   ocaml(Parsing) = 29c3f123280f8e6e639cfb025b3c9a3f
   ocaml(Pa_type_conv) = 917c39ac24d30438f1e78e6e58840e45
   ocaml(Pervasives) = 88cb1505c8bdf9a4dcd2cdf3452732b4
   ocaml(Printf) = 807ecd3a1538992580464c03462c9964
   ocaml(Queue) = 56b5e04dcda600ae0cdf49a37f17fcd9
   ocaml(Set) = c4be5d24d30c129dd60d2739e54db7dd
   ocaml(Stream) = 91a43ea7fb16bf36f3f10c0dc7d08a0e
   ocaml(String) = ecc403546c1c50056801131811c39017
   ocaml(Sys) = 21bf525b2b3f3a46a54b96163adfe387
   ocaml(Unix) = 0596a58544f8cd88fed5bf5432a53d43
   ocaml(runtime) = 3.11.1

  ocaml-bin-prot-devel-1.2.20-1.fc12.x86_64.rpm
   ocaml-bin-prot-devel = 1.2.20-1.fc12
   ocaml-bin-prot-devel(x86-64) = 1.2.20-1.fc12
  =
   ocaml-bin-prot = 1.2.20-1.fc12

* %check is present and all tests pass:
   Ran: 67 tests in: 0.35 seconds.
   OK

   msgs: 100000  msg length: 461
   write time: 2.548s  write rate:  39244.14 msgs/s  write throughput: 17.25 MB/s
   read time: 2.619s   read rate:  38181.64 msgs/s   read throughput: 16.79 MB/s

* owns the directories it creates.
* doesn't own any directories it shouldn't.
X license files are duplicated.
* 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.
? a .ml file is included
* .so files have no rpath

Comment 12 Richard W.M. Jones 2009-11-09 10:03:45 UTC

I looked at the actual code and what the author has done is
to take an earlier BSD licensed program ("tywith") and has
substantially derived from and rewritten that code to produce
this package.

The new package is entirely licensed under LGPLv2+ with
the standard OCaml linking exception.

In all the files that were derived from tywith there is an
acknowledgement of the earlier provenance of the code,
but there are no separate files that are still BSD licensed.

Therefore I have changed the license to:

License:        LGPLv2+ with exceptions

I made the other changes as you suggested.

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-bin-prot-1.2.20-2.fc11.src.rpm  

* Mon Nov  9 2009 Richard W.M. Jones <rjones> - 1.2.20-2
- The final license of the code is LGPLv2+ with the OCaml linking
  exception.  It was derived from earlier BSD code.
- Don't duplicate the license files across base and -devel packages.
- Add note to spec about inclusion of *.ml file in -devel package.

Comment 13 Jason Tibbitts 2010-01-08 07:42:14 UTC

Not sure why I didn't see your reply, but here's mine, two months late.  Sorry about that.

Looks good to me.  APPROVED

Comment 14 Richard W.M. Jones 2010-01-08 09:39:24 UTC

No problem, thanks for looking at it.

Comment 15 Richard W.M. Jones 2010-01-08 09:40:31 UTC

New Package CVS Request
=======================
Package Name: ocaml-bin-prot
Short Description: Read and write OCaml values in a type-safe binary protocol
Owners: rjones
Branches: F-12
InitialCC:

Comment 16 Kevin Fenzi 2010-01-09 04:36:27 UTC

cvs done.

Comment 17 Richard W.M. Jones 2010-01-11 12:43:36 UTC

Added to F12 and Rawhide.

(I also bumped the version up to 1.2.21).

Rawhide: http://koji.fedoraproject.org/koji/taskinfo?taskID=1913644

Comment 18 Fedora Update System 2010-01-12 23:40:25 UTC

ocaml-bin-prot-1.2.21-1.fc12.1 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.