Bug 480037 - Review Request: cups-pk-helper - PolicyKit support for system-config-printer
Review Request: cups-pk-helper - PolicyKit support for system-config-printer
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Clasen
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-14 12:20 EST by Marek Kašík
Modified: 2009-01-30 01:05 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-30 01:05:13 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mclasen: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Marek Kašík 2009-01-14 12:20:08 EST
Spec URL: http://mkasik.fedorapeople.org/cups-pk-helper.spec
SRPM URL: http://mkasik.fedorapeople.org/cups-pk-helper-0.0.3-1.fc11.src.rpm

Description: Use PolicyKit to define policies for accessing the cups functionality in system-config-printer.
Originally developed by Vincent Untz.
(see https://fedoraproject.org/wiki/Features/CupsPolicyKitIntegration for more details)

To test this feature, one need to use patched system-config-printer (see links below). You can also test this feature in Fedora 10.

http://mkasik.fedorapeople.org/cups-pk-helper-0.0.3-1.fc10.src.rpm
http://mkasik.fedorapeople.org/system-config-printer-1.1.1-3.fc10.src.rpm
http://mkasik.fedorapeople.org/system-config-printer-1.1.1-3.fc11.src.rpm
Comment 1 Matthias Clasen 2009-01-14 12:40:27 EST
Builds fine in mock.

rpmlint says:

cups-pk-helper.i386: W: no-documentation
cups-pk-helper.i386: W: non-conffile-in-etc /etc/dbus-1/system.d/org.opensuse.CupsPkHelper.Mechanism.conf
cups-pk-helper.i386: E: description-line-too-long cups-pk-helper is an application, which helps system-config-printer to use PolicyKit infrastructure

The warnings are ignorable, the line-too-long error should be fixed.
Comment 2 Matthias Clasen 2009-01-14 12:45:19 EST
Here are my proposals for Summary / Description:

Summary: A helper that makes system-config-printer use PolicyKit

%description
cups-pk-helper is an application which makes cups configuration
interfaces available under control of PolicyKit.
Comment 3 Jochen Schmitt 2009-01-14 13:26:21 EST
Good
+ Package naming fits with naming guidelines
+ Basename of SPEC file matches with package name
+ License tag contains valid license
+ License GPLv2 is a valid OSS license
+ Local build works fine
+ Could download tar ball with spectool
+ Tar ball in source rpm matches with upstream
(md5sum: db1c91dcb3c9f7ef3270fe29c25178a4)
+ Consistently use of rpm macros
+ Buildroot will be clean on begin of %install and %clean
+ Local build works fine
+ Rpmlint doens't complaints debuginfo rpm
+ Local install works fine
+ Local uninstall wworks fine
+ Package contains no subpcakges
+ Build works fine on koji
+ Pakcage contains no files belong to others
+ Files Permissions looks right
+ Package contains valid %changelog stanza

Bad: 
- Rpmlint complaints source rpm:
cups-pk-helper.src: E: description-line-too-long cups-pk-helper is an application, which helps system-config-printer to use PolicyKit infrastructure
- Rpmlint complaints binary rpm:
cups-pk-helper.x86_64: W: no-documentation
cups-pk-helper.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/org.opensuse.CupsPkHelper.Mechanism.conf
cups-pk-helper.x86_64: E: description-line-too-long cups-pk-helper is an application, which helps system-config-printer to use PolicyKit infrastructure
- License tag should be GPLv2+, because the copyright notes in the source files aloow use of later versions of the GPL.
- Verbatin copy of the license text is not included in the rpm, but available in the original source tree.
- %doc stanza contains no files

question:
* Can I run a test without a printer. Is there a text plan for this package.
Comment 4 Matthias Clasen 2009-01-14 14:09:19 EST
Formal review:

rpmlint: see above
package name: ok
spec file name: ok
packaging guidelines: 
 - Looks to me like the dbus-glib and PolicyKit requires are redundant, 
   since they are already pulled in by library dependencies
 - should include AUTHORS and NEWS as %doc
license: ok
license field: should be GPLv2+ (note the +), since the source files say "or 
   any later version"
license file: must be included
spec file language: ok
spec file legible: ok
upstream sources: ok
buildable: ok
excludearch: ok
build deps: ok
locale handling: ok
shared libraries: ok
relocatable: not applicable
directory ownership: ok
duplicate files: ok
permissions: ok
%clean: ok
macro use: ok
permissible content: ok
large docs: ok
%doc content: ok
header files: ok
static libs: ok
pkgconfig files: ok
shared libs: ok
devel package: not applicable
libtool archives: ok
desktop files: not applicable
file ownership: ok
%install: ok
utf8 filenames: ok

summary:
- fix summary/description
- fix license tag
- include license file
- remove redundant requires
- include %doc files
Comment 5 Matthias Clasen 2009-01-14 14:26:37 EST
Jochen, testing this requires a patched system-config-printer that we don't have yet. See https://fedoraproject.org/wiki/Features/CupsPolicyKitIntegration
Comment 6 Marek Kašík 2009-01-15 09:11:45 EST
Hi,
thank you for comments. I modified the package according to your comments and I uploaded it to the same location (the same addresses). Original versions are stored in "http://mkasik.fedorapeople.org/old/".

  Regards

    Marek


P.S.: I had to modify also the scp-policykit.patch patch in http://mkasik.fedorapeople.org/system-config-printer-1.1.1-3.*.src.rpm. I forget to remove one line from the patch.
Comment 7 Matthias Clasen 2009-01-15 09:39:13 EST
Looks fine now. Approved
Comment 8 Marek Kašík 2009-01-19 05:59:57 EST
New Package CVS Request
=======================
Package Name: cups-pk-helper
Short Description: A helper that makes system-config-printer use PolicyKit
Owners: mkasik
Branches:
InitialCC:
Comment 9 Kevin Fenzi 2009-01-19 17:56:15 EST
cvs done.
Comment 10 Marek Kašík 2009-01-20 08:49:59 EST
Hi,
I committed this package to CVS and I built it.

  Marek

Note You need to log in before you can comment on or make changes to this bug.