Bug 469955

Some issues I noticed at my first check:

- Why do you need a Conflict with pam_fprint, are Obsolutes/Provides not better fitted here?
https://fedoraproject.org/wiki/Packaging/Conflicts

- Source0 is not a URL and it is not explained how to create the source https://fedoraproject.org/wiki/Packaging/SourceURL

- Is it intentional that the config files in /etc are not marked as %config?

Btw. the release string of the linked libfprint seems to be wrong, it should be 0.1.pre.fc10 instead of 1.pre.fc10

(In reply to comment #1)
> Some issues I noticed at my first check:
> 
> - Why do you need a Conflict with pam_fprint, are Obsolutes/Provides not better
> fitted here?
> https://fedoraproject.org/wiki/Packaging/Conflicts

It's not stable/tested enough to replace pam_fprint. So I'm currently only conflicting with it.

> - Source0 is not a URL and it is not explained how to create the source
> https://fedoraproject.org/wiki/Packaging/SourceURL

True, will fix.

> - Is it intentional that the config files in /etc are not marked as %config?

Yes and no. %{_sysconfdir}/fprintd.conf should be marked as %config, but it doesn't actually do anything useful yet, so it's better to just have it replaced for now. I should add a comment about it.

> Btw. the release string of the linked libfprint seems to be wrong, it should be
> 0.1.pre.fc10 instead of 1.pre.fc10

That should be fixed, I updated libfprint in F11:
http://koji.fedoraproject.org/koji/taskinfo?taskID=938201

All fixed.

http://hadess.fedorapeople.org/fprintd/fprintd.spec
http://hadess.fedorapeople.org/fprintd/fprintd-0.1-2.gitaf42ec70f3.fc10.src.rpm

Koji scratch build worked:
http://koji.fedoraproject.org/koji/taskinfo?taskID=946393

Here is what rpmlint says on the rpms: 

fprintd.i386: W: non-conffile-in-etc /etc/fprintd.conf

You said you wanted to make this conf ?

fprintd.i386: W: non-conffile-in-etc /etc/dbus-1/system.d/net.reactivated.Fprint.conf

This is ignorable

fprintd-devel.i386: W: summary-not-capitalized fprintd development documentation

I always use a more-or-less standardized summary of "Development files for %{name}" for -devel packages.

fprintd-devel.i386: W: invalid-license GFDLv1.1+

No + there, I think.

printd-pam.i386: W: no-documentation

Thats sad, but ignorable.

Taking a first look at the spec file:

Requires:       %{name} = %{version}

Not sure if theres a policy about this, but I always do
Requires: %{name} = %{version}-%{release}
to avoid surprises


Going to do a formal review in a bit.

(In reply to comment #4)
> Here is what rpmlint says on the rpms: 
> 
> fprintd.i386: W: non-conffile-in-etc /etc/fprintd.conf
> 
> You said you wanted to make this conf ?

No, I added a comment about it:
# This file should be marked as config when it does something useful

> fprintd.i386: W: non-conffile-in-etc
> /etc/dbus-1/system.d/net.reactivated.Fprint.conf
> 
> This is ignorable
> 
> fprintd-devel.i386: W: summary-not-capitalized fprintd development
> documentation
> 
> I always use a more-or-less standardized summary of "Development files for
> %{name}" for -devel packages.

OK, will change.

> fprintd-devel.i386: W: invalid-license GFDLv1.1+
> 
> No + there, I think.

But the docs say:
"Permission is granted to copy, distribute and/or modify this document under
the terms of the GNU Free Documentation License, Version 1.1 or any later
version  [...]"

> printd-pam.i386: W: no-documentation
> 
> Thats sad, but ignorable.

I'll write a little something to go upstream and in the package.

(In reply to comment #5)
> Taking a first look at the spec file:
> 
> Requires:       %{name} = %{version}
> 
> Not sure if theres a policy about this, but I always do
> Requires: %{name} = %{version}-%{release}
> to avoid surprises

Yes, will fix.

two more informal comments on the spec before I run down the checklist:
- I don't think the explicit Requires: PolicyKit is needed, library deps 
  should take care of that
- The Conflicts: pam_fprint should probably have a comment explaining your
  rationale from comment #2


rpmlint run: see above

package name: ok
spec file name: ok
packaging guidelines: ok
license: ok
license field: not ok
  the license field says GPLv2+, and the source files say so too,
  but the license file is GPL3. What gives ?
license file: ok
spec file language: ok
spec file legible: ok
upstream sources: ok
buildable: ok
ExcludeArch: ok
BuildRequires: ok
locale handling: ok
shared libs: ok
relocatable: ok
directory ownership: not ok
  -devel should require gtk-doc, for /usr/share/gtk-doc/html
  -pam should require pam, for /lib/security
duplicate files: ok
permissions: not ok
  the %files sections for pam and devel need %defattr
%clean: ok
macro use: ok
permissible content: ok
large docs: ok
%doc content: ok
headers: ok
static libs: ok
pc files: ok
shared libs: ok
devel deps: ok
libtool archives: ok
gui apps: ok
file ownership: ok
%install: ok
utf8 filenames: ok

summary: some small fixes left

Fixed Require: %{name} = %{version}-%{release}
Changed summary for devel files
Added README for pam

(In reply to comment #7)
> two more informal comments on the spec before I run down the checklist:
> - I don't think the explicit Requires: PolicyKit is needed, library deps 
>   should take care of that

Removed

> - The Conflicts: pam_fprint should probably have a comment explaining your
>   rationale from comment #2

I removed the Conflicts. The configuration is different, so I'll add some release notes.

> rpmlint run: see above
> 
> package name: ok
> spec file name: ok
> packaging guidelines: ok
> license: ok
> license field: not ok
>   the license field says GPLv2+, and the source files say so too,
>   but the license file is GPL3. What gives ?

Fixed, it's supposed to be a GPLv2 COPYING file.

> license file: ok
> spec file language: ok
> spec file legible: ok
> upstream sources: ok
> buildable: ok
> ExcludeArch: ok
> BuildRequires: ok
> locale handling: ok
> shared libs: ok
> relocatable: ok
> directory ownership: not ok
>   -devel should require gtk-doc, for /usr/share/gtk-doc/html

Added.

>   -pam should require pam, for /lib/security

Already dragged in through library deps (PAM libs are in the pam package)

> duplicate files: ok
> permissions: not ok
>   the %files sections for pam and devel need %defattr

Done.

> %clean: ok
> macro use: ok
> permissible content: ok
> large docs: ok
> %doc content: ok
> headers: ok
> static libs: ok
> pc files: ok
> shared libs: ok
> devel deps: ok
> libtool archives: ok
> gui apps: ok
> file ownership: ok
> %install: ok
> utf8 filenames: ok
> 
> summary: some small fixes left

All done!

http://hadess.fedorapeople.org/fprintd/fprintd.spec
http://hadess.fedorapeople.org/fprintd/fprintd-0.1-3.git43fe72a2aa.fc10.src.rpm

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

Looks fine now. Approved.

Might be nice to add

/usr/share/dbus/interfaces/net.reactivated.Fprint.xml 

at some point

New Package CVS Request
=======================
Package Name: fprintd
Short Description: D-Bus service for Fingerprint reader access
Owners: hadess
Branches: F-11
InitialCC:

cvs done.

Build in rawhide, thanks all.