[Bug 429006] Review Request: gnome-subtitles - Subtitle editor for Gnome

Mon Mar 24 12:03:39 UTC 2008

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

Summary: Review Request: gnome-subtitles - Subtitle editor for Gnome

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

------- Additional Comments From akahl at iconmobile.com  2008-03-24 08:03 EST -------
Package Review
==============

* rpmlint output:
  gnome-subtitles.x86_64: W: non-conffile-in-etc
/etc/gconf/schemas/gnome-subtitles.schemas
OK according to
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-6c2101d8f810cc95c677c8c27f43573b0bc23cb1

-------------------------------------------------------------------------------
  gnome-subtitles.x86_64: W: unstripped-binary-or-object
  /usr/lib64/gnome-subtitles/libgstreamer_playbin.so
  gnome-subtitles-debuginfo.x86_64: E: empty-debuginfo-package

The first one is caused by the fact the shared object, the assemblies and the
mono executable are installed as *_DATA defined in Makefile.am and
src/Makefile.am:
-rw-r--r-- /usr/lib64/gnome-subtitles/GStreamerPlaybin.dll
-rw-r--r-- /usr/lib64/gnome-subtitles/gnome-subtitles.exe
-rw-r--r-- /usr/lib64/gnome-subtitles/libgstreamer_playbin.so
-rw-r--r-- /usr/lib64/gnome-subtitles/sublib.dll

Thus find-debuginfo.sh cannot find anything since it searches for executable
files after install only which causes the second error, an empty debuginfo
package.
There is an example of how to properly handle mono applications with
automake at
http://www.mono-project.com/Guidelines:Application_Deployment#Auto-tools
where they use *_SCRIPTS to have automake properly handle mono
executables. I've attached a patch made according to the guide that
fixes the issue, you can use it if you want, just an additional
autoreconf call needs to be added in build before configure.
Please also inform upstream about the issue and offer them the patch.
-------------------------------------------------------------------------------

* The package is named according to the Package Naming Guidelines
* The spec file name matches the base package
* Fedora approved license: GPLv2+
* License field in the package spec file matches the actual license
* COPYING included
* The spec file is written in American English
* The spec file is legible
* The sources used to build the package matches the upstream source:
SHA1 0c484af7934a6a2fbed41139221489fbcd687000  gnome-subtitles-0.7.2.tar.gz
* Builds locally on dist-f8 x86_64
* BuildRequires look sane
* Proper locale handling
-------------------------------------------------------------------------------
E Package does not build for ppc64:
http://koji.fedoraproject.org/koji/taskinfo?taskID=527292
mono-devel is not available for ppc64. 
- Please add an ExcludeArch: ppc64
- After this package has been approved, please open a new report (with component
  gnome-subtitles) which tells that gnome-subtitles does not support ppc64 and
  make the bug block FE-ExcludeArch-ppc64.
--------------------------------------------------------------------------------
* No libraries in dynamic linker's default paths
* No relocatable libraries
* Package owns all directories it creates
* No duplicates in %files
E File permissions are not sane, see above
* %clean starts with build root clean
-------------------------------------------------------------------------------
E Consistent use of macros
Please replace:
- rm with %{__rm}
- make with %{__make}
-------------------------------------------------------------------------------
* The package contains code
* Documentation not large, no -doc subpackage needed
* No header files
* No static libs
* No pkgconfig files
* No suffixed library files
* No -devel package
* No libtool archives
* Desktop file handled properly (BuildRequires, %install, vendor id)
* Package does not own files or directories already owned by other packages
* %install starts with build root clean
* All filenames are valid UTF-8

Additional Checks
=================
* Latest version is being packaged: 0.7.2
* Dist tag is present                                                          

* Group tag is valid                                                           

* Build root is correct:                                                       

%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)                 

* Scriptlets look sane                                                         

E Does not build in mock (due to ppc64, see above)
* GConf schema files handled properly:
  - %configure --disable-schemas-install present
  - GConf2 in Requires({pre,post,preun})
  - Scriplets in %pre, %post and %preun present
* Scrollkeeper documentation handled properly:
  - scrollkeeper in BuildRequires, Requries(post{,un})
  - Scriplets in %post and %postun present
* Requires is complete, all dependencies resolved automatically

Mono-specific Checks
====================
* Assembly location is correct:
%{_libdir}/%{name}
* No global assemblies are registered with gacutil
* Package builds without source version of mono
* No pkgconfig files
E debuginfo error (see above)
* No prebuilt assemblies distributed
* No library distributed
W Distributing .DLLs from other projects
-------------------------------------------------------------------------------
gnome-subtitles includes code from
SubLib (http://sublib.sf.net)
NCharDet (http://www.conceptdevelopment.net/Localization/NCharDet)
Fuse Media Centre (http://sf.net/projects/fusemc)

If I get
http://fedoraproject.org/wiki/Packaging/Mono#head-6635c736011474fcbbc900285de0d9b0977cde1b
right, this is considered poor practice, if not even a
blocker. However since upstream is not building those libraries into
separate assemblies but directly into in their source, thus taking
over responsibility to fix everything themselves, I consider this to
be OK.
-------------------------------------------------------------------------------
* _libdir not redefined
* Package not noarch

-- 
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, or are watching someone who is.