https://bugzilla.redhat.com/show_bug.cgi?id=863795
Bug ID: 863795 QA Contact: extras-qa@fedoraproject.org Severity: medium Version: rawhide Priority: medium CC: notting@redhat.com, package-review@lists.fedoraproject.org Assignee: nobody@fedoraproject.org Summary: Review Request: kadu - An Gadu-Gadu client for online messaging Regression: --- Story Points: --- Classification: Fedora OS: Linux Reporter: karlikt@gmail.com Type: --- Documentation: --- Hardware: All Mount Type: --- Status: NEW Component: Package Review Product: Fedora
Spec URL: http://karlik.fedorapeople.org/kadu.spec SRPM URL: http://karlik.fedorapeople.org/kadu-0.12.2-1.fc19.src.rpm Description: Kadu is a dynamically evolving instant messenger compatible with the Gadu-Gadu protocol.
It is rereview request. Package is dead because of inactivity of the maintainer. This package was retired on 2012-08-06 due to failure to build for multiple releases. Rereview is required according to http://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers#Cla... (the package is deprecated more than two weeks)
Fedora Account System Username: karlik
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Volker Fröhlich volker27@gmx.at changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |volker27@gmx.at
--- Comment #1 from Volker Fröhlich volker27@gmx.at --- Use %global instead of %define.
There's a libiris bundle. Rex managed to create a separate package: https://bugzilla.redhat.com/show_bug.cgi?id=749885
Don't BR phonon, sqlite and the likes. Only BR the devel sub-packages.
Since you can't go for EPEL 5, please remove buildroot definition, clean section and the first rm in the install section. defattr is no longer necessary either.
What's the idea of having -k with make?
I can't see why several sub-packages should be GPLv3+. Sub-packages should use the ?_isa macro when requiring the base package.
You can use the name macro in Source0. You can drop "-n kadu-%{version}" from the setup macro.
I noticed this warning:
+ desktop-file-install --vendor fedora --delete-original --dir /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/kadu.desktop /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/fedora-kadu.desktop: warning: value "Instant Messenger" for key "Comment" in group "Desktop Entry" looks redundant with value "Instant Messenger" of key "GenericName" /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/fedora-kadu.desktop: warning: value "Komunikator internetowy" for key "Comment[pl]" in group "Desktop Entry" looks redundant with value "Komunikator internetowy" of key "GenericName[pl]"
The locales are not handled properly, please compare http://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files
I personally consider the files section a bit over-detailled.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #2 from Karol Trzcionka karlikt@gmail.com --- Spec URL: http://karlik.fedorapeople.org/kadu.spec SRPM URL: http://karlik.fedorapeople.org/kadu-0.12.2-2.fc19.src.rpm
I do some cleanup. Most of the problems was the heritage of previous maintainer.
Use %global instead of %define. DONE
There's a libiris bundle. Rex managed to create a separate package: https://bugzilla.redhat.com/show_bug.cgi?id=749885 I've disabled temporarily jabber_protocol because there is not origin libiris but it is modified. I try to contact with devs, maybe it would be exception: http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Needing_unrelea... It doesn't provide own iris library (in fact, it is compiled and statically linked).
Don't BR phonon, sqlite and the likes. Only BR the devel sub-packages. DONE
Since you can't go for EPEL 5, please remove buildroot definition, clean section and the first rm in the install section. defattr is no longer necessary either. DONE
What's the idea of having -k with make? I didn't notice it, removed.
I can't see why several sub-packages should be GPLv3+. Sub-packages should use the ?_isa macro when requiring the base package. I'm not the lawyer, I think it was added because the external plugins are GPLv3+, the core client is GPL2v+.
You can use the name macro in Source0. You can drop "-n kadu-%{version}" from the setup macro. DONE
I noticed this warning:
+ desktop-file-install --vendor fedora --delete-original --dir /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/kadu.desktop /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/fedora-kadu.desktop: warning: value "Instant Messenger" for key "Comment" in group "Desktop Entry" looks redundant with value "Instant Messenger" of key "GenericName" /home/makerpm/rpmbuild/BUILDROOT/kadu-0.12.2-1.fc16.x86_64/usr/share/applications/fedora-kadu.desktop: warning: value "Komunikator internetowy" for key "Comment[pl]" in group "Desktop Entry" looks redundant with value "Komunikator internetowy" of key "GenericName[pl]" I think it is not a problem, the desktop file is provided by upstream.
The locales are not handled properly, please compare http://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files DONE.
I personally consider the files section a bit over-detailled. I think now it's better.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #3 from Volker Fröhlich volker27@gmx.at --- Sorry, my bad, I only looked at kadu-0.12.2.tar.bz2 when I browsed through the licenses.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Karol Trzcionka karlikt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- External Bug ID| |FedoraHosted 230
--- Comment #4 from Karol Trzcionka karlikt@gmail.com --- I've asked for bundled library exception (libiris).
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Karol Trzcionka karlikt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- External Bug ID|FedoraHosted 230 |
--- Comment #5 from Karol Trzcionka karlikt@gmail.com --- I'm not sure if there is correct tracker in bugzilla, so link: https://fedorahosted.org/fpc/ticket/230
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #6 from Dawid Gajownik gajownik@gmail.com --- I would suggest changing in %build section
%{cmake} -DCMAKE_BUILD_TYPE=Debug
to
%{cmake} -DCMAKE_BUILD_TYPE=RelWithDebInfo
or removing option CMAKE_BUILD_TYPE altogether because RelWithDebInfo is a default option.
http://www.kadu.im/w/English:CMakeConfiguration (polish page has more details http://www.kadu.im/w/Instalacja_ze_%C5%BAr%C3%B3de%C5%82)
Debug disables all the optimizations and spills lots of debugging output on the console while the program is running.
RelWithDebInfo allows to create dubug packages and is more performance wise for the user.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #7 from Dawid Gajownik gajownik@gmail.com --- + /usr/bin/cmake -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_INSTALL_PREFIX:PATH=/usr -DINCLUDE_INSTALL_DIR:PATH=/usr/include -DLIB_INSTALL_DIR:PATH=/usr/lib64 -DSYSCONF_INSTALL_DIR:PATH=/etc -DSHARE_INSTALL_PREFIX:PATH=/usr/share -DLIB_SUFFIX=64 -DBUILD_SHARED_LIBS:BOOL=ON -DCMAKE_USE_PTHREADS:BOOL=ON -DBUILD_DESCRIPTION=Fedora .
[...]
-- Configuring done -- Generating done CMake Warning: Manually-specified variables were not used by the project:
BUILD_DESCRIPTION CMAKE_USE_PTHREADS INCLUDE_INSTALL_DIR LIB_INSTALL_DIR LIB_SUFFIX SHARE_INSTALL_PREFIX SYSCONF_INSTALL_DIR
I think that you can also remove '-DCMAKE_USE_PTHREADS:BOOL=ON' and '-DBUILD_DESCRIPTION=Fedora' option from the spec file.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags| |fedora-review?
--- Comment #8 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- (In reply to comment #1)
- desktop-file-install --vendor fedora --delete-original --dir
Also, please drop --vendor fedora. It is forbidden by the guidelines now.
I'll try to do a full review this weekend.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |dominik@greysector.net
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #9 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- MUST: rpmlint must be run on the source rpm and all binary rpms the build produces. The output should be posted in the review.
$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result kadu-mime_tex.x86_64: I: enchant-dictionary-not-found pl kadu-mime_tex.x86_64: W: spelling-error %description -l en_US tex -> Tex, ex, text kadu-mime_tex.x86_64: W: spelling-error %description -l en_US mathematic -> mathematics, ma thematic, ma-thematic kadu-mime_tex.x86_64: W: no-documentation kadu-messagessplitter.x86_64: W: no-documentation kadu-panelkadu.x86_64: W: no-documentation kadu-senthistory.x86_64: W: no-documentation kadu-networkping.x86_64: W: no-documentation kadu-import_history.x86_64: W: summary-ended-with-dot C Imports history from origin GG client. kadu-import_history.x86_64: W: spelling-error %description -l en_US Importhistory -> Import history, Import-history, Prehistory kadu-import_history.x86_64: W: no-documentation kadu.src:9: W: setup-not-in-prep kadu.src:17: W: macro-in-comment %files kadu.src:22: W: macro-in-comment %files kadu-devel.x86_64: W: no-documentation kadu.x86_64: W: no-manual-page-for-binary kadu kadu-kadu_completion.x86_64: W: no-documentation kadu-nextinfo.x86_64: W: no-documentation kadu-lednotify.x86_64: W: obsolete-not-provided kadu-led_notify kadu-lednotify.x86_64: W: no-documentation kadu-globalhotkeys.x86_64: W: spelling-error Summary(en_US) hotkeys -> hot keys, hot-keys, hotcakes kadu-globalhotkeys.x86_64: W: spelling-error %description -l en_US hotkeys -> hot keys, hot-keys, hotcakes kadu-globalhotkeys.x86_64: W: no-documentation kadu-anonymous_check.x86_64: W: spelling-error Summary(en_US) lookup -> lockup, hookup, look up kadu-anonymous_check.x86_64: W: spelling-error %description -l en_US lookup -> lockup, hookup, look up kadu-anonymous_check.x86_64: W: no-documentation 15 packages and 0 specfiles checked; 0 errors, 25 warnings.
MUST: The package must meet the Packaging Guidelines .
Please incorporate Dawid's and my comments.
MUST: The License field in the package spec file must match the actual license.
Why is -devel package GPLv3+ when main package is GPLv2+?
MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use sha256sum for this task as it is used by the sources file once imported into git. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this.
$ sha256sum -c SHA256SUM globalhotkeys-0.12-32.tar.gz: OK lednotify-0.12-33.tar.gz: OK messagessplitter-0.12-5.tar.gz: OK networkping-0.12-4.tar.gz: OK nextinfo-0.12-9.tar.gz: OK panelkadu-0.12-10.tar.gz: OK senthistory-0.12-11.tar.gz: OK anonymous_check-0.11.0-1.tar.bz2: OK import_history-0.12.0.tar.bz2: OK kadu-0.12.2.tar.bz2: OK kadu_completion-0.12.0-1.tar.bz2: OK mime_tex-0.12.0-2.tar.bz2: OK $ cat SHA256SUM 3696940aa1a0d9f6ad1576873bc0ab6a4e237cac24c627f0a7284410cdfba72d globalhotkeys-0.12-32.tar.gz 2c7216bfde19beb2aafcbe4868a5a50ae743eaf9046ef9d65d5ba9609ecade7f lednotify-0.12-33.tar.gz ceba4eee8f387fc2c4447886769414db383e1d51039a54ba7afe3c5740c12adf messagessplitter-0.12-5.tar.gz 891ebea7a086f415f2d4f7cdaf8ce1407ae522272b17dd91d99d8a73ce7e7ffc networkping-0.12-4.tar.gz 3a88fb13f334762723e792338c58a0ee42090abec577aa702b5653a3324c0d32 nextinfo-0.12-9.tar.gz 784c2596d9a698b46b8c3a99c96bc556b4370453f57a99aa6a93d9b07d8d5733 panelkadu-0.12-10.tar.gz eb8919dcadf65dcc192ea3de1ad263ef8eff2b080339f5619856dbcb96c60aa0 senthistory-0.12-11.tar.gz 80c4dbf28524b9c1b168982ff6b5ae3254c802fc1661eba8228e12eee0b05408 anonymous_check-0.11.0-1.tar.bz2 f831f97936798ea9ec03334d1ab04fd76506a422d6887addbd92f15b7bd8bc67 import_history-0.12.0.tar.bz2 a1fad5444fedb7af82d64d7e13daa65d3edd553b90efdaef8bf230647d9c1067 kadu-0.12.2.tar.bz2 0c6e361a070c098e0fabdd7a7a893292ffd1e3f373cbce7e9f05a9fc893d0869 kadu_completion-0.12.0-1.tar.bz2 f3f831108a4db4ea1cb7979efdad0d7a6db1b380ef38dade38561e4f8a1b6fa8 mime_tex-0.12.0-2.tar.bz2
MUST: Packages must NOT bundle copies of system libraries.
https://fedorahosted.org/fpc/ticket/230 resolution must be followed.
MUST: If a package includes something as %doc, it must not affect the runtime of the application. To summarize: If it is in %doc, the program must run properly if it is not present.
Are the docs in %{_datadir}/kadu used at runtime? If not, please include them as %doc instead.
SHOULD: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it.
GPLv3+ plugins don't come with a copy of GPLv3. Please ask upstream(s) to include one with each tarball.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
Karol Trzcionka karlikt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |928576
--- Comment #10 from Karol Trzcionka karlikt@gmail.com --- Spec URL: http://karlik.fedorapeople.org/kadu.spec SRPM URL: http://karlik.fedorapeople.org/kadu-0.12.2-3.fc20.src.rpm
Dawid's patches applied.
License in -devel fixed.
HISTORY and README moved to %doc, others are used by "About" window at runtime.
iris library in another review request: bz928576
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #11 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- I think the external modules should be moved to separate packages (or at least a separate kadu-plugins package), especially since they have independently-versioned tarballs. At the very least, the subpackages should carry the versions of their upstream tarballs instead of kadu's main package version. Of course, at the moment, this would mean introducing epochs at least for some of them.
What do you think?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #12 from Karol Trzcionka karlikt@gmail.com --- It might be but I'm not sure if it is needed. As I can see all external modules have the version lesser or equal the kadu's version. Well... I think versioning is hard dependent on core package. It might be splitted but I can't see any necessity at the moment. It would be useful only if the subpackages had faster release cycle than core.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #13 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- I saw some activity in libiris upstream recently. Could you retry building against the latest libiris source?
https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #14 from Dawid Gajownik gajownik@gmail.com --- Please add below line Requires: qca-ossl%{?_isa}
otherwise encryption plugin will not load.
https://bugzilla.redhat.com/show_bug.cgi?id=863795 Bug 863795 depends on bug 928576, which changed state.
Bug 928576 Summary: Review Request: iris-kadu - A library for working with the XMPP/Jabber protocol https://bugzilla.redhat.com/show_bug.cgi?id=928576
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |NOTABUG
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW Assignee|dominik@greysector.net |nobody@fedoraproject.org
--- Comment #15 from Dominik 'Rathann' Mierzejewski dominik@greysector.net --- Dropping due to no response from reporter. Feel free to ping me if a new review is needed.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Luke luton@wp.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |luton@wp.pl
--- Comment #16 from Luke luton@wp.pl --- I'd be really grateful to have kadu packaged for Fedora, if possible, just trying it here instead of opening a new bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Iwicki Artur fedora@svgames.pl changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |fedora@svgames.pl
--- Comment #17 from Iwicki Artur fedora@svgames.pl --- Do you need kadu itself, or just something supporting the Gadu-Gadu network? Pidgin can be used for the latter.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
--- Comment #18 from Luke luton@wp.pl --- (In reply to Iwicki Artur from comment #17)
Do you need kadu itself, or just something supporting the Gadu-Gadu network? Pidgin can be used for the latter.
Sure, pidgin supports gg-network, however, I'd still prefer kadu since I've been using it since around 2004 and would like to keep history etc. by simply copying the ~/.kadu dir.
https://bugzilla.redhat.com/show_bug.cgi?id=863795
Dominik 'Rathann' Mierzejewski dominik@greysector.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |
package-review@lists.fedoraproject.org