Bug 450410 - Review Request: multiget - A multi-thread http/ftp file downloader
Summary: Review Request: multiget - A multi-thread http/ftp file downloader
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-06-07 21:44 UTC by Guido Ledermann
Modified: 2008-08-06 10:17 UTC (History)
5 users (show)

Fixed In Version: 1.2.0-3.fc8
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-07-30 19:59:16 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Guido Ledermann 2008-06-07 21:44:15 UTC
Spec URL: http://www.soglatec.de/fedora/multiget.spec
SRPM URL: http://www.soglatec.de/fedora/multiget-1.2.0-1.fc9.src.rpm
Description: multiget is an easy-to-use GUI file downloader. It's programmed in C++ and has a GUI based on wxWidgets. It supports HTTP/FTP protocols which covers the requirements of most users. It supports multi-task with multi-thread on multi-server. It supports resuming downloads if the Web server supports it, and if you like, you can reconfig the thread number without stopping the current task. It's also support SOCKS 4,4a,5 proxy, ftp proxy, http proxy.

There has already been a review for a previous version:
https://bugzilla.redhat.com/show_bug.cgi?id=231786

Comment 1 manuel wolfshant 2008-06-08 01:20:54 UTC
 Guido, once a program has been accepted in Fedora, the review for newer
versions is not usually needed.
 You should get in touch with multiget's Fedora maintainer and ask him to update
to the newer version and / or offer co-maintainership.

Comment 2 Guido Ledermann 2008-06-08 07:42:30 UTC
Manuel, the package is orphaned and Mamoru Tasaka, asked if I could take the
package, agreed. Furthermore there were significant changes to the spec
necessary to get 1.2.0 work. Christoph Wickert offered sponsorship to me and
this should be the first Package for me to maintain (though there has been no
development in recent days).  

Comment 3 Mamoru TASAKA 2008-06-12 06:59:42 UTC
The package on comment 0 does not build.
http://koji.fedoraproject.org/koji/taskinfo?taskID=658540

I just tried to rebuild this package, however
- This package seems to apply a patch on Makefiles after calling
  configure, which is not good in general. You should apply a
  patch against "Makefile.in"s. Also, $RPM_OPT_FLAGS differs between
  on i386 system and on x86_64 system.

- The line like "#%configure" does not prevent configure from being
  executed (actually from build.log you can see that configure is
  actually called). This is rpm "feature".

Comment 4 Guido Ledermann 2008-06-12 09:47:12 UTC
Thanks for your comments, Mamoru. I've changed the spec and the patch. You can
find them at
Spec URL: http://www.soglatec.de/fedora/multiget.spec
SRPM URL: http://www.soglatec.de/fedora/multiget-1.2.0-2.fc9.src.rpm

Comment 5 Mamoru TASAKA 2008-06-12 15:13:11 UTC
Well, from where can we confirm the license of this package?

Comment 6 Mamoru TASAKA 2008-06-12 15:17:05 UTC
(In reply to comment #5)
> Well, from where can we confirm the license of this package?
Scratch this comment :(

Comment 7 Mamoru TASAKA 2008-06-12 16:53:07 UTC
Well, after all the license issue is pending.

It seems that this package is licensed under at least GPLv2, however
now we distinguish between GPLv2 and GPLv3 and GPLv2+.
Would you ask the upstream to clarify under what license this package
is released?

Comment 8 Mamoru TASAKA 2008-06-12 17:10:30 UTC
One of the reason I am asking is that while some files (including 
src/mainframe.cpp ) says that they are licensed under GPLv2+,
src/mainframe.cpp also includes:
----------------------------------------------------------------------
     9  /*
    10   *  This program is free software; you can redistribute it and/or modify
    11   *  it under the terms of the GNU General Public License as published by
    12   *  the Free Software Foundation; either version 2 of the License, or
    13   *  (at your option) any later version.
  1235          info.SetVersion(wxT("v1.2.0"));
  1236          info.SetLicence(wxT("GPLv2"));
  1237          info.SetWebSite(wxT("http://multiget.sourceforge.net"));
------------------------------------------------------------------------
which seems to be saying as "GPLv2" (although the header of the file
also says GPLv2+....)

Comment 9 Guido Ledermann 2008-06-12 17:13:45 UTC
(In reply to comment #8)
> One of the reason I am asking is that while some files (including 
> src/mainframe.cpp ) says that they are licensed under GPLv2+,
> src/mainframe.cpp also includes:

I see. I've just sent an email and will get back when I have some news.

Comment 10 Mamoru TASAKA 2008-06-12 17:49:07 UTC
Thanks. Then for 1.2.0-2:

Except for license issue:
* Source3
  - Please specify the URL of Source3 (or write as a comment
    how you received Source3)

* Buildroot
  - Please follow
    http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot

* BuildRequires
  - Currently gtk2-devel seems to pull automake, however
    for fixing broken install-sh symlink I think explicitly
    adding "BuildRequires: automake" is preferred.

* Perl module BRs:
  - For perl modules BuildRequires, please follow
    http://fedoraproject.org/wiki/Packaging/Perl#requiresandprovides
    (In short, "BuildRequires: perl-XML-Parser" must be
     "BuildRequires: perl(XML::Parser)" )

* Patch0 vs %optflags vs %configure
  - You seem to add %Patch0 to honor %optflags, however this Patch0
    is not needed.
    The reason your srpm does not honor %optflags is that %configure
    is called at %prep, not at %build. Actually %configure sets
    CPPFLAGS, however written at %prep, all those variables are
    reset.
    Move %configure to %build and then Patch0 is no longer needed.

* Macros
  - Use macros. /usr/share must be %{_datadir}.

* Timestamps
  - When using "cp" or "install" commands, add "-p" option to keep
    timestamps on installed files.


Comment 11 Guido Ledermann 2008-06-12 19:20:25 UTC
(In reply to comment #10)
> * Source3
>   - Please specify the URL of Source3 (or write as a comment
>     how you received Source3)

Wrote a comment in the spec. Actually there isn't an icon provided within the
source of 1.2.0. So I took the one from 1.1.4

> * Buildroot
>   - Please follow
>     http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot

Done.

> * BuildRequires
>   - Currently gtk2-devel seems to pull automake, however
>     for fixing broken install-sh symlink I think explicitly
>     adding "BuildRequires: automake" is preferred.

Fixed.

> * Perl module BRs:
>   - For perl modules BuildRequires, please follow
>     http://fedoraproject.org/wiki/Packaging/Perl#requiresandprovides
>     (In short, "BuildRequires: perl-XML-Parser" must be
>      "BuildRequires: perl(XML::Parser)" )

Fixed. Will be carefully done in future packages.

> * Patch0 vs %optflags vs %configure
>   - You seem to add %Patch0 to honor %optflags, however this Patch0
>     is not needed.
>     The reason your srpm does not honor %optflags is that %configure
>     is called at %prep, not at %build. Actually %configure sets
>     CPPFLAGS, however written at %prep, all those variables are
>     reset.
>     Move %configure to %build and then Patch0 is no longer needed.

Okay. Got it. So this patch is obsolete now.

> * Macros
>   - Use macros. /usr/share must be %{_datadir}.

Done.

> * Timestamps
>   - When using "cp" or "install" commands, add "-p" option to keep
>     timestamps on installed files.

Fixed.

Find the latest here:
Spec URL: http://www.soglatec.de/fedora/multiget.spec
SRPM URL: http://www.soglatec.de/fedora/multiget-1.2.0-3.fc9.src.rpm


Comment 12 Mamoru TASAKA 2008-06-13 17:34:21 UTC
Okay, as it seems that at least this is licensed under GPLv2 (however
I really want the reply from the upstream), this package itself is okay.

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few (or no) work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora package collection review requests which are waiting for someone to
review can be checked on:
http://fedoraproject.org/PackageReviewStatus/NEW.html
(NOTE: please don't choose "Merge Review")


Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------


Comment 13 Guido Ledermann 2008-06-19 18:05:10 UTC
From the reply of the upstream we can admit that this program is licensed under
GPLv2.

Comment 14 Mamoru TASAKA 2008-06-28 16:20:02 UTC
ping?

Comment 15 Guido Ledermann 2008-06-28 18:55:11 UTC
Sorry for the delay. My review will be posted during the next week.

Comment 16 Guido Ledermann 2008-06-29 07:58:22 UTC
Here is my pre-review of a package.

https://bugzilla.redhat.com/show_bug.cgi?id=448717

Comment 17 Mamoru TASAKA 2008-06-29 08:52:40 UTC
Okay, your pre-review seems good to some extent for initial comments.

------------------------------------------------------------------------
     This package (multiget) 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 Christoph Wickert 2008-06-29 13:39:05 UTC
(In reply to comment #17)

> Then I will sponsor you.

As written in comment #2 I am willing to sponsor Guido, in fact I already did,
so I'm removing FE-NEEDSPINSOR blocker now. Guido and me know each other from
real life and we are both German, which simplifies communication a lot.

Nevertheless thanks a lot for offering sponsorship to Guido and for helping out
with this review.

Comment 19 Mamoru TASAKA 2008-06-29 13:57:53 UTC
(In reply to comment #18)
> As written in comment #2 I am willing to sponsor Guido, in fact I already did,
> so I'm removing FE-NEEDSPINSOR blocker now. Guido and me know each other from
> real life and we are both German, which simplifies communication a lot.

Ah, that is much better! Thank you.

Comment 20 Guido Ledermann 2008-06-29 21:14:24 UTC
Package Change Request
======================
Package Name: multiget
Updated Fedora Owners: guidoledermann

Comment 21 Kevin Fenzi 2008-06-30 16:11:19 UTC
cvs done.

Comment 22 Fedora Update System 2008-07-02 17:05:53 UTC
multiget-1.2.0-3.fc9 has been submitted as an update for Fedora 9

Comment 23 Fedora Update System 2008-07-02 17:07:50 UTC
multiget-1.2.0-3.fc8 has been submitted as an update for Fedora 8

Comment 24 Fedora Update System 2008-07-02 17:09:18 UTC
multiget-1.2.0-3.fc8 has been submitted as an update for Fedora 8

Comment 25 Fedora Update System 2008-07-03 03:15:51 UTC
multiget-1.2.0-3.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update multiget'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-6036

Comment 26 Fedora Update System 2008-07-30 19:59:14 UTC
multiget-1.2.0-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Fedora Update System 2008-07-30 20:05:25 UTC
multiget-1.2.0-3.fc9 has been pushed to the Fedora 9 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.