Bug 250240

Summary: Review Request: linkage - Lightweight bittorent client
Product: [Fedora] Fedora Reporter: Adel Gadllah <adel.gadllah>
Component: Package ReviewAssignee: Denis Leroy <denis>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: denis, fedora-package-review, notting
Target Milestone: ---Flags: denis: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-09-09 09:06:45 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
failed build log
none
mock root none

Description Adel Gadllah 2007-07-31 10:35:58 UTC
Spec URL: http://tgmweb.at/gadllah/linkage.spec
SRPM URL: http://tgmweb.at/gadllah/linkage-0.1.2-1.fc7.src.rpm
Description:
Linkage a bittorent lightweight client, it includes most 
common features found in other BitTorrent clients. 
Features include DHT, PEX, UPnP, desktop notifications 
and torrent creation

Comment 1 Adel Gadllah 2007-07-31 11:58:23 UTC
Fixed the plugin location in a better/cleaner way:
http://tgmweb.at/gadllah/linkage.spec
http://tgmweb.at/gadllah/linkage-0.1.2-2.fc7.src.rpm

Comment 2 Adel Gadllah 2007-07-31 12:01:40 UTC
Remove the .la files in the new location:
http://tgmweb.at/gadllah/linkage.spec
http://tgmweb.at/gadllah/linkage-0.1.2-3.fc7.src.rpm



Comment 3 manuel wolfshant 2007-07-31 13:32:23 UTC
mock build ends with:
mkdir .libs
 g++ -DHAVE_CONFIG_H -I. -I.. -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-protector --param=ssp-
buffer-size=4 -m64 -mtune=generic -I/usr/include/gtkmm-2.4
-I/usr/lib64/gtkmm-2.4/include -I/usr/include/glibmm-2.4
-I/usr/lib64/glibmm-2.4/include -I/usr/include/gdkmm-2.4
-I/usr/lib64/gdkmm-2.4/include -I/usr/include/pangomm-1.4 -
I/usr/include/atkmm-1.6 -I/usr/include/gtk-2.0 -I/usr/include/sigc++-2.0
-I/usr/lib64/sigc++-2.0/include -I/usr/incl
ude/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/lib64/gtk-2.0/include
-I/usr/include/cairomm-1.0 -I/usr/include/pa
ngo-1.0 -I/usr/include/cairo -I/usr/include/atk-1.0 -I/usr/include/libtorrent
-I/usr/include/dbus-1.0 -I/usr/lib64/d
bus-1.0/include -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
-DPLUGIN_DIR=\"/usr/lib64/linkage/plugins\" -O
2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generi
c -MT liblinkage_1_la-keyfile.lo -MD -MP -MF .deps/liblinkage_1_la-keyfile.Tpo
-c gtkmm_extra/keyfile.cc  -fPIC -DPI
C -o .libs/liblinkage_1_la-keyfile.o
gtkmm_extra/keyfile.cc:28: error: redefinition of 'struct
Glib::Container_Helpers::TypeTraits<bool>'
/usr/include/glibmm-2.4/glibmm/containerhandle_shared.h:322: error: previous
definition of 'struct Glib::Container_H
elpers::TypeTraits<bool>'
make[3]: *** [liblinkage_1_la-keyfile.lo] Error 1
make[3]: Leaving directory `/builddir/build/BUILD/linkage-0.1.2/lib'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/builddir/build/BUILD/linkage-0.1.2/lib'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/linkage-0.1.2'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.76956 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.76956 (%build)


Comment 4 Adel Gadllah 2007-07-31 13:46:11 UTC
works fine here (F7 x86_64 in mock)
can you attach your build.log file?

Comment 5 manuel wolfshant 2007-07-31 14:20:17 UTC
Created attachment 160324 [details]
failed build log

This is the log after trying to build in devel/x86_64

Comment 6 manuel wolfshant 2007-07-31 14:39:24 UTC
Created attachment 160325 [details]
mock root

log for mock buildroot when build failed

Comment 7 Adel Gadllah 2007-07-31 14:47:13 UTC
ok, thx for the log. seems that it does not build against the gtkmm24 version in
rawhide. 
added maintainer to CC...
any ideas?

Comment 8 Adel Gadllah 2007-08-09 11:08:53 UTC
upstream has fixed this in svn should I ship a svn snapshot or wait for the release?

Comment 9 Mamoru TASAKA 2007-08-11 16:11:59 UTC
(In reply to comment #8)
> upstream has fixed this in svn should I ship a svn snapshot or wait for the
release?

One way is to extract the needed patch from CVS.



Comment 10 Adel Gadllah 2007-08-11 17:47:01 UTC
(In reply to comment #9)
> (In reply to comment #8)
> > upstream has fixed this in svn should I ship a svn snapshot or wait for the
> release?
> 
> One way is to extract the needed patch from CVS.
> 
it isn't just a small patch but it moves the configuration stuff to use gconf... 

Comment 11 Adel Gadllah 2007-08-22 07:40:26 UTC
Updated to new upstream version, fixes the rawhide build issues.
http://tgmweb.at/gadllah/linkage.spec
http://tgmweb.at/gadllah/linkage-0.1.4-1.fc7.src.rpm

Comment 12 Adel Gadllah 2007-08-23 09:52:26 UTC
Fixed many spec file issues and a runtime bug:
http://tgmweb.at/gadllah/linkage.spec
http://tgmweb.at/gadllah/linkage-0.1.4-2.fc7.src.rpm

Comment 13 Denis Leroy 2007-08-23 12:08:35 UTC
Hi Adel,

1. There as some unused-direct-shlib-dependency issues on liblinkage-1.so.0.0.3.

See discussions at
http://www.redhat.com/archives/fedora-maintainers/2006-June/msg00176.html
http://www.redhat.com/archives/fedora-packaging/2007-January/msg00158.html

There's a "hack" used in a number of packages already (alliance, clamav), that
simply patches the "-Wl,--as-needed" option into libtool right after configure.
That looks to me like the simplest option. 

2. The --prefix and --libdir options to configure are not necessary

3. libupnp-devel BR is not needed. The package looks for libgupnp, which appears
to be something completely different (and not in fedora as of yet).


There's also a libtorrent vs rb_libtorrent conflict that needs to be addressed
at some point, though that's outside the scope of this review.


Comment 14 Adel Gadllah 2007-08-23 12:32:02 UTC
(In reply to comment #13)
> Hi Adel,
> 
> 1. There as some unused-direct-shlib-dependency issues on liblinkage-1.so.0.0.3.
> 
> See discussions at
> http://www.redhat.com/archives/fedora-maintainers/2006-June/msg00176.html
> http://www.redhat.com/archives/fedora-packaging/2007-January/msg00158.html
> 
> There's a "hack" used in a number of packages already (alliance, clamav), that
> simply patches the "-Wl,--as-needed" option into libtool right after configure.
> That looks to me like the simplest option. 
>
 
ok thx, added this hack for now.

> 2. The --prefix and --libdir options to configure are not necessary

removed.

> 3. libupnp-devel BR is not needed. The package looks for libgupnp, which appears
> to be something completely different (and not in fedora as of yet).

upstream changed the upnp lib in 0.1.4 removed this for now (package not in fedora)

Updated package:

http://tgmweb.at/gadllah/linkage.spec
http://tgmweb.at/gadllah/linkage-0.1.4-3.fc7.src.rpm



Comment 15 Denis Leroy 2007-08-24 07:57:56 UTC
Some more nitpicks :-)

- Patches are traditionally named something like
<name>-<version>-tag.patch, where version if the upstream version the
patch was generated from (and not necessarily that of the spec
itself). Could you rename the patch to "linkage-0.1.4-plugindir.patch" ?

- Exporting the '-Wl,--as-needed' option will not work, due to a
flaw/bug/feature in libtool (remember all compiler and linker flags
are processed by libtool) : it'll move the linker option at the end of
the command-line and that won't work (see discussions in the link I
posted above).

I used the following line after configure:

# clean unused-direct-shlib-dependencies
sed -i -e 's! -shared ! -Wl,--as-needed\0!g' libtool

- Also, you don't need to set CFLAGS and CXXFLAGS before calling make.


Review will be approved after these fixes.


Comment 17 Denis Leroy 2007-08-24 10:33:26 UTC
Review:
OK: rpmlint
  W: linkage non-conffile-in-etc /etc/gconf/schemas/linkage.schemas (that's fine)
  W: linkage-devel no-documentation (that's fine also)
OK: name is according to guidelines
OK: package meets guidelines
OK: License is GPLv2+
OK: Source code matches upstream
OK: Compiles and works
OK: Calls ldconfig
OK: BRs ok, builds in mock
OK: locale handled correctly
OK: owns all created directories
OK: %files section is clean
OK: headers and *.so are in -devel package
OK: *.la removed
OK: desktop file handled correctly
OK: seems to work just fine, tested on i386 and x86_64

APPROVED





Comment 18 Adel Gadllah 2007-08-24 11:34:04 UTC
New Package CVS Request
=======================
Package Name: linkage
Short Description: Lightweight bittorent client
Owners: drago01
Branches: FC-6 F7 devel
InitialCC: 
Cvsextras Commits: no

Comment 19 Denis Leroy 2007-08-24 12:16:34 UTC
Watch out, it won't build on FC-6: it doesn't have libnotify >= 0.4.4

Also, for F7, be sure to mark the License as GPL rather than GPLv2+...


Comment 20 Adel Gadllah 2007-08-24 12:20:42 UTC
thx for catching this,
why should I mark the License GPL on F7 ?
-------------------------------------------
New Package CVS Request
=======================
Package Name: linkage
Short Description: Lightweight bittorent client
Owners: drago01
Branches: F7 devel
InitialCC: 
Cvsextras Commits: no

Comment 21 Kevin Fenzi 2007-08-24 15:37:22 UTC
cvs done.

Comment 22 Tom "spot" Callaway 2007-08-24 15:39:35 UTC
Umm, you need to mark the License as GPLv2+ on all branches.

Comment 23 Adel Gadllah 2008-04-20 07:50:11 UTC
Package Change Request
======================
Package Name: linkage
New Branches: F-9

Comment 24 Kevin Fenzi 2008-04-22 17:08:27 UTC
All packages have been branched for F-9 now. 

cvs done.