Bug 476622 - Review Request: ocaml-pa-do - OCaml syntax extension for delimited overloading
Review Request: ocaml-pa-do - OCaml syntax extension for delimited overloading
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2008-12-16 04:49 EST by Richard W.M. Jones
Modified: 2009-03-18 15:16 EDT (History)
2 users (show)

See Also:
Fixed In Version: 0.8.4-4.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2009-03-18 06:11:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)

  None (edit)
Description Richard W.M. Jones 2008-12-16 04:49:44 EST
Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8-1.fc10.src.rpm
Description: OCaml syntax extension for delimited overloading
Comment 1 Jason Tibbitts 2008-12-16 20:50:28 EST
This fails to build for me:

- build examples comp_diff.opt
+ ocamlopt.opt -warn-error A -dtypes -inline 3 -I . -I ../src -I +camlp4 -o 
   comp_diff.opt nums.cmxa graphics.cmxa comp_diff.cmx
/usr/bin/ld: cannot find -lX11
Comment 2 Richard W.M. Jones 2008-12-18 05:16:49 EST
Thanks for looking at this.  There are several build
failures which I've been tracking with upstream.  (The
one you found is a missing BR, but there are real build
failures later on - for some reason they only affect Koji
builds, not 'rpmbuild' on the command line).

So best to leave this bug alone until they have had a
chance to fix the build problems.
Comment 3 Jason Tibbitts 2008-12-18 13:12:48 EST
I'm not completely sure why this was submitted for review when you know it doesn't build, but I'll indicate that it can't be reviewed.  Please clear the whiteboard when this is ready for a review.
Comment 4 Richard W.M. Jones 2008-12-18 14:46:13 EST
Jason, this was just an oversight.  It does build if you use 'rpmbuild'.
It was when I was trying to make a scratch-build in Koji that I discovered
first the missing BR, then much more serious problems which I've been
trying since then to resolve with upstream.


I will clear the whiteboard when this is really ready.
Comment 5 Richard W.M. Jones 2008-12-20 09:40:29 EST
I don't know why, but this has been resolved upstream with
their latest version.

Koji scratch build:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.1-1.fc11.src.rpm

* Sat Dec 20 2008 Richard W.M. Jones <rjones@redhat.com> - 0.8.1-1
- New upstream release 0.8.1.
- Run omake with the --verbose option to try to debug Koji failures.
Comment 6 Richard W.M. Jones 2008-12-20 09:45:28 EST
rpmlint says:

ocaml-pa-do.x86_64: E: no-binary

which we think is OK for OCaml packages (bug 433783).
Comment 7 Jason Tibbitts 2008-12-20 18:26:26 EST
The tarball in this package doesn't match the one I get from https://forge.ocamlcore.org/frs/download.php/101/pa_do-0.8.1.tar.gz
Comment 8 Richard W.M. Jones 2008-12-20 19:09:13 EST
Oh, that's really strange.  What happens here, as far as I can tell
is the .../101/... in the URI is significant.   The purported filename
doesn't matter:

wget --no-check-certificate 'https://forge.ocamlcore.org/frs/download.php/101/furbie'
==> furbie size 99353, same as ocaml-pa-do 0.8

wget --no-check-certificate 'https://forge.ocamlcore.org/frs/download.php/102/fooble'
==> fooble size 102485, same as ocaml-pa-do 0.8.1

What I did was to just update the %{version} tag without bumping
this implicit "release number" in the Source URL.

The following SRPM should fix this, by correcting the source URL:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.1-2.fc11.src.rpm
Comment 9 Richard W.M. Jones 2009-01-13 08:46:48 EST
Here is a new upstream version:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.2-1.fc11.src.rpm

* Tue Jan 13 2009 Richard W.M. Jones <rjones@redhat.com> - 0.8.2-1
- New upstream version 0.8.2.

Koji scratch build:
Comment 10 Richard W.M. Jones 2009-02-08 05:00:56 EST
Another new upstream release:

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.3-2.fc11.src.rpm

* Sun Feb  8 2009 Richard W.M. Jones <rjones@redhat.com> - 0.8.3-2
- New upstream version 0.8.3.
- Missing BR pdflatex.

Koji scratch build:
Comment 11 Jason Tibbitts 2009-03-11 17:48:41 EDT
forge.ocamlcore.org's certificate is invalid (or at least self-signed); I recommend using http links so tools like spectool will work.

There's no separate %check section, but I think the tests are running during %build.  However, I'm not really sure how to read the output.  Are they being run, or just built?  Any hints for me?

It looks like 0.8.4 is out.  I doubt it will make much difference packaging wise so I'll go ahead and review this and you can update if you like.

The license file is duplicated; current guidelines forbid this.

* source files match upstream.  sha256sum:
* 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.
X 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(Pa_do) = 971be223d707310f2a458c64d3419d67
   ocaml(Pa_do_nums) = 091e99b411e6b077e65bf341508e57f2
   ocaml(Pa_do_top) = 5ec8bd77ec745470937cd44f0e4c37e2
   ocaml(Pa_infix) = 01c10d0e5d9d4982c560905f9590445e
   ocaml-pa-do = 0.8.3-2.fc11
   ocaml-pa-do(x86-64) = 0.8.3-2.fc11
   ocaml(Arg) = b6513be035dc9c8a458c189cd8841700
   ocaml(Array) = 9c9fa5f11e2d6992c427dde4d1168489
   ocaml(Big_int) = b094bddd70d11f4b8592f3957a8b3d9f
   ocaml(Buffer) = 23af67395823b652b807c4ae0b581211
   ocaml(CamlinternalLazy) = ed280fb9736e9200aa47db73c5ff077f
   ocaml(CamlinternalOO) = f83f268cd1a00c37180b9b1fb9306031
   ocaml(Camlp4) = bb930f7c2bed5d057c794fe07dc8596a
   ocaml(Camlp4_config) = 80b5d58834366711574a5ec4dfb123fd
   ocaml(Camlp4_import) = 901773aae9273de4d3a05d3a93dde334
   ocaml(Char) = 3da72249626c7db769beafc97036cb4f
   ocaml(Consistbl) = caff227b73e80a299bc9064d8932a731
   ocaml(Digest) = 310db9d3dd12d84178f002a532644c84
   ocaml(Env) = 85c160706e01672db948b045d51d0663
   ocaml(Filename) = 7cd172f02b7ee9b8d7bda3bb92144951
   ocaml(Format) = b7ba3152a5eec5609d6ab86e6c51eebb
   ocaml(Hashtbl) = ee2a3220e38a4350c5bc131ce9f3f6ce
   ocaml(Ident) = d2f1896a13d2b6ab5a7f039f2e1e4baf
   ocaml(Int32) = b2545c419b6b6a173cac4c0a3e7e0277
   ocaml(Int64) = d501d6e89fdce41c79f274fb464995d5
   ocaml(Lazy) = 4c7ed568fa7b5f73a2aa02eeb0e5e12b
   ocaml(Lexing) = 4d17267334f1a6c75730dc3fae21fb9b
   ocaml(List) = a0e2e49d266ff302f8667651a43f71ba
   ocaml(Location) = 0d236ae3a37e3f5f553fe29e883ac46d
   ocaml(Longident) = af7a4daa7675e00536bcf34c30f1ef8e
   ocaml(Map) = d6ea0139afe59a16df7b23d35e571de7
   ocaml(Nat) = 3ba7c2bfbc706aa841271c572dbb55de
   ocaml(Nativeint) = 7233ce5207a538fea4f0c61ed411ea2c
   ocaml(Num) = a130968f082cd5c0b9fd83b97c9603c1
   ocaml(Obj) = c827f726ce05da709cf7de58fc15e324
   ocaml(Parsing) = 29c3f123280f8e6e639cfb025b3c9a3f
   ocaml(Path) = 3f80ef0865cd9994e2dcb1444d86c8b9
   ocaml(Pervasives) = 88cb1505c8bdf9a4dcd2cdf3452732b4
   ocaml(Primitive) = ffb9c662271efdee731a555268b835a9
   ocaml(Printf) = 807ecd3a1538992580464c03462c9964
   ocaml(Queue) = 56b5e04dcda600ae0cdf49a37f17fcd9
   ocaml(Random) = 462fc826fd1ae9df8d15e3cb798cba9d
   ocaml(Ratio) = 5ee67f3f53c78b1d40c5da48028935f3
   ocaml(Set) = c4be5d24d30c129dd60d2739e54db7dd
   ocaml(Stream) = 91a43ea7fb16bf36f3f10c0dc7d08a0e
   ocaml(String) = ecc403546c1c50056801131811c39017
   ocaml(Sys) = 21bf525b2b3f3a46a54b96163adfe387
   ocaml(Topdirs) = 259bd544fdba007c4f0fb2efdbf8e3e2
   ocaml(Toploop) = 85ab2f8a53c5adc2ef86abcfd6f2aa92
   ocaml(Types) = 51884d3e170a51d2c53e50c054df93c5
   ocaml(Warnings) = f8edde181ba3c5ccbccdbdcf0e922d3a
   ocaml(runtime) = 3.11.0

   ocaml-pa-do-devel = 0.8.3-2.fc11
   ocaml-pa-do-devel(x86-64) = 0.8.3-2.fc11
   ocaml-pa-do = 0.8.3-2.fc11

? Not sure about the test suite.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X LICENSE file is 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.
* .o and .ml files not included (a syntax extension, so .cmo files are OK)
Comment 12 Richard W.M. Jones 2009-03-16 08:32:22 EDT
Let me first just put forward the new version.  I will
address the other issues in a moment ...

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.4-1.fc10.src.rpm

* Mon Mar 16 2009 Richard W.M. Jones <rjones@redhat.com> - 0.8.4-1
- New upstream version 0.8.4.
- Use http URLs instead of https URLs.
- Min version of OCaml required is 3.10, not 3.11.  This will let us
  distribute on Fedora 10.
Comment 13 Richard W.M. Jones 2009-03-16 08:40:05 EDT
It looks to me like tests are only building in the %build
section, and in fact I can run 'omake test' by hand and
that appears to run the tests.  I have added a new %check
section which does this.

I've also removed the LICENSE file from the -devel package,
so it isn't duplicated now.

I think that addresses all your concerns.

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.4-2.fc10.src.rpm

* Mon Mar 16 2009 Richard W.M. Jones <rjones@redhat.com> - 0.8.4-2
- Add check section which runs the tests.
- Don't duplicate LICENSE file in the -devel subpackage as well.
Comment 14 Jason Tibbitts 2009-03-16 20:48:34 EDT
Hmm, this failed to build:

+ ocamlopt.opt -warn-error A -dtypes -inline 3 -pp 'camlp4 -parser oq -printer a -I ../src pa_do.cmo    pa_infix.cmo nums.cma pa_do_           nums.cmo' -I . -I ../src -I +camlp4 -c ad-hoc-newton.ml
File "ad-hoc-newton.ml", line 1, characters 0-1:
Warning X: bad source file name: "Ad-hoc-newton" is not a valid module name.
File "ad-hoc-newton.ml", line 1, characters 0-1:
Error: Error-enabled warnings (1 occurrences)
- exit examples ad-hoc-newton.cmi, 0.10 sec, code 2
*** omake: 236/316 targets are up to date
*** omake: failed (21.62 sec, 36/36 scans, 67/115 rules, 169/444 digests)
*** omake: targets were not rebuilt because of errors:
      depends on: examples/ad-hoc-newton.ml
      depends on: examples/ad-hoc-newton.ml
      depends on: examples/ad-hoc-newton.ml
Comment 15 Richard W.M. Jones 2009-03-17 06:18:22 EDT
My fault for not testing this properly on OCaml 3.11
in Rawhide.

The updated package below contains patches to fix the
problems.  There is still one test which fails, but
I think it's a bug in camlp4.

Spec URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do.spec
SRPM URL: http://www.annexia.org/tmp/ocaml/ocaml-pa-do-0.8.4-3.fc11.src.rpm

* Tue Mar 17 2009 Richard W.M. Jones <rjones@redhat.com> - 0.8.4-3
- Patch module name which is illegal in OCaml 3.11.
- Patch complex tests.
Comment 16 Jason Tibbitts 2009-03-17 12:36:32 EDT
Just to be sure, are you expecting six failures in the op_concrete test?  I'm not sure if your above comment expects a single failure, or some number of failures in a single test group.
Comment 17 Richard W.M. Jones 2009-03-17 12:49:51 EDT
Was hoping not to get any failures at all :-)  But yes
at the moment it's 6 failures:

+Group: Concrete syntax tests
  +Group: Simple concrete syntax tests
    +Test: /+/
      File "op_concrete.ml", line 12, characters 29-50:
      Test: while executing camlp4, the following error was encountered:
Parse error: [relative_precedence] expected after [associativity] (in [pa_infix_declaration])

    +Test: /*/
      File "op_concrete.ml", line 22, characters 29-50:
      Test: while executing camlp4, the following error was encountered:
Parse error: [implem] expected after [semi] (in [implem])

  +Group: Alphabetic operators.
    +Test: o
      File "op_concrete.ml", line 36, characters 36-53:
      Test: while executing camlp4, the following error was encountered:
Parse error: [implem] expected after [semi] (in [implem])

    +Test: plus
      File "op_concrete.ml", line 41, characters 35-58:
      Test: while executing camlp4, the following error was encountered:
Parse error: [implem] expected after [semi] (in [implem])

    +Test: subset
      File "op_concrete.ml", line 46, characters 35-62:
      Test: while executing camlp4, the following error was encountered:
Parse error: [implem] expected after [semi] (in [implem])

    +Test: incr
      File "op_concrete.ml", line 51, characters 36-54:
      Test: while executing camlp4, the following error was encountered:
Parse error: [implem] expected after [semi] (in [implem])

6 tests failed on 14.
Comment 18 Jason Tibbitts 2009-03-17 18:40:31 EDT
OK, that matches what I see.  If you're OK with those failures then I don't see a problem.

Comment 19 Richard W.M. Jones 2009-03-17 19:11:40 EDT
Thanks for reviewing this Jason.

New Package CVS Request
Package Name: ocaml-pa-do
Short Description: OCaml syntax extension for delimited overloading
Owners: rjones
Branches: F-10
Comment 20 Kevin Fenzi 2009-03-17 23:40:23 EDT
cvs done.
Comment 21 Fedora Update System 2009-03-18 06:10:28 EDT
ocaml-pa-do-0.8.4-4.fc10 has been submitted as an update for Fedora 10.
Comment 22 Fedora Update System 2009-03-18 15:15:58 EDT
ocaml-pa-do-0.8.4-4.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.