Bug 767042

Successful Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3581663

Taking for review.

If you are going to build this package for Fedora only, then there are several elements of the spec file that are not needed.  These lines:

%global _use_internal_dependency_generator 0
%global __find_requires /usr/lib/rpm/ocaml-find-requires.sh
%global __find_provides /usr/lib/rpm/ocaml-find-provides.sh

are still specified on the wiki even though they haven't been necessary for a few Fedora releases.  In fact, if you remove these, you'll see Requires on libc and libm show up that don't show up with these lines in place.  Unfortunately, a Provides of dlljs_of_ocaml.so() also shows up, but it can be filtered like this:

%global __provides_exclude_from ^%{_libdir}/ocaml/stublibs/.*\.so$

All current versions of Fedora were released with ocaml 3.12.0, so you can omit the ">= 3.10.0" on the BuildRequires of ocaml.

Finally, the "rm -rf $RPM_BUILD_ROOT" at the top of %install and the %defattr in %files can be omitted.

If you are going to build for EPEL, disregard everything above this line.

Legend:
+: OK
-: must be fixed
=: should be fixed (at your discretion)
N: not applicable

MUST:
[-] rpmlint output:
js-of-ocaml.x86_64: W: unstripped-binary-or-object /usr/bin/js_of_ocaml
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/regexp.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/regexp.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/libjs_of_ocaml.a
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/form.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/form.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/js.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/js.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/event_arrows.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/event_arrows.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/dom_events.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/dom_events.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/dom.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/dom.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/xmlHttpRequest.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/xmlHttpRequest.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/runtime.js
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/file.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/file.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/url.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/url.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/firebug.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/firebug.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/CSS.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/CSS.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/dom_html.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/dom_html.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/lwt_js.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/lwt_js.mli
js-of-ocaml.x86_64: W: devel-file-in-non-devel-package /usr/lib64/ocaml/js_of_ocaml/json.mli
js-of-ocaml.x86_64: E: incorrect-fsf-address /usr/lib64/ocaml/js_of_ocaml/json.mli
js-of-ocaml.x86_64: W: no-manual-page-for-binary js_of_ocaml
js-of-ocaml.x86_64: W: ocaml-naming-policy-not-applied /usr/lib64/ocaml/js_of_ocaml/regexp.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/viewer_js.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/wiki/main.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/svg.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/converter.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/main.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene_json.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/planet/planet.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/wysiwyg/main.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/viewer_common.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_file.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_lexer.mll
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_lexer.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/wiki/wikicreole.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/cubes/cubes.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/viewer.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_graph.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_file.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_parser.mly
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene_extents.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_render.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/wiki/wiki_syntax.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/viewer.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/viewer_common.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/wiki/wikicreole.mll
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_render.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene_json.mli
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/scene_extents.ml
js-of-ocaml-doc.noarch: E: incorrect-fsf-address /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/dot_graph.ml
js-of-ocaml-doc.noarch: W: hidden-file-or-dir /usr/share/doc/js-of-ocaml-doc-1.0.9/examples/graph_viewer/.depend
2 packages and 1 specfiles checked; 45 errors, 19 warnings.

The unstripped binary is okay.  As you noted, programs containing ocaml bytecode do not survive being stripped.  However, you also have to worry about prelink.  You need to add a file to /etc/prelink.conf.d to blacklist the binary.  See the ocaml-ocamlnet spec file for an example of how to do this.

I assume the devel-file-in-non-devel-package warnings are because those devel files are needed in the translation process.  Is that right?

Please talk to upstream about fixing the FSF's address.  This is not a blocker, though.

I'm not sure what to think of the ocaml naming policy warning.  I guess rpmlint wants this package to be named ocaml-js-of-ocaml?

I doubt the hidden file is needed.  What do you think?
[=] follows package naming guidelines: why did you change the underscores to hyphens?  Since upstream uses underscores, this package is granted the exception mentioned in https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Separators.  Reverting back to underscores would also let you use %{name} in more places in the spec file, and also drop the "-n js_of_ocaml-%{version}" from the %setup invocation.
[+] spec file base name matches package name
[+] package meets the packaging guidelines: but consider adding "-p" to the "cp" invocations, as per https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps.
[+] package uses a Fedora approved license
[+] license field matches the actual license: not quite.  The LICENSE file mentions exceptions, so the license should be "LGPLv2+ with exceptions".  Also, some of the examples are GPLv2+ (e.g., examples/graph_viewer/scene.mli), and one is WTFPL (examples/boulderdash/boulderdash.ml).  I think the -doc subpackage will need to have a license along the lines of "(LGPLv2+ with exceptions) and GPLv2+ and WTFPL".
[+] license file is included in %doc
[+] spec file is in American English
[+] spec file is legible
[+] sources match upstream: md5sum is e3ddba862f046859eb0d0456853d195a for both
[+] package builds on at least one primary arch (tried x86_64)
[-] appropriate use of ExcludeArch: this spec file must contain this line:

ExclusiveArch: %{ocaml_arches}

[+] all build requirements in BuildRequires
[N] spec file handles locales properly
[N] ldconfig in %post and %postun
[+] no bundled copies of system libraries
[N] no relocatable packages
[+] package owns all directories that it creates
[+] no files listed twice in %files
[+] proper permissions on files
[+] consistent use of macros
[+] code or permissible content
[+] large documentation in -doc
[+] no runtime dependencies in %doc
[N] header files in -devel
[N] static libraries in -static
[N] .so in -devel: this package has an .so, but it is in the right place
[N] -devel requires main package
[+] package contains no libtool archives
[N] package contains a desktop file, uses desktop-file-install
[+] package does not own files/dirs owned by other packages
[+] all filenames in UTF-8

SHOULD:
[N] query upstream for license text
[N] description and summary contains available translations
[+] package builds in mock: tried fedora-rawhide-i386
[+] package builds on all supported arches: tried i386 and x86_64
[-] package functions as described: only minimal testing
[+] sane scriptlets
[+] subpackages require the main package
[N] placement of pkgconfig files
[+] file dependencies versus package dependencies
[=] package contains man pages for binaries/scripts

Updated spec at same URL to incorporate these fixes:
1. Use RPM features not available on EPEL to simplify spec
2. Add /usr/bin/js_of_ocaml to prelink black list
3. Removed examples/graph_viewer/.depend
4. Add ExclusiveArch

diff --git a/js-of-ocaml.spec b/js-of-ocaml.spec
index 18db785..16c048d 100644
--- a/js-of-ocaml.spec
+++ b/js-of-ocaml.spec
@@ -1,8 +1,6 @@
 %global opt %(test -x %{_bindir}/ocamlopt && echo 1 || echo 0)
 %global debug_package %{nil}
-%global _use_internal_dependency_generator 0
-%global __find_requires /usr/lib/rpm/ocaml-find-requires.sh
-%global __find_provides /usr/lib/rpm/ocaml-find-provides.sh
+%global __provides_exclude_from ^%{_libdir}/ocaml/stublibs/.*\.so$
 
 Name:           js-of-ocaml
 Version:        1.0.9
@@ -16,12 +14,13 @@ Source0:        http://ocsigen.org/download/js_of_ocaml-%{version}.tar.gz
 # Allow examples to be built with the distro packaged js-of-ocaml
 Source1:        Makefile.common.js-of-ocaml.examples
 
-BuildRequires:  ocaml >= 3.10.0
+BuildRequires:  ocaml
 BuildRequires:  ocaml-findlib-devel
 BuildRequires:  ocaml-ocamldoc
 BuildRequires:  ocaml-camlp4-devel
 BuildRequires:  ocaml-lwt >= 2.3.0
 BuildRequires:  chrpath
+ExclusiveArch: %{ocaml_arches}
 
 %description
 js_of_ocaml is a compiler of OCaml byte-code to JavaScript.
@@ -52,7 +51,6 @@ reference and examples for %{name}.
 make
 
 %install
-rm -rf $RPM_BUILD_ROOT
 export DESTDIR=$RPM_BUILD_ROOT
 export OCAMLFIND_DESTDIR=$RPM_BUILD_ROOT%{_libdir}/ocaml
 mkdir -p $OCAMLFIND_DESTDIR $OCAMLFIND_DESTDIR/stublibs
@@ -70,17 +68,22 @@ cp lib/dlljs_of_ocaml.so $OCAMLFIND_DESTDIR/stublibs/
 strip $OCAMLFIND_DESTDIR/stublibs/dll*.so
 chrpath --delete $OCAMLFIND_DESTDIR/stublibs/dll*.so
 
+# NB. Do NOT strip the binaries and prevent prelink from stripping them too.
+# (https://bugzilla.redhat.com/show_bug.cgi?id=435559)
+mkdir -p $RPM_BUILD_ROOT/etc/prelink.conf.d
+echo '-b /usr/bin/js_of_ocaml' > $RPM_BUILD_ROOT/etc/prelink.conf.d/js-of-ocaml.conf
+
 # docs
-find examples '(' -name '*.cmi' -or -name '*.cmo' -or -name '*.byte' ')' -exec rm '{}' ';'
+find examples '(' -name '*.cmi' -or -name '*.cmo' -or -name '*.byte' -or -name '.depend' ')' -exec rm '{}' ';'
 # Replacing Makefile.common allows the user to build the examples
 cp %{SOURCE1} examples/Makefile.common
 
 %files
-%defattr(-,root,root,-)
 %doc LICENSE README
 %{_libdir}/ocaml/js_of_ocaml/
 %{_libdir}/ocaml/stublibs/*.so
 %{_bindir}/js_of_ocaml
+%config(noreplace) /etc/prelink.conf.d/js-of-ocaml.conf

(In reply to comment #3)
> I assume the devel-file-in-non-devel-package warnings are because those devel
> files are needed in the translation process.  Is that right?

Indeed. I meant to say this in my mysterious "This is way I didn't split produce a separate -debug package" sentence in comment 0 as well. (Apparently my command of the English language comes and goes ..)

> [=] follows package naming guidelines: why did you change the underscores to hyphens?

I used hyphens because that's what Debian did:
http://packages.debian.org/sid/js-of-ocaml
Say you're trying to build some upstream open source project and the upstream website says "Run apt-get ... to install the build dependencies". I run into this often enough that I'd like to keep the package name consistent with Debian unless there's a strong reason not to.

> [-] package functions as described: only minimal testing

The bundled examples are built so the OCaml to Javascript compiler is minimally tested. I can't run the generated code since they are interactive programs that assumes a user clicking in a browser.

I have personally built a large non trivial program and ran it successfully with this js-of-ocaml package though:
http://blog.scottt.tw/2011/12/compiling-web-interface-of-ppcmemarmmem.html

I can see how collecting simple OCaml programs and running the generated code with a command line Javascript interpreter like /usr/bin/js could work but lack the motivation to do that since upstream didn't.

> [=] package contains man pages for binaries/scripts

Upstream didn't include a man page. I can't find one I can conveniently steal from other distros so I punted here as well.

(In reply to comment #5)
> Indeed. I meant to say this in my mysterious "This is way I didn't split
> produce a separate -debug package" sentence in comment 0 as well. (Apparently
> my command of the English language comes and goes ..)

Heh.  I'd say your command of English is quite good.  It's certainly far better than my command of Mandarin Chinese, which I used to speak (but not read or write) semi-fluently 20-some years ago.  Today I have trouble putting a coherent sentence together.

> I used hyphens because that's what Debian did:

Good enough.  Thanks for the explanation.

And I screwed up on some of the pluses and minuses.  Sorry about that.  This is what happens when I cut & paste from a template.  I'm going to make the template have nothing in between the brackets so I'm forced to think about the contents of every single one in the future.  Let me draw your attention to 2 more items, with the correct character between the brackets this time:

MUST:
[=] package meets the packaging guidelines: but consider adding "-p" to the
"cp" invocations, as per
https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps.
[-] license field matches the actual license: not quite.  The LICENSE file
mentions exceptions, so the license should be "LGPLv2+ with exceptions".  Also,
some of the examples are GPLv2+ (e.g., examples/graph_viewer/scene.mli), and
one is WTFPL (examples/boulderdash/boulderdash.ml).  I think the -doc
subpackage will need to have a license along the lines of "(LGPLv2+ with
exceptions) and GPLv2+ and WTFPL".

Everything else looks fine.

(In reply to comment #6)
Updated spec at same URL:
1. Preserve timestamps with "cp -p"
2. Update license fields

diff --git a/js-of-ocaml.spec b/js-of-ocaml.spec
index 16c048d..7ca3b82 100644
--- a/js-of-ocaml.spec
+++ b/js-of-ocaml.spec
@@ -7,7 +7,7 @@ Version:        1.0.9
 Release:        1%{?dist}
 Summary:        An OCaml to Javascript compiler

-License:        LGPLv2+
+License:        LGPLv2+ with exceptions
 URL:            http://ocsigen.org/js_of_ocaml
 Source0:        http://ocsigen.org/download/js_of_ocaml-%{version}.tar.gz

@@ -38,6 +38,7 @@ Its key features are the following:
 Summary:        User manual and other documentation for %{name}
 Group:          Development/Libraries
 BuildArch:      noarch
+License:        (LGPLv2+ with exceptions) and GPLv2+ and WTFPL

 %description    doc
 The %{name}-doc package contains the user manual, API
@@ -56,14 +57,14 @@ export OCAMLFIND_DESTDIR=$RPM_BUILD_ROOT%{_libdir}/ocaml
 mkdir -p $OCAMLFIND_DESTDIR $OCAMLFIND_DESTDIR/stublibs
 D=$RPM_BUILD_ROOT%{_libdir}/ocaml/js_of_ocaml
 mkdir -p $D
-cp lib/META \
+cp -p lib/META \
     lib/*.{cmi,cma,a,mli} \
     lib/syntax/pa_js.cmo \
     runtime/runtime.js \
     $D
 mkdir -p $RPM_BUILD_ROOT%{_bindir}
-cp compiler/js_of_ocaml $RPM_BUILD_ROOT%{_bindir}/
-cp lib/dlljs_of_ocaml.so $OCAMLFIND_DESTDIR/stublibs/
+cp -p compiler/js_of_ocaml $RPM_BUILD_ROOT%{_bindir}/
+cp -p lib/dlljs_of_ocaml.so $OCAMLFIND_DESTDIR/stublibs/

 strip $OCAMLFIND_DESTDIR/stublibs/dll*.so
 chrpath --delete $OCAMLFIND_DESTDIR/stublibs/dll*.so
@@ -76,7 +77,7 @@ echo '-b /usr/bin/js_of_ocaml' > $RPM_BUILD_ROOT/etc/prelink.conf.d/js-of-ocaml.
 # docs
 find examples '(' -name '*.cmi' -or -name '*.cmo' -or -name '*.byte' -or -name '.depend' ')' -exec rm '{}' ';'
 # Replacing Makefile.common allows the user to build the examples
-cp %{SOURCE1} examples/Makefile.common
+cp -p %{SOURCE1} examples/Makefile.common

Looks good.  This package is APPROVED.

New Package SCM Request
=======================
Package Name: js-of-ocaml
Short Description: An OCaml to Javascript compiler
Owners: scottt
Branches: f16
InitialCC:

Git done (by process-git-requests).

js-of-ocaml-1.0.9-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/js-of-ocaml-1.0.9-1.fc16

js-of-ocaml-1.0.9-1.fc16 has been pushed to the Fedora 16 testing repository.

js-of-ocaml-1.0.9-1.fc16 has been pushed to the Fedora 16 stable repository.