Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 676941 - Review Request: eiskaltdcpp-gtk - GTK+ frontend to DC++ client library eiskaltdcpp
Summary: Review Request: eiskaltdcpp-gtk - GTK+ frontend to DC++ client library eiskal...
Keywords:
Status: CLOSED DEFERRED
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard: NotReady
Depends On:
Blocks: FE-DEADREVIEW
TreeView+ depends on / blocked
 
Reported: 2011-02-11 23:02 UTC by Tirtha Chatterjee
Modified: 2015-07-21 13:08 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-07-21 13:08:24 UTC


Attachments (Terms of Use)

Description Tirtha Chatterjee 2011-02-11 23:02:07 UTC
Spec URL: http://www.4shared.com/file/4Se1Dk4F/eiskaltdcpp-gtk.html
SRPM URL: http://www.4shared.com/file/sAFNlTul/eiskaltdcpp-gtk-220-1fc14src.html

Description: EiskaltDC++ is a cross-platform program that
uses the Direct Connect and ADC protocol.
It is compatible with other DC clients, such as the
original DC from Neomodus, DC++ and derivatives.
EiskaltDC++ also inter-operates with all common DC hub software. 

This package provides the GTK+ frontend to the EiskaltDC++ library.

Comment 1 Patryk Obara 2011-02-19 20:47:39 UTC
(This is not normal review, because I am novice packager, but a bit of of
feedback):

1) host files somewhere other than 4shared.com - it's impossible to download them using wget :(
Public dropbox dir or hosting files on http://fedorapeople.org would work better :)

2) Source0 url: better use:
http://eiskaltdc.googlecode.com/files/eiskaltdcpp-%{version}.tar.bz2

3) unfortunately, %build step failed for me:

CMake Error at /usr/share/cmake/Modules/FindOpenSSL.cmake:93 (MESSAGE):
  Could NOT find OpenSSL
Call Stack (most recent call first):
  CMakeLists.txt:70 (find_package)

4) I don't think you need this:
%install
rm -rf %{buildroot}

buildroot is always cleaned before %install step

5) Why does %dir do? I can't find it in guidelines/documentation anywhere - I don't think it's necessary.

6) change %doc to
%doc COPYING LICENSE
You could also add ChangeLog files in doc

7) update date in %changelog, it says February 2010 ;)

Comment 2 Patryk Obara 2011-02-19 22:51:38 UTC
Actually, I was wrong about point (4). By looking at different reviews I see that is required after all.

Comment 3 Susi Lehtola 2011-02-19 23:16:36 UTC
(In reply to comment #2)
> Actually, I was wrong about point (4). By looking at different reviews I see
> that is required after all.

It is not required, the reviews you have looked at have been performed incorrectly, were done a long time ago or were targeting EPEL.

http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
http://fedoraproject.org/wiki/Packaging/Guidelines#.25clean

Comment 4 Patryk Obara 2011-02-19 23:30:32 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > Actually, I was wrong about point (4). By looking at different reviews I see
> > that is required after all.
> 
> It is not required, the reviews you have looked at have been performed
> incorrectly, were done a long time ago or were targeting EPEL.

I was misguided by last comment in this, quite recent, review: https://bugzilla.redhat.com/show_bug.cgi?id=673919
(there is no mention in there, that it targets EPEL)
> At the beginning of %install, each package MUST run rm -rf %{buildroot} - OK

Thanks for clarification, Jussi :)

Comment 5 Michael Schwendt 2011-04-01 16:57:37 UTC
Some more items:


> Requires:       gtk2 >= 2.10
> Requires:       glib2 >= 2.10
> Requires:       libglade2 >= 2.4
> Requires:       gettext
> Requires:       libeiskaltdcpp = %{version}-%{release}

https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

For a Fedora Packager it is very important to understand rpmbuild's automatic dependencies on library SONAMEs.


> 5) What does %dir do?

It includes a directory (and just the dir, not any files within the dir) in the RPM package with proper ownership and permissions. 
https://fedoraproject.org/wiki/Packaging:UnownedDirectories
Under certain circumstances, %dir and recursive inclusion of a directory compete with eachother with regard to adding an entry for the directory to the package.


> %files -f %{name}.lang
> %{_datadir}/locale/*/LC_MESSAGES/eiskaltdcpp-gtk.mo

https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files


> %{_mandir}/man1/eiskaltdcpp-gtk.1.gz

Acceptable, but please notice that the gzip compression is applied by rpmbuild and may change. So, %{_mandir}/man1/eiskaltdcpp-gtk.1* would be safer.


> make install/strip DESTDIR=%{buildroot}

Please investigate what this "/strip" here does.


> rm -f %{buildroot}/%{_libdir}/libeiskaltdcpp.so.2.2
> rm -f %{buildroot}/%{_datadir}/locale/*/LC_MESSAGES/libeiskaltdcpp.mo

Two commands that ask for an explanation in a comment.


> %{_datadir}/applications/eiskaltdcpp-gtk.desktop

https://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files

[...]

https://fedoraproject.org/wiki/Packaging:Guidelines
https://fedoraproject.org/wiki/Packaging:ReviewGuidelines
https://fedoraproject.org/wiki/PackageMaintainers

Comment 6 Jason Tibbitts 2012-05-09 22:18:58 UTC
No response from the submitter, and in addition I cannot download the spec or srpm at all.  Marking as notready; I'll close this if nothing reviewable appears soon.

Comment 7 Miroslav Suchý 2015-07-21 13:08:24 UTC
Closing per #6. Feel free to reopen if you want to continue.


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