This service will be undergoing maintenance at 00:00 UTC, 2016-09-28. It is expected to last about 1 hours
Bug 250240 - Review Request: linkage - Lightweight bittorent client
Review Request: linkage - Lightweight bittorent client
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Denis Leroy
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-07-31 06:35 EDT by Adel Gadllah
Modified: 2008-04-22 13:08 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-09 05:06:45 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
denis: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
failed build log (13.17 KB, application/octet-stream)
2007-07-31 10:20 EDT, manuel wolfshant
no flags Details
mock root (24.83 KB, text/plain)
2007-07-31 10:39 EDT, manuel wolfshant
no flags Details

  None (edit)
Description Adel Gadllah 2007-07-31 06:35:58 EDT
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 07:58:23 EDT
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 08:01:40 EDT
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 09:32:23 EDT
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 09:46:11 EDT
works fine here (F7 x86_64 in mock)
can you attach your build.log file?
Comment 5 manuel wolfshant 2007-07-31 10:20:17 EDT
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 10:39:24 EDT
Created attachment 160325 [details]
mock root

log for mock buildroot when build failed
Comment 7 Adel Gadllah 2007-07-31 10:47:13 EDT
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 07:08:53 EDT
upstream has fixed this in svn should I ship a svn snapshot or wait for the release?
Comment 9 Mamoru TASAKA 2007-08-11 12:11:59 EDT
(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 13:47:01 EDT
(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 03:40:26 EDT
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 05:52:26 EDT
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 08:08:35 EDT
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 08:32:02 EDT
(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 03:57:56 EDT
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 06:33:26 EDT
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 07:34:04 EDT
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 08:16:34 EDT
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 08:20:42 EDT
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 11:37:22 EDT
cvs done.
Comment 22 Tom "spot" Callaway 2007-08-24 11:39:35 EDT
Umm, you need to mark the License as GPLv2+ on all branches.
Comment 23 Adel Gadllah 2008-04-20 03:50:11 EDT
Package Change Request
======================
Package Name: linkage
New Branches: F-9
Comment 24 Kevin Fenzi 2008-04-22 13:08:27 EDT
All packages have been branched for F-9 now. 

cvs done.

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