[Bug 223586] Review Request: strigi - A desktop search program for KDE

Fri May 4 23:40:20 UTC 2007

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: strigi - A desktop search program for KDE

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

------- Additional Comments From Kevin at tigcc.ticalc.org  2007-05-04 19:40 EST -------
MUST Items: 
+ rpmlint output OK (see comment #22)
+ named and versioned according to the Package Naming Guidelines
+ spec file name matches base package name
+ Packaging Guidelines:
  + License LGPL OK
  + No known patent problems
  + No emulator, no firmware, no binary-only or prebuilt components
  + Complies with the FHS
  + proper changelog, tags, BuildRoot, Requires, BuildRequires, Summary, 
Description
  + no non-UTF-8 characters
  + relevant documentation is included
  + RPM_OPT_FLAGS are used
  + debuginfo package is valid
  + no static libraries nor .la files
  + no duplicated system libraries
  + no rpaths, at least on i386 (I ran strings on a few files)
  + no configuration files, so %config guideline doesn't apply
  + no init scripts, so init script guideline doesn't apply
  + desktop file is valid and installed correctly
  + no timestamp-clobbering file commands
  + _smp_mflags used
  + scriptlets are valid
  + not a web application, so web application guideline doesn't apply
+ complies with all the legal guidelines
MUST FIX: The License tag says "GPL", but the license is actually LGPL 
according to COPYING.
+ COPYING included as %doc
+ spec file written in American English
+ spec file is legible
+ source matches upstream:
  MD5SUM: b976b4f3cf451fc53cd773c338d78994
  SHA1SUM: 0bc32ab1668492ad44d061fd4d5753ec0344cb41
+ builds on at least one arch (FC6 i386)
+ no known non-working arches, so no ExcludeArch needed
+ all build dependencies listed in CMakeLists.txt are listed in BuildRequires, 
however:
SHOULD FIX: openssl-devel is no longer required, please remove BR
SHOULD FIX: expat-devel is not actually required either: CMakeLists.txt 
requires libxml2-devel, then goes on to require expat-devel OR libxml2-devel, 
so this actually means libxml2-devel is required and expat-devel is not really 
used (and rpm -q --requires strigi confirms that)
+ no translations in original tarball, so translation/locale guidelines don't 
apply
+ ldconfig correctly called in %post and %postun
+ package not relocatable
+ ownership correct (owns package-specific directories, doesn't own directories 
owned by another package) except for the following:
MUST FIX: strigiclient.desktop is installed into /usr/share/applications/kde. 
This is owned by kdelibs, which is not a direct or indirect dependency of 
strigi. So this has to be fixed one way or the other. As Strigi is in no way a 
KDE app (strigiclient is a Qt 4 app), please simply install the .desktop file 
into /usr/share/applications instead.
+ no duplicate files in %files
+ permissions set properly
+ %clean section present and correct
+ macros used where possible (%cmake macro does not work according to comment 
#18)
+ no non-code content
+ no large documentation files
+ %doc files not required at runtime
+ all header files in -devel
+ no static libraries, so no -static package needed
+ Requires: pkgconfig present
+ /usr/lib/*.so symlinks are correctly in -devel, however:
MUST FIX: The .so files in /usr/lib/strigi are NOT symlinks, they're loadable 
plugins, so they must be in the main package, not the -devel package.
+ -devel requires %{name} = %{version}-%{release} 
+ no .la files
+ the only GUI program in the package, strigiclient, has a .desktop file
+ buildroot is deleted at the beginning of %install
+ all filenames are valid UTF-8

SHOULD Items: 
+ license already included upstream
+ no translations for description and summary provided by upstream
+ package builds in mock (or at least used to) according to comment #1 (I may 
run the current package through mock later, but that's best done once the 
redundant BRs are removed so we can be sure they really were redundant. ;-) )
+ package builds successfully on my live system, and I checked for missing BRs 
(found none)
+ can't test non-i386 architectures, but all arches are supposed to be 
supported by upstream
+ package appears to work
+ scriptlets are sane
+ no subpackages other than -devel, so "Usually, subpackages other than devel 
should require the base package using a fully versioned dependency." is 
irrelevant
+ .pc files are in -devel
+ no file dependencies

Nitpicks:
* The guidelines say the Summary should not end with a period. This, however, 
doesn't apply to the %description. The sentence at the end of your main 
package %description should end with a period. (The %description devel is not a 
complete sentence and thus should not end with a period though.)
* Why did you set strigiclient.desktop to only get shown in KDE?
* "accessesed" should read "accessed".
* This one's a question more than a request to fix something: How is 
strigidaemon supposed to get run? Is this supposed to be done by the clients 
which need it (such as strigiapplet)? The strigiclient in any case doesn't 
offer to start the daemon, it just spits out warnings on the console when run 
without the daemon running, and some operations even crash in that case. Not 
that I personally care, I only need the indexers to build KDE 4 alpha 1 (used 
by the KDE 4 metadata classes), I hate background-indexing daemons, but people 
who actually want to use the daemon will need it started somehow.

Please fix your License tag, install the .desktop file into 
just %{_datadir}/applications, move %{_libdir}/strigi/ from -devel to the main 
package and remove the redundant BRs, then I'll approve this. (Also please 
consider fixing the nitpicks, but those aren't blockers.)

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.