Bug 200348 - Review Request: libgadu - Gadu-Gadu protocol support library
Review Request: libgadu - Gadu-Gadu protocol support library
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Thorsten Leemhuis (ignored mailbox)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2006-07-26 21:32 EDT by Piotr Drąg
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-04 09:53:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Make aclocal shut up (1.72 KB, patch)
2006-08-03 13:58 EDT, Dawid Gajownik
no flags Details | Diff
Fixes for the spec file (3.35 KB, patch)
2006-08-03 14:00 EDT, Dawid Gajownik
no flags Details | Diff

  None (edit)
Description Piotr Drąg 2006-07-26 21:32:04 EDT
Spec URL: http://pmail.pl/~raven/libgadu.spec
SRPM URL: http://pmail.pl/~raven/libgadu-20060726-1.src.rpm
Description:

Hi, it's my first package and I'm looking for review and sponsor. :)

libgadu is intended to make it easy to add Gadu-Gadu communication
support to your software. It's needed by gaim-gadugadu package, which provides support for Gadu-Gadu protocol in Gaim IM client.
Comment 1 Michał Bentkowski 2006-07-27 07:21:05 EDT
Hi :) Thanks for packaging it, I wanted to do it, too :)
I'm not a sponsor, but I can give you some advices:
 * as far as I know, license of libgadu is only LGPL
 * you should include COPYING file to %%doc, and also, include more docs
in the main package, not in -devel
 * I don't think using of libgadu-current.tar.gz is good. This file changes
every day and md5sum of upstream source won't match md5 of source included
in SRPM
 * you should change doc files charset to utf8 (by iconv)
 * change version in changelog entry. According to changelog inital release is
20060717-1, but in fact it is 20060726-1
Comment 2 Piotr Drąg 2006-07-27 11:02:03 EDT
Thanks for the advicies:

>* as far as I know, license of libgadu is only LGPL

See it's official site. :)

>* you should include COPYING file to %%doc, and also, include more docs
in the main package, not in -devel

COPYING file is not included in upstream tarball, and more docs are not useful
(it's related only to ekg program)

>* I don't think using of libgadu-current.tar.gz is good. This file changes
every day and md5sum of upstream source won't match md5 of source included
in SRPM

Have you any idea?

>* you should change doc files charset to utf8 (by iconv)

OK, I'll do it, but please help me, I'm new to RPM.

>* change version in changelog entry. According to changelog inital release is
20060717-1, but in fact it is 20060726-1

Ops, my ommision, I will correct it in next release.
Comment 3 Piotr Drąg 2006-07-27 12:09:51 EDT
Thanks for the advices. I've updated spec.

Spec URL: http://pmail.pl/~raven/libgadu.spec
SRPM URL: http://pmail.pl/~raven/libgadu-20060726-1.src.rpm
Comment 4 Piotr Drąg 2006-07-27 12:29:07 EDT
Sorry, I didn't remember to add changelog entry and bump release.

Spec URL: http://pmail.pl/~raven/libgadu.spec
SRPM URL: http://pmail.pl/~raven/libgadu-20060726-2.src.rpm
Comment 5 Chris Weyl 2006-07-28 00:34:04 EDT
(In reply to comment #2)
> >* I don't think using of libgadu-current.tar.gz is good. This file changes
> every day and md5sum of upstream source won't match md5 of source included
> in SRPM
> 
> Have you any idea?

I tried looking for versioned downloads -- if I spoke any Polish I suspect I
might have had better luck.  But Michał is right; this poses a "challenge" for
the review.

> >* you should change doc files charset to utf8 (by iconv)
> 
> OK, I'll do it, but please help me, I'm new to RPM.

You can use a tool called iconv.  See, e.g., "file-not-utf8" at
http://www.fedoraproject.org/wiki/PackagingTips/Perl

Comment 6 Michał Bentkowski 2006-07-28 06:36:12 EDT
(In reply to comment #5)
> I tried looking for versioned downloads -- if I spoke any Polish I suspect I
> might have had better luck.  But Michał is right; this poses a "challenge" for
> the review.

I speak Polish very well (much better than English ;)) but it looks bad.
Official site doesn't include versioned downloads. Maybe, he should copy
tarball to his own server?

> You can use a tool called iconv.  See, e.g., "file-not-utf8" at
> http://www.fedoraproject.org/wiki/PackagingTips/Perl

It has already done in new spec.

Comment 7 Chris Weyl 2006-07-28 11:19:38 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > I tried looking for versioned downloads -- if I spoke any Polish I suspect I
> > might have had better luck.  But Michał is right; this poses a "challenge" for
> > the review.
> 
> I speak Polish very well (much better than English ;)) but it looks bad.
> Official site doesn't include versioned downloads. Maybe, he should copy
> tarball to his own server?

Hrm.  Well, one thing to try would be to email upstream and ask if they could
start keeping versioned packages somewhere.  The viability of that depends on
how often the "current" tarball changes, I suspect.  But it is critical to be
able to point somewhere and say "this package is version foo" authoritatively.

If they don't respond, well, then we figure out Plan B :)
Comment 8 Dawid Gajownik 2006-08-03 13:58:33 EDT
Created attachment 133570 [details]
Make aclocal shut up
Comment 9 Dawid Gajownik 2006-08-03 14:00:16 EDT
Created attachment 133571 [details]
Fixes for the spec file
Comment 10 Dawid Gajownik 2006-08-03 14:14:21 EDT
I'm not a sponsor so this review is informal:

* You should pass option --disable-bind or --enable-bind to configure script.
Without it rebuild of a package in different build environment can lead to
different dependencies/features of final RPM.
* Debian package adds option --enable-pthread. IMHO it's worth to enable it, too.
* Add CFLAGS_LIBGADU="$CFLAGS" to configure script. Right now package is being
built without gcc optimizations/security features.
* Mark proper files as %lang(pl). Please see
http://www.redhat.com/archives/fedora-extras-list/2006-August/msg00090.html for
more information
* Replace %files with %%files. Hint: you should run rpmlint on srpms, too:

[rpm-build@X RPMS]$ rpmlint ../SRPMS/libgadu-20060726-2.src.rpm
W: libgadu macro-in-%changelog files
[rpm-build@X RPMS]$

I have changed $RPM_BUILD_ROOT to %{buildroot} because it's shorter. You can
revert this change if you don't like it.
Comment 11 Piotr Drąg 2006-08-03 15:59:40 EDT
Thanks!

Spec URL: http://pmail.pl/~raven/libgadu.spec
SRPM URL: http://pmail.pl/~raven/libgadu-20060802-1.src.rpm

Upstream didn't agree to release standalone libgadu tarballs. :/ Maybe I could
use CVS? Otherwise I have to package main ekg program (which has problems with
UTF-8), with libgadu as subpackage. :(
Comment 12 Michał Bentkowski 2006-08-03 16:24:12 EDT
(In reply to comment #11)
> Upstream didn't agree to release standalone libgadu tarballs. :/ Maybe I could
> use CVS? Otherwise I have to package main ekg program (which has problems with
> UTF-8), with libgadu as subpackage. :(

I don't think so. If your reviewer agrees, you'll can copy upstream source
to your own server. I think this is the best solution.
Comment 13 Chris Weyl 2006-08-03 16:53:16 EDT
(In reply to comment #12)
> (In reply to comment #11)
> > Upstream didn't agree to release standalone libgadu tarballs. :/ Maybe I could
> > use CVS? Otherwise I have to package main ekg program (which has problems with
> > UTF-8), with libgadu as subpackage. :(
> 
> I don't think so. If your reviewer agrees, you'll can copy upstream source
> to your own server. I think this is the best solution.

I don't know if there's precedent, but given the situation I don't believe
hosting a dated copy of the tarball on a different host is unreasonable.

I'd make sure that the tarball itself is dated tho -- e.g.
libgadu-20060801.tar.gz or somesuch.
Comment 14 Michał Bentkowski 2006-08-05 11:05:59 EDT
Rpmlint output to the newest srpm:
W: libgadu macro-in-%changelog configure
W: libgadu macro-in-%changelog lang

You have to simply change %configure to %%configure and %lang(pl) to %%lang(pl).

Comment 15 Michał Bentkowski 2006-08-05 11:14:26 EDT
And one another thing:
shouldn't libgadu.so.3.5 be stripped? Make install strips it, but you unstrips
it in %%files section. You can fix it in two ways, either change defattr
macro to %defattr(-,root,root,-) or add %attr(0755,root,root) before
libgadu.so.3.5 file.
Comment 16 Dominik 'Rathann' Mierzejewski 2006-09-03 15:11:31 EDT
What's the problem with packaging ekg instead? I have a working package if
anyone is interested.
Comment 17 Chris Weyl 2006-09-03 18:31:55 EDT
(In reply to comment #16)
> What's the problem with packaging ekg instead? I have a working package if
> anyone is interested.

I don't think there'd be a problem packaging ekg -- I'm sure ekg and libgadu are
not mutually exclusive.  Go ahead and submit it if you like.  :)
Comment 18 Dominik 'Rathann' Mierzejewski 2006-09-03 19:51:54 EDT
Unfortunately, there is, because ekg includes it. libgadu was written by ekg
developers. And ekg does have numbered releases, hence my suggestion to package
ekg instead. I have an ekg src.rpm that produces both ekg and libgadu.
Comment 19 Dominik 'Rathann' Mierzejewski 2006-09-04 09:09:15 EDT
Until such a time that libgadu is released separately from ekg and ekg is fixed
to build agains external libgadu, Piotr and I have decided to derive libgadu
from ekg. I've submitted ekg in bug 205127.

When libgadu is released separately, we can revisit this.

Note You need to log in before you can comment on or make changes to this bug.