[Bug 460244] Review Request: alt-ergo - Alt-Ergo automatic theorem prover

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=460244





--- Comment #1 from Alan Dunn <amdunn at gmail.com>  2008-08-26 20:15:14 EDT ---
Bugzilla was inoperative at the time that I originally produced this package,
so I solicited a review by email (from David Wheeler, who will vouch for this).

He wrote:


* I could NOT rebuild on i386; it seems to fail on ocaml-ocamlgraph. Perhaps
the ocaml-ocamlgraph I'm using is not the latest version, but in any case, I
got this error: "Cannot find file graph.cmxa".  I rebuilt
http://www.duke.edu/~amd34/ocamlgraph/ocaml-ocamlgraph-0.99c-2.fc9.src.rpm to
create ocaml-ocamlgraph, and it's true, there's no .cmxa file:
$ rpm -qil ocaml-ocamlgraph
...
/usr/lib/ocaml/ocamlgraph
/usr/lib/ocaml/ocamlgraph/META
/usr/lib/ocaml/ocamlgraph/graph.cma
/usr/lib/ocaml/ocamlgraph/graph.cmi
/usr/share/doc/ocaml-ocamlgraph-0.99c/LICENSE

...

which I fixed in version 2 by adding ocaml-ocamlgraph-devel to BuildRequires
(it is not yet in the repositories, which implies that I can't really use mock)

* In "cp %{SOURCE1} %{SOURCE2} .", say "cp -p" instead.  You should try to keep
the timestamps where you can.  Indeed, if you got these files from elsewhere,
try to preserve THEIR timestamps.

which I also changed, and the following two comments

* The spec file says this, and I couldn't figure out what "->" meant;
 can you make it clearer by replacing "->" with the word(s) you mean?
 # Avoid a Makefile patch by just adding this empty file -> autoconf
 # doesn't complain (better than ignoring all status from configure)

* I think the "iconv" should be run during _build_, not _install_.
Also, you have 3 semicolon-separated commands on one line to invoke it,
which kindof hides the "iconv".  I'd prefer to have that as a sequence
of 3 commands on 3 lines.  I'm not big on cuddling multiple commands
on a line anyway, but this sequence hides the important command:
"mv CeCILL-C CeCILL-C.iso8859; iconv -f ISO-8859-1 -t UTF-8 < CeCILL-C.iso8859
> CeCILL-C; rm CeCILL-C.iso8859"

which I also changed. He then gave a "more formal review":

+ rpmlint output
 It outputs:
 alt-ergo.i386: W: invalid-license CeCILL-C
 But this is an error in rpmlint (CeCILL-C is a recent addition),
 already explained in the spec file

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
 Yes.  CeCILL-C was just added to the acceptable list.
+ license matches the actual package license
+ %doc includes license file
  Yes, /usr/share/doc/alt-ergo-0.8/CeCILL-C
+ spec file written in American English
+ spec file is legible
+ upstream sources match sources in the srpm
 Yes, sha1sum:
 6a073b88d799e3dfcc7e13dfc1386c6422ce9ab8
+ package successfully builds on at least one architecture
 i386.  Can't try koji until ocamlgraph is in.
??? ExcludeArch bugs filed
??? BuildRequires list all build dependencies
 (Not yet)
n/a %find_lang instead of %{_datadir}/locale/*
n/a binary RPM with shared library files must call ldconfig in %post and
%postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
n/a header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
n/a libfoo.so must go in -devel
n/a -devel must require the fully versioned base
n/a packages should not contain libtool .la files
n/a packages containing GUI apps must include %{name}.desktop file
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
??? reviewer should build the package in mock
 Don't know how to do that yet; ocamlgraph not available in yum
??? the package should build into binary RPMs on all supported architectures
 Don't know how to do that yet; ocamlgraph not available in yum
+ review should test the package functions as described
 Tried using gwhy; looks great!
n/a scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.