Bug 473835 - Review Request: autoarchive - Simple backup tool
Review Request: autoarchive - Simple backup tool
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Julian Aloofi
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-11-30 19:10 EST by Fabian Affolter
Modified: 2009-08-04 20:31 EDT (History)
5 users (show)

See Also:
Fixed In Version: 0.1.2-2.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-08-04 20:31:25 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
julian.fedora: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Fabian Affolter 2008-11-30 19:10:22 EST
Spec URL: http://fab.fedorapeople.org/packages/SRPMS/autoarchive.spec
SRPM URL: http://fab.fedorapeople.org/packages/SRPMS/autoarchive-0.1.1-1.fc9.src.rpm

Project URL: http://autoarchive.sourceforge.net/

Description:
AutoArchive is a simple utility for making backups more easily. It
uses tar for creating archives. The idea of the program is that every 
information needed for making a backup is in one file - the archive 
spec file. Path to this file is passed as a parameter to 'aa' command 
which reads informations from it and creates desired backup.

Koji scratch builds:
http://koji.fedoraproject.org/koji/taskinfo?taskID=964944

rpmlint output:
[fab@laptop024 noarch]$ rpmlint auto*
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[fab@laptop024 SRPMS]$ rpmlint auto*
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 1 Gratien D'haese 2009-01-16 11:08:36 EST
Home page of project: http://autoarchive.sourceforge.net/
File Section on SourceForge:
http://sourceforge.net/project/platformdownload.php?group_id=239510

Analysis Report
================

** OK ** - MUST: rpmlint must be run on every package.
$ rpmlint -i -v SPECS/autoarchive.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.

$ rpmlint -i -v ~/Download/autoarchive-0.1.1-1.fc9.src.rpm 
autoarchive.src: I: checking
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

** OK ** - MUST: The package meets the Packaging Guidelines. Package name "autoarchive" is acceptable and does not yet exist in the list of registered packages.

** OK ** - MUST: The spec file name matches the base package %{name}, in the format %{name}.spec.

** OK ** - MUST: The package is licensed with a Fedora approved license (GPLv3+) and
meets the Licensing Guidelines. 

** FIX ** - MUST: The License field in the package spec file matches the actual license.
** OK **	  The file COPYING matches the spec License line.
!! Warning !!	  The file PKG-INFO mentions as license GNU GPL which does not match GPLv3+
!! Warning !!	  The source files do not match the license:
# archiver.py
#
# Project: AutoArchive
# License: GNU GPL

** OK ** - MUST: The license file from the source package is included in %doc.

** OK ** - MUST: The spec file is in American English.

** OK ** - MUST: The spec file for the package is legible.

** OK ** - MUST: The sources used to build the package matches the upstream source by
e5d447c99c056027778ea7abf4d4c91e  Download/autoarchive-0.1.1-1.fc9.src.rpm
8cede45be633221fca031b4825ede1ea  Download/autoarchive-0.1.1.tar.bz2
extracted tarball (from rpm -ivh ~/Download/autoarchive-0.1.1-1.fc9.src.rpm) equals:
$ md5sum autoarchive-0.1.1.tar.bz2
8cede45be633221fca031b4825ede1ea  autoarchive-0.1.1.tar.bz2

** OK ** - MUST: The package successfully compiles and builds into binary rpms on i386
$ rpmbuild -ba ../SPECS/autoarchive.spec 
error: Failed build dependencies:
	python-setuptools-devel is needed by autoarchive-0.1.1-1.fc9.noarch
$ grep Requires ../SPECS/autoarchive.spec 
BuildRequires:  python
BuildRequires:  python-setuptools-devel
(first installing python-setuptools-devel)
$ rpmbuild -ba ../SPECS/autoarchive.spec
succeeds.
$ rpmlint -v -i RPMS/noarch/autoarchive-0.1.1-1.fc9.noarch.rpm 
autoarchive.noarch: I: checking
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

** N/A ** - MUST: If the package does not successfully compile, build or work 
on an architecture, then those architectures should be listed in the spec
in ExcludeArch.

** OK ** - MUST: Not all build dependencies are listed in BuildRequires:
$ rpm -qp --requires RPMS/noarch/autoarchive-0.1.1-1.fc9.noarch.rpm 
/usr/bin/python  
python(abi) = 2.5

!! Warning !!	Not all Requirements are listed - lzma seems to be used and is not part
		of the Packaging/FullExceptionList (it is up to you to decide).

** OK ** - MUST: The spec file handles locales properly with the %find_lang macro.

** N/A ** - MUST: Every binary RPM package (or subpackage) which stores shared
library files (not just symlinks) in any of the dynamic linker's default paths,
must call ldconfig in %post and %postun.

** N/A ** - MUST: If the package is designed to be relocatable, the packager must
state this fact in the request for review, along with the rationalization for
relocation of that specific package.

** OK ** - MUST: The package owns all directories that it creates.
$ rpm -qpl RPMS/noarch/autoarchive-0.1.1-1.fc9.noarch.rpm 
/usr/bin/aa
/usr/lib/python2.5/site-packages/AutoArchive
/usr/lib/python2.5/site-packages/AutoArchive/__init__.py
/usr/lib/python2.5/site-packages/AutoArchive/__init__.pyc
/usr/lib/python2.5/site-packages/AutoArchive/__init__.pyo
/usr/lib/python2.5/site-packages/AutoArchive/aautils.py
/usr/lib/python2.5/site-packages/AutoArchive/aautils.pyc
/usr/lib/python2.5/site-packages/AutoArchive/aautils.pyo
/usr/lib/python2.5/site-packages/AutoArchive/archive_spec.py
/usr/lib/python2.5/site-packages/AutoArchive/archive_spec.pyc
/usr/lib/python2.5/site-packages/AutoArchive/archive_spec.pyo
/usr/lib/python2.5/site-packages/AutoArchive/archiver.py
/usr/lib/python2.5/site-packages/AutoArchive/archiver.pyc
/usr/lib/python2.5/site-packages/AutoArchive/archiver.pyo
/usr/lib/python2.5/site-packages/AutoArchive/meta.py
/usr/lib/python2.5/site-packages/AutoArchive/meta.pyc
/usr/lib/python2.5/site-packages/AutoArchive/meta.pyo
/usr/lib/python2.5/site-packages/AutoArchive/options.py
/usr/lib/python2.5/site-packages/AutoArchive/options.pyc
/usr/lib/python2.5/site-packages/AutoArchive/options.pyo
/usr/lib/python2.5/site-packages/autoarchive-0.1.1-py2.5.egg-info
/usr/share/doc/autoarchive-0.1.1
/usr/share/doc/autoarchive-0.1.1/COPYING
/usr/share/doc/autoarchive-0.1.1/NEWS
/usr/share/doc/autoarchive-0.1.1/README
/usr/share/doc/autoarchive-0.1.1/README.sk
/usr/share/man/man1/aa.1.gz


** OK ** - MUST: The package does not contain any duplicate files in the %files
listing.

** OK ** - MUST: Permissions on files are set properly. The %files section includes a
%defattr(...) line.

** OK ** - MUST: The package has a %clean section, which contains or $RPM_BUILD_ROOT
or %{buildroot}.

** OK ** - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines.

** OK ** - MUST: The package contains code, no content.

** N/A ** - MUST: Large documentation files should go in a -doc subpackage.

** OK ** - MUST: Files included as %doc do not affect the runtime of the application.
** N/A ** - MUST: Header files must be in a -devel package.
** N/A ** - MUST: Static libraries must be in a -static package.
** N/A  **- MUST: Packages containing pkgconfig(.pc) files must 'Requires:
pkgconfig'.
** N/A ** - MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), then library files that end in .so (without suffix) must go in
a -devel package.

** N/A ** - MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency: Requires: %{name} =%{version}-%{release}

** OK ** - MUST: The package does not contain any .la libtool archives.

** N/A ** - MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section.

** OK ** - MUST: The packages does not own files or directories already owned by
other packages.

** OK ** - MUST: At the beginning of %install, the package runs rm -rf $RPM_BUILD_ROOT.

** FIX ** - MUST: All filenames in rpm packages are valid UTF-8.
$ file /usr/share/doc/autoarchive-0.1.1/COPYING
/usr/share/doc/autoarchive-0.1.1/COPYING: ASCII English text

Keeping the original date/time of documentation file is probably a good idea
(no guidelines about this)

A simple solution :
     # Convert to utf-8
     for file in COPYING NEWS README README.sk; do
       mv $file timestamp
       iconv -f ISO-8859-1 -t UTF-8 -o $file timestamp
       touch -r timestamp $file
     done


** N/A ** - SHOULD: If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to include it.

** N/A ** - SHOULD: The description and summary sections in the package spec file
should contain translations for supported Non-English languages, if available.

** OK ** - SHOULD: The the package builds in mock.
$ mock -r default rebuild /home/gdha/RPM/SRPMS/autoarchive-0.1.1-1.fc9.src.rpm
builds are succssfully.

** OK ** - SHOULD: The package should compile and build into binary rpms on all
supported architectures.
$ koji build --arch=i386 --scratch dist-f10 ../../SRPMS/autoarchive-0.1.1-1.fc9.src.rpm 
Uploading srpm: ../../SRPMS/autoarchive-0.1.1-1.fc9.src.rpm
[====================================] 100% 00:00:02  24.74 KiB  10.11 KiB/sec
Created task: 1058745
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=1058745
Watching tasks (this may be safely interrupted)...
1058745 build (dist-f10, autoarchive-0.1.1-1.fc9.src.rpm): free
1058745 build (dist-f10, autoarchive-0.1.1-1.fc9.src.rpm): free -> open (x86-1.fedora.phx.redhat.com)
  1058746 buildArch (autoarchive-0.1.1-1.fc9.src.rpm, i386): free
  1058746 buildArch (autoarchive-0.1.1-1.fc9.src.rpm, i386): free -> open (x86-2.fedora.phx.redhat.com)
  1058746 buildArch (autoarchive-0.1.1-1.fc9.src.rpm, i386): open (x86-2.fedora.phx.redhat.com) -> closed
  0 free  1 open  1 done  0 failed
1058745 build (dist-f10, autoarchive-0.1.1-1.fc9.src.rpm): open (x86-1.fedora.phx.redhat.com) -> closed
  0 free  0 open  2 done  0 failed

1058745 build (dist-f10, autoarchive-0.1.1-1.fc9.src.rpm) completed successfully


** OK ** - SHOULD: The package functions as described.
$ cat .my_config.aa 
# AutoArchive file for users configuration files
[General]
name: users-config
path: /home/makerpm
include_files: .*
include_files: *
exclude_files: *.gz
[makerpm@localhost ~]$ aa .my_config.aa 
/usr/bin/aa: *.gz: No such file or directory


** N/A  **- SHOULD: If scriptlets are used, those scriptlets must be sane.

** N/A ** - SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.

** N/A ** - SHOULD: The placement of pkgconfig(.pc) files depends on their usecase,
and this is usually for development purposes, so should be placed in a -devel
pkg.

** N/A ** - SHOULD: If the package has file dependencies outside of /etc, /bin,
/sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the
file instead of the file itself.

Issues:
- Would be nice to provide some example config-files with extention .aa
- command name "aa" does not remind me of "autoarchive", perhaps better
  use autoarchive
- The name autoarchiver is sometimes written as AutoArchiver. Why the
  difference?
- FIX the above mentioned items
- Summary line (in spec file) is a bit simplistic "Simple backup tool".
  Does not give me any hint about autoarchiver. Please give a better
  summary.
Comment 2 Fabian Affolter 2009-03-18 13:46:44 EDT
Thanks for the review.

(In reply to comment #1)
> ** FIX ** - MUST: The License field in the package spec file matches the actual
> license.
> ** OK **   The file COPYING matches the spec License line.
> !! Warning !!   The file PKG-INFO mentions as license GNU GPL which does not
> match GPLv3+
> !! Warning !!   The source files do not match the license:
> # archiver.py
> #
> # Project: AutoArchive
> # License: GNU GPL

I changed the license to GPL+, removed the needless COPYING, and informed upstream about the discrepancy.
http://sourceforge.net/tracker/?func=detail&atid=1110082&aid=2691699&group_id=239510

> !! Warning !! Not all Requirements are listed - lzma seems to be used and is
> not part
>   of the Packaging/FullExceptionList (it is up to you to decide).
%files section

added

> ** FIX ** - MUST: All filenames in rpm packages are valid UTF-8.
> $ file /usr/share/doc/autoarchive-0.1.1/COPYING
> /usr/share/doc/autoarchive-0.1.1/COPYING: ASCII English text
> 
> Keeping the original date/time of documentation file is probably a good idea
> (no guidelines about this)
> 
> A simple solution :
>      # Convert to utf-8
>      for file in COPYING NEWS README README.sk; do
>        mv $file timestamp
>        iconv -f ISO-8859-1 -t UTF-8 -o $file timestamp
>        touch -r timestamp $file
>      done

The files are already UTF-8, aren't they?

> Issues:
> - Would be nice to provide some example config-files with extention .aa

There is an example configuration mentioned in the README. Pinged upstream about that
https://sourceforge.net/tracker2/?func=detail&aid=2692252&group_id=239510&atid=1110082

> - command name "aa" does not remind me of "autoarchive", perhaps better
>   use autoarchive

This is upstream's call and not the one of the package maintainers.  I agree with you that the name would be better 'autoarchive' than 'aa'.
https://sourceforge.net/tracker2/?func=detail&aid=2692266&group_id=239510&atid=1110082

> - The name autoarchiver is sometimes written as AutoArchiver. Why the
>   difference?

The project is called 'AutoArchive' and the application stuff 'autoarchive'.  For me this seams a normal way to go. 'AutoArchive' for the python stuff is feasible, from my point of view. 

> - FIX the above mentioned items

done

> - Summary line (in spec file) is a bit simplistic "Simple backup tool".
>   Does not give me any hint about autoarchiver. Please give a better
>   summary.  

As fare as I know is the intention of the summary to provide only very basic information about the tool.  But changed a bit.


Here are the updated files:

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/autoarchive.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/autoarchive-0.1.1-2.fc10.src.rpm
Comment 3 Fabian Affolter 2009-04-15 07:14:32 EDT
* Wed Apr 15 2009 Fabian Affolter <fabian@bernewireless.net> - 0.1.2-1
- Upstream renamed some parts from aa to autoarchive (symlinks)
- Added examples
- Updated to new upstream version 0.1.2


Here are the updated files:

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/autoarchive.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/autoarchive-0.1.2-1.fc10.src.rpm
Comment 4 Julian Aloofi 2009-07-26 14:38:35 EDT
rpmlint output of all files is clean:
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

MUST: The package does not yet exist in Fedora. The Review Request is not a
duplicate. OK
MUST: The spec file for the package is legible and macros are used
consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the 
Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license.
OK
MUST: 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 must be included in %doc. NEEDSWORK
-Please add COPYING to the doc section

MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package
that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. N/A
MUST: Permissions on files must be set properly. OK
MUST: Clean section exists. OK
MUST: Large documentation files must go in a -doc subpackage. N/A
MUST: All relevant items are included in %doc. Items in %doc do not affect
runtime of application. NEEDSWORK
-As I said before, you need to include COPYING

MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files
ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. N/A
MUST: Desktop files are installed properly. N/A
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from
upstream, the packager should query upstream to include it. OK
SHOULD: The package builds in mock. OK 


Notes:
-Your package should BuildRequire: python-devel instead of python
Comment 5 Susi Lehtola 2009-07-26 15:11:46 EDT
A couple of additional notes - I just sponsored Julian.


(In reply to comment #4)
> Notes:
> -Your package should BuildRequire: python-devel instead of python  

Actually, python-setuptools-devel pulls in python-devel, so that's not a problem. But it's better to BR it exclusively.

**

(The ./ in front of setup.py is unnecessary when run via python.)

**

I suggest using autoarchive instead of %{name} in the %files section for consistency.

**

Requires: lzma is not enough IMHO. The package seems to need also tar, gzip and bzip2.
Comment 6 Fabian Affolter 2009-07-26 17:16:05 EDT
Thank you guys. 

(In reply to comment #4)
> MUST: 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 must be included in %doc. NEEDSWORK
> -Please add COPYING to the doc section

After the review of Gratien I removed COPYING.  But the fixed version came back with 0.1.2.  COPYING added

(In reply to comment #5)
> (In reply to comment #4)
> > Notes:
> > -Your package should BuildRequire: python-devel instead of python  
> 
> Actually, python-setuptools-devel pulls in python-devel, so that's not a
> problem. But it's better to BR it exclusively.

For newer Fedora releases that's true.  For releases < F-11 the guidelines told another story.  Changed.

> (The ./ in front of setup.py is unnecessary when run via python.)

Removed

> I suggest using autoarchive instead of %{name} in the %files section for
> consistency.

Changed

> Requires: lzma is not enough IMHO. The package seems to need also tar, gzip and
> bzip2.  

Yes, you are right, the 'archiver class' shows points that let indicate that all this tools are needed.

Here are the updated files:

Spec URL: http://fab.fedorapeople.org/packages/SRPMS/autoarchive.spec
SRPM URL:
http://fab.fedorapeople.org/packages/SRPMS/autoarchive-0.1.2-2.fc11.src.rpm
Comment 7 Julian Aloofi 2009-07-26 19:12:24 EDT
(In reply to comment #6)
> > Requires: lzma is not enough IMHO. The package seems to need also tar, gzip and
> > bzip2.  
> 
> Yes, you are right, the 'archiver class' shows points that let indicate that
> all this tools are needed.

You are still using the old spec file in the Spec URL link, but the one in the SRPM is good.
By the way, tar gzip and bzip2 are in the Base group anyway, you can't install Fedora without them. I wonder why these packages are not in the exception list in the Guidelines.

I can't see any further problems with the package.

APPROVED
Comment 8 Fabian Affolter 2009-07-27 02:57:54 EDT
New Package CVS Request
=======================
Package Name: autoarchive
Short Description:  Simple backup tool
Owners: fab
Branches: F-10 F-11
InitialCC:
Comment 9 Kevin Fenzi 2009-07-28 00:30:15 EDT
cvs done.
Comment 10 Fedora Update System 2009-07-28 02:01:56 EDT
autoarchive-0.1.2-2.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/autoarchive-0.1.2-2.fc10
Comment 11 Fedora Update System 2009-07-28 02:02:03 EDT
autoarchive-0.1.2-2.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/autoarchive-0.1.2-2.fc11
Comment 12 Fedora Update System 2009-07-28 14:25:10 EDT
autoarchive-0.1.2-2.fc10 has been pushed to the Fedora 10 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 autoarchive'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-8063
Comment 13 Fedora Update System 2009-07-28 14:26:06 EDT
autoarchive-0.1.2-2.fc11 has been pushed to the Fedora 11 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 autoarchive'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-8067
Comment 14 Fedora Update System 2009-08-04 20:31:19 EDT
autoarchive-0.1.2-2.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 15 Fedora Update System 2009-08-04 20:31:32 EDT
autoarchive-0.1.2-2.fc10 has been pushed to the Fedora 10 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.