Spec URL: http://miskatonic.cs.nmsu.edu/pub/osgGtk.spec SRPM URL: http://miskatonic.cs.nmsu.edu/pub/osgGtk-0.1.2-1.fc9.src.rpm Description: osgGtk provides widgets that can be used to integrate OpenSceneGraph (OSG) into Gtk+/gtkmm applications.
Just some quick comments - '--vendor fedora' is obsolete for new packages - I think that 'Requires: hicolor-icon-theme' is needed - To make your life easier in the future, change 'Source: http://download.sourceforge.net/osggtk/osggtk-0.1.2.tar.bz2' to 'Source: http://download.sourceforge.net/osggtk/osggtk-%{version}.tar.bz2'. Same in '%setup -q -n osggtk-0.1.2'
> -n osggtk-0.1.2 -n %{name}-%{version} is the default.
Here's an updated spec and srpm: http://miskatonic.cs.nmsu.edu/pub/osgGtk.spec http://miskatonic.cs.nmsu.edu/pub/osgGtk-0.1.3-1.fc10.src.rpm And the changelog: - Improved package summaries and descriptions - Added hicolor-icon-theme to viewer requires - Removed fedora vendor tag for desktop files - Added desktop file validation on install - Fixed license tag - Parameterized prep directory - Fixed unowned directories issue in devel files
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@freebook osggtk-0.1.3]$ desktop-file-validate osgviewerGtk.desktop osgviewerGtk.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated [rishi@freebook osggtk-0.1.3]$ desktop-file-validate osgviewerGtkmm.desktop osgviewerGtkmm.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated [rishi@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
(In reply to comment #4) > MUST Items: > > xx - does not follow Naming Guidelines > I would be willing to listen to your rationale for > preferring otherwise. :-) I had one at one point, but I forget what it was. Must not have been compelling. If I had to guess it was just to remain consistent with the naming of the package, which was named to be consistent with the other OSG libraries like osgDB, osgFX, osgParticle, osgShadow, osgTerrain, et. al. Although I have AC_INIT([osgGtk]) for some reason autoconf insists on creating lowercase tarballs. If I could figure out a way to get it to create tarballs named osgGtk I would. > 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'. Got it changed. > + 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. I would kind of like to keep it in there for EPEL support and in case anyone tries to backport it. > + 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. Ah, fixed it upstream. It was missing the openscenegraph dependency in the .pc file. > + 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. Actually, osgGtkmm doesn't have a dependency on osgGtk, either for libraries or headers. > + You could consider using '%{__install} -p' consistently through the > %install stanza. Got it fixed. > 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. Got it fixed upstream. > xx - redundant and extra build dependencies listed > + pkgconfig is brought in by all the -devel packages providing *.pc files I'd like to leave this one in for now to support backporting and EPEL > xx - file and directory ownership > + The -devel and osgGtkmm-devel sub-packages should have > 'Requires: gtk-doc' as it needs /usr/share/gtk-doc. Added > OK - macros used consistently > + Both %{name} and osgGtk are used. You could consider using %{name} > throughout. I added it to the -devel and -viewer requires > + 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 Got it fixed > 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 > Same as above to support EPEL and backporting. > 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@freebook osggtk-0.1.3]$ desktop-file-validate osgviewerGtk.desktop > osgviewerGtk.desktop: warning: key "Encoding" in group "Desktop Entry" is > deprecated > [rishi@freebook osggtk-0.1.3]$ desktop-file-validate > osgviewerGtkmm.desktop > osgviewerGtkmm.desktop: warning: key "Encoding" in group "Desktop Entry" > is deprecated > [rishi@freebook osggtk-0.1.3]$ > The key "Encoding" is deprecated on all supported versions of Fedora. > Please consider removing it. Got it fixed upstream. > SHOULD Items: > > 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 Got it fixed. > xx - subpackages other than -devel do not use fully versioned dependency > + The osgGtkmm subpackage does not use a fully versioned dependency on > osgGtk. osgGtkmm doesn't have a dependency on osgGtk. Let me know what you think on these items, and then I'll push out a new upstream release and post a new spec/srpm. 1. package name 2. Keeping versioned OSG BuildRequires 3. Keeping pkgconfig requires in -devel I think the rest is either fixed upstream or in the spec.
(In reply to comment #5) > Let me know what you think on these items, and then I'll push out a new > upstream release and post a new spec/srpm. Can we have a fresh Spec/SRPM pair?
(In reply to comment #6) > (In reply to comment #5) > > > Let me know what you think on these items, and then I'll push out a new > > upstream release and post a new spec/srpm. > > Can we have a fresh Spec/SRPM pair? I pushed out a new upstream release that fixes things like the license in the Makefiles, the pkgconfig .pc files, etc. This spec has all the above mentioned items fixed. I'd really like to keep the name at osgGtk to not only match the osg library names, but to also make it easier when I figure out how to get autoconf to name the dist packages osgGtk-0.1.4.tar.bz2. I've tried to keep the case notation consistent since programmers #include<osgGtk.h> or #include<osgGtkmm.h> http://miskatonic.cs.nmsu.edu/pub/osgGtk.spec http://miskatonic.cs.nmsu.edu/pub/osgGtk-0.1.4-1.fc10.src.rpm
Just touching base to make sure you saw that I pushed out a new release with the fixes.
Ping?
pbrobinson's scratch build of gtk-sharp-beans?#bbba0aef7a4bd673006df5b9a85a5763d9c026bc for epel7-archbootstrap and git://pkgs.fedoraproject.org/gtk-sharp-beans?#bbba0aef7a4bd673006df5b9a85a5763d9c026bc failed http://koji.fedoraproject.org/koji/taskinfo?taskID=12037959
pbrobinson's scratch build of gtk-murrine-engine?#b4bddbf235c281d671c99e8008f609b6d4e314d0 for epel7-archbootstrap and git://pkgs.fedoraproject.org/gtk-murrine-engine?#b4bddbf235c281d671c99e8008f609b6d4e314d0 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=12037955
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
Sorry, for having ignored this review for so many years. I no longer have time to work on this. Therefore, I have cleared the fedora-review flag and reset the assignee.
So, Rick are you still interested in packaging this?
This is an automatic action taken by review-stats script. The ticket submitter failed to clear the NEEDINFO flag in a month. As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews we consider this ticket as DEADREVIEW and proceed to close it.