[Bug 224458] Review Request: libsilc (dependency of gaim)

[bugzilla at redhat.com]

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: libsilc (dependency of gaim)


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


tibbs at math.uh.edu changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stu at nosnilmot.com
         AssignedTo|nobody at fedoraproject.org    |tibbs at math.uh.edu
               Flag|                            |fedora-review?




------- Additional Comments From tibbs at math.uh.edu  2008-06-03 18:20 EST -------
CC'ing Stu, who seems to be the current maintainer of this package.

I think this does still need a review, because it's still required by libpurple
which in turn is needed by a pile of things.  I'll take a look.

First off, let's look at the rpmlint complaints:
  libsilc-devel.x86_64: W: no-documentation
Yeah, the documentation is in the -doc package.

  libsilc.x86_64: W: unused-direct-shlib-dependency 
   /usr/lib64/libsilcclient-1.1.so.2.0.1 /lib64/libdl.so.2
Not a huge deal, you might be able to fix this with the libtool tweak from
http://fedoraproject.org/wiki/PackageMaintainers/CommonRpmlintIssues#unused-direct-shlib-dependency
if it bothers you.

Then there are 249 of these:
  libsilc.x86_64: W: undefined-non-weak-symbol 
   /usr/lib64/libsilcclient-1.1.so.2.0.1 silc_hash_ptr
  libsilc.x86_64: W: undefined-non-weak-symbol 
   /usr/lib64/libsilcclient-1.1.so.2.0.1 silc_packet_set_keys
It seems that libsilcclient doesn't actually link against libsilc, which would
seem to be a bug to me.  Am I missing something?

Anyway, those don't stop me from doing a full review.

The way dependencies are filtered is a bit fragile because it ignores any
rpmbuild customizations involving the dependency generator.  I don't think it's
a significant issue, but something akin to what many Perl modules which
generates the filter on the fly might work better.  There's some discussion at 
http://fedoraproject.org/wiki/Packaging/Perl#Filtering_Requires:_and_Provides
In any case, could you add a comment to the spec with a note on why you need to
filter the dependencies?

>From the README file I'd expect to see silc and silcd binaries somewhere, and
something other than libsilc packages to hold them, but the source doesn't seem
to include any actual applications.  I guess if it grew some then the base
package would be misnamed but we don't usually worry about naming issues in
merge reviews unless there are egregious issues.

It might be nice to clarify the meaning of SILC in %description.   Currently you
either have to install the package and read the docs or search the network to
figure out just what this package is supposed to do.

* source files match upstream:
   fcfb34cd0bb858a711ba0af4a2fce60ae64e485b35c67dcdad764cc6a5feac1f  
   silc-toolkit-1.1.7.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
? description could use some clarification.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
? rpmlint has several possibly valid complaints.
* final provides and requires are sane:
  libsilc-1.1.7-1.fc10.x86_64.rpm
   libsilc-1.1.so.2()(64bit)
   libsilcclient-1.1.so.2()(64bit)
   libsilc = 1.1.7-1.fc10
  =
   /sbin/ldconfig

  libsilc-devel-1.1.7-1.fc10.x86_64.rpm
   pkgconfig(silc) = 1.1.7
   libsilc-devel = 1.1.7-1.fc10
  =
   libsilc = 1.1.7-1.fc10
   pkgconfig

  libsilc-doc-1.1.7-1.fc10.x86_64.rpm
   libsilc-doc = 1.1.7-1.fc10
  =

* %check is not present; no test suite upstream.
* shared libraries present; ldconfig called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets OK (ldconfig)
* code, not content.
* %docs are in a -doc subpackage.
* %docs are not necessary for the proper functioning of the package.
* headers are in the -devel subpackage.
* pkgconfig files are in the -devel subpackage.
* no static libraries.
* no libtool .la files.

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