[Bug 459751] Review Request: osgGtk - Gtk and Gtkmm widgets for OpenSceneGraph

Fri Feb 27 05:16:02 UTC 2009

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=459751

--- Comment #4 from Debarshi Ray <debarshi.ray at gmail.com>  2009-02-27 00:15:57 EDT ---
MUST Items: 

OK - rpmlint is clean

xx - does not follow Naming Guidelines
    + It would be better to name this package 'osggtk' instead of 'osgGtk'.
      The tarball is named 'osggtk' and Fedora's other OpenSceneGraph
      packages are named 'osgcal' and 'osgal'. Therefore having a completely
      lower-case name would be more consistent. But since you are also the
      upstream author, I would be willing to listen to your rationale for
      preferring otherwise. :-)

OK - spec file is named as %{name}.spec

xx - package does not meet Packaging Guidelines
    + Although the current Source0 URL works, according to
      https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
      the Source0 tag should have 'downloads' and not 'download'.
    + Even Fedora 9 has OpenSceneGraph-devel >= 2.2.0 for sometime now.
      According to
      https://fedoraproject.org/wiki/Packaging:Guidelines#Requires no need to
      add it if its not really required.
    + Why is there a runtime dependency on 'OpenSceneGraph-devel >= 2.2.0' for
      osgGtkmm-devel? If it is because the osgGtkmm header files need the
      OpenSceneGraph headers, then the osgGtkmm-1.0.pc should mention it.
    + The osgGtkmm sub-package does not explicitly require osgGtk. Now I can
      understand that RPM is going to autogenerate the dependency on the
      shared library, but

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package
      does that "subpackages other than -devel should also require the base
      package using a fully versioned dependency". I find that you have done
      so for all the other sub-packages, but only not for osgGtkmm. I confess
      that I do not know the rationale behind this guideline. In the meantime,
      I will try to find out the reason.
    + You could consider using '%{__install} -p' consistently through the
      %install stanza.

OK - license meets Licensing Guidelines

xx - License field does not meet actual license
    + Going by the license notices in the source code:
      (i)  Makefile.am, examples/Makefile.am, osgGtk/Makefile.am,
           osgGtkmm/Makefile.am are under LGPLv3.
      (ii) the others are under GPLv3.
      Since you are the upstream author, for the Makefile.ams please consider
      marking them as GPLv3 or use the license notices in the autogenerated
      Makefile.ins.

OK - upstream license file included in %doc
OK - spec file uses American English
OK - spec file is legible
OK - sources match upstream sources
OK - package builds successfully
OK - ExcludeArch not needed

xx - redundant and extra build dependencies listed
    + pkgconfig is brought in by all the -devel packages providing *.pc files

OK - no locales
OK - %post and %postun invoke ldconfig
OK - package is not relocatable

xx - file and directory ownership
    + The -devel and osgGtkmm-devel sub-packages should have
      'Requires: gtk-doc' as it needs /usr/share/gtk-doc.

OK - no duplicates in %file
OK - file permissions set properly
OK - %clean present

OK - macros used consistently
    + Both %{name} and osgGtk are used. You could consider using %{name}
      throughout.
    + Using both %{buildroot} and $RPM_BUILD_ROOT is looked down upon. See

https://fedoraproject.org/wiki/Packaging:Guidelines#Using_.25.7Bbuildroot.7D_and_.25.7Boptflags.7D_vs_.24RPM_BUILD_ROOT_and_.24RPM_OPT_FLAGS

OK - contains code and permissable content
OK - -doc is not needed
OK - contents of %doc does not affect the runtime
OK - header files in -devel
OK - no static libraries

OK - devel has *.pc file and requires pkgconfig
    + Even though
      https://fedoraproject.org/wiki/Packaging:Guidelines#Pkgconfig_Files
      lays down that the -devel sub-package must have 'Requires: pkgconfig' if
      it includes a *.pc file, Fedora 11 onwards rpm-4.6 autogenerates this
      runtime dependency and the ones on the other -devel subpackages mentioned
      in the *.pc file. So please consider removing them from Fedora 11 and
      onwards using a %if %endif pair.
      In osgGtk-devel:
      Requires: gtk2-devel
      Requires: gtkglext-devel
      Requires: OpenSceneGraph-devel >= 2.2.0
      Requires: pkgconfig
      In osgGtkmm-devel:
      Requires: gtkmm24-devel
      Requires: gtkglextmm-devel
      Requires: pkgconfig

OK - library files without suffix in -devel
OK - -devel requires base package
OK - no libtool archives

xx - %{name}.desktop file is invalid
    + According to https://fedoraproject.org/wiki/Packaging/Guidelines#desktop
      desktop-file-validate must be run on the .desktop file, and it says:
      [rishi at freebook osggtk-0.1.3]$ desktop-file-validate osgviewerGtk.desktop
      osgviewerGtk.desktop: warning: key "Encoding" in group "Desktop Entry" is
deprecated
      [rishi at freebook osggtk-0.1.3]$ desktop-file-validate
osgviewerGtkmm.desktop 
      osgviewerGtkmm.desktop: warning: key "Encoding" in group "Desktop Entry"
is deprecated
      [rishi at freebook osggtk-0.1.3]$ 
      The key "Encoding" is deprecated on all supported versions of Fedora.
      Please consider removing it.

OK - does not own files or directories owned by other packages
OK - buildroot correctly prepped
OK - all file names valid UTF-8

SHOULD Items:

OK - upstream provides license text
xx - no translations for description and summary
OK - package builds in mock successfully
OK - package builds on all supported architectures
OK - package functions as expected

xx - scriptlets are not sane
    + Would be good if you could use the Gtk+ icon cache scripts from

https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#GTK.2B_icon_cache

xx - subpackages other than -devel do not use fully versioned dependency
    + The osgGtkmm subpackage does not use a fully versioned dependency on
      osgGtk.

OK - pkgconfig files in -devel
OK - no file dependencies

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.