[Fedora-haskell-list] [Bug 630208] Review Request: ghc-csv - CSV loader and dumper

Sat Sep 11 17:29:21 UTC 2010

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=630208

Dave Ludlow <dave at adsllc.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |dave at adsllc.com
               Flag|                            |fedora-review?, needinfo+,
                   |                            |needinfo?(mathstuf at gmail.co
                   |                            |m)

--- Comment #1 from Dave Ludlow <dave at adsllc.com> 2010-09-11 13:29:20 EDT ---
I am utterly unfamiliar with Haskell.  I'm happy to negotiate on any point,
given my ignorance.

+ OK
- Not OK
? Questionable
/ N/A

[-] MUST: rpmlint must be run on every package.

RPMLINT

ghc-csv.src: W: spelling-error %description -l en_US franca -> francs, franc,
franc a
ghc-csv.src: W: strange-permission csv-0.1.1.tar.gz 0640L
ghc-csv.src: W: strange-permission ghc-csv.spec 0640L
ghc-csv.x86_64: W: spelling-error %description -l en_US franca -> francs,
franc, franc a
ghc-csv-devel.x86_64: W: spelling-error %description -l en_US franca -> francs,
franc, franc a
ghc-csv-prof.x86_64: E: devel-dependency ghc-csv-devel
ghc-csv-prof.x86_64: W: spelling-error %description -l en_US franca -> francs,
franc, franc a
ghc-csv-prof.x86_64: W: no-documentation
ghc-csv-prof.x86_64: W: devel-file-in-non-devel-package
/usr/lib64/ghc-6.12.3/csv-0.1.1/libHScsv-0.1.1_p.a
4 packages and 1 specfiles checked; 1 errors, 8 warnings.

The strange permissions are exactly that.  Wouldn't 0644 make more sense?
Since ghc-csv-prof is a profiling package, I assume that its errors and
warnings are irrelevant.  A comment to this effect in the .spec would be nice.

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license and meet
the Licensing Guidelines.
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The license is in %doc as provided.
[+] MUST: The spec file must be written in American English.
[-] MUST: The spec file for the package MUST be legible.

There is apparently some pretty good voodoo in ghc-rpm-macros.  I'd like to see
some comments describing that this is the reason for common_summary being
defined buy only appearing once and the outright lack of %files and %doc - at
least, I'm assuming that that's the reason.  As-is, I'd say that this .spec
file is not legible to someone without an existing ghc background.

[+] MUST: The sources used to build the package must match the upstream source.

312cdb7d59528a161034b3397af10266  ~/rpmbuild/SOURCES/csv-0.1.1.tar.gz
312cdb7d59528a161034b3397af10266  csv-0.1.1.tar.gz

[+] MUST: The package MUST successfully compile and build into binary rpms.
[/] MUST: If the package does not successfully compile, build or work...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: The spec file MUST handle locales properly.
[?] MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths, must
call ldconfig in %post and %postun.

Is it fair to say that this doesn't apply to
/usr/lib64/ghc-6.12.3/csv-0.1.1/libHScsv-0.1.1-ghc6.12.3.so?

[+] MUST: Packages must NOT bundle copies of system libraries.
[+] MUST: If the package is designed to be relocatable...
[+] MUST: A package must own all directories that it creates.
[?] MUST: A Fedora package must not list a file more than once in the spec
file's %files listings.

I'm assuming that ghc-rpm-macros enforces this, but I have no easy way of
knowing.  Legibility is not there.

[-] MUST: Permissions on files must be set properly. Executables should be set
with executable permissions, for example. Every %files section must include a
%defattr(...) line.

The 0640 permissions found by rpmlint are probably not "proper".  There is no
legible %files section or comment.

[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[+] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: If a package includes something as %doc...
[+] MUST: Header files must be in a -devel package.
[?] MUST: Static libraries must be in a -static package.

ghc-csv-devel contains /usr/lib64/ghc-6.12.3/csv-0.1.1/libHScsv-0.1.1.a; I'm
unsure if this is qualifies as a static library.

[+] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1)...
[+] MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency
[+] MUST: Packages must NOT contain any .la libtool archives
[/] MUST: Packages containing GUI applications must include a %{name}.desktop
file
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[/] SHOULD: If the source package does not include license text...
[/] SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all
supported architectures. [28]

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

[?] SHOULD: The reviewer should test that the package functions as described. A
package should not segfault instead of running, for example.

I don't have the background to do any significant testing.

[+] SHOULD: If scriptlets are used, those scriptlets must be sane.
[+] SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
[/] SHOULD: The placement of pkgconfig(.pc)...
[/] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin...
[/] SHOULD: your package should contain man pages...

BLOCKERS

ghc-csv.src: W: strange-permission csv-0.1.1.tar.gz 0640L
ghc-csv.src: W: strange-permission ghc-csv.spec 0640L
[-] MUST: The spec file for the package MUST be legible.  At a minimum, please
provide a comment with a link to some documentation on the RPM macro voodoo in
the .spec file.

As I said before, I'm willing to be flexible since this is an area I'm totally
unfamiliar with.  Overall, it looks pretty good.

-- 
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.