Bug 450408 - Review Request: trickle - Portable lightweight userspace bandwidth shaper
Review Request: trickle - Portable lightweight userspace bandwidth shaper
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-07 16:25 EDT by Nicoleau Fabien
Modified: 2008-07-03 23:41 EDT (History)
4 users (show)

See Also:
Fixed In Version: 1.07-3.fc9
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-03 23:39:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mtasaka: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)
failed build while using mock (17.81 KB, application/octet-stream)
2008-06-07 20:57 EDT, manuel wolfshant
no flags Details
Patch to compile also on ppc64 for 1.06 (378 bytes, patch)
2008-06-07 23:53 EDT, Mamoru TASAKA
no flags Details | Diff
Patch to fix dlopened module path (660 bytes, patch)
2008-06-16 11:31 EDT, Mamoru TASAKA
no flags Details | Diff

  None (edit)
Description Nicoleau Fabien 2008-06-07 16:25:17 EDT
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/trickle.spec
SRPM URL: http://nicoleau.fabien.free.fr/rpms/srpms.fc9/trickle-1.06-1.fc9.src.rpm
Description:
trickle is a portable lightweight userspace bandwidth shaper. It can run in collaborative mode (together with trickled) or in stand alone mode.
trickle works by taking advantage of the unix loader preloading. Essentially it provides, to the application, a new version of the functionality that is required to send and receive data through sockets. It then limits traffic based on delaying the sending and receiving of data over a socket. trickle runs entirely in userspace and does not require root privileges.

rpmlint is OK on rpm, debuginfo and src.rpm. Package build is OK under mock.

This is my 3rd package but I'm still seeking a sponsor.
Comment 1 manuel wolfshant 2008-06-07 20:57:11 EDT
The srpm you have submitted does not build for me in mock. I'll attach the build
log.

Just for the record: 10 months ago I've packaged trickle too, and following some
links from Mandriva I have found out that
http://monkey.org/~marius/trickle/trickle-1.07.tar.gz gives a newer version of
the software. I know that it's not listed in the main web page of the project
but it's been available for a very long time.
Comment 2 manuel wolfshant 2008-06-07 20:57:47 EDT
Created attachment 308625 [details]
failed build while using mock
Comment 3 manuel wolfshant 2008-06-07 21:16:39 EDT
build also fails in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=652376 (scratch build,
started at http://koji.fedoraproject.org/koji/taskinfo?taskID=652374 )
Comment 4 Nicoleau Fabien 2008-06-07 21:36:21 EDT
thx for your comments. It seems that build fails only on x86_64 (I only have a
32bits computer). I'll try to find a computer on x86_64 to test build and use
1.07 version.
Comment 5 Mamoru TASAKA 2008-06-07 23:53:49 EDT
Created attachment 308631 [details]
Patch to compile also on ppc64 for 1.06

Just having tried to compile this package.

- srpm on comment 0 does not build even on i386:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=652380

- This is due to parallel make issue, removing %{?_smp_mflags}
  will pass compilation on i386/x86_64/ppc, not on ppc64

- With the attached patch 1.06 builds 1.06 also on ppc64.
  (No problem with this patch also on i386/x86_64/ppc)

- For 1.07. parallel make still fails (at least for -j4).
  The attached patch seems no longer needed.

Again I just tried to compile this package. I would appreciate it
if Wolfy would continue to review this package.
Comment 6 Nicoleau Fabien 2008-06-08 07:32:11 EDT
Spec updated : http://nicoleau.fabien.free.fr/rpms/SPECS/trickle.spec
New SRPM     :
http://nicoleau.fabien.free.fr/rpms/fc9.i386/trickle-1.07-1.fc9.i386.rpm

I'm now using version 1.07 and I removed %{?_smp_mflags}.
Comment 7 Mamoru TASAKA 2008-06-15 09:46:08 EDT
Well, for 1.07-1:

* License
  - Many codes are licensed under 4 clause BSD, so the license
    tag must be "BSD with advertising".

* Parallel make
  - Please write a note on the spec file that this package is
    parallel make unsafe.

* ldconfig
  - Calling /sbin/ldconfig on scriptlets are not needer for
    this package.
    trickle-overload.so is not under default library search paths,
    and it is only dlopen'ed.

* 64 bit issue
  - (Although I have 32 bit machine only) 
    I guess this package doesn't work on 64 bit machine.
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
configure:
----------------------------------------------------
 21008  clibdir="$prefix/lib/trickle"
 21009  
 21010  cat >>confdefs.h <<_ACEOF
 21011  #define SYSCONFDIR "$csysconfdir"
 21012  _ACEOF
 21013  
 21014  cat >>confdefs.h <<_ACEOF
 21015  #define LIBDIR "$clibdir"
 21016  _ACEOF
----------------------------------------------------

trickle.c:
----------------------------------------------------
    40  #define LIBNAME "trickle-overload.so"
    51          char *trypaths[]  = {
    52                  LIBNAME,
    53                  LIBDIR "/" LIBNAME,
    54                  NULL
    55          };
   103          for (pathp = trypaths; *pathp != NULL; pathp++)
   104                  if (lstat(*pathp, &sb) == 0)
   105                          break;
   106  
   107          path = *pathp;
   108          if (path == NULL)
   109                  errx(1, "Could not find overload object");
----------------------------------------------------

trickle.spec:
----------------------------------------------------
    59  %{_bindir}/trickled
    60  %{_libdir}/%{name}/trickle-overload.so
    61  %{_mandir}/man1/%{name}.1.gz
----------------------------------------------------

On 64 bit machine, %_libdir is /usr/lib64.
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Comment 8 Nicoleau Fabien 2008-06-15 17:23:00 EDT
Updated :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/trickle.spec
SRPM URL: http://nicoleau.fabien.free.fr/rpms/srpms.fc9/trickle-1.07-2.fc9.src.rpm


Changelog :
* Sun Jun 15 2008 Nicoleau Fabien <nicoleau.fabien@gmail.com> 1.07-2
- Licence changed
- ldconfig no more used
- dir macro used for libdir/name
- config.h file modified (/lib/ hardcoded)

With config.h modification, I think that this package can run udner x86_64

rpmlint output :
[builder@FEDOBOX tmp]$ ls
trickle-1.07-2.fc9.i386.rpm  trickle-1.07-2.fc9.src.rpm 
trickle-debuginfo-1.07-2.fc9.i386.rpm
[builder@FEDOBOX tmp]$ rpmlint -i *.rpm
[builder@FEDOBOX tmp]$

src.rpm rebuild under mock is OK
Comment 9 manuel wolfshant 2008-06-15 17:48:22 EDT
Unfortunately your patch for x86_64 does not work correctly. Here is the result
of running trickle on an x86_64 system (tested in F7, but I am almost sure the
same behaviour happens in newer fedoras):
[wolfy@wolfy tmp]$ trickle --d 100 wget
http://nicoleau.fabien.free.fr/rpms/srpms.fc9/trickle-1.07-2.fc9.src.rpm
trickle: Could not find overload object

and the reason is probably here:

[wolfy@wolfy tmp]$ strings  `which trickle`|grep overload.so
/usr/lib/trickle/trickle-overload.so
trickle-overload.so
Comment 10 Nicoleau Fabien 2008-06-15 18:22:16 EDT
I don't understand where this path is written. 
Perhaps in libtool or ltmain.sh.
I saw differents RPMs of trickle for other distributions that uses /usr/lib64,
but nothing is done in the spec file to use it.
Comment 11 manuel wolfshant 2008-06-15 19:52:18 EDT
On my system config.h has (taken from 
/var/lib/mock/fedora-7-x86_64/root/builddir/build/BUILD/trickle-1.07/config.h
after the build):
     #define LIBDIR "/usr/lib/trickle"
even if your sed command is not applied.
(I have no idea why the config.h file is generated with /usr/lib instead of
/usr/local/lib)

Everything works Ok on my system if I replace your sed line with:
     %{__sed} -i -e s@"LIBDIR \"\/usr\/lib\/trickle\""@"LIBDIR
\"%{_libdir}/%{name}\""@ config.h
I've tested on F7/x86_64 and devel/i386&x86_64
Comment 12 Nicoleau Fabien 2008-06-16 03:02:33 EDT
ok, then I'll try to have a more "generic" sed line to handle all possible
cases, something like :
 
%{__sed} -i -e s@"LIBDIR \".*/trickle\""@"LIBDIR \"%{_libdir}/%{name}\""@ config.h
Comment 13 Mamoru TASAKA 2008-06-16 11:31:45 EDT
Created attachment 309512 [details]
Patch to fix dlopened module path

Please don't fix config.h but fix configure{,in}.

As I already wrote in comment 7, clibdir is defined in configure{,.in}.
configure creates config.guess and config.guess creates config.h

The attached patch should fix dlopen'ed module path issue on 64 bit
machine.

Also, to aviod autotool automated call, the following line is needed
after applying the attached patch:
------------------------------------------------------------------------
touch -r configure aclocal.m4 Makefile.in stamp-h.in
------------------------------------------------------------------------
Comment 14 Nicoleau Fabien 2008-06-16 15:36:28 EDT
Updated :
Spec URL: http://nicoleau.fabien.free.fr/rpms/SPECS/trickle.spec
SRPM URL: http://nicoleau.fabien.free.fr/rpms/srpms.fc9/trickle-1.07-3.fc9.src.rpm

Package now contains Mamoru Tasaka's patch and provides a configuration example.

[builder@FEDOBOX tmp]$ rpmlint -i trickle-1.07-3.fc9.i386.rpm 
[builder@FEDOBOX tmp]$ rpmlint -i trickle-debuginfo-1.07-3.fc9.i386.rpm 
[builder@FEDOBOX tmp]$ rpmlint -i trickle-1.07-3.fc9.src.rpm 
[builder@FEDOBOX tmp]$ 
Rebuild under mock is OK.
Comment 15 manuel wolfshant 2008-06-16 17:34:24 EDT
Works perfectly for me in F7/x86_64, C5.2/x86_64 and C4/x86_64 (using libevent
from epel), rpmlint has no complains.

Nicoleau, could you please consider bilding this package in EPEL, too ? I'll be
glad to [co-]maintain it for EPEL [together with you, or even alone if you are
not interested in EPEL). Thanks.
Comment 16 Nicoleau Fabien 2008-06-16 17:46:48 EDT
manuel wolfshant, thanks for your comment. 

trickle seems working fine, but I don't know if the package can be considered as
finished (It could be great to have an init file for trickled).

As I said in my First comment, It's my third package (actually I have 5 packages
in review) and still need a sponsor. I'm quite "new" in packaging, but I'm very
intersting in packagin trickle for EPEL (I'm CentOS) user too. So I'm okay to
build it for EPEL with you.
Comment 17 Mamoru TASAKA 2008-06-17 12:29:21 EDT
Well,
+ This package itself is okay now.
+ You have some other review requests, which seem good (to some extent)
  for first quick glance

Some miscs.
* Commenting out macros
----------------------------------------------------------------------------
# Parallel make is unsafe for this package, so %{?_smp_mflags} is not used
----------------------------------------------------------------------------
  - In comments, use %% to avoid macros expansion like
----------------------------------------------------------------------------
# Parallel make is unsafe for this package, so %%{?_smp_mflags} is not used
----------------------------------------------------------------------------

(In reply to comment #16)
> (It could be great to have an init file for trickled).
I will leave it you for now. If you write a draft for init script for
trickled and you feel you need a help, please open a new bug against
trickle (once trickle is imported into Fedora) and add me to cc list.
Or I guess wolfy will help you a lot.

By the way, you can start from
http://fedoraproject.org/wiki/Packaging/SysVInitScript

-------------------------------------------------------------------------
                This package (trickle) is APPROVED by me
-------------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Get a Fedora Account".
At a point a mail should be sent to sponsor members which notifies
that you need a sponsor. At the stage, please also write on
this bug for confirmation that you requested for sponsorship and
your FAS (Fedora Account System) name. Then I will sponsor you.

If you want to import this package into Fedora 8/9, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Comment 18 Nicoleau Fabien 2008-06-17 16:32:35 EDT
Thx for help mtasaka.
I confirm that I am requesting for sponsorship.
My FAS name is 'eponyme'.

PS : must I rebuild a -4 for trickle to set %%{?_smp_mflags} instead of
%{?_smp_mflags} in comment ?
Comment 19 manuel wolfshant 2008-06-17 18:13:09 EDT
Nicoleau: neah, just fix the spec and cvs commit the fixed version (when times
come...). Please don't forget to ask for epel branches when you make the request
for branch creation
Comment 20 Mamoru TASAKA 2008-06-18 00:17:58 EDT
Nicoleau: Now I am sponsoring you. Please follow "Join" wiki again.
Comment 21 Nicoleau Fabien 2008-06-18 05:10:25 EDT
New Package CVS Request
=======================
Package Name: trickle
Short Description: Portable lightweight userspace bandwidth shaper
Owners: eponyme
Branches: F-8 F-9 EL-4 EL-5
InitialCC: mtasaka
Cvsextras Commits: yes
Comment 22 Mamoru TASAKA 2008-06-18 05:17:50 EDT
I guess wolfy (FAS name) want to be in CC members.
Comment 23 Nicoleau Fabien 2008-06-18 06:53:57 EDT
oups, sorry wolfy. 
Should I also add wolfy in EPEL owners too ?
Comment 24 Mamoru TASAKA 2008-06-18 07:00:03 EDT
(In reply to comment #23)
> oups, sorry wolfy. 
> Should I also add wolfy in EPEL owners too ?

Ah.. sorry I meant I guess wolfy wants to be in Owners list, so yes
(not CC list)
Comment 25 Nicoleau Fabien 2008-06-18 07:05:46 EDT
New Package CVS Request
=======================
Package Name: trickle
Short Description: Portable lightweight userspace bandwidth shaper
Owners: eponyme,wolfy
Branches: F-8 F-9 EL-4 EL-5
InitialCC: mtasaka
Cvsextras Commits: yes
Comment 26 manuel wolfshant 2008-06-18 07:43:26 EDT
Thank you, Tasaka-san.
Comment 27 Jason Tibbitts 2008-06-18 22:52:38 EDT
CVS done.
Comment 28 Fedora Update System 2008-06-19 15:15:37 EDT
trickle-1.07-3.fc9 has been submitted as an update for Fedora 9
Comment 29 Fedora Update System 2008-06-19 15:17:05 EDT
trickle-1.07-3.fc8 has been submitted as an update for Fedora 8
Comment 30 Mamoru TASAKA 2008-06-20 03:33:07 EDT
Okay, now closing.
Comment 31 Fedora Update System 2008-07-03 23:39:27 EDT
trickle-1.07-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 32 Fedora Update System 2008-07-03 23:41:29 EDT
trickle-1.07-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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