Bug 630208

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.

(In reply to comment #1)
> I am utterly unfamiliar with Haskell.  I'm happy to negotiate on any point,
> given my ignorance.
> 
> 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.

The strange permissions are a relic of my umask of 027. When put into git and the lookaside, they get the proper 644 (git only tracks +x or -x and the lookaside has 644).

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

The spec magic is standard ghc-rpm-macros voodoo. The bulk of it is generated by the cabal2spec tool.

The ghc_lib_package macro is in the following file:

    http://pkgs.fedoraproject.org/gitweb/?p=ghc-rpm-macros.git;a=blob;f=ghc-rpm-macros.ghc

%files is generated by grepping the cabal file for the license file that gets installed and adding it to %doc. It also makes the information for the -devel and -prof (which is akin to a -debuginfo of sorts) packages. Haskell is predictable since cabal standardizes many things and the directories and such are generated given %{pkg_name} and %{version}.

> [?] 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?

The ghc_lib_package runs the ghc-pkg recache which registers the library with ghc's linking.

> [?] 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.

rpm -qlp ghc-csv-*.<arch>.rpm

The COPYING file should be in 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: 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.

Haskell is a statically built language for some things. Hopefully ghc-7.0 will help get us proper shared libraries across the board.

(In reply to comment #2)
> The strange permissions are a relic of my umask of 027. When put into git and
> the lookaside, they get the proper 644 (git only tracks +x or -x and the
> lookaside has 644).

Git and lookasides are nice, but from a package review standpoint I'm only interested in the file permissions that wind up on user's hard drives.

I find no suitable guidance in the FHS at http://www.pathname.com/fhs/

Looking at https://fedoraproject.org/wiki/Packaging/Guidelines#File_Permissions tells me that only file permissions must be set "properly."  I cannot think of any realistic instance where this would cause improper behavior and it can only affect developers in any event, so I'll concede that it's a style issue instead of "improper."

I'd prefer to see you reset the tarball and .spec file permissions to "typical" 0644 values before building the package, but I won't block on it.

> > [-] MUST: The spec file for the package MUST be legible.

I just stumbled upon https://fedoraproject.org/wiki/Packaging:Haskell and it satisfies my concerns.  I would prefer to see that link as a comment within the .spec file, but I won't block on it either.

APPROVED.

(In reply to comment #1)
> I am utterly unfamiliar with Haskell.  I'm happy to negotiate on any point,
> given my ignorance.

Thanks for your good comments and input.

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

Good point: we should document that in the fedora wiki
and add a link to our spec files.

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

Agreed - ditto as above.

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

Not obvious again but you can see the filelists on the src build dir.

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

It does unfortunately but ghc still assumes static libs by
default so they can't be subpackaged yet.

New Package SCM Request
=======================
Package Name: ghc-csv
Short Description: CSV loader and dumper
Owners: mathstuf
Branches: f14
InitialCC: haskell-sig

Oops, forgot f13.

New Package SCM Request
=======================
Package Name: ghc-csv
Short Description: CSV loader and dumper
Owners: mathstuf
Branches: f13
InitialCC: haskell-sig

Git done (by process-git-requests).

I assume you also want f14? 
Processed with that as well as f13

Yes, thanks. Filed an update.

ghc-csv-0.1.1-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/ghc-csv-0.1.1-1.fc13

ghc-csv-0.1.1-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/ghc-csv-0.1.1-1.fc14

ghc-csv-0.1.1-1.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update ghc-csv'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/ghc-csv-0.1.1-1.fc13

ghc-csv-0.1.1-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

ghc-csv-0.1.1-1.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.