Bug 575480 - Review Request: imvirt - Detects several virtualizations
Summary: Review Request: imvirt - Detects several virtualizations
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Steve Traylen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-03-20 22:52 UTC by Miroslav Suchý
Modified: 2010-03-24 12:54 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-24 12:54:41 UTC
steve.traylen: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Miroslav Suchý 2010-03-20 22:52:34 UTC
SPEC:
http://miroslav.suchy.cz/fedora/imvirt/imvirt.spec
SRPM:
http://miroslav.suchy.cz/fedora/imvirt/imvirt-0.9.0-pre1.el6.src.rpm

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2065370

rpmlint is ... 
$ rpmlint /home/msuchy/rpmbuild/SRPMS/imvirt-0.9.0-pre1.el6.src.rpm
imvirt.src: W: spelling-error Summary(en_US) virtualizations -> conceptualizations, visualizations, conceptualization
imvirt.src: W: spelling-error %description -l en_US virtualization -> actualization, visualization, conceptualization
imvirt.src: W: spelling-error %description -l en_US VMware -> Firmware, Stemware, Vaporware
imvirt.src: W: spelling-error %description -l en_US Xen -> Xe, En, Xes
imvirt.src: W: spelling-error %description -l en_US virtualized -> ritualized, actualized, virtual
imvirt.src: W: invalid-url Source0: imvirt-0.9.0-pre1.tar.gz
1 packages and 0 specfiles checked; 0 errors, 6 warnings.
Which shoul all be OK.

Comment 1 Pavel Alexeev 2010-03-21 10:35:27 UTC
Source is not URL and no comments why and how get it.

Comment 2 Miroslav Suchý 2010-03-21 12:36:23 UTC
Fixed. Spec and srpm is on same location.

Comment 3 Steve Traylen 2010-03-21 16:07:09 UTC
Hi Miroslav,

 First I think its much easier for reviewers if you always bump the release
 numbers during the review.


Here's a review:

Review: imvirt
Date:   21st March 2011
Mock Build: F14, x86_64 builds.

* YES: rpmlint output
rpmlint SPECS/imvirt.spec RPMS/x86_64/imvirt-* SRPMS/imvirt-0.9.0-pre1.fc14.src.rpm 
imvirt.x86_64: W: spelling-error Summary(en_US) virtualizations -> conceptualizations, visualizations, actualization
imvirt.x86_64: W: spelling-error %description -l en_US virtualization -> actualization, visualization, contextualization
imvirt.x86_64: W: spelling-error %description -l en_US virtualized -> ritualized, actualized, virtual
imvirt.src: W: spelling-error Summary(en_US) virtualizations -> conceptualizations, visualizations, actualization
imvirt.src: W: spelling-error %description -l en_US virtualization -> actualization, visualization, contextualization
imvirt.src: W: spelling-error %description -l en_US virtualized -> ritualized, actualized, virtual
3 packages and 1 specfiles checked; 0 errors, 6 warnings.

All of these words are in very common usage within this context.
They will become words soon enough.

* YES: Named according to the Package Naming Guidelines.
* YES: spec file name same as  base package %{name}.
* NO: Packaging Guidelines.

The Source tar ball is imvirt-0.9.0-pre1.tar.gz which is presumably
a pre-release to 0.9.0. This needs to be handled in the release.
See: http://fedoraproject.org/wiki/PackageNamingGuidelines#Pre-Release_packages

* YES: Approved license in .spec file.
GPLv2+
* COMMENT: License on Source code.
CLearly licensed this way.
* YES: Include LICENSE file or similar if it exist.
COPYFILE file present.
* YES: Written in American English.
* YES: Spec file legible. 
* YES: Included source must match upstream source.
$ md5sum imvirt-0.9.0-pre1.tar.gz ../SOURCES/imvirt-0.9.0-pre1.tar.gz 
698d022b778aaf0d07ba67fa357da464  imvirt-0.9.0-pre1.tar.gz
698d022b778aaf0d07ba67fa357da464  ../SOURCES/imvirt-0.9.0-pre1.tar.gz

Is the Source URL possible to define exactly the example here?
http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net


* YES: Build on one architecture.
* YES: Not building on an architecture must highlighted.
* YES: Build dependencies must be listed in BuildRequires.
* YES: Handle locales properly. 
* YES: ldconfig must be called on shared libs.
* YES: No bundled copies of system libraries.
* YES: Package must state why relocatable if relocatable.
* YES: A package must own all directories that it creates
* YES:  No duplicate files in %files listings. 
* YES:  Permissions on files must be set properly. %defattr
* YES:  %clean section contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
* YES:  Each package must consistently use macros.
* YES:  The package must contain code, or permissable content.
* YES:  Large documentation files must go in a -doc subpackage.
No large docs.  
* YES:  %doc  must not affect the runtime of the application. 
* YES:  Header files must be in a -devel package.
NO headers.
* YES:  Static libraries must be in a -static package.
No statics.
* YES:  Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
No pkgconfig
* YES:  Then library files that end in .so 
         (without suffix) must go in a -devel package. 
No .sos
* YES:  devel packages must require the exact base package
None
* YES:  No .la libtool archives
None
* YES:  GUI apps should have %{name}.desktop file
No gui
* YES:  No files or directories already owned by other packages. 
None are.
* YES:  %install must run rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
It does.
* YES:  All filenames in rpm packages must be valid UTF-8.
Yes.


So just handling of the fact this is a pre-release 
and if you can change the URL do so.

Comment 4 Steve Traylen 2010-03-21 17:21:44 UTC
I think

Vendor:     IBH IT-Service GmbH (http://www.ibh.de/) 

should come out.

Comment 5 Miroslav Suchý 2010-03-22 10:12:46 UTC
SPEC:
http://miroslav.suchy.cz/fedora/imvirt/
SRPM:
http://miroslav.suchy.cz/fedora/imvirt/imvirt-0.9.0-0.1.pre1.el6.src.rpm

I fixed that release tag according to guidelines.
I removed Vendor tag.

But I did not change Source0 url since that template does not work in this case.
I tried
 http://downloads.sourceforge.net/imvirt/imvirt-0.9.0-pre1.tar.gz
 http://downloads.sourceforge.net/imvirt/imvirt-0.9.0.tar.gz
and both returns 404 not found. So I will stick with that one which works.

Comment 6 Steve Traylen 2010-03-22 10:26:20 UTC
APPROVED but could you add a "-p" to 

install -m644 -D ImVirt.pm $RPM_BUILD_ROOT%{perl_vendorlib}/ImVirt.pm

post review to preserve the timestamp.

 Steve

Comment 7 Miroslav Suchý 2010-03-22 22:03:36 UTC
New Package CVS Request
=======================
Package Name: imvirt
Short Description: Detects several virtualizations
Owners: msuchy
Branches: F-12, F-13, EL-5
InitialCC:

Comment 8 Kalev Lember 2010-03-23 06:20:48 UTC
(In reply to comment #5)
> But I did not change Source0 url since that template does not work in this
> case.
> I tried
>  http://downloads.sourceforge.net/imvirt/imvirt-0.9.0-pre1.tar.gz
>  http://downloads.sourceforge.net/imvirt/imvirt-0.9.0.tar.gz
> and both returns 404 not found. So I will stick with that one which works.    

Try this one:
http://downloads.sourceforge.net/apt-dater/%{name}-%{version}-pre1.tar.gz

Comment 9 Miroslav Suchý 2010-03-23 08:32:09 UTC
that url works. thx kalev.

Comment 10 Kevin Fenzi 2010-03-24 03:33:50 UTC
CVS done (by process-cvs-requests.py).


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