Bug 477313 - Review Request: ocaml-preludeml - OCaml utility functions
Summary: Review Request: ocaml-preludeml - OCaml utility functions
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2008-12-20 12:33 UTC by Richard W.M. Jones
Modified: 2009-01-16 23:44 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2009-01-15 14:25:14 UTC
Type: ---
j: fedora-review+
kevin: fedora-cvs+

Attachments (Terms of Use)
Parallel mandlebrot set (1.02 KB, text/plain)
2008-12-20 13:09 UTC, Richard W.M. Jones
no flags Details

Description Richard W.M. Jones 2008-12-20 12:33:22 UTC
Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml-0.1-0.1.20080922.fc11.src.rpm
Prelude.ml is a collection of utility functions for OCaml programs.
Of particular interest are high level parallel combinators, which make
multicore parallelization of OCaml programs easy.

(Note that despite the "0.1" version number, this package is very
mature - it just doesn't have any upstream releases).

Koji scratch build:

rpmlint is silent.

Comment 1 Richard W.M. Jones 2008-12-20 12:58:26 UTC
Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml.spec

Just a tiny change to the META file.

Comment 2 Richard W.M. Jones 2008-12-20 13:09:07 UTC
Created attachment 327532 [details]
Parallel mandlebrot set

An auto-parallelizing mandlebrot set program, which
uses this library.

Adapted from:

Compile using:
  ocamlfind opt -package preludeml -linkpkg mandlebrot.ml -o mandlebrot

Run using:
  ./mandlebrot 5000 test.pgm
  eog test.pgm
(It will auto-detect the number of cores in your system and spread
the work across them.)

Comment 3 Jason Tibbitts 2008-12-21 20:47:12 UTC
You should prod upstream to get licensing information into all of the source files, not just the top level one.

There is no need to extract the license text like that.  In fact, my understanding is that you shouldn't do so, especially since you insist on needlessly duplicating this generated license file between the two subpackages.  I'll ask the licensing experts for clarification.

* source files match upstream (compared manually).
* 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.
* 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(Prelude) = bc560abdad61c8ad43c135a42a46cab8
   ocaml-preludeml = 0.1-0.2.20080922.fc11
   ocaml-preludeml(x86-64) = 0.1-0.2.20080922.fc11
   ocaml(Array) = 9c9fa5f11e2d6992c427dde4d1168489
   ocaml(Bigarray) = fc2b6c88ffd318b9f111abe46ba99902
   ocaml(Buffer) = 23af67395823b652b807c4ae0b581211
   ocaml(Complex) = 73899d718b62e5534e8737bb363dbf71
   ocaml(Filename) = 7cd172f02b7ee9b8d7bda3bb92144951
   ocaml(Hashtbl) = ee2a3220e38a4350c5bc131ce9f3f6ce
   ocaml(Int32) = b2545c419b6b6a173cac4c0a3e7e0277
   ocaml(Int64) = d501d6e89fdce41c79f274fb464995d5
   ocaml(Lexing) = 4d17267334f1a6c75730dc3fae21fb9b
   ocaml(List) = a0e2e49d266ff302f8667651a43f71ba
   ocaml(Map) = d6ea0139afe59a16df7b23d35e571de7
   ocaml(Marshal) = 02be0525cda0ca38ef8d49584e7769d6
   ocaml(Nativeint) = 7233ce5207a538fea4f0c61ed411ea2c
   ocaml(Netbuffer) = b2846209efee896cd2574779d0d1d42d
   ocaml(Netchannels) = b370fbc569ce2ec79307482cf4263a41
   ocaml(Netdate) = 30913487673f7f8869f808f49a7fcadd
   ocaml(Obj) = c827f726ce05da709cf7de58fc15e324
   ocaml(Pcre) = 4fd7bbc136a5cb4f7df9321ec6092b80
   ocaml(Pervasives) = 88cb1505c8bdf9a4dcd2cdf3452732b4
   ocaml(Printf) = 807ecd3a1538992580464c03462c9964
   ocaml(Random) = 462fc826fd1ae9df8d15e3cb798cba9d
   ocaml(String) = ecc403546c1c50056801131811c39017
   ocaml(Sys) = 21bf525b2b3f3a46a54b96163adfe387
   ocaml(Unix) = 0596a58544f8cd88fed5bf5432a53d43
   ocaml(runtime) = 3.11.0

   ocaml-preludeml-devel = 0.1-0.2.20080922.fc11
   ocaml-preludeml-devel(x86-64) = 0.1-0.2.20080922.fc11
   ocaml-preludeml = 0.1-0.2.20080922.fc11

* owns the directories it creates.
* doesn't own any directories it shouldn't.
O license file is duplicated; it should not be, but this is not a blocker.
* 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 and .ml files not included
* this is a syntax extension, so the .o file os OK in the main package.

Comment 4 Richard W.M. Jones 2008-12-21 22:08:11 UTC
Do you think I should just not add the src/* files to the package?
They are not (currently) necessary to build anyway.  In fact my personal
opinion is that the split is ill-advised.

I'll raise the issue of licensing upstream now anyway.

Comment 5 Richard W.M. Jones 2008-12-22 14:55:43 UTC
Upstream have added a LICENSE file and also added licensing
info to all of the src/* files.

I'm just starting a 2 week vacation, so I will have only irregular
contact until the beginning of January.

Comment 6 Richard W.M. Jones 2008-12-22 14:56:24 UTC
Set to NEEDINFO of me, to look at when I get back.

Comment 7 Richard W.M. Jones 2009-01-13 13:33:39 UTC
Here's an updated version which includes the license
information and bumps everything to the latest version:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml-0.1-0.4.20090113.fc11.src.rpm
* Tue Jan 13 2009 Richard W.M. Jones <rjones> - 0.1-0.4.20090113
- New upstream version 20090113, resolving licensing issues.
- Use omake to build.
- Added a check section to run automated tests.

Comment 8 Richard W.M. Jones 2009-01-13 14:30:36 UTC
An even more updated version that runs the automated
tests and builds in Koji:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-preludeml-0.1-0.10.20090113.fc11.src.rpm
* Tue Jan 13 2009 Richard W.M. Jones <rjones> - 0.1-0.10.20090113
- Tests require ounit & ruby.

Koji scratch build:

Comment 9 Jason Tibbitts 2009-01-14 01:35:48 UTC
OK, this one still builds for me, and I guess I'm lucky to be building on the one platform where the tests pass, because I get:
  Ran: 333 tests in: 0.72 seconds.

I don't see anything in particular to complain about, so:


Comment 10 Richard W.M. Jones 2009-01-14 09:14:22 UTC
(In reply to comment #9)
> OK, this one still builds for me, and I guess I'm lucky to be building on the
> one platform where the tests pass, because I get:
>   Ran: 333 tests in: 0.72 seconds.
>   OK

Yes, the tests only work on x86-64.

1/333 fails on 32 bit platforms, and lots of things fail on ppc64.
I'm going to raise the issue with upstream.

Thanks for your patience looking at this package.

Comment 11 Richard W.M. Jones 2009-01-14 09:16:08 UTC
New Package CVS Request
Package Name: ocaml-preludeml
Short Description: OCaml utility functions
Owners: rjones
Branches: F-10

Comment 12 Kevin Fenzi 2009-01-14 21:24:38 UTC
cvs done.

Comment 13 Fedora Update System 2009-01-15 14:24:37 UTC
ocaml-preludeml-0.1-0.10.20090113.fc10 has been submitted as an update for Fedora 10.

Comment 14 Richard W.M. Jones 2009-01-15 14:25:14 UTC
Thanks everyone, this is now built in F-10 and Rawhide.

Comment 15 Fedora Update System 2009-01-16 23:44:03 UTC
ocaml-preludeml-0.1-0.10.20090113.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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