Bug 226465

Preliminary Review:-

rpmlint on SRPM and RPM gave me
-------------------------------------
system-config-nfs.src:18: W: unversioned-explicit-obsoletes redhat-config-nfs
system-config-nfs.src: W: invalid-license GPL
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/propertiesWindow.py
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/system-config-nfs.glade
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/nfsServer.py
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/nfsExports.py
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/nfsServerSettings.py
system-config-nfs.noarch: E: script-without-shebang
/usr/share/system-config-nfs/mainWindow.py
system-config-nfs.noarch: W: incoherent-version-in-changelog 1.3.27 1.3.27-1.fc8
system-config-nfs.noarch: W: invalid-license GPL
system-config-nfs.noarch: W: obsolete-not-provided redhat-config-nfs
system-config-nfs.noarch: W: conffile-without-noreplace-flag
/etc/pam.d/system-config-nfs
system-config-nfs.noarch: W: conffile-without-noreplace-flag
/etc/security/console.apps/system-config-nfs
--------------------------------------------------

also,
  good to use %defattr(-,root,root,-)

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

Using xdg-open would be better than htmlview:

http://www.redhat.com/archives/fedora-devel-list/2007-October/msg00025.html
http://scop.fedorapeople.org/patches/xdg-utils/system-config-nfs.patch

(In reply to comment #1)
> Preliminary Review:-
> 
> rpmlint on SRPM and RPM gave me
> -------------------------------------
> system-config-nfs.src:18: W: unversioned-explicit-obsoletes redhat-config-nfs

added version

> system-config-nfs.src: W: invalid-license GPL

checked and changed to GPLv2+

> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/propertiesWindow.py
> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/system-config-nfs.glade
> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/nfsServer.py
> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/nfsExports.py
> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/nfsServerSettings.py
> system-config-nfs.noarch: E: script-without-shebang
> /usr/share/system-config-nfs/mainWindow.py

made files be installed without executable bits set unless necessary

> system-config-nfs.noarch: W: incoherent-version-in-changelog 1.3.27 1.3.27-1.fc8

added (non-dist-tagged) releases to changelog entries

> system-config-nfs.noarch: W: invalid-license GPL
> system-config-nfs.noarch: W: obsolete-not-provided redhat-config-nfs

idem

> system-config-nfs.noarch: W: conffile-without-noreplace-flag
> /etc/pam.d/system-config-nfs
> system-config-nfs.noarch: W: conffile-without-noreplace-flag
> /etc/security/console.apps/system-config-nfs

aded %config(noreplace)

> --------------------------------------------------
> 
> also,
>   good to use %defattr(-,root,root,-)

changed

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

Package is building right now (this is a merge review).


(In reply to comment #2)
> Using xdg-open would be better than htmlview:
> 
> http://www.redhat.com/archives/fedora-devel-list/2007-October/msg00025.html
> http://scop.fedorapeople.org/patches/xdg-utils/system-config-nfs.patch

xdg-open and htmlview are tried in this order, changed requirements suitably for
Fedora >= 7.

NB: Ville, is using xdg-open instead of htmlview something required by the
packaging guidelines? Otherwise this would have warranted its own bug.

Forgot this: the version building is system-config-nfs-1.3.29-1.fc8.

No, xdg-open is not packaging guidelines material at the moment.  Nevertheless
it's something I'd point out during a review of any package where applicable. 
Thanks for using it.

Thanks for updates.
Current CVS package build fine in mock i386 but rpmlint gave me
system-config-nfs.noarch: E: non-executable-script
/usr/share/system-config-nfs/exports.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-nfs.noarch: W: obsolete-not-provided redhat-config-nfs
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.

Also,
 Is there any need to use following in SPEC?
   ExclusiveOS: Linux

I am bit confused whether we really need 
Requires(post): hicolor-icon-theme
Requires(postun): hicolor-icon-theme

because
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda
suggests no such usage of Requires needed.

(In reply to comment #7)
> Thanks for updates.
> Current CVS package build fine in mock i386 but rpmlint gave me
> system-config-nfs.noarch: E: non-executable-script
> /usr/share/system-config-nfs/exports.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.

removed exports.py (very old test script which should have been removed for a
long time)

> system-config-nfs.noarch: W: obsolete-not-provided redhat-config-nfs
> 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.

This is the case.

> Also,
>  Is there any need to use following in SPEC?
>    ExclusiveOS: Linux

Not that I knew. Removed.

(In reply to comment #8)
> I am bit confused whether we really need 
> Requires(post): hicolor-icon-theme
> Requires(postun): hicolor-icon-theme
> 
> because
>
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5735e8477bdd569ad73ea2c49bda
> suggests no such usage of Requires needed.

removed hicolor-icon-theme requirement, calling gtk-update-icon-cache with full path

system-config-nfs-1.3.30-1.fc8 is building with these changes

warning: File listed twice:
/usr/share/system-config-nfs/pixmaps/system-config-nfs.png

You need to remove following line from %files in SPEC
%{_datadir}/%{name}/pixmaps/system-config-nfs.png

(In reply to comment #10)
> warning: File listed twice:
> /usr/share/system-config-nfs/pixmaps/system-config-nfs.png
> 
> You need to remove following line from %files in SPEC
> %{_datadir}/%{name}/pixmaps/system-config-nfs.png

fixed.

Note that I found that the hicolor-icon-theme requirement is needed because I
ship files beneath /usr/share/icons/hicolor, see:

http://fedoraproject.org/wiki/Packaging/Guidelines#head-a5931a7372c4a00065713430984fa5875513e6d4

I'll re-add it for this and my other packages that have such icons like this
("uncolored", i.e. without pre/post/preun/postun):

Requires: hicolor-icon-theme

system-config-nfs-1.3.31-1.fc8 is building with these changes right now

Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM but Not for RPM.
system-config-nfs.noarch: W: obsolete-not-provided redhat-config-nfs
These messages can be ignored for this package.
+ source files match upstream.
8dae249bcc43a43603f3386d1761b786  system-config-nfs-1.3.32.tar.bz2
+ 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.
+ 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.
+ no .la files.
+ translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ gtk-update-icon-cache scriptlets are used.
+ Desktop files are handled correctly.
+ package system-config-nfs-1.3.32-1.fc8 ->
  Provides: config(system-config-nfs) = 1.3.32-1.fc8
  Requires: /usr/bin/python config(system-config-nfs) = 1.3.32-1.fc8
hicolor-icon-theme nfs-utils pygtk2 pygtk2-libglade python >= 2.0 rhpl usermode
>= 1.36 xdg-utils
+ GUI app.

APPROVED.

Closing this review as Approved package is already built in rawhide.

system-config-nfs-1.3.33-1.fc7 has been pushed to the Fedora 7 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 system-config-nfs'

system-config-nfs-1.3.33-1.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.