Bug 177635 - Review Request: libtorrent
Review Request: libtorrent
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
David Lawrence
Depends On:
Blocks: FE-ACCEPT 177636
  Show dependency treegraph
Reported: 2006-01-12 11:05 EST by Chris Chabot
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-01-14 14:25:45 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Chris Chabot 2006-01-12 11:05:30 EST
SPEC: http://www.xs4all.nl/~chabotc/libtorrent.spec
SRPM: http://www.xs4all.nl/~chabotc/libtorrent-0.8.2-1.src.rpm


LibTorrent is a BitTorrent library written in C++ for *nix, with a focus                                                     
on high performance and good code. The library differentiates itself                                                         
from other implementations by transfering directly from file pages to                                                        
the network stack. On high-bandwidth connections it is able to seed at                                                       
3 times the speed of the official client.
Comment 1 Rudolf Kastl 2006-01-13 03:49:43 EST
test build on x86_64 rawhide succeeded and spec looks good to me.
Comment 2 Chris Chabot 2006-01-13 17:15:00 EST
Also did a mock on fc4 and devel, both completed fine.

Still hoping someone will be in a reviewing mood, don't suppose you have the
time Rudolf? :-)
Comment 3 Hans de Goede 2006-01-14 11:59:43 EST
As requested a review, first look over the specfile while doing a build yields:
-Summary must not start with "A ..." remove "A .."
-Summary line exceeds 80 chars (nitpick)
-Is the requires openssl nescesarry? "ldd /usr/lib64/libtorrent.so" gives:
        libcrypto.so.6 => /lib64/libcrypto.so.6 (0x00002aaaaac68000)
        libsigc-2.0.so.0 => /usr/lib64/libsigc-2.0.so.0 (0x00002aaaaaea8000)
        libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x00002aaaaafae000)
        libm.so.6 => /lib64/libm.so.6 (0x00002aaaab1ab000)
        libc.so.6 => /lib64/libc.so.6 (0x00002aaaab32d000)
        libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00002aaaab566000)
        libdl.so.2 => /lib64/libdl.so.2 (0x00002aaaab674000)
        libz.so.1 => /usr/lib64/libz.so.1 (0x00002aaaab778000)
        /lib64/ld-linux-x86-64.so.2 (0x0000555555554000)
 and "rpm -qf /lib64/libcrypto.so.6" gives:
 so openssl is already automaticly required, or does it require additonal
 non .so files provided by openssl?
-%package devel, Summary s/envirioment/environment/
-%package devel, Group should be Development/Libraries .
-%package devel, Description Header, include files seems a bit double to me.
 %dir %{_includedir}/torrent
 can be replaced by just (nitpick again)
 rpm will then own that dir and pickup all files under it automaticly

It compiles without an warnings on 64bit, good! rpmlint also likes it. Please
fix the above and I'll do a full review soon.
Comment 4 Chris Chabot 2006-01-14 12:23:33 EST
* Sat Jan 14 2006 - Chris Chabot <chabotc@xs4all.nl> - 0.8.2-2                 
- Improved general summary & devel package description                         
- Simplified devel package includedir files section                            
- Removed openssl as requires, its implied by .so dependency   
- Correct devel package Group

SPEC: http://www.xs4all.nl/~chabotc/libtorrent.spec
SRPM: http://www.xs4all.nl/~chabotc/libtorrent-0.8.2-2.src.rpm

Thanks for the time & review so far!

ps you forgot to set the bug to blocking FE-REVIEW? :-)
Comment 5 Hans de Goede 2006-01-14 13:38:10 EST
The formal review steps:

MUST review items:
- Builds cleanly on FC5 devel.
- rpmlint has no output / complaints
- Source included matches upsteam source (md5sum)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence (GPL) is fedora extra's compatible & is included in spec
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- Proper use of ldconfig
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- Proper -devel package
- No .la files
- Package owns directories properly

Should items:
- Includes upstream licence file (COPYING)
- No insane scriplets, or scriplets at all
- No unnescesarry requires

Looks good to me, changing blockerbug to FE-ACCEPT and assigning to me.
Comment 6 Chris Chabot 2006-01-14 14:25:45 EST
Commited, added entry to owners list and build cleanly on FC5 extra's in the
buildsystem, closing bug.

Thanks for the review!

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