Bug 226466

Summary: Merge Review: system-config-printer
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: twaugh
Target Milestone: ---Flags: panemade: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 0.7.74.4-2.fc8 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-10-05 16:01:44 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Nobody's working on this, feel free to take it 2007-01-31 21:06:35 UTC
Fedora Merge Review: system-config-printer

http://cvs.fedora.redhat.com/viewcvs/devel/system-config-printer/
Initial Owner: twaugh

Comment 1 Parag AN(पराग) 2007-09-28 15:04:27 UTC
picking up this for review.

rpmlint output on SRPM and RPMs is not clean

On SRPM it showed
------------------------------------
system-config-printer.src:30: W: prereq-use system-config-printer-libs =
%{version}-%{release}
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

system-config-printer.src:44: W: prereq-use python
The use of PreReq is deprecated. In the majority of cases, a plain Requires
is enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.

On RPM it showed
------------------------------------
system-config-printer.i386: I: checking
system-config-printer.i386: W: non-conffile-in-etc
/etc/security/console.apps/system-config-printer
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

system-config-printer.i386: W: non-conffile-in-etc
/etc/xdg/autostart/redhat-print-applet.desktop
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

system-config-printer.i386: W: non-conffile-in-etc /etc/pam.d/system-config-printer
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

system-config-printer.i386: E: zero-length
/usr/share/doc/system-config-printer-0.7.74.3/NEWS

===> You can drop this file.

system-config-printer.i386: E: non-executable-script
/usr/share/system-config-printer/cupsd.py 0644
This text file contains a shebang or is located in a path dedicated for
executables, but lacks the executable bits and cannot thus be executed.  If
the file is meant to be an executable script, add the executable bits,
otherwise remove the shebang or move the file elsewhere.

system-config-printer.i386: W: obsolete-not-provided system-config-printer-gui
If a package is obsoleted by a compatible replacement, the obsoleted package
must also be provided in order to provide clean upgrade paths and not cause
unnecessary dependency breakage.  If the obsoleting package is not a compatible
replacement for the old one, leave out the provides.

system-config-printer.i386: E: no-binary
The package should be of the noarch architecture because it doesn't contain
any binaries.

===> good to add following line in SPEC 
     BuildArch: noarch

    
system-config-printer.i386: W: dangerous-command-in-%post rm

On -libs RPM it showed
-----------------------------------
system-config-printer-libs.i386: I: checking
system-config-printer-libs.i386: W: spurious-executable-perm
/usr/share/doc/system-config-printer-libs-0.7.74.3/pycups-1.9.27/examples/cupstree.py
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

system-config-printer-libs.i386: W: non-conffile-in-etc
/etc/dbus-1/system.d/newprinternotification.conf
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

system-config-printer-libs.i386: E: non-executable-script
/usr/share/system-config-printer/cupshelpers.py 0644
This text file contains a shebang or is located in a path dedicated for
executables, but lacks the executable bits and cannot thus be executed.  If
the file is meant to be an executable script, add the executable bits,
otherwise remove the shebang or move the file elsewhere.

system-config-printer-libs.i386: W: summary-ended-with-dot Common code for the
graphical and non-graphical pieces.
Summary ends with a dot.

system-config-printer-libs.i386: W: doc-file-dependency
/usr/share/doc/system-config-printer-libs-0.7.74.3/pycups-1.9.27/examples/cupstree.py
/usr/bin/python
An included file marked as %doc creates a possible additional dependency in
the package.  Usually, this is not wanted and may be caused by eg. example
scripts with executable bits set included in the package's documentation.


Also,
  1) update buildroot tag to either of value given below
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
%{_tmppath}/%{name}-%{version}-%{release}-root
 
  2) use %defattr(-,root,root,-)

  3) preserve timestamps by using -p option 
    eg. cp -p or install -p.
    

Update package. Better to provide new SPEC and SRPM links for this package
before actually committing in CVS.

Comment 2 Tim Waugh 2007-09-28 17:07:42 UTC
I don't think /etc/xdg/autostart/redhat-print-applet.desktop should be marked as
a config file.  Other files in that directory are not marked as config files.

/usr/share/system-config-printer/cupshelpers.py: fixed upstream, will be in the
next release (0.7.74.4).

system-config-printer-libs.i386: W: doc-file-dependency
Not an extra dependendency, and it's a useful example file to have executable.

http://twaugh.fedorapeople.org/system-config-printer/system-config-printer-0.7.74.3-1.0.0.1.fc8.src.rpm
http://twaugh.fedorapeople.org/system-config-printer/system-config-printer.spec

Comment 3 Parag AN(पराग) 2007-10-01 07:18:18 UTC
With new updated SRPM,
rpmlint on RPM gave me
--------------------------------------------------
system-config-printer.i386: W: non-conffile-in-etc
/etc/xdg/autostart/redhat-print-applet.desktop
==> This can be ignored.

system-config-printer.i386: E: no-binary
==> ok as we need system-config-printer-libs as i386 package and we can't use
noarch type then in SPEC file.

system-config-printer.i386: W: conffile-without-noreplace-flag
/etc/security/console.apps/system-config-printer
===> Is this(noreplace) not needed?

system-config-printer.i386: W: dangerous-command-in-%post rm
==> Do you have any other workaround to avoid using rm?

rpmlint on --libs RPM gave me
---------------------------------------------------------
system-config-printer-libs.i386: W: spurious-executable-perm
/usr/share/doc/system-config-printer-libs-0.7.74.3/pycups-1.9.27/examples/cupstree.py
===> any reason to have this as valid. I saw most of packages installing example
 files with 0644 mode.

system-config-printer-libs.i386: E: non-executable-script
/usr/share/system-config-printer/cupshelpers.py 0644
===> This MUST be solved. As you said its upstream. Can you provide a new SRPM
with upstream fix?

system-config-printer-libs.i386: W: conffile-without-noreplace-flag
/etc/dbus-1/system.d/newprinternotification.conf
===> Is this(noreplace) not needed?

system-config-printer-libs.i386: W: doc-file-dependency
/usr/share/doc/system-config-printer-libs-0.7.74.3/pycups-1.9.27/examples/cupstree.py
/usr/bin/python
===> ==> But user can use this with python command also right?


Comment 4 Tim Waugh 2007-10-02 17:04:04 UTC
system-config-printer.i386: W: conffile-without-noreplace-flag
/etc/security/console.apps/system-config-printer
===> Is this(noreplace) not needed?

I copied that from some other package that places a file in that directory. 
Changed.

system-config-printer.i386: W: dangerous-command-in-%post rm
==> Do you have any other workaround to avoid using rm?

The rm is necessary (it removes a cache file), and not dangerous.

system-config-printer-libs.i386: W: spurious-executable-perm
/usr/share/doc/system-config-printer-libs-0.7.74.3/pycups-1.9.27/examples/cupstree.py
===> any reason to have this as valid. I saw most of packages installing example
 files with 0644 mode.

Okay, changed.

system-config-printer-libs.i386: E: non-executable-script
/usr/share/system-config-printer/cupshelpers.py 0644
===> This MUST be solved. As you said its upstream. Can you provide a new SRPM
with upstream fix?

New upstream packaged.

system-config-printer-libs.i386: W: conffile-without-noreplace-flag
/etc/dbus-1/system.d/newprinternotification.conf
===> Is this(noreplace) not needed?

Copied from another package that places a file in that directory.  Changed,
we'll see if anyone complains.

http://twaugh.fedorapeople.org/system-config-printer/system-config-printer-0.7.74.4-1.fc8.src.rpm
http://twaugh.fedorapeople.org/system-config-printer/system-config-printer.spec

Comment 5 Parag AN(पराग) 2007-10-03 04:22:56 UTC
1) I don't see any use of adding update-desktop-database as no MimeKey specified
in any of installed desktop files.
check
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-de6770dd9867fcd085a73a4700f6bcd0d10294ef

2) macro usage is not consistent. 
check
http://fedoraproject.org/wiki/Packaging/Guidelines#head-f3d77b27a5d29dfc1f5600ef3fc836f2e317badf

Rest looks ok.

Comment 7 Parag AN(पराग) 2007-10-04 05:37:25 UTC
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM but Not for RPM.
system-config-printer.i386: W: non-conffile-in-etc
/etc/xdg/autostart/redhat-print-applet.desktop
system-config-printer.i386: E: no-binary
These messages can be ignored for this package.
system-config-printer.i386: W: no-version-in-last-changelog
system-config-printer-libs.i386: W: no-version-in-last-changelog
==> These can be corrected at time of cvs import.
+ source files match upstream.
20b18338b46531b1b28602deb44aa07d  pycups-1.9.27.tar.bz2
a332f8598c0a4afcc72123856733e273  system-config-printer-0.7.74.4.tar.bz2
b265be98565333de000309e14deea408  system-config-printer.console
f5be82be51135cf46de5955695f67c0d  system-config-printer.pam
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ Compiler flags are honoured correctly.
+ defattr usage is correct.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists but --libs exists.
+ no .la files.
+ translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Desktop files are handled correctly in Makefile only so no need to have
dekstop-file-install called in SPEC.
+ package system-config-printer-0.7.74.4-1.0.0.1.fc8 ->
  Provides: config(system-config-printer) = 0.7.74.4-1.0.0.1.fc8
desktop-printing = 0.20-7.fc7 system-config-printer-gui = 0.6.152
  Requires: /bin/sh /usr/bin/env /usr/bin/python config(system-config-printer) =
0.7.74.4-1.0.0.1.fc8 dbus-x11 desktop-file-utils >= 0.2.92 pirut pygobject2
pygtk2 >= 2.4.0 pygtk2-libglade system-config-printer-libs =
0.7.74.4-1.0.0.1.fc8 usermode >= 1.37
+ package  system-config-printer-libs-0.7.74.4-1.0.0.1.fc8 ->
  Provides: config(system-config-printer-libs) = 0.7.74.4-1.0.0.1.fc8 cups.so
pycups = 1.9.27
  Requires: /usr/bin/env config(system-config-printer-libs) =
0.7.74.4-1.0.0.1.fc8 foomatic libc.so.6 libc.so.6(GLIBC_2.0)
libc.so.6(GLIBC_2.1) libc.so.6(GLIBC_2.1.3) libc.so.6(GLIBC_2.2)
libc.so.6(GLIBC_2.3) libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) libcups.so.2
libpthread.so.0 libpthread.so.0(GLIBC_2.0) libpython2.5.so.1.0 python
python(abi) = 2.5 rtld(GNU_HASH)
+ GUI app.
APPROVED.


Remember->
rpmlint complained some more extra messages now
system-config-printer.i386: W: no-version-in-last-changelog
system-config-printer-libs.i386: W: no-version-in-last-changelog
==> These can be corrected at time of cvs import.


Comment 8 Tim Waugh 2008-03-25 09:33:52 UTC
Package Change Request
======================
Package Name: system-config-printer
New Branches: F-9
Early branch request for F-9.
https://www.redhat.com/archives/fedora-devel-list/2008-March/msg02041.html

Comment 9 Kevin Fenzi 2008-03-25 16:38:17 UTC
cvs done.