Bug 676939 - Review Request: libeiskaltdcpp - A client library for the DC file sharing protocol
Review Request: libeiskaltdcpp - A client library for the DC file sharing pro...
Status: CLOSED DEFERRED
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity medium
: ---
: ---
Assigned To: Nobody's working on this, feel free to take it
Fedora Extras Quality Assurance
NotReady
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-11 17:53 EST by Tirtha Chatterjee
Modified: 2015-07-21 09:07 EDT (History)
6 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Tirtha Chatterjee 2011-02-11 17:53:59 EST
Spec URL: http://www.4shared.com/file/A2XmVfEu/libeiskaltdcpp.html
SRPM URL: http://www.4shared.com/file/ZAuOC-KF/libeiskaltdcpp-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 core EiskaltDC++ library.
Comment 1 Martin Gieseking 2011-02-16 04:11:02 EST
Hi,

since you're not a member of the Fedora packager group yet (as far as I can see), you need a sponsor who approves you. Here are some more information on how to join the packager group: 

http://fedoraproject.org/wiki/PackageMaintainers/Join
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

BTW, if you don't have access to a web server to upload your files to, you can
also use something like Dropbox for example. Downloading files from a
filehoster is pretty annoying and might prevent reviewers from taking a look at
your submission. Please provide direct links to the SRPM and the spec file.
Comment 2 Sergio Belkin 2011-02-18 08:38:38 EST
Hi,

I think that you should use tools like koji and mock in order to be sure that package is built. Currently it is failing on koji:

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:91 (MESSAGE):
  Could NOT find OpenSSL (missing: OPENSSL_LIBRARIES OPENSSL_INCLUDE_DIR)

AFAIS, openssl-devel is missing in BuildRequires.

Also you should fix the permission on tarball file. Remove the execution mode bits. And on spec file: use %post -p <command> instead of using:  %post <command>, the same goes for the %postun section.

Besides you may find the following pages really useful:

http://fedoraproject.org/wiki/Common_Rpmlint_issues
http://fedoraproject.org/wiki/How_to_create_an_RPM_package
http://fedoraproject.org/wiki/Packaging:Guidelines
http://fedoraproject.org/wiki/Using_Mock_to_test_package_builds
http://fedoraproject.org/wiki/PackageMaintainers/UsingKoji

Still I am not a sponsor but I hope that it helps :)
Comment 3 Michael Schwendt 2011-03-25 10:25:05 EDT
I wonder about a few additional things:


> Name:           libeiskaltdcpp
> Source0:        http://eiskaltdc.googlecode.com/files/eiskaltdcpp-2.2.0.tar.bz2
>

Library packages do not need to start with a "lib" prefix:
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming

It seems the upstream name is not libeiskaltdcpp, and other distributions have not called their packages like that either.


> Requires:       openssl >= 0.9.8

As Sergio has pointed out, "BuildRequires: openssl-devel" seems to be missing, but the explicit dependency on "openssl" should be revisited according to:
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires


> %description

I would consider the description better, if it were much more like the current Summary. That is, if it described that this is a library package and what that library can do.

Instead, it refers to a "cross-platform program" and "other DC clients", even mentions their names. This is quite confusing. If the library offers an API for the DC and ADC protocols and could be used by developers for programs other than EiskaltDC++, the description could be made much more concise.


> %post
> /sbin/ldconfig
>
> %postun
> /sbin/ldconfig

This would execute /sbin/ldconfig via a /bin/sh script and add an additional dependency on /bin/sh. The following would execute /sbin/ldconfig directly and add a dependency on /sbin/ldconfig:

%post -p /sbin/ldconfig

%postun -p /sbin/ldconfig


> make install/strip DESTDIR=%{buildroot}

Without having tried to build this, the "strip" here is suspicious. No files should not be stripped, because rpmbuild does that when extracting the debuginfo details.


> %files -f %{name}.lang
> %defattr(-,root,root,-)
> %{_libdir}/libeiskaltdcpp.so.2.2
> %doc COPYING
> %doc LICENSE

No libeiskaltdcpp-devel package? Where are the header files for this library? And where is the libeiskaltdcpp.so symlink needed for compiling software? Isn't this an ordinary library with a public API to develop with?

Also, the install appears to be incomplete, because the symlink for
%{_libdir}/libeiskaltdcpp.so.2 is missing currently.
Comment 4 nucleo 2011-04-21 20:27:39 EDT
Hi,

I think that no need to create three independent packages as in this bug and bug 676941 and bug 676943 because they use the same sources tarball

http://eiskaltdc.googlecode.com/files/eiskaltdcpp-2.2.0.tar.xz (Smallest Compressed Archive)
see https://fedoraproject.org/wiki/Packaging/SourceURL#Referencing_Source

eiskaltdcpp-gtk and eiskaltdcpp-qt can be subpackages of eiskaltdcpp
Main package eiskaltdcpp can contain common files.

See for example eiskaltdcpp in russianfedora repository:
http://mirror.yandex.ru/fedora/russianfedora/russianfedora/free/fedora/updates/14/SRPMS/eiskaltdcpp-2.2.1-1.fc14.src.rpm
Comment 5 Miroslav Suchý 2015-07-21 09:07:39 EDT
Closing due long inactivity. 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.