Bug 464308 - Review Request: apt-mirror - APT sources mirroring tool
Review Request: apt-mirror - APT sources mirroring tool
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: manuel wolfshant
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-27 11:48 EDT by Simon
Modified: 2015-03-10 11:18 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-10-20 18:15:19 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
wolfy: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Simon 2008-09-27 11:48:55 EDT
Spec URL:
http://cassmodiah.fedorapeople.org/apt-mirror-0.4.5/apt-mirror.spec

SRPM URL: 
http://cassmodiah.fedorapeople.org/apt-mirror-0.4.5/apt-mirror-0.4.5-1.fc9.src.rpm

Description:
A small and efficient tool that lets you mirror a part of or
the whole Debian GNU/Linux distribution or any other apt sources.

Main features:
 * It uses a config similar to apts <sources.list>
 * It's fully pool comply
 * It supports multithreaded downloading
 * It supports multiple architectures at the same time
 * It can automatically remove unneeded files
 * It works well on overloaded channel to internet
 * It never produces an inconsistent mirror including while mirroring
 * It works on all POSIX complied systems with perl and wget
Comment 1 manuel wolfshant 2008-09-27 12:43:53 EDT
I would have allowed rpmbuild to compress the man page, but I guess that your method works too. 
A couple of questions :
- Why did you trim the sources list and not use the one included in the package?
- Where did you get the license tag from? I see no mention of a specific license anywhere in the tarball or on the project's web site. If there is none, I am afraid that (please correct me if I am wrong) I cannot approve your package until you get a clarification from the author (either via mail or by releasing an updated version)



Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one supported architecture.
     Tested on: devel/x86_64
 [x] Rpmlint output:
source RPM: empty
binary RPM:empty
 [x] Package is not relocatable.
 [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines.
 [!] License field in the package spec file matches the actual license.
--> see issue 1
 [x] If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided in the spec URL.
     SHA1SUM of package: b6dc641f4f12871810280981abb7bdfa5b4dfbb8 apt-mirror_0.4.5.orig.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [!] Package must own all directories that it creates.
See issue 2
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf %{buildroot}.
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
 [-] Package contains a properly installed %{name}.desktop file if it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64
 [?] Package should compile and build into binary rpms on all supported architectures.
     Tested on:devel/x86_64
 [x] Package functions as described.
 [-] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.

=== Issues ===
1. License tag
     License type as declared by our spec : GPLv2
     License type as declared by the sources : I have not find any place specifying it
Could you please explain why did you use GPLv2 as license?
2. Your last line in %files packages only the folders below /var/spool/apt-mirror but not the directory itself, thus leaving it unowned.
3. There is no need for perl as a specific "Requires". rpmbuild adds /usr/bin/perl automatically.
Comment 2 Simon 2008-09-28 15:08:33 EDT
hi manuel

1. the mirror.list in the source is too old...
sarge is the current oldstable and I would rather work with releases than with codenames
oldstable = sarge
stable = etch
testing = lenny
in a few weeks (or months we don't know it) is lenny stable and sarge will run out of support
oldstable = etch 
stable = lenny
testing = squeeze


2. the license is not tricky ;-)
----
This package was debianized by Dmitry N. Hramtsov <hdn@nsu.ru> on
Sat, 27 Jul 2002 12:44:33 +0700.

It was downloaded from http://apt-mirror.sourceforge.net/

Upstream Author(s): Dmitry N. Hramtsov <hdn@nsu.ru>

Copyright: GPLv2
----
This is from the tarball of version 0.3.0; the first public version.
The upstream author and the debian package author is the same person :-)
no changes to the current release


3. Removed "Requires: perl"


4.Correct %files-section


5. I don't know a better way to extract a manpage from a perl script


6.
SPEC (updated):
http://cassmodiah.fedorapeople.org/apt-mirror-0.4.5/apt-mirror.spec

SRPM:
http://cassmodiah.fedorapeople.org/apt-mirror-0.4.5/apt-mirror-0.4.5-2.fc9.src.rpm
Comment 3 manuel wolfshant 2008-09-28 16:52:56 EDT
OK to replies 1, 3-5.
Yet I still fail to see where is the license specified. Whatever it was at version 0.3.0, you have packaged 0.4.5 and despite reading once again the source and the web site, I cannot locate a single place referencing a license. The closest thing that I could find is http://packages.debian.org/changelogs/pool/main/a/apt-mirror/apt-mirror_0.4.5-1/apt-mirror.copyright, but as this is just _related_ to the package and not included as such... I do not know what to say. Calling in fedora-legal for help.
Comment 4 Susi Lehtola 2008-09-29 09:40:01 EDT
The sourceforge project page at

http://sourceforge.net/projects/apt-mirror/

states that the license is GPL. Maybe you should contact the author at hdn@post.nsu.ru to get him to clear up the license on the project website.
Comment 5 manuel wolfshant 2008-09-29 12:28:57 EDT
  I have no doubt that the application is GPL, otherwise Debian would not have claimed that on their server. But AFAIK we need a clear proof for that (if I am mistaken, please correct me and I will gladly approve the package. I jumped into the review less than an hour after submission because I WANT it in Fedora).
  Not to mention that GPL has several flavors and the license tag in the spec must specify the correct one.
Comment 6 Tom "spot" Callaway 2008-10-08 12:45:58 EDT
http://packages.debian.org/changelogs/pool/main/a/apt-mirror/apt-mirror_0.4.5-1/apt-mirror.copyright is enough proof of licensing (GPLv2+), because the debian package maintainer is the same as upstream.

You need to update the licensing tag to reflect this (currently, you have GPLv2, which is not correct).

You should still email the author and ask him to include a proper license attribution in the header of the apt-mirror script, and a copy of the GPL (COPYING).

Lifting FE-Legal.
Comment 7 manuel wolfshant 2008-10-08 17:32:59 EDT
package APPROVED. please do not forget to change the license tag to GPLv2+ before uploading to CVS
Comment 9 Simon Wesp 2008-10-12 09:58:46 EDT
Package Name: apt-mirror
Short Description: APT sources mirroring tool
Owners: cassmodiah
Branches: F9
InitialCC:
Comment 10 manuel wolfshant 2008-10-12 10:37:37 EDT
Simon, if possible, please consider maintaining EPEL branches, too. TIA.
Comment 11 Simon Wesp 2008-10-12 11:32:53 EDT
Package Name: apt-mirror
Short Description: APT sources mirroring tool
Owners: cassmodiah
Branches: F-9 EL-4 EL-5
InitialCC:
Comment 12 Kevin Fenzi 2008-10-12 22:12:20 EDT
cvs done.
Comment 13 Fedora Update System 2008-10-16 10:45:28 EDT
apt-mirror-0.4.5-3.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/apt-mirror-0.4.5-3.fc9
Comment 14 Fedora Update System 2008-10-20 18:15:16 EDT
apt-mirror-0.4.5-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 15 Mosaab Alzoubi 2015-03-10 09:30:30 EDT
Package Change Request
======================
Package Name: apt-mirror
New Branches: master f21 f22
Owners: moceap
Comment 16 Gwyn Ciesla 2015-03-10 11:18:22 EDT
Is this an unretirement request?  If so, has there been a re-review?

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